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

Handle invalid requirement as parameter to uninstall #9052

Closed
wants to merge 245 commits into from
Closed

Handle invalid requirement as parameter to uninstall #9052

wants to merge 245 commits into from

Conversation

GuyTuval
Copy link
Contributor

@GuyTuval GuyTuval commented Oct 25, 2020

Implemented error handling when passing a URL or an archive to pip uninstall.
The current implementation, when given a list of libraries in pip uninstall, will not uninstall any library if at least one of them is an archive or a URL.

@GuyTuval GuyTuval marked this pull request as ready for review October 26, 2020 16:00
Copy link
Contributor

@NoahGorny NoahGorny left a comment

Choose a reason for hiding this comment

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

Just a minor comment, otherwise LGTM

src/pip/_internal/commands/uninstall.py Outdated Show resolved Hide resolved
Copy link
Member

@xavfernandez xavfernandez left a comment

Choose a reason for hiding this comment

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

You will also have to rebase your PR on master to fix the CI 👍

src/pip/_internal/commands/uninstall.py Outdated Show resolved Hide resolved
Copy link
Member

@xavfernandez xavfernandez left a comment

Choose a reason for hiding this comment

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

script.pip is a special helper that actually spawn a process.

tests/functional/test_uninstall.py Outdated Show resolved Hide resolved
tests/functional/test_uninstall.py Outdated Show resolved Hide resolved
@GuyTuval
Copy link
Contributor Author

GuyTuval commented Nov 5, 2020

script.pip is a special helper that actually spawn a process.

Roger that, @xavfernandez. How do I rebase my PR on master, so the CI will get fixed and I will be allowed to merge? :)

@xavfernandez
Copy link
Member

Roger that, @xavfernandez. How do I rebase my PR on master, so the CI will get fixed and I will be allowed to merge? :)

something like:

git fetch origin
git checkout failed-uninstall-parameter-4958
git rebase origin/master
git push GuyTuval failed-uninstall-parameter-4958 -f

should work (given that origin remote is https://github.com/pypa/pip/ and GuyTuval remote is https://github.com/GuyTuval/pip).

@GuyTuval GuyTuval changed the title Handle archives and URLs as parameters to uninstall Handle invalid requirement as parameter to uninstall Nov 6, 2020
@GuyTuval
Copy link
Contributor Author

Hi @xavfernandez ! 😃
It seems the requested changes haven't been approved yet.
Can you check if they are all met and approve/disapprove accordingly?

news/4958.feature.rst Outdated Show resolved Hide resolved
src/pip/_internal/commands/uninstall.py Outdated Show resolved Hide resolved
@GuyTuval
Copy link
Contributor Author

Hi @uranusjr 😁
Would you mind reviewing this PR? I believe it offers a nice improvement to uninstall command

@GuyTuval
Copy link
Contributor Author

GuyTuval commented Feb 4, 2021

Hi @pradyunsg, I am humbly pumping this PR as I don't want it to get lost.
Can you review/merge the PR?

@pradyunsg
Copy link
Member

Thanks for the nudge here @GuyTuval! :)

@BrownTruck
Copy link
Contributor

Hello!

I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the master branch into this pull request or rebase this pull request against master then it will be eligible for code review and hopefully merging!

@BrownTruck BrownTruck added the needs rebase or merge PR has conflicts with current master label Feb 19, 2021
@pypa-bot pypa-bot removed the needs rebase or merge PR has conflicts with current master label Feb 21, 2021
@GuyTuval
Copy link
Contributor Author

Hi @pradyunsg!
Is it possible to merge this PR?
If there is a reason I'm not aware of that postpones the merge of this PR I would appreciate it if you lmk 😅

minrk and others added 3 commits March 12, 2021 13:12
adds VERBOSE custom log level between DEBUG and INFO

used when `-v` is given. Now require -vv to enable full debug output.

Messages can be logged with VERBOSE level to promote them to `-v` output instead of `-vv`
The UTF-8 encoding was assumed in an sdist, but without explicit
specifying, extraction may fail on obscure systems where the default
encoding is not UTF-8.

Co-Authored-By: Chris Hunt <[email protected]>
The practical difference is the mismatch detection is not performed at
most once for every invocation, thus only warned once if there are any
mismatches.
@GuyTuval
Copy link
Contributor Author

Hello @pradyunsg !
Would you mind merging this PR? I have fixed every given CR and don't want it to get forgotten 😅

sbidoul and others added 28 commits July 18, 2021 19:58
Co-authored-by: Pradyun Gedam <[email protected]>
POSIX is problematic when the environment is not configured properly.
In pip 20.3, the new resolver will be the default.
Therefore, this commit updates user docs to change
instructions for testing, migration, etc.

Towards #8937.
Co-authored-by: Xavier Fernandez <[email protected]>
@GuyTuval GuyTuval closed this Jul 18, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 27, 2021
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.