Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32k
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Open
bitdancer wants to merge1 commit intopython:mainChoose a base branch
base:main
Could not load branches
Branch not found:{{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline, and old review comments may become outdated.
+23 −10
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
Like cygwin, MUSL defaults to utf-8 if no variables are set. I have noidea if the existing tests pass on cygwin, but I made the modificationssuch that I shouldn't break it if is. The additional checks needed forMUSL are guarded by DEFAULT_LOCALE_IS_C being False. Based on thisflag, we expect utf-8 for the encodings and no coercion message, aslong as LC_ALL is not set to C. (That looks like a bit of an issue withthe test structure, but I'm not going to attempt to "fix" that.)DEFAULT_ENCODING is intentionally not given a default since it is onlyused when DEFAULT_LOCALE_IS_C is False, and if you use the flag you'llneed to set it.After reading through issue 30672, looking at the source, and running atest on Android, I *think* the current situation is that coercion willbe done if the local is set to POSIX regardless of platform. However,if the platform doesn't make POSIX equivalent to C, the encodings whencoercion is disabled will not be the same as for C (it is utf-8 onandroid, for example). This means the tests would fail if POSIX wereadded unconditionally to the EXPECTED_C_LOCALE_EQUIVALENTS as envisionedin the issue. This *could* be fixed with another flag, but I'm not sureit is worth the effort. I'm not even sure Python is behaving optimallyin this case (assuming my analysis is correct). So I just altered thecomment and add POSIX if and only if the platform is linux.
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading.Please reload this page.
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, Ithink 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. Thiscould 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.