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

Consider system packages as installed if the venv includes them #8359

Merged
merged 6 commits into from
Aug 28, 2023

Conversation

abompard
Copy link
Contributor

@abompard abompard commented Aug 24, 2023

This changeset adds a getter to env.virtual_env.VirtualEnv to check whether it has access to system packages, and overrides the is_path_relative_to_lib method to take it into account.

This will prevent Poetry from reinstalling system packages in the venv when they are already installed with a compatible version.

Resolves: #6035

  • Added tests for changed code.
  • Updated documentation for changed code. (N/A)

This changesets adds a getter to `env.virtual_env.VirtualEnv` to check
whether it has access to system packages, and overrides the
`is_path_relative_to_lib` method to take it into account.

This will prevent Poetry from reinstalling system packages in the venv
when they are already installed with a compatible version.

Fixes: python-poetry#6035

Signed-off-by: Aurélien Bompard <[email protected]>
@abompard abompard force-pushed the fix-6035 branch 5 times, most recently from 46b1fc8 to baf4106 Compare August 24, 2023 14:33
Copy link
Member

@radoering radoering left a comment

Choose a reason for hiding this comment

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

IMO, it would be nice to have a test in test_installed_repository.py that checks that a package from system site packages does not have source_type == "directory".

src/poetry/utils/env/virtual_env.py Outdated Show resolved Hide resolved
src/poetry/utils/env/virtual_env.py Outdated Show resolved Hide resolved
@abompard
Copy link
Contributor Author

IMO, it would be nice to have a test in test_installed_repository.py that checks that a package from system site packages does not have source_type == "directory".

I thought that as well, but in that file the virtual environment is entirely mocked, so I thought it wouldn't be terribly useful as I didn't change any of the calls in repository. What do you think?

@radoering
Copy link
Member

I think it makes sense if we set up the test a little differently. Please take a look at the test I just added.

@abompard
Copy link
Contributor Author

I think it makes sense if we set up the test a little differently. Please take a look at the test I just added.

Oh yeah that's a good idea! Thanks :-)

src/poetry/utils/env/virtual_env.py Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Mar 3, 2024

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Poetry wants to reinstall every system package to the venv when using virtualenvs.options.system-site-packages
2 participants