RFR: 8263028: Windows build fails due to several treat-warning-as-errors

classic Classic list List threaded Threaded
12 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: 8263028: Windows build fails due to several treat-warning-as-errors

Yi Yang
cl.exe(19.28.29334) can not build JDK on windows_x64 because it treats many warnings as errors thus prohibiting further compilation. (See detailed failure logs on JBS)

1. methodMatcher.cpp

cl.exe can not handle advanced usage of sscanf(i.e. regex-like sscanf) correctly. This looks like an msvc compiler bug, it has been there for a long while, so I temporarily disable these warnings in a limited region. Outside of this region, the compiler still treats them as errors. This change does not affect the functionality of MethodMatcher::parse_method_pattern, it can parse class name and method name in a desired manner.

2. vm_version_x86.cpp

Some comments contain characters(Register Trademark) that cannot be represented in the current code page (936). Replacing them with ASCII-characters makes the compiler happy.

Test manually!

Best Regards,
Yang

-------------

Commit messages:
 - fix build failure on windows

Changes: https://git.openjdk.java.net/jdk/pull/3107/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3107&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8263028
  Stats: 41 lines in 2 files changed: 5 ins; 0 del; 36 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3107.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3107/head:pull/3107

PR: https://git.openjdk.java.net/jdk/pull/3107
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8263028: Windows build fails due to several treat-warning-as-errors

Ioi Lam-2
On Sun, 21 Mar 2021 04:22:42 GMT, Yi Yang <[hidden email]> wrote:

> cl.exe(19.28.29334) can not build JDK on windows_x64 because it treats many warnings as errors thus prohibiting further compilation. (See detailed failure logs on JBS)
>
> 1. methodMatcher.cpp
>
> cl.exe can not handle advanced usage of sscanf(i.e. regex-like sscanf) correctly. This looks like an msvc compiler bug, it has been there for a long while, so I temporarily disable these warnings in a limited region. Outside of this region, the compiler still treats them as errors. This change does not affect the functionality of MethodMatcher::parse_method_pattern, it can parse class name and method name in a desired manner.
>
> 2. vm_version_x86.cpp
>
> Some comments contain characters(Register Trademark) that cannot be represented in the current code page (936). Replacing them with ASCII-characters makes the compiler happy.
>
> Best Regards,
> Yang

The problem in methodMatcher.cpp is caused by this:

#define RANGEBASE "\x1\x2\x3\x4\x5\x6\x7\x8\xa\xb\xc\xd\xe\xf" \
    "\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f" \
    "\x21\x22\x23\x24\x25\x26\x27\x2a\x2b\x2c\x2d"  .....

