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

bootstrap_impl=script doesn't allow overriding libs in the runtime's site-packages directory #2064

Closed
rickeylev opened this issue Jul 15, 2024 · 0 comments · Fixed by #2073
Closed
Labels
core-rules Issues concerning the stubs for native rules type: bug

Comments

@rickeylev
Copy link
Collaborator

rickeylev commented Jul 15, 2024

In #2032, it was pointed out that if you depend on a newer version of pip than what the runtime has, the newer version isn't respected.

This is because, with the bootstrap_impl=script backend, all the pypi paths are append to sys.path. This results in e.g. [stdlib, site-packages, @pypi/pip]. In the old system_python bootstrap, the ordering would be [@pypi/pip, stdlib, site-packages].

The original intent with changing the ordering as to make sure shadowing the stdlib didn't happen, which was relatively easy to do when repo's are on the path. However, a side-effect of appending is that, e.g. a newer version of pip isn't respected.

So, to fix this, we just need to put the pypi entries after the stdlib, but before site-packages.

I'm not sure if there's an API to identify which sys.path entry is the site-packages entry. Worst case, we'll just have to sniff the values and try to insert in the appropriate place.

Notes:

  • There is site-packages and dist-packages. dist-packages appears to occur as part of the distribution (e.g. part of the OS python install)
  • site.getsitepackages() appears to have the list of directories
  • I couldn't find a simple way to divine the stdlib paths. Construction of them uses various substrings and looks non-trivial; see https://docs.python.org/3/library/sys_path_init.html#sys-path-init
  • IMHO: Search forward on sys.path for {site,dist}-packages. Insert before that entry. Maybe add a sanity check to ensure it's after stdlib-looking entries
@rickeylev rickeylev added type: bug core-rules Issues concerning the stubs for native rules labels Jul 15, 2024
github-merge-queue bot pushed a commit that referenced this issue Jul 19, 2024
Previously, all the user import paths were put at the end of sys.path.
This was done so that
user import paths didn't hide stdlib modules. However, a side-effect is
that user import
paths came after the runtime's site-packages directory. This prevented
user imports from
overriding non-stdlib modules included in a runtime (e.g. pip).

To fix, we look for the runtime site-packages directory, then insert the
user import paths
before it. A test is used to ensure that the ordering is `[stdlib, user,
runtime site-packages]`

Also fixes a bug introduced by #2076: safe path was being disabled by
default

Fixes #2064
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core-rules Issues concerning the stubs for native rules type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant