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

use usersite for getting currently installed packages #14

Merged
merged 2 commits into from
Aug 1, 2023

Conversation

airportyh
Copy link
Collaborator

@airportyh airportyh commented Jul 31, 2023

Why?

There's a bug in our Python module setup where removing a package using poetry will remove it from pyproject.toml but not .pythonlibs. It happens because while .pythonlibs is listed as a user site package directory and is considered a standard lib directory, it is not searched as a lib directory for gathering the list of already installed packages.

There's a secondary problem: a bug in poetry where for a "GenericEnv", getting the set of paths never includes the user site package directory because the shell out command to python includes the -I which means isolate and implies
-s : don't add user site directory to sys.path; also PYTHONNOUSERSITE.

I'd like to upstream this change in the future, but that would require a larger change (passing a config parameter to some functions and adding them to various call sites.

Changes

  1. Added the user site directory to the paths searched for installed packages if POETRY_USE_USER_SITE is on
  2. Use the POETRY_USE_USER_SITE flag for the other place too, that way we can opt-in to this feature
  3. Disable the -I flag for when getting paths for the generic environment

@airportyh airportyh requested review from a team and cdmistman and removed request for a team July 31, 2023 16:05
@airportyh airportyh changed the base branch from th-use-custom-pip to replit-1.5 July 31, 2023 18:51
@cdmistman
Copy link

a bug in poetry where for a "GenericEnv", getting the set of paths never includes the user site package directory because the shell out command to python includes the -I which means isolate and implies
-s : don't add user site directory to sys.path; also PYTHONNOUSERSITE.

I'd like to upstream this change in the future, but that would require a larger change (passing a config parameter to some functions and adding them to various call sites.

Can we at least file the bug upstream and link here? maybe somebody else will pick up the issue, and you can point them to this PR for inspiration

@airportyh
Copy link
Collaborator Author

a bug in poetry where for a "GenericEnv", getting the set of paths never includes the user site package directory because the shell out command to python includes the -I which means isolate and implies
-s : don't add user site directory to sys.path; also PYTHONNOUSERSITE.
I'd like to upstream this change in the future, but that would require a larger change (passing a config parameter to some functions and adding them to various call sites.

Can we at least file the bug upstream and link here? maybe somebody else will pick up the issue, and you can point them to this PR for inspiration

It might be hard to step a reproducible test case for them since they don't have the other 2 changes in this PR. I plan to try to upstream it to their 1.6 alpha properly soon. Just want to get some prod-level testing in first.

@airportyh airportyh merged commit 8ccde84 into replit-1.5 Aug 1, 2023
15 checks passed
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