-
-
Notifications
You must be signed in to change notification settings - Fork 32.8k
gh-90548: Allow Alpine/MUSL to pass test_c_locale_coercion. #134454
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Like cygwin, MUSL defaults to utf-8 if no variables are set. I have no idea if the existing tests pass on cygwin, but I made the modifications such that I shouldn't break it if is. The additional checks needed for MUSL are guarded by DEFAULT_LOCALE_IS_C being False. Based on this flag, we expect utf-8 for the encodings and no coercion message, as long as LC_ALL is not set to C. (That looks like a bit of an issue with the test structure, but I'm not going to attempt to "fix" that.) DEFAULT_ENCODING is intentionally not given a default since it is only used when DEFAULT_LOCALE_IS_C is False, and if you use the flag you'll need to set it. After reading through issue 30672, looking at the source, and running a test on Android, I *think* the current situation is that coercion will be done if the local is set to POSIX regardless of platform. However, if the platform doesn't make POSIX equivalent to C, the encodings when coercion is disabled will not be the same as for C (it is utf-8 on android, for example). This means the tests would fail if POSIX were added unconditionally to the EXPECTED_C_LOCALE_EQUIVALENTS as envisioned in the issue. This *could* be fixed with another flag, but I'm not sure it is worth the effort. I'm not even sure Python is behaving optimally in this case (assuming my analysis is correct). So I just altered the comment and add POSIX if and only if the platform is linux.
cc @ncoghlan |
!buildbot Alpine |
🤖 New build scheduled with the buildbot fleet by @zware for commit 2ad34c6 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F134454%2Fmerge The command will test the builders whose names match following regular expression: The builders matched are:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, and the Alpine buildbot worker agrees (the overall build is still a failure, but the locale tests pass).
…thonGH-134454) Like cygwin, MUSL defaults to utf-8 if no variables are set. I have no idea if the existing tests pass on cygwin, but I made the modifications such that I shouldn't break it if is. The additional checks needed for MUSL are guarded by DEFAULT_LOCALE_IS_C being False. Based on this flag, we expect utf-8 for the encodings and no coercion message, as long as LC_ALL is not set to C. (That looks like a bit of an issue with the test structure, but I'm not going to attempt to "fix" that.) DEFAULT_ENCODING is intentionally not given a default since it is only used when DEFAULT_LOCALE_IS_C is False, and if you use the flag you'll need to set it. After reading through issue 30672, looking at the source, and running a test on Android, I *think* the current situation is that coercion will be done if the local is set to POSIX regardless of platform. However, if the platform doesn't make POSIX equivalent to C, the encodings when coercion is disabled will not be the same as for C (it is utf-8 on android, for example). This means the tests would fail if POSIX were added unconditionally to the EXPECTED_C_LOCALE_EQUIVALENTS as envisioned in the issue. This *could* be fixed with another flag, but I'm not sure it is worth the effort. I'm not even sure Python is behaving optimally in this case (assuming my analysis is correct). So I just altered the comment and add POSIX if and only if the platform is linux.
Thanks @bitdancer for the PR, and @zware for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14. |
…thonGH-134454) Like cygwin, MUSL defaults to utf-8 if no variables are set. I have no idea if the existing tests pass on cygwin, but I made the modifications such that I shouldn't break it if is. The additional checks needed for MUSL are guarded by DEFAULT_LOCALE_IS_C being False. Based on this flag, we expect utf-8 for the encodings and no coercion message, as long as LC_ALL is not set to C. (That looks like a bit of an issue with the test structure, but I'm not going to attempt to "fix" that.) DEFAULT_ENCODING is intentionally not given a default since it is only used when DEFAULT_LOCALE_IS_C is False, and if you use the flag you'll need to set it. After reading through issue 30672, looking at the source, and running a test on Android, I *think* the current situation is that coercion will be done if the local is set to POSIX regardless of platform. However, if the platform doesn't make POSIX equivalent to C, the encodings when coercion is disabled will not be the same as for C (it is utf-8 on android, for example). This means the tests would fail if POSIX were added unconditionally to the EXPECTED_C_LOCALE_EQUIVALENTS as envisioned in the issue. This *could* be fixed with another flag, but I'm not sure it is worth the effort. I'm not even sure Python is behaving optimally in this case (assuming my analysis is correct). So I just altered the comment and add POSIX if and only if the platform is linux. (cherry picked from commit bcdac03) Co-authored-by: R. David Murray <[email protected]>
GH-138600 is a backport of this pull request to the 3.14 branch. |
…H-134454) (#138600) Co-authored-by: R. David Murray <[email protected]>
Like cygwin, MUSL defaults to utf-8 if no variables are set. I have no idea if the existing tests pass on cygwin, but I made the modifications such that I shouldn't break it if is. The additional checks needed for MUSL are guarded by DEFAULT_LOCALE_IS_C being False. Based on this flag, we expect utf-8 for the encodings and no coercion message, as long as LC_ALL is not set to C. (That looks like a bit of an issue with the test structure, but I'm not going to attempt to "fix" that.) DEFAULT_ENCODING is intentionally not given a default since it is only used when DEFAULT_LOCALE_IS_C is False, and if you use the flag you'll need to set it.
After reading through issue 30672, looking at the source, and running a test on Android, I think the current situation is that coercion will be done if the local is set to POSIX regardless of platform. However, if the platform doesn't make POSIX equivalent to C, the encodings when coercion is disabled will not be the same as for C (it is utf-8 on android, for example). This means the tests would fail if POSIX were added unconditionally to the EXPECTED_C_LOCALE_EQUIVALENTS as envisioned in the issue. This could be fixed with another flag, but I'm not sure it is worth the effort. I'm not even sure Python is behaving optimally in this case (assuming my analysis is correct). So I just altered the comment and add POSIX if and only if the platform is linux.