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

[John Coomes]

Erik Joelsson (erik.joelsson at oracle.com) 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.

Looks good.  One minor request:  in linux/makefiles/gcc.make, you
moved the setting of STRIP under the SPEC conditional.  Might as well
fold it into the CROSS_COMPILE_ARCH conditional that's already there.

-John

> 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