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

[Kelly O'Hair]

The only issue I see is that it's using cygpath -m -a   and not   cygpath -s -m -a
which I think means that the path could have spaces in it.

Otherwise, looks fine.

-kto

On Feb 9, 2012, at 1:33 AM, Erik Joelsson wrote:

> 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