-
-
Notifications
You must be signed in to change notification settings - Fork 33.1k
gh-138614: site._get_path
to respect non-default implementation name
#138610
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
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
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.
- When is it an issue?
- Please open an issue first to discuss the rationale.
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
site._get_path
to respect non-default implementation namesite._get_path
to respect non-default implementation name
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.
Please add a NEWS entry, something like:
:mod:`site`: correctly synchronize the ``purelib`` installation
scheme with the one defined in :mod:`sysconfig`.
:mod:`site`: correctly synchronize the ``purelib`` installation scheme with | ||
the one defined in :mod:`sysconfig`. |
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.
After some thoughts, what I wrote is a bit too obscure. Let's mention the public API:
:mod:`site`: correctly synchronize the ``purelib`` installation scheme with | |
the one defined in :mod:`sysconfig`. | |
:mod:`site`: correctly synchronize the ``purelib`` installation scheme with | |
the one defined in :mod:`sysconfig`. This affects the default user-specific | |
site-packages directory path (see :func:`site.getusersitepackages`). |
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.
Following the maintainer's feedback, you can remove the NEWS entry.
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.
Do we have any actual user-facing downstream issues? If so, we can keep the NEWS entry (with @picnixz's proposed change), otherwise I think we should just drop it.
cc @ShaharNaveh
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.
We also need to update https://docs.python.org/3/library/site.html#site.USER_SITE:
Path to the user site-packages for the running Python. Can be None if getusersitepackages() hasn’t been called yet. Default value is ~/.local/lib/pythonX.Y[t]/site-packages for UNIX and non-framework macOS builds, ~/Library/Python/X.Y/lib/python/site-packages for macOS framework builds, and %APPDATA%\Python\PythonXY\site-packages on Windows. The optional “t” indicates the free-threaded build. This directory is a site directory, which means that .pth files in it will be processed.
It shouldn't be "Python" but whatever implementation it is being used.
Strictly speaking, We do have an issue of synchronization that I'm ok to fix, but it won't be visible to end-users so we may just forget about NEWS entries. I'll wait for @FFY00 feedback. |
Yes, it is meant to be patched by other implementations, to minimize the amount patching required in the rest of the code, and help keep everything in sync.
I don't think we need a NEWS entry. |
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.
@ShaharNaveh, thanks for working on this!
Alright, will fix the PR in a bit |
fix #138614
site.py
hardcodes "python" instead of using{implementation_lower}
#138614