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

Test: Add coverage for wheel filename normalization. #422

Merged
merged 1 commit into from
Apr 19, 2021

Conversation

pelson
Copy link
Contributor

@pelson pelson commented Apr 19, 2021

I noticed that #387 did not include a test for _ in the wheel name, nor for anything with upper case characters. This kind of filename can be generated with python -m build and pip wheel --no-deps.

cc @pfmoore as the author of the original PR

@pfmoore
Copy link
Member

pfmoore commented Apr 19, 2021

Hmm, are wheel filenames with uppercase in them even valid? https://packaging.python.org/specifications/binary-distribution-format/ says

In distribution names, any run of -_. characters (HYPHEN-MINUS, LOW LINE and FULL STOP) should be replaced with _ (LOW LINE). This is equivalent to PEP 503 normalisation followed by replacing - with _.

That's not actually self-consistent as PEP 503 converts everything to lower case, whereas the text before that doesn't say that should happen. The reference to PEP 503 wasn't in the original wheel PEP (it was added in pypa/packaging.python.org#844, which references https://discuss.python.org/t/escaping-versions-for-wheel-sdist-and-dist-info-names/5605).

Either way, I think we probably have to accept that we'll see mixed-case distribution names in the wild, and normalising them when parsing is the correct thing to do, so these tests make sense even if it turns out that they are testing how the code handles technically-invalid input.

@brettcannon brettcannon merged commit 64f82b5 into pypa:main Apr 19, 2021
@brettcannon
Copy link
Member

Thanks!

@pelson pelson deleted the test/uppercase branch April 21, 2021 11:25
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.

3 participants