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

Diagnostic error when processing package already installed with invalid requirement #12953

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

notatallshaw
Copy link
Member

@notatallshaw notatallshaw commented Sep 4, 2024

Fixes #12950
Fixes #12979

Creates a new exception message which gives the user a much clearer output:

$ python -m pip install --dry-run celery
Requirement already satisfied: celery in ./.venv/lib/python3.12/site-packages (4.4.7)
error: invalid-installed-package

× Cannot process installed package celery 4.4.7 in '/home/damian/opensource/contributions/pip/.venv/lib/python3.12/site-packages' because it has an invalid requirement:
│ Expected matching RIGHT_PARENTHESIS for LEFT_PARENTHESIS, after version specifier
│     pytz (>dev)
│          ~^
╰─> Starting with pip 24.1, packages with invalid requirements can not be processed.

hint: To proceed this package must be uninstalled using 'pip<24.1', some other Python package tool, or manually deleted.

Looking for feedback as I've never created a pip exception before and not sure how this would be tested.

@notatallshaw
Copy link
Member Author

I've improved the message to make it a little bit more accurate (not always about uninstallation) and more actionable (gives the location it is installed).

Ready for review/merge.

Copy link
Contributor

@edmorley edmorley left a comment

Choose a reason for hiding this comment

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

Thank you!

I like the revised wording of the error message (since the exception can occur even when not uninstalling).

@edmorley
Copy link
Contributor

edmorley commented Sep 9, 2024

Could you update the PR description (and also the example in #12950 (comment)) to use the new wording (that's broader than just uninstall)?

@notatallshaw notatallshaw changed the title Diagnostic error on pip uninstall of invalid package Diagnostic error when processing package already installed with invalid requirement Sep 12, 2024
@notatallshaw
Copy link
Member Author

Could you update the PR description (and also the example in #12950 (comment)) to use the new wording (that's broader than just uninstall)?

Done

@edmorley
Copy link
Contributor

Can this be merged? It would be great to get it into 24.3.

@notatallshaw
Copy link
Member Author

notatallshaw commented Sep 27, 2024

One moment, I'm going to update this PR to also handle the exception in #12979, it's the same issue type (invalid installed package), but the exception is happening in a different location because it's slightly different (invalid version instead of invalid requirement).

@notatallshaw notatallshaw force-pushed the diagnostic-error-on-pip-uninstall branch from 820b09f to 22cc32d Compare September 28, 2024 01:32
@notatallshaw
Copy link
Member Author

Done, now also fixes #12979, ready for re-review / merge (once #12964 lands anyway, which is currently blocking CI)

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