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

[Tim Bell]

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