Testing revealed that the fix solved the immediate problem of removing the error message, but a similar problem occurred when the resulting JTREG_BASIC_OPTIONS variable is then used. Specifically the problem is that the path returned from the shell execution (still) contains spaces, so the resulting string also needs to be quoted. This version has passed the typical CI job:
http://cr.openjdk.java.net/~mikael/webrevs/8201263/webrev.01/open/webrev/ <http://cr.openjdk.java.net/~mikael/webrevs/8201263/webrev.01/open/webrev/> Cheers, Mikael > On Apr 6, 2018, at 3:17 PM, Erik Joelsson <erik.joels...@oracle.com> wrote: > > Looks good. > > /Erik > > > On 2018-04-06 15:04, Mikael Vidstedt wrote: >> Please review this change which addresses a minor issue when running tests >> on some Windows machines. >> >> Bug: https://bugs.openjdk.java.net/browse/JDK-8201263 >> Webrev: http://cr.openjdk.java.net/~mikael/webrevs/8201263/webrev.00/open/ >> <http://cr.openjdk.java.net/~mikael/webrevs/8201263/webrev.00/open/> >> >> * Background (from the bug) >> >> When running tests using Make on a (Windows) machine where VS120COMNTOOLS is >> set an error message is generated. >> >> /bin/sh: -c: line 0: unexpected EOF while looking for matching `"' >> /bin/sh: -c: line 1: syntax error: unexpected end of file >> >> The message is generated by this logic in test/TestCommon.gmk: >> >> ifneq ($(VS120COMNTOOLS), ) >> JTREG_BASIC_OPTIONS += -e:VS120COMNTOOLS=$(shell $(GETMIXEDPATH) >> "$(VS120COMNTOOLS)") >> endif >> >> The problem is that the VS120COMNTOOLS variable typically looks something >> like: >> >> $ echo $VS120COMNTOOLS >> C:\Program Files (x86)\Microsoft Visual Studio 12.0\Common7\Tools\ >> >> Of particular interest is the fact that it has a trailing backslash. When >> used in the make file that means it will all be expanded to something like: >> >> JTREG_BASIC_OPTIONS += ... $(shell cygwin -m "c:\<path>\Common7\Tools\") >> >> When that in turn gets executed the backslash will escape the last double >> quote which means that the quotes are no longer paired. >> >> >> * Fix >> >> The fix removes the trailing backslash (if there is one). I verified the fix >> locally, and I’m running more CI testing on it now. >> >> Cheers, >> Mikael >> >
