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

Erik Joelsson erik.joelsson at
Wed Feb 8 07:36:02 PST 2012

New webrev up with the changes I detailed below:
177 lines changed: 89 ins; 29 del; 59 mod; 3970 unchg


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

More information about the hotspot-runtime-dev mailing list