Skip to content
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

Explicitly use "locale" encoding for .pth files #4265

Merged
merged 6 commits into from
Mar 8, 2024

Conversation

abravalheri
Copy link
Contributor

For the time being .pth files in UTF-8 cause problems, see python/cpython#77102.
CPython maintainers' recommendation is to use the default encoding of the underlying system.

The motivation of this PR is to help with the encoding warnings.

Summary of changes

  • Use "locale" encoding for .pth files.
  • Add TODO comments as reminders of other parts that may be moved to UTF-8, but there might be a risk.
  • Introduce the setuptools.compat.py39 module. This tries to follow the methodology in Move compatibility modules into a compat package. #4212.

Closes

Pull Request Checklist

@abravalheri abravalheri marked this pull request as ready for review March 7, 2024 11:12
@@ -576,7 +576,7 @@ def check_pth_processing(self):
_one_liner(
"""
import os
f = open({ok_file!r}, 'w')
f = open({ok_file!r}, 'w', encoding="utf-8")
Copy link
Contributor Author

@abravalheri abravalheri Mar 7, 2024

Choose a reason for hiding this comment

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

Throw-away file only used internally. It should not make a difference if we change its encoding.

@@ -491,7 +491,7 @@ def check_site_dir(self): # noqa: C901 # is too complex (12) # FIXME
try:
if test_exists:
os.unlink(testfile)
open(testfile, 'w').close()
open(testfile, 'wb').close()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

"Touch" operation... can be done in the binary mode to avoid messing with encoding.

@abravalheri abravalheri merged commit 965636e into pypa:main Mar 8, 2024
21 of 23 checks passed
@abravalheri abravalheri deleted the locale-encoding branch March 8, 2024 09:53
# ``encoding="locale"``, this can be used to suppress the warning.
# However, please try to use UTF-8 when possible
# (.pth files are the notorious exception: python/cpython#77102, pypa/setuptools#3937).
LOCALE_ENCODING = "locale" if sys.version_info >= (3, 10) else None
Copy link
Contributor

@Avasam Avasam Mar 8, 2024

Choose a reason for hiding this comment

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

@abravalheri Should this comment be the variable's docstring? So it can be picked-up by editors and the likes? For example:
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Initially I thought about doing that, but then I changed it back, because this module is not really targeting external users (it is not really part of the public API...)

jaraco added a commit that referenced this pull request Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants