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

Rewrite protect_pip_from_modification_on_windows #7558

Closed
wants to merge 2 commits into from

Conversation

uranusjr
Copy link
Member

@uranusjr uranusjr commented Jan 6, 2020

Follow up on #7358 to implement a better logic with less edge cases. The implementation is put in its own module and functions so it can be improved on in the future.

The logic I have in mind is:

  • Check whether the command (sys.argv[0]) looks like a path. If it does not (and therefore will be resolved by the shell as a command), use shutil.which to resolve it.
  • Check if the path is a file (to exclude the python src\pip case).
  • Check if the file is named like a pip wrapper script.
  • Emit the error.

To make it usable for the detection logic.
@uranusjr
Copy link
Member Author

uranusjr commented Jan 6, 2020

This PR does not contain any of these yet, but

  • I want to put the logic up for feedback.
  • Make sure it’s OK to move looks_like_path like this.

@chrahunt
Copy link
Member

chrahunt commented Jan 7, 2020

IMO, taking a "move the file out of the way if deletion fails" approach would be better, then we can remove all of these pip-specific checks and workarounds. Similar to what is suggested in #7533, except not necessarily scheduling deletion on next reboot.

@uranusjr
Copy link
Member Author

uranusjr commented Jan 7, 2020

Yeah that makes sense. I’m abandoning this for now so this can be discussed more in detail first.

@uranusjr uranusjr closed this Jan 7, 2020
@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Feb 6, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Feb 6, 2020
@uranusjr uranusjr deleted the pip-detect-executable-redo branch September 28, 2020 14:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants