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

Support overriding corrupt virtual environments #2198

Closed
arcivanov opened this issue Sep 25, 2021 · 4 comments
Closed

Support overriding corrupt virtual environments #2198

arcivanov opened this issue Sep 25, 2021 · 4 comments
Labels

Comments

@arcivanov
Copy link
Contributor

The fix for #2185 in #2186 introduced changes to the behavior of Virtualenv that was not the intention of the fix. Such changes may affect existing environments in unexpected ways. Specifically:

  1. The uninstall now would uninstall from platlib. However Virtualenv never installed (and does not now install) the embedded into the platlib and if a distro was installed there, the pre-2186 behavior was to leave platlib completely undisturbed. The During seeding properly uninstall present versions of the wheels #2186 modified this behavior without a warning, reason, an ability to test such change (none of the embeds go to platlib) while violating the principle of least surprise.

  2. Prior to During seeding properly uninstall present versions of the wheels #2186 during installation if the target directory was present (for whatever reason), the logic would remove such target. Now that target must not exist or the installation would fail. A virtual env may contain extraneous directories/symlinks that are not associated with a distro (that would be handled by an uninstall), if the distro was installed in development mode, which deals in egg-links, symlinks and pth-files.

@arcivanov arcivanov added the bug label Sep 25, 2021
@gaborbernat
Copy link
Contributor

gaborbernat commented Sep 25, 2021

  1. I consider this behaviour of bug, fixed by the PR in question and as such, I'll veto restoring it until someone can provide use cases where this would be hurtful.
  2. PR with tests welcome 👍

PS. In future please create 1 issue per one topic.

@gaborbernat gaborbernat changed the title Restore unintended cleanup behavior after #2186 Support overriding corrupt virtual environments Sep 25, 2021
arcivanov added a commit to arcivanov/virtualenv that referenced this issue Sep 25, 2021
1. Do not disturb platdir
2. Do delete a target directory on install if it is present and
wasn't a part of the distro that could've been uninstalled

fixes pypa#2198
@arcivanov
Copy link
Contributor Author

1. I consider this behaviour of bug, fixed by the PR in question and as such, I'll veto restoring it until someone can provide use cases where this would be hurtful.

Pursuant to PEP 0427 if a dist us a pure python dist, it will always be installed in the purelib and not platlib. This is the behavior observed by pip as well: https://github.com/pypa/pip/blob/main/src/pip/_internal/operations/install/wheel.py#L457

Virtualenv does not analyze whether embedded wheels are purelib (incidentally they are so far) thus it would never install anything into platlib. If any version of the embedded dists suddenly became non-pure, Virtualenv would have no ability to install them pursuant to PEP 0427 anyway.

Thus,

  1. either Virtualenv needs to analyze the METADATA of the dist's dist-info and then pick the install directory as PIP does; or,
  2. maintaining the current behavior of assuming the embeds are always pure, they would never actually end up in the platlib anyway by Virtualenv's seeder.

I.e. either the seeding needs to be fixed to account for pure vs non-pure as PIP does and PEP specifies, and then looking at platlib makes sense or deleting from platlib potentially touches some other tool's packages which Virtualenv knows nothing about and int

It's either or but it can't be both.

@arcivanov
Copy link
Contributor Author

Actually, I've exhausted the amount of the time I can dedicate to arguing on the Internet today. Withdrawn. Good luck.

@gaborbernat
Copy link
Contributor

Pursuant to PEP 0427 if a dist us a pure python dist, it will always be installed in the purelib and not platlib.

Seems you're assuming that seed packages will always be pure python, which might be true today it might not be the case forever. Feels to me it's no real downside in assuming either pure python or platform-dependent implementation can happen, and probably the correct solution.

  1. either Virtualenv needs to analyze the METADATA of the dist's dist-info and then pick the install directory as PIP does; or,

Yes, this is a bug that should be fixed at some point; though for now, all seed packages are pure python so the issue isn't manifesting yet. A PR doing this would be accepted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants