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