Skip to content

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

Merged
pganssle merged 2 commits into
pypa:masterfrom
Infinidat:issue1350
May 17, 2018
Merged

Use canonicalize_name to look for .dist-info in wheel files#1360
pganssle merged 2 commits into
pypa:masterfrom
Infinidat:issue1350

Conversation

@wiggin15

Copy link
Copy Markdown

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

Comment thread setuptools/wheel.py
canonicalize_name(dirname).startswith(
canonicalize_name(self.project_name))):
return dirname
raise ValueError("unsupported wheel format. .dist-info not found")

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Author

Updated with force push.

@pganssle

Copy link
Copy Markdown
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