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

[Phil Race]

+1

-phil.

On 05/12/2016 11:02 AM, Tim Bell wrote:
> 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
>>>
>>>
>>
>