RFR: JDK-8065704 Set LC_ALL=C for all relevant commands in the build system

[Erik Joelsson]

On 2019-10-04 04:37, Magnus Ihse Bursie wrote:
> On 2019-10-02 17:40, Erik Joelsson wrote:
>> Hello Magnus,
>>
>> The change looks good, but should perhaps also include removal of the 
>> few scattered specific instances of LANG=C and LC_ALL=C in the 
>> makefiles:
>>
>> make/common/JavaCompilation.gmk:    export LC_ALL=C ; ( $(CAT) $$< && 
>> $(ECHO) "" ) \
>>
>> make/autoconf/basics.m4:  DATE_WHEN_CONFIGURED=`LANG=C date`
>>
>> Also compare.sh has a bunch which could be replaced with one export 
>> at the top.
> Ok, here's an updated version where I've deleted all other LC_* and 
> LANG assignments in the build system. FWIW, I found a few instances in 
> test code, but I did not change that.
>
> http://cr.openjdk.java.net/~ihse/JDK-8065704-LC_ALL/webrev.02
>
The removal in RunTestPrebuilt.gmk reminded me that we do not run 
configure before running tests, and in that situation spec.gmk.in is not 
used either. The same export needs to be added somewhere in the 
run-test-prebuilt chain of invocations.

>> I would expect a full round of builds-infra to be run for this, as 
>> well as explicit compare builds as subtle things could be changing.
> Yes, of course. My original patch has already made the rounds on the 
> builds-infra test suite without any issues, but I'll do a rerun with 
> this second version of the patch as well.
>
Thanks!

/Erik

> /Magnus
>>
>> /Erik
>>
>> On 2019-10-02 02:06, Magnus Ihse Bursie wrote:
>>> From the bug report:
>>> We should prefix LC_ALL=C for most, maybe all, tools we use when 
>>> building.
>>>
>>> This probably means we should run "export LC_ALL=C" early in the 
>>> configure script as well.
>>> ---
>>>
>>> The fix itself is trivial. While I know we've had several issues 
>>> regarding localization, I could not find any specific instances now 
>>> that I was looking for them. I searched JBS for a while but could 
>>> not dig up anything that was reproducible. So, unfortunately, I have 
>>> been unable to verify that this solves any actual problems. That 
>>> being said, I believe this is a prudent fix that should have been in 
>>> place long time ago. But if anyone can give me a concrete example 
>>> that breaks so that I can verify that this helps, please let me know.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8065704
>>> WebRev: http://cr.openjdk.java.net/~ihse/JDK-8065704-LC_ALL/webrev.01
>>>
>>> /Magnus
>