RFR: JDK-8156837: RC resource compilation on windows generates false build failure reports
erik.joelsson at oracle.com
Thu May 12 11:06:40 UTC 2016
Good point, the quotes are certainly not necessary. Removed and updated
review in place. Verified that it still works.
On 2016-05-12 12:49, Vadim Pakhnushev wrote:
> 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...
> 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
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8156837
>> Webrev: http://cr.openjdk.java.net/~erikj/8156837/webrev.01/
More information about the build-dev