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:09:32 UTC 2012


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