RFR: JDK-8156837: RC resource compilation on windows generates false build failure reports

[Erik Joelsson]

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
>
>