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
