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

Provide a better error message when uninstalling packages without dist-info/RECORD #9949

Merged
merged 1 commit into from
Jun 2, 2021

Conversation

hroncok
Copy link
Contributor

@hroncok hroncok commented May 5, 2021

Fixes #8954

@hroncok
Copy link
Contributor Author

hroncok commented May 5, 2021

Open question: What if INSTALLER says pip? In that case, the error would read:

ERROR: Cannot uninstall foobar 0.1, RECORD file not found. Hint: The package was installed by pip.

And that is a bit silly. Should INSTALLER with pip in it be treated the same way as missing INSTALLER?

@uranusjr
Copy link
Member

uranusjr commented May 5, 2021

The message should probably say something else if INSTALLER says pip. The pip value is also unreliable in practice since a lot of Conda packages incorrectly leave that file in their .dist-info directory (and they are unwilling to invest in effort fixing it because “everything works” from Conda’s point of view).

One possibility is to say declare the package outright broken and suggest re-installing it with pip install --force-reinstall --no-deps <name>==<version>.

@hroncok
Copy link
Contributor Author

hroncok commented May 6, 2021

One possibility is to say declare the package outright broken and suggest re-installing it with pip install --force-reinstall --no-deps <name>==<version>.

That is getting rather complex and I am not sure I want to go that way.


If pip value is unreliable in practice, I think treating it like the case without INSTALLER is appropriate here. That would still be an improvement over the status quo. The suggestion can be added later.

@pfmoore
Copy link
Member

pfmoore commented May 6, 2021

I'm a little annoyed to think that we can't improve our users' experience by using a file that's explicitly designed to tell us when we manage an installation, just because some other project (conda in this case) is in violation of the standard.

@uranusjr's suggestion seems like the right approach to me (yes, it's a bit passive aggressive towards conda but 🤷). If pip does own the installation, then --force-reinstall --no-deps will fix the issue. So we should be giving that advice.

@hroncok
Copy link
Contributor Author

hroncok commented May 10, 2021

Pushed a fixup commit for easier review (the intention is to squash it).

@hroncok hroncok force-pushed the error_msg_missing_record branch from 2203bc8 to f77649e Compare May 10, 2021 19:57
@hroncok
Copy link
Contributor Author

hroncok commented May 10, 2021

Squashed.

@hroncok
Copy link
Contributor Author

hroncok commented May 19, 2021

Approved and CI passed. Ready to be merged?

@encukou
Copy link
Contributor

encukou commented Jun 2, 2021

Is there anything I can do to help move this along?

@pfmoore pfmoore merged commit 3c1d181 into pypa:main Jun 2, 2021
@hroncok hroncok deleted the error_msg_missing_record branch June 2, 2021 12:32
@hroncok
Copy link
Contributor Author

hroncok commented Jun 2, 2021

Thanks @pfmoore

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.

Better error message for missing RECORD file
4 participants