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

Improve uninstall logic to handle file removal failure e.g. pip install -U pip on Windows #7567

Closed
uranusjr opened this issue Jan 7, 2020 · 11 comments
Labels
C: uninstall The logic for uninstallation of packages type: enhancement Improvements to functionality

Comments

@uranusjr
Copy link
Member

uranusjr commented Jan 7, 2020

When pip is uninstalling (or upgrading/reinstalling) itself, it would try to remove the wrapper script generated by the installation. This causes problems on Windows when you run something like pip upgrade pip because it is an error to delete the executable of a running process.

Currently pip tries to avoid this by detecting how the user invokes pip, and error-ing out early before the uninstallation happens on certain situations (pip._internal.utils.misc.protect_pip_from_modification_on_windows). But the checks themselves have many edge cases, and we’ve been either not catching things (#6841) or overly strict (#7358). I believe it is possible to implement a comprehensive check (#7558), but it would be quite complex. @chrahunt suggested in #7558 (comment) that it would be a good idea to get rid of these special cases, and fix the removal failures instead.

Since pip already moves them to a temporary directory during uninstallation (so it can rollback on e.g. upgrade failures), the missing piece here is to not error on when the stashed files fail to be deleted. This is done by TempDirectory.cleanup, which simply calls rmtree on the temp dir, so one approach to solve this would be to add an onerror handler that moves the failed-to-be-deleted file somewhere (or just keep it and print out its location).

@triage-new-issues triage-new-issues bot added the S: needs triage Issues/PRs that need to be triaged label Jan 7, 2020
@pradyunsg
Copy link
Member

/cc @pfmoore

@pfmoore
Copy link
Member

pfmoore commented Jan 15, 2020

This certainly seems like a viable idea. A quick test by putting a try...except pass around the rmtree here does cause pip install -U to work cleanly. Obviously a real fix should be more careful (maybe even to the extent of special-casing pip*.exe, as they are the only files where we expect deletion to fail?)

I don't think I had ever realised we moved the files before upgrading. Good catch!

I'm slightly sad that we'll routinely leave clutter behind in $TEMP, but not sad enough to say we shouldn't do this 🙂

@uranusjr
Copy link
Member Author

as they are the only files where we expect deletion to fail?

I hit this often by accident, e.g. upgrading flake8 when it is in use by VS Code. I accept them as user errors, but it’d help improve my workflow a lot if we do this for all files.

@pfmoore
Copy link
Member

pfmoore commented Jan 15, 2020

Fair enough, I don't have a problem with that. Maybe trap "Access denied" and filename is *.exe to just hit "trying to delete an in-use Windows exe wrapper"? But honestly, "don't get upset if cleanup fails" is just as good a principle, so I'm not going to argue here.

@uranusjr uranusjr added C: uninstall The logic for uninstallation of packages type: enhancement Improvements to functionality labels Jul 29, 2020
@triage-new-issues triage-new-issues bot removed the S: needs triage Issues/PRs that need to be triaged label Jul 29, 2020
@uranusjr
Copy link
Member Author

I decided to implement something for this since this came up again and I don’t have better things to do now 🙂

@SigmundVik
Copy link

SigmundVik commented Nov 11, 2021

I ran into the same issue when switching between different versions of pip. With the knowledge from this thread the workaround is obvious, just use python.exe to invoke pip:

[BOOTCAMP] [none] Thu 11-11-21 16:06 8.600 23G z:\>pip --version
pip 21.3 from C:\tools\Python39\lib\site-packages\pip (python 3.9)

[BOOTCAMP] [none] Thu 11-11-21 16:07 0.640 23G z:\>pip install pip==21.2.4
Collecting pip==21.2.4
  Using cached pip-21.2.4-py3-none-any.whl (1.6 MB)
Installing collected packages: pip
  Attempting uninstall: pip
    Found existing installation: pip 21.3
    Uninstalling pip-21.3:
      Successfully uninstalled pip-21.3
ERROR: Could not install packages due to an OSError: [WinError 5] Access is denied: 'C:\\tools\\Python39\\~3ripts\\pip.exe'
Consider using the `--user` option or check the permissions.


[BOOTCAMP] [none] Thu 11-11-21 16:07 8.632 23G z:\>pip --version
pip 21.2.4 from C:\tools\Python39\lib\site-packages\pip (python 3.9)

[BOOTCAMP] [none] Thu 11-11-21 16:07 0.634 23G z:\>python -m pip install pip==21.3
Collecting pip==21.3
  Using cached pip-21.3-py3-none-any.whl (1.7 MB)
Installing collected packages: pip
  Attempting uninstall: pip
    Found existing installation: pip 21.2.4
    Uninstalling pip-21.2.4:
      Successfully uninstalled pip-21.2.4
Successfully installed pip-21.3

@pradyunsg
Copy link
Member

This has been fixed in #10560 and will be in the next release. Users will benefit from the fix in releases after that.

Users hitting this should run python -m pip install --upgrade pip --force-reinstall to fix their Python installation.

If you're unable to do so for some reason, download get-pip.py and run it with --force-reinstall.

@pradyunsg
Copy link
Member

I'd suggest reporting that to the brotli package.

@pradyunsg
Copy link
Member

@89z let's discuss that in #10950. :)

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
C: uninstall The logic for uninstallation of packages type: enhancement Improvements to functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants
@uranusjr @SigmundVik @pfmoore @pradyunsg and others