Looks good.
/Erik
On 2018-04-06 21:24, Mikael Vidstedt wrote:
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/%7Emikael/webrevs/8201263/webrev.01/open/webrev/>
Cheers,
Mikael
On Apr 6, 2018, at 3:17 PM, Erik Joelsson <erik.joels...@oracle.com
<mailto: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/%7Emikael/webrevs/8201263/webrev.00/open/>
<http://cr.openjdk.java.net/~mikael/webrevs/8201263/webrev.00/open/
<http://cr.openjdk.java.net/%7Emikael/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
