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

Correctly(?) escape distribution names for wheels #72

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

trishankatdatadog
Copy link

Thanks for pip2pi, @wolever!

Unfortunately, I found that it wasn't correctly escaping distribution names in wheels according to PEP 427:

pypa/pip#5006

Attached is patch that hopefully correctly fixes it. I know @pfmoore and @dstufft must be busy, but I would greatly appreciate it if they could very quickly review. Thanks, everyone!

Cc: @ofek @truthbk

@pfmoore
Copy link

pfmoore commented Mar 19, 2018

Honestly, I can't really say for sure, as I don't know the codebase or what the project is trying to do.

If you have a project name, you can get the correct wheel name by escaping as in the wheel spec. To that extent the first hunk of the code is using the correct escaping (although without tracking where to_safe_name is used, I can't confirm it's only being used for that purpose). As for the second chunk, I'm not clear what that's doing at all, so I can't comment, sorry.

@trishankatdatadog
Copy link
Author

Not a problem, totally understood. Thanks for your time, @pfmoore, much appreciated!

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