-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Abort installation on metadata mismatch #3153
Abort installation on metadata mismatch #3153
Conversation
4595e27
to
10d1d6c
Compare
@@ -421,6 +421,14 @@ def run_egg_info(self): | |||
self.pkg_info()["Version"], | |||
])) | |||
self._correct_build_location() | |||
else: | |||
metadata_name = canonicalize_name(self.pkg_info()["Name"]) | |||
if self.req.project_name.lower() != metadata_name: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're canonicalizing stuff, why not canonicalize instead of .lower()
'ing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well self.req.project_name.lower()
is equivalent to the canonical name but I agree this is not clear (and only true for the current definition of canonicalize_name
). I'll canonicalize_name
both terms.
10d1d6c
to
7b47538
Compare
It might make sense to call this out as a backwards incompatible change, but I'm not dead set on it. |
If setup.py egg_info produces metadata for a different project name than self.req.project_name, abort the installation. Fixes pypa#3143
7b47538
to
7e02e5c
Compare
@dstufft Moved the changelog to backward incompatible |
Abort installation on metadata mismatch
metadata_name = canonicalize_name(self.pkg_info()["Name"]) | ||
if canonicalize_name(self.req.project_name) != metadata_name: | ||
raise InstallationError( | ||
'Running setup.py (path:%s) egg_info for package %s ' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this is after merge, but thinking the message should mention the url (if present) with the #egg fragment, or otherwise I'm not sure people will connect the dots that the problem is simply due to the fragment being wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pip errors just after cloning the repository, so the error seemed sufficient to me:
Obtaining djpyfs from git+https://github.com/pmitros/django-pyfs.git#egg=djpyfs (from -r /home/xfernandez/some_requirements.txt (line 7))
Cloning https://github.com/pmitros/django-pyfs.git to ./src/djpyfs
Running setup.py (path:/home/xfernandez/.virtualenvs/d694c7c01647e1aa/src/djpyfs/setup.py) egg_info for package djpyfs produced metadata for project name django-pyfs
I don't know for sure, but concerned this will break many builds. It could be worth only warning for the next release. |
A warning could indeed help with the transition but on the other end the fix is pretty simple and I'm tempted to think it isn't that frequent... |
If setup.py egg_info produces metadata for a different project name than
self.req.project_name, abort the installation.
Fixes #3143