Erik:
Looks good to me as well.
/Tim
On 05/12/16 04:06, Erik Joelsson wrote:
Good point, the quotes are certainly not necessary. Removed and
updated review in place. Verified that it still works.
/Erik
On 2016-05-12 12:49, Vadim Pakhnushev wrote:
Erik,
I wonder why in the Launcher-java.base.gmk first -I flag parameter is
not quoted, but subsequent flags parameters are quoted?
I guess the quotes are unnecessary here...
Vadim
On 12.05.2016 13:22, Erik Joelsson wrote:
When using the RC tool on Windows, we also run CL with the same
arguments to generate dependencies on included files. In some cases,
extra include directories are provided in the RC_FLAGS, using the
flag -i (lower case). This flag works for the RC tool, but not for
CL, which requires -I (upper case). The upper case version works for
RC as well so we should switch to using upper case.
Another problem uncovered by this is that NativeCompilation.gmk
isn't handling this failure well. It calls CL through the
ExecuteWithLog macro. This macro will copy the log to the
failure-logs dir if the command returns non zero. Because of the
above problem, the command returns non zero. NativeCompilation.gmk
ignores this so the build continues. If the build then fails for any
other reason, the failure report will find these failure logs and
fool the user into thinking this was the cause of the build failure.
I propose to fix this by changing NativeCompilation.gmk to not
ignore the return code of CL and let the output of CL through, but
filtered the same way as when we do normal compilation. Then the
build will actually fail if CL fails here so there is no discrepancy
between failure-logs and the actual failure. I also removed the
piping to a new file since ExecuteWithLog already pipes output to a
file so we can just use that.
To make this work, we must also ensure that no bad arguments reach
CL here. This is done by filtering known bad arguments (currently
only -l0x409 which I chose to filter with +l%, note that this is
lower case l and not upper case i), and of course changing -i to -I
in all places.
Bug: https://bugs.openjdk.java.net/browse/JDK-8156837
Webrev: http://cr.openjdk.java.net/~erikj/8156837/webrev.01/
/Erik