where the literal `\x25` is the `%` character. It seems like VC++ tries to interpret the `%` character even when it's inside square brackets, like

      if (1 == sscanf(line, "%1022[[);/" RANGEBASE "]%n", sig+1, &bytes_read)) {
->
      if (1 == sscanf(line, "%1022[[);.....#$%&.....]%n", sig+1, &bytes_read)) {
                                             ^

The [C++ reference](https://en.cppreference.com/w/c/io/fscanf) is unclear about how characters like `%` can be escaped inside square brackets (or whether they should be escaped at all).

Trying to use sscanf for this purpose makes the code hard to understand and non portable. It's better to ditch sscanf and read the characters byte-by-byte. That way, you can get rid of the original `PRAGMA_DISABLE_MSVC_WARNING(4819)` as well.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3107
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8263028: Windows build fails due to several treat-warning-as-errors

Yi Yang
On Mon, 22 Mar 2021 21:10:33 GMT, Ioi Lam <[hidden email]> wrote:

> The problem in methodMatcher.cpp is caused by this:
>
> ```
> #define RANGEBASE "\x1\x2\x3\x4\x5\x6\x7\x8\xa\xb\xc\xd\xe\xf" \
>     "\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f" \
>     "\x21\x22\x23\x24\x25\x26\x27\x2a\x2b\x2c\x2d"  .....
> ```
>
> where the literal `\x25` is the `%` character. It seems like VC++ tries to interpret the `%` character even when it's inside square brackets, like
>
> ```
>       if (1 == sscanf(line, "%1022[[);/" RANGEBASE "]%n", sig+1, &bytes_read)) {
> ->
>       if (1 == sscanf(line, "%1022[[);.....#$%&.....]%n", sig+1, &bytes_read)) {
>                                              ^
> ```
>
> The [C++ reference](https://en.cppreference.com/w/c/io/fscanf) is unclear about how characters like `%` can be escaped inside square brackets (or whether they should be escaped at all).
>
> Trying to use sscanf for this purpose makes the code hard to understand and non portable. It's better to ditch sscanf and read the characters byte-by-byte. That way, you can get rid of the original `PRAGMA_DISABLE_MSVC_WARNING(4819)` as well.

The explanation makes sense. We can parse class name and method name via byte-by-byte stream instead of advanced regex-like sscanf. For this reason, I also put a FIXME comment above MethodMatcher::parse_method_pattern.

This build failure also appears in the [downstream JDK](https://github.com/alibaba/dragonwell11/issues/70), blocking further development. So the purpose of this PR is to address these treat-warning-as-error problems. I'd like to rewrite this function in another PR.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3107
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8263028: Windows build fails due to several treat-warning-as-errors

Ioi Lam-2
On Tue, 23 Mar 2021 02:22:00 GMT, Yi Yang <[hidden email]> wrote:

> > The problem in methodMatcher.cpp is caused by this:
> > ```
> > #define RANGEBASE "\x1\x2\x3\x4\x5\x6\x7\x8\xa\xb\xc\xd\xe\xf" \
> >     "\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f" \
> >     "\x21\x22\x23\x24\x25\x26\x27\x2a\x2b\x2c\x2d"  .....
> > ```
> >
> >
> > where the literal `\x25` is the `%` character. It seems like VC++ tries to interpret the `%` character even when it's inside square brackets, like
> > ```
> >       if (1 == sscanf(line, "%1022[[);/" RANGEBASE "]%n", sig+1, &bytes_read)) {
> > ->
> >       if (1 == sscanf(line, "%1022[[);.....#$%&.....]%n", sig+1, &bytes_read)) {
> >                                              ^
> > ```
> >
> >
> > The [C++ reference](https://en.cppreference.com/w/c/io/fscanf) is unclear about how characters like `%` can be escaped inside square brackets (or whether they should be escaped at all).
> > Trying to use sscanf for this purpose makes the code hard to understand and non portable. It's better to ditch sscanf and read the characters byte-by-byte. That way, you can get rid of the original `PRAGMA_DISABLE_MSVC_WARNING(4819)` as well.
>
> The explanation makes sense. We can parse class name and method name via byte-by-byte stream instead of advanced regex-like sscanf. For this reason, I also put a FIXME comment above MethodMatcher::parse_method_pattern.
>
> This build failure also appears in the [downstream JDK](https://github.com/alibaba/dragonwell11/issues/70), blocking further development. So the purpose of this PR is to address these treat-warning-as-error problems. I'd like to rewrite this function in another PR.

I think we should add `#pragmas` one only as a last resort. We need to understand why the problem is happening.

This code has been there for a long time. I wonder what happened to cause it to fail in your build system. Could it be related to a particular version of VC++? I checked the build logs from Oracle's CI as well as GitHub actions

19.27.29111 Oracle  -- OK
19.28.29334 Alibaba -- warnings
19.28.29910 GitHub  -- OK

Or, is it related to the system language setting of your build machine (e.g., could it be set to Chinese?)

Most importantly, does the `#pragma` actually make the code work, or does it merely hides the problem? I.e., will sscanf fail at runtime when it sees the `%&`? Have you run any tests to validate that the affected code works with your toolchain?

-------------

PR: https://git.openjdk.java.net/jdk/pull/3107
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8263028: Windows build fails due to several treat-warning-as-errors

Yi Yang
On Tue, 23 Mar 2021 03:06:42 GMT, Ioi Lam <[hidden email]> wrote:

>>> The problem in methodMatcher.cpp is caused by this:
>>>
>>> ```
>>> #define RANGEBASE "\x1\x2\x3\x4\x5\x6\x7\x8\xa\xb\xc\xd\xe\xf" \
>>>     "\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f" \
>>>     "\x21\x22\x23\x24\x25\x26\x27\x2a\x2b\x2c\x2d"  .....
>>> ```
>>>
>>> where the literal `\x25` is the `%` character. It seems like VC++ tries to interpret the `%` character even when it's inside square brackets, like
>>>
>>> ```
>>>       if (1 == sscanf(line, "%1022[[);/" RANGEBASE "]%n", sig+1, &bytes_read)) {
>>> ->
>>>       if (1 == sscanf(line, "%1022[[);.....#$%&.....]%n", sig+1, &bytes_read)) {
>>>                                              ^
>>> ```
>>>
>>> The [C++ reference](https://en.cppreference.com/w/c/io/fscanf) is unclear about how characters like `%` can be escaped inside square brackets (or whether they should be escaped at all).
>>>
>>> Trying to use sscanf for this purpose makes the code hard to understand and non portable. It's better to ditch sscanf and read the characters byte-by-byte. That way, you can get rid of the original `PRAGMA_DISABLE_MSVC_WARNING(4819)` as well.
>>
>> The explanation makes sense. We can parse class name and method name via byte-by-byte stream instead of advanced regex-like sscanf. For this reason, I also put a FIXME comment above MethodMatcher::parse_method_pattern.
>>
>> This build failure also appears in the [downstream JDK](https://github.com/alibaba/dragonwell11/issues/70), blocking further development. So the purpose of this PR is to address these treat-warning-as-error problems. I'd like to rewrite this function in another PR.
>
>> > The problem in methodMatcher.cpp is caused by this:
>> > ```
>> > #define RANGEBASE "\x1\x2\x3\x4\x5\x6\x7\x8\xa\xb\xc\xd\xe\xf" \
>> >     "\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f" \
>> >     "\x21\x22\x23\x24\x25\x26\x27\x2a\x2b\x2c\x2d"  .....
>> > ```
>> >
>> >
>> > where the literal `\x25` is the `%` character. It seems like VC++ tries to interpret the `%` character even when it's inside square brackets, like
>> > ```
>> >       if (1 == sscanf(line, "%1022[[);/" RANGEBASE "]%n", sig+1, &bytes_read)) {
>> > ->
>> >       if (1 == sscanf(line, "%1022[[);.....#$%&.....]%n", sig+1, &bytes_read)) {
>> >                                              ^
>> > ```
>> >
>> >
>> > The [C++ reference](https://en.cppreference.com/w/c/io/fscanf) is unclear about how characters like `%` can be escaped inside square brackets (or whether they should be escaped at all).
>> > Trying to use sscanf for this purpose makes the code hard to understand and non portable. It's better to ditch sscanf and read the characters byte-by-byte. That way, you can get rid of the original `PRAGMA_DISABLE_MSVC_WARNING(4819)` as well.
>>
>> The explanation makes sense. We can parse class name and method name via byte-by-byte stream instead of advanced regex-like sscanf. For this reason, I also put a FIXME comment above MethodMatcher::parse_method_pattern.
>>
>> This build failure also appears in the [downstream JDK](https://github.com/alibaba/dragonwell11/issues/70), blocking further development. So the purpose of this PR is to address these treat-warning-as-error problems. I'd like to rewrite this function in another PR.
>
> I think we should add `#pragmas` one only as a last resort. We need to understand why the problem is happening.
>
> This code has been there for a long time. I wonder what happened to cause it to fail in your build system. Could it be related to a particular version of VC++? I checked the build logs from Oracle's CI as well as GitHub actions
>
> 19.27.29111 Oracle  -- OK
> 19.28.29334 Alibaba -- warnings
> 19.28.29910 GitHub  -- OK
>
> Or, is it related to the system language setting of your build machine (e.g., could it be set to Chinese?)
>
> Most importantly, does the `#pragma` actually make the code work, or does it merely hides the problem? I.e., will sscanf fail at runtime when it sees the `%&`? Have you run any tests to validate that the affected code works with your toolchain?

I have confirmed that this problem is related to a specific msvc version(At least it happens in msvc 19.28.29334, I have not tested other msvc versions). After applying this patch, I can build successfully, both on upstream JDK and Alibaba JDK.

I have written a minimal reproducible demo. When the /WX(Treats all compiler warnings as errors) option is turned on, the compiler issued the same warnings. After disabling these warnings via `#pragma`, the compiler will not complain about anything, the execution result is exactly as I expected. Besides, all test cases under `compiler/compilercontrol/` are passed except ClearDirectivesFileStackTest.java which has been problem-listed before. So I believe this is a false positive of the compilation warning, turning it off or on will not affect the runtime behavior.

FYI: See detailed jtreg log and minimal reproducible demo on JBS attachments.

Thanks,
Yang

-------------

PR: https://git.openjdk.java.net/jdk/pull/3107
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8263028: Windows build fails due to several treat-warning-as-errors

Jorn Vernee-2
On Tue, 23 Mar 2021 04:38:59 GMT, Yi Yang <[hidden email]> wrote:

>>> > The problem in methodMatcher.cpp is caused by this:
>>> > ```
>>> > #define RANGEBASE "\x1\x2\x3\x4\x5\x6\x7\x8\xa\xb\xc\xd\xe\xf" \
>>> >     "\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f" \
>>> >     "\x21\x22\x23\x24\x25\x26\x27\x2a\x2b\x2c\x2d"  .....
>>> > ```
>>> >
>>> >
>>> > where the literal `\x25` is the `%` character. It seems like VC++ tries to interpret the `%` character even when it's inside square brackets, like
>>> > ```
>>> >       if (1 == sscanf(line, "%1022[[);/" RANGEBASE "]%n", sig+1, &bytes_read)) {
>>> > ->
>>> >       if (1 == sscanf(line, "%1022[[);.....#$%&.....]%n", sig+1, &bytes_read)) {
>>> >                                              ^
>>> > ```
>>> >
>>> >
>>> > The [C++ reference](https://en.cppreference.com/w/c/io/fscanf) is unclear about how characters like `%` can be escaped inside square brackets (or whether they should be escaped at all).
>>> > Trying to use sscanf for this purpose makes the code hard to understand and non portable. It's better to ditch sscanf and read the characters byte-by-byte. That way, you can get rid of the original `PRAGMA_DISABLE_MSVC_WARNING(4819)` as well.
>>>
>>> The explanation makes sense. We can parse class name and method name via byte-by-byte stream instead of advanced regex-like sscanf. For this reason, I also put a FIXME comment above MethodMatcher::parse_method_pattern.
>>>
>>> This build failure also appears in the [downstream JDK](https://github.com/alibaba/dragonwell11/issues/70), blocking further development. So the purpose of this PR is to address these treat-warning-as-error problems. I'd like to rewrite this function in another PR.
>>
>> I think we should add `#pragmas` one only as a last resort. We need to understand why the problem is happening.
>>
>> This code has been there for a long time. I wonder what happened to cause it to fail in your build system. Could it be related to a particular version of VC++? I checked the build logs from Oracle's CI as well as GitHub actions
>>
>> 19.27.29111 Oracle  -- OK
>> 19.28.29334 Alibaba -- warnings
>> 19.28.29910 GitHub  -- OK
>>
>> Or, is it related to the system language setting of your build machine (e.g., could it be set to Chinese?)
>>
>> Most importantly, does the `#pragma` actually make the code work, or does it merely hides the problem? I.e., will sscanf fail at runtime when it sees the `%&`? Have you run any tests to validate that the affected code works with your toolchain?
>
> I have confirmed that this problem is related to a specific msvc version(At least it happens in msvc 19.28.29334, I have not tested other msvc versions). After applying this patch, I can build successfully, both on upstream JDK and Alibaba JDK.
>
> I have written a minimal reproducible demo. When the /WX(Treats all compiler warnings as errors) option is turned on, the compiler issued the same warnings. After disabling these warnings via `#pragma`, the compiler will not complain about anything, the execution result is exactly as I expected. Besides, all test cases under `compiler/compilercontrol/` are passed except ClearDirectivesFileStackTest.java which has been problem-listed before. So I believe this is a false positive of the compilation warning, turning it off or on will not affect the runtime behavior.
>
> FYI: See detailed jtreg log and minimal reproducible demo on JBS attachments.
>
> Thanks,
> Yang

I happened to have the exact same version of cl.exe (19.28.29334) but have not been seeing this issue, and I can't reproduce it with the reproducer you attached to the JBS issue either.

I do note that the reproducer has a bunch of `LS` characters where `LF` are expected, and Visual Studio asks if I want to normalize them, but, whether I do that or not doesn't change the outcome. I just don't get the same warnings.

So, it seems like there might be an environment/configuration problem somewhere, and these warnings are just a symptom of that. It might be worth it to investigate further.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3107
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8263028: Windows build fails due to several treat-warning-as-errors

Jorn Vernee-2
On Tue, 23 Mar 2021 13:04:26 GMT, Jorn Vernee <[hidden email]> wrote:

>> I have confirmed that this problem is related to a specific msvc version(At least it happens in msvc 19.28.29334, I have not tested other msvc versions). After applying this patch, I can build successfully, both on upstream JDK and Alibaba JDK.
>>
>> I have written a minimal reproducible demo. When the /WX(Treats all compiler warnings as errors) option is turned on, the compiler issued the same warnings. After disabling these warnings via `#pragma`, the compiler will not complain about anything, the execution result is exactly as I expected. Besides, all test cases under `compiler/compilercontrol/` are passed except ClearDirectivesFileStackTest.java which has been problem-listed before. So I believe this is a false positive of the compilation warning, turning it off or on will not affect the runtime behavior.
>>
>> FYI: See detailed jtreg log and minimal reproducible demo on JBS attachments.
>>
>> Thanks,
>> Yang
>
> I happened to have the exact same version of cl.exe (19.28.29334) but have not been seeing this issue, and I can't reproduce it with the reproducer you attached to the JBS issue either.
>
> I do note that the reproducer has a bunch of `LS` characters where `LF` are expected, and Visual Studio asks if I want to normalize them, but, whether I do that or not doesn't change the outcome. I just don't get the same warnings.
>
> So, it seems like there might be an environment/configuration problem somewhere, and these warnings are just a symptom of that. It might be worth it to investigate further.

Based on Ioi's suggestion I decided to try with a different locale as well. I tried setting my system locale to `Chinese (Simplified, China)` and with that I was able to reproduce the warnings you report, so it indeed seems to be an issue with locale settings. AFAIK only `en-us` is supported.

I've had problems in the past as well because I had the wrong locale set, and some of the tests were failing because of that. So, maybe rather than disabling the warnings, it might be more prudent to change the system locale of the used build systems to prevent similar issues in the future (FWIW, the display language doesn't seem to affect `cl` so that could still be whatever is convenient).

-------------

PR: https://git.openjdk.java.net/jdk/pull/3107
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8263028: Windows build fails due to several treat-warning-as-errors

Yi Yang
On Tue, 23 Mar 2021 13:47:39 GMT, Jorn Vernee <[hidden email]> wrote:

> Based on Ioi's suggestion I decided to try with a different locale as well. I tried setting my system locale to something else and with that I was able to reproduce the warnings you report, so it _could_ be an issue with locale settings. AFAIK only `en-us` is supported. Maybe you could confirm/check your locale settings as well? (can run `systeminfo` to get the current setting)
>
> I've had problems in the past as well because I had the wrong locale set, and some of the tests were failing because of that. So, maybe rather than disabling the warnings, it might be more prudent to change the system locale of the used build systems to prevent similar issues in the future (FWIW, the display language doesn't seem to affect `cl` so that could still be whatever is convenient).

Hi Jorn,

Sorry for the delayed response. I set the locale of my Cygwin environment to en-us via  `export LC_ALL="en_US.UTF-8"`, these warnings are generated when compiling as well as before. Should I change this setting globally instead of just changing it in Cygwin?

Anyway, it seems that this problem is caused by the locale setting because as you mentioned, this problem appears when you change the locale setting to Chinese. Setting the locale to English does not have this problem. I checked the building document, but there is no mention of the need to set the locale option to en-us before building JDK. If this is really a necessary step for building, I think we should add this step in the building document, otherwise, I think we should fix this problem in HotSpot.

Best Regards
Yang

-------------

PR: https://git.openjdk.java.net/jdk/pull/3107
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8263028: Windows build fails due to several treat-warning-as-errors

Magnus Ihse Bursie-3
On Sun, 28 Mar 2021 04:40:46 GMT, Yi Yang <[hidden email]> wrote:

>> Based on Ioi's suggestion I decided to try with a different locale as well. I tried setting my system locale to something else and with that I was able to reproduce the warnings you report, so it _could_ be an issue with locale settings. AFAIK only `en-us` is supported. Maybe you could confirm/check your locale settings as well? (can run `systeminfo` to get the current setting)
>>
>> I've had problems in the past as well because I had the wrong locale set, and some of the tests were failing because of that. So, maybe rather than disabling the warnings, it might be more prudent to change the system locale of the used build systems to prevent similar issues in the future (FWIW, the display language doesn't seem to affect `cl` so that could still be whatever is convenient).
>
>> Based on Ioi's suggestion I decided to try with a different locale as well. I tried setting my system locale to something else and with that I was able to reproduce the warnings you report, so it _could_ be an issue with locale settings. AFAIK only `en-us` is supported. Maybe you could confirm/check your locale settings as well? (can run `systeminfo` to get the current setting)
>>
>> I've had problems in the past as well because I had the wrong locale set, and some of the tests were failing because of that. So, maybe rather than disabling the warnings, it might be more prudent to change the system locale of the used build systems to prevent similar issues in the future (FWIW, the display language doesn't seem to affect `cl` so that could still be whatever is convenient).
>
> Hi Jorn,
>
> Sorry for the delayed response. I set the locale of my Cygwin environment to en-us via  `export LC_ALL="en_US.UTF-8"`, these warnings are generated when compiling as well as before. Should I change this setting globally instead of just changing it in Cygwin?
>
> Anyway, it seems that this problem is caused by the locale setting because as you mentioned, this problem appears when you change the locale setting to Chinese. Setting the locale to English does not have this problem. I checked the building document, but there is no mention of the need to set the locale option to en-us before building JDK. If this is really a necessary step for building, I think we should add this step in the building document, otherwise, I think we should fix this problem in HotSpot.
>
> Best Regards
> Yang

@kelthuzadx Hi Yang,

Setting locale to US English used to be documented as a build requirement. When the "new" build-infra system was introduced several years ago, we thought that all locale-dependent issues were solved, and removed that requirement. Later on, issues crept in on non-Windows platforms, but these were handled by setting LC_ALL=C in the build system itself while building.

The problem with requiring US English as locale on Windows is that you cannot set that for a single process, but must change the entire system locale for the user (which also often requires a reboot). Otoh, if we do *not* require US English, the test matrix grows almost without bounds, and we might run into a lot of weird problems (like this one!).

So I'm not really comfortable with just patching around this issue, since:
a) it does not occur in what is at least the "recommended" locale, and
b) more issues are likely to creep up in the future (in fact, there might already be testing issues as Jorn says)

On the other hand, I am not really comfortable either with just stating in the build document that US English is the only supported Windows locale, since it has such far-reaching consequences for the individual developers.

In short, I'm torn between two bad solutions, but I'm definitely leaning towards the latter. If only there were some way of setting the locale just for cl.exe! :-(

-------------

PR: https://git.openjdk.java.net/jdk/pull/3107
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8263028: Windows build fails due to several treat-warning-as-errors

Yi Yang
On Mon, 29 Mar 2021 09:56:09 GMT, Magnus Ihse Bursie <[hidden email]> wrote:

>> @kelthuzadx Hi Yang,
>>
>> Setting locale to US English used to be documented as a build requirement. When the "new" build-infra system was introduced several years ago, we thought that all locale-dependent issues were solved, and removed that requirement. Later on, issues crept in on non-Windows platforms, but these were handled by setting LC_ALL=C in the build system itself while building.
>>
>> The problem with requiring US English as locale on Windows is that you cannot set that for a single process, but must change the entire system locale for the user (which also often requires a reboot). Otoh, if we do *not* require US English, the test matrix grows almost without bounds, and we might run into a lot of weird problems (like this one!).
>>
>> So I'm not really comfortable with just patching around this issue, since:
>> a) it does not occur in what is at least the "recommended" locale, and
>> b) more issues are likely to creep up in the future (in fact, there might already be testing issues as Jorn says)
>>
>> On the other hand, I am not really comfortable either with just stating in the build document that US English is the only supported Windows locale, since it has such far-reaching consequences for the individual developers.
>>
>> In short, I'm torn between two bad solutions, but I'm definitely leaning towards the latter. If only there were some way of setting the locale just for cl.exe! :-(
>
> I searched the net once more for setting the locale, and this time I found some creative workarounds on superuser. The suggestion is to create a *secondary* user account, and set US English as locale for that account. Then you can go back to your main account, and us the "Run as..." functionality to execute an arbitrary command as that user.
>
> This could be done by: `%comspec% runas /profile /user:yourotheruser "the_application_you_want_ to_run_in_english"` or using the GUI (shift+right click on the icon, select `Run as different user`).
>
> I assume you would be able to start a cygwin shell like this, and have all processes started in that shell belonging to this US English user.
>
> @kelthuzadx Can you please verify if this method works? If so, I believe it is convenient enough for us to be able to require US English locale for building on Windows.

Hi Magnus,

> I searched the net once more for setting the locale, and this time I found some creative workarounds on superuser. The suggestion is to create a secondary user account, and set US English as locale for that account. Then you can go back to your main account, and us the "Run as..." functionality to execute an arbitrary command as that user.

> This could be done by: %comspec% runas /profile /user:yourotheruser "the_application_you_want_ to_run_in_english" or using the GUI (shift+right click on the icon, select Run as different user).

Thanks for your investigations and kind suggestions. It is more troublesome to add new a user to the Chinese system and set its system locale to English. Instead of doing this, I prefer to directly change the system locale to English.

When I set the system locale to English(`Control Panel->Change date, time,...->Administrative->Change System locale->English`), and it indeed works for building! No warnings were generated. All works fine.

> a) it does not occur in what is at least the "recommended" locale, and

> b) more issues are likely to creep up in the future (in fact, there might already be testing issues as Jorn says)

> On the other hand, I am not really comfortable either with just stating in the build document that US English is the only supported Windows locale, since it has such far-reaching consequences for the individual developers.

You convinced me, I agree with you that stating these has far-reaching consequences and your internal test matrix will become incredibly heavy. However, I think we can add a section in the FAQ or other places in the building document to give a solution for such problems as much as possible, e.g.

Q: Why I can not build JDK on a non-English system? What should I do next?
A: Maybe you can change your system locale to English and try again

Just IMHO, :-)

Best Regards,
Yang

-------------

PR: https://git.openjdk.java.net/jdk/pull/3107
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8263028: Windows build fails due to several treat-warning-as-errors

Magnus Ihse Bursie


On 2021-03-29 16:44, Yi Yang wrote:
> On Mon, 29 Mar 2021 09:56:09 GMT, Magnus Ihse Bursie <[hidden email]> wrote:
>
>> This could be done by: %comspec% runas /profile /user:yourotheruser "the_application_you_want_ to_run_in_english" or using the GUI (shift+right click on the icon, select Run as different user).
> Thanks for your investigations and kind suggestions. It is more troublesome to add new a user to the Chinese system and set its system locale to English. Instead of doing this, I prefer to directly change the system locale to English.
I understand that you find the easier to just change your local
settings. Could I nevertheless trouble you to try out the method
described above, i.e. to create a English locale user, and run the build
from a shell started by

%comspec% runas /profile /user:yourotheruser c:\cygwin64\bin\bash.exe

? If that works, I'd like to document it in the build manual as an
alternative for those who do not want to change the locale of their main
user account.

> You convinced me, I agree with you that stating these has far-reaching consequences and your internal test matrix will become incredibly heavy. However, I think we can add a section in the FAQ or other places in the building document to give a solution for such problems as much as possible, e.g.

I have opened https://bugs.openjdk.java.net/browse/JDK-8264425 about
updating the build documentation. I suggest that this PR/issue can now
be closed.

/Magnus
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8263028: Windows build fails due to several treat-warning-as-errors

Yi Yang
In reply to this post by Yi Yang
On Mon, 29 Mar 2021 14:42:00 GMT, Yi Yang <[hidden email]> wrote:

>> I searched the net once more for setting the locale, and this time I found some creative workarounds on superuser. The suggestion is to create a *secondary* user account, and set US English as locale for that account. Then you can go back to your main account, and us the "Run as..." functionality to execute an arbitrary command as that user.
>>
>> This could be done by: `%comspec% runas /profile /user:yourotheruser "the_application_you_want_ to_run_in_english"` or using the GUI (shift+right click on the icon, select `Run as different user`).
>>
>> I assume you would be able to start a cygwin shell like this, and have all processes started in that shell belonging to this US English user.
>>
>> @kelthuzadx Can you please verify if this method works? If so, I believe it is convenient enough for us to be able to require US English locale for building on Windows.
>
> Hi Magnus,
>
>> I searched the net once more for setting the locale, and this time I found some creative workarounds on superuser. The suggestion is to create a secondary user account, and set US English as locale for that account. Then you can go back to your main account, and us the "Run as..." functionality to execute an arbitrary command as that user.
>
>> This could be done by: %comspec% runas /profile /user:yourotheruser "the_application_you_want_ to_run_in_english" or using the GUI (shift+right click on the icon, select Run as different user).
>
> Thanks for your investigations and kind suggestions. It is more troublesome to add new a user to the Chinese system and set its system locale to English. Instead of doing this, I prefer to directly change the system locale to English.
>
> When I set the system locale to English(`Control Panel->Change date, time,...->Administrative->Change System locale->English`), and it indeed works for building! No warnings were generated. All works fine.
>
>> a) it does not occur in what is at least the "recommended" locale, and
>
>> b) more issues are likely to creep up in the future (in fact, there might already be testing issues as Jorn says)
>
>> On the other hand, I am not really comfortable either with just stating in the build document that US English is the only supported Windows locale, since it has such far-reaching consequences for the individual developers.
>
> You convinced me, I agree with you that stating these has far-reaching consequences and your internal test matrix will become incredibly heavy. However, I think we can add a section in the FAQ or other places in the building document to give a solution for such problems as much as possible, e.g.
>
> Q: Why I can not build JDK on a non-English system? What should I do next?
> A: Maybe you can change your system locale to English and try again
>
> Just IMHO, :-)
>
> Best Regards,
> Yang

In order to avoid disturbing others, I will comment on the https://bugs.openjdk.java.net/browse/JDK-8264425

-------------

PR: https://git.openjdk.java.net/jdk/pull/3107