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

[Erik Joelsson]

New webrev up with the changes I detailed below:

http://cr.openjdk.java.net/~erikj/7141244/webrev.01/
177 lines changed: 89 ins; 29 del; 59 mod; 3970 unchg

/Erik

On 2012-02-08 09:14, 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.
>
> /Erik