GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — gh-90548: Allow Alpine/MUSL to pass test_c_locale_coercion. by bitdancer · Pull Request #134454 · python/cpython

Conversation

bitdancer commented May 21, 2025 •

edited by bedevere-app Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

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.

Issue: Multiple test failures on Alpine 3.15 / musl-1.2.2-r7 #90548

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Allow Alpine/MUSL to pass test_c_locale_coercion.

2ad34c6

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.

bedevere-app Bot added the tests Tests in the Lib/test dir label

May 21, 2025

bedevere-app Bot mentioned this pull request

May 21, 2025

Multiple test failures on Alpine 3.15 / musl-1.2.2-r7 #90548

Closed

bedevere-app Bot added the awaiting core review label

May 21, 2025

bitdancer requested a review from ncoghlan

May 21, 2025 17:44

vstinner added the skip news label

May 22, 2025

vstinner commented May 22, 2025

Copy link

Copy Markdown

Member

cc @ncoghlan

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

zware commented Sep 2, 2025

Copy link

Copy Markdown

Member

!buildbot Alpine

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bedevere-bot commented Sep 2, 2025

Copy link

Copy Markdown

🤖 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: Alpine

The builders matched are:

AMD64 Alpine Linux PR

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

zware approved these changes

Sep 2, 2025

View reviewed changes

zware left a comment

Copy link

Copy Markdown

Member

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).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

bedevere-app Bot added awaiting merge and removed awaiting core review labels

Sep 2, 2025

zware merged commit bcdac03 into python:main

Sep 6, 2025

bedevere-app Bot removed the awaiting merge label

Sep 6, 2025

zware pushed a commit to zware/cpython that referenced this pull request

Sep 6, 2025


          pythongh-90548: Allow Alpine/MUSL to pass test_c_locale_coercion. (py…

63491de

…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.

zware added the needs backport to 3.14 bugs and security fixes label

Sep 6, 2025

miss-islington-app Bot commented Sep 6, 2025

Copy link

Copy Markdown

Thanks @bitdancer for the PR, and @zware for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request

Sep 6, 2025


          pythongh-90548: Allow Alpine/MUSL to pass test_c_locale_coercion. (py…

2413bdf

…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]>

bedevere-app Bot commented Sep 6, 2025

Copy link

Copy Markdown

GH-138600 is a backport of this pull request to the 3.14 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bedevere-app Bot removed the needs backport to 3.14 bugs and security fixes label

Sep 6, 2025

hugovk pushed a commit that referenced this pull request

Sep 7, 2025


          [3.14] gh-90548: Allow Alpine/MUSL to pass test_c_locale_coercion. (G…

096fa04

…H-134454) (#138600)

Co-authored-by: R. David Murray <[email protected]>

kumaraditya303 pushed a commit to miss-islington/cpython that referenced this pull request

Sep 9, 2025


          [3.14] pythongh-90548: Allow Alpine/MUSL to pass test_c_locale_coerci…

3f9a6e9

…on. (pythonGH-134454) (python#138600)

Co-authored-by: R. David Murray <[email protected]>

lkollar pushed a commit to lkollar/cpython that referenced this pull request

Sep 9, 2025


          pythongh-90548: Allow Alpine/MUSL to pass test_c_locale_coercion. (py…

a3ce193

…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.

Reviewers

zware

zware approved these changes

ncoghlan

Awaiting requested review from ncoghlan

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news tests

Tests in the Lib/test dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

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.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

bitdancer commented May 21, 2025 • edited by bedevere-app Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vstinner commented May 22, 2025

Uh oh!

zware commented Sep 2, 2025

Uh oh!

bedevere-bot commented Sep 2, 2025

Uh oh!

zware left a comment

Choose a reason for hiding this comment

Uh oh!

miss-islington-app Bot commented Sep 6, 2025

Uh oh!

bedevere-app Bot commented Sep 6, 2025

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bitdancer commented May 21, 2025 •

edited by bedevere-app Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.