Review for 7141244: build-infra merge: Include $(SPEC) in makefiles and make variables overridable

[David Holmes]

Hi Erik,

On 8/02/2012 6:14 PM, Erik Joelsson wrote:
> Thanks for looking at this!
>
> On 2012-02-08 02:47, David Holmes wrote:
>> It doesn't make sense to me to include SPEC in make/Makefile and
>> make/Defs.make because Makefile includes Defs.make. You only need the
>> -include in Defs.make (unless SPEC is going to define GAMMADIR or
>> ALT_OUTPUTDIR - in which case include it in the Makefile not Defs.make)
>>
> I checked this again and you are right, we don't set anything that
> warrants including SPEC in make/Makefile. I will move it to defs only.
>> So this seems really ugly to me. If these were all set as Make
>> variables on a top-level make invocation then you wouldn't need to do
>> any of these tests. If the SPEC file is always going to set these
>> variables then why not either include SPEC or else do these
>> definitions eg:
>>
>> ifeq ($(SPEC),)
>> CC = ...
>> CXX = ..
>> ...
>> endif
>> # else SPEC already defined these
>>
>> this might need some refactoring to group the necessary settings
>> together.
>>
> This was how I initially did it, but I wasn't sure on the best solution.
> I also forgot about command line overriding normal assignments. With an
> explicit check for SPEC it's very obvious what we are trying to achieve.
> I will look into this and try to group things more neatly together for
> it. Hope to publish a new webrev later today.

This reminded me that one of the things on my wishlist for the new build 
was that I can still override something in SPEC by setting it on the 
command-line invocation of make.

Cheers,
David

> /Erik