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

Use canonicalize_name to look for .dist-info in wheel files #1360

Merged
merged 2 commits into from
May 17, 2018

Conversation

wiggin15
Copy link

wheel files contain a directory that ends with ".dist-info", but its full name may not exactly match the name of the package as written in the wheel filename. e.g. on Windows, the wheel file name may use different letter cases. Using canonicalize_name on both the package name from the wheel filename and the directories in the zip to find a match fixes the problem.
See issue #1350

The general logic was taken from the pip code: https://github.com/pypa/pip/blob/0ae0109901f63b7f133229a9fee41332dd7366b4/src/pip/_internal/wheel.py#L270

canonicalize_name(dirname).startswith(
canonicalize_name(self.project_name))):
return dirname
raise ValueError("unsupported wheel format. .dist-info not found")
Copy link
Member

Choose a reason for hiding this comment

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

This line is still not covered. Can you add a test for it?

@wiggin15
Copy link
Author

I updated the PR with the new test.

pganssle added a commit to Infinidat/setuptools that referenced this pull request May 15, 2018
@wiggin15
Copy link
Author

@pganssle it looks like you edited the commit and it's without the new test now. How do we proceed?

wiggin15 pushed a commit to Infinidat/setuptools that referenced this pull request May 16, 2018
@pganssle
Copy link
Member

@wiggin15 Damn, that's entirely my bad. Do you still have your version of the code? Can you do a force push? I might be able to recover the original test from my git history.

@wiggin15
Copy link
Author

Updated with force push.

@pganssle
Copy link
Member

LGTM.

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

Successfully merging this pull request may close these issues.

2 participants