Skip to content

gh-126357: Remove gettext import guards #126358

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

Merged
merged 1 commit into from
Nov 3, 2024

Conversation

tomasr8
Copy link
Member

@tomasr8 tomasr8 commented Nov 3, 2024

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @sobolevn for commit 75bae45 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Nov 3, 2024
Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this affect other implementations like PyPy?

@tomasr8
Copy link
Member Author

tomasr8 commented Nov 3, 2024

I don't know enough about PyPy to be able to answer that :/ Do you know how I could verify that?

@sobolevn
Copy link
Member

sobolevn commented Nov 3, 2024

You can take a look at the source code (https://github.com/pypy/pypy prefered way) or ping one of PyPy maintainers :)

I suspect that it won't, but just to be sure.

@tomasr8
Copy link
Member Author

tomasr8 commented Nov 3, 2024

Ok so I applied this patch to the vendored stdlib in PyPy and built pypy3.11 without any issues. Importing these modules at runtime does not cause any issues either so I think it should be fine.

The 4 buildbot failures also seem unrelated to this change.

Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM then! Thanks for checking!

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@serhiy-storchaka
Copy link
Member

I think that if there are some bootstrapping issues with the gettext module, they most likely should be solved in the gettext module. If there are some issues.

@serhiy-storchaka serhiy-storchaka merged commit ac556a2 into python:main Nov 3, 2024
108 of 116 checks passed
@tomasr8 tomasr8 deleted the gettext-import branch November 3, 2024 19:22
picnixz pushed a commit to picnixz/cpython that referenced this pull request Dec 8, 2024
ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants