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

Erik Joelsson erik.joelsson at oracle.com
Thu Feb 9 09:33:48 UTC 2012


New webrev:
http://cr.openjdk.java.net/~erikj/7141244/webrev.02/ 
<http://cr.openjdk.java.net/%7Eerikj/7141244/webrev.02/>
177 lines changed: 89 ins; 29 del; 59 mod; 3970 unchg

Changes since last time:

* Moved the , to after $(SPEC)
* Changed comment in gcc/sparcWorks.make according to suggestion from 
Fredrik.

Haven't changed anything regarding the nmake files.

/Erik

On 2012-02-09 10:09, Erik Joelsson wrote:
>
> On 2012-02-09 03:51, David Holmes wrote:
>> make/defs.make:
>>
>> + 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.
>>
> I guess it's an unconscious habit from java where you rather do 
> "".equals(something) to avoid NPE. I will switch it around. And the 
> assumption is correct. We used -include at first, but I figured that 
> we wanted to know if the include failed at least on the root level 
> Makefile.
>> make/windows/makefiles/compile.make:
>>
>> 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:
>>
>> CXX=cl.exe
>> MT=mt.exe
>> RC=rc.exe
>> LD=link.exe
>>
>> together and use the same "if (,$(SPEC))" approach?
>>
> Grouping them together would certainly look nicer, but MT isn't set 
> for every possible compiler version. Not sure if that matters since we 
> don't support older versions anyway, right?
>
> As for testing for SPEC, this is nmake and the SPEC file is only 
> gnumake compatible. CXX, MT, RC and LD are sent in to nmake on the 
> command line from gnumake. They are then generated into local.make 
> which is in turn included by sub invocations of nmake. Sending in SPEC 
> as well seemed redundant to me, but we could send it in as a flag 
> signaling that configure should be in control. Wouldn't look obviously 
> better to me though. I'm open for suggestions.
>
> /Erik



More information about the build-dev mailing list