Review for 7141244: build-infra merge: Include $(SPEC) in makefiles and make variables overridable
David Holmes
david.holmes at oracle.com
Thu Feb 9 10:39:32 UTC 2012
Added back runtime to the cc list
On 9/02/2012 7:33 PM, 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.
That's okay - it was just an observation.
Seems okay to me. Let's see if we can get Kelly or someone else from HS
to comment.
David
-----
> /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