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

David Holmes david.holmes at
Wed Feb 8 18:51:18 PST 2012

On 9/02/2012 1:36 AM, Erik Joelsson wrote:
> New webrev up with the changes I detailed below:
> 177 lines changed: 89 ins; 29 del; 59 mod; 3970 unchg


+ ifneq (,$(SPEC))
+   include $(SPEC)
+ endif

Having the blank first looks odd. I assume you aren't using -inlcude 
because you want to see errors if SPEC is set but not found.


The definitions of MT=mt.exe in each block for the different VS versions 
seems redundant. If we factor this out is there any reason not to group:


together and use the same "if (,$(SPEC))" approach?


> /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

More information about the hotspot-runtime-dev mailing list