Review for 7141244: build-infra merge: Include $(SPEC) in makefiles and make variables overridable
erik.joelsson at oracle.com
Thu Feb 9 01:09:32 PST 2012
On 2012-02-09 03:51, David Holmes wrote:
> + 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.
> 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?
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.
More information about the build-dev