Several fixes to WHEEL metadata handling
#588
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The default policies of Message and BytesGenerator always produce LF line endings, so there's never a CRLF to match. If there were, the existing code would have incorrectly replaced CRLF with CR, and then test_wheelfile_line_endings() would have failed.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #588 +/- ##
==========================================
- Coverage 72.66% 72.17% -0.49%
==========================================
Files 13 13
Lines 1090 1071 -19
==========================================
- Hits 792 773 -19
Misses 298 298 ☔ View full report in Codecov by Sentry. |
|
Could you please add a note to the changelog ( |
There's an existing test for this case, which verifies that the original build tag is removed from the wheel filename and is *not* removed from the WHEEL metadata. Presumably this is not what was intended. BUILD_NUM_RE assumed that it would only match at end of file, but there's at least a newline in the way. Fix it, and the test.
For symmetry with `wheel pack`. The functionality works already; we just need to fix the command-line validation.
We're about to change pack() to emit WHEEL metadata with LF-terminated lines and a trailing blank line. In some test_pack() parameterizations we expect to see the original WHEEL file from the test fixture, and in some, a modified one. Handle the difference by comparing WHEEL contents parsed with email.parser, instead of byte strings.
The PyPA Binary Distribution Format spec says the WHEEL metadata file is "in the same basic key: value format" as the METADATA file. METADATA in turn is governed by the Core Metadata Specifications, which say: "In the absence of a precise definition, the practical standard is set by what the standard library email.parser module can parse using the compat32 policy." wheel.bdist_wheel accordingly uses email.generator.BytesGenerator to generate WHEEL, but the CLI `pack` and `tags` commands opt to hand-implement WHEEL parsing and mutation. Their mutation functions, set_tags() and set_build_number(), append any new headers to the existing WHEEL file. Since WHEEL tends to have a trailing blank line (separating the headers from the nonexistent body), the new headers end up in the "body" and are ignored by any tool that parses WHEEL with email.parser. In addition, both functions assume that WHEEL uses CRLF line endings, while bdist_wheel (and email.generator with the compat32 policy) specifically always writes LF line endings. This turns out okay for set_tags(), which rewrites the whole file, but set_build_number() ends up appending a CRLF-terminated line to a file with LF-terminated lines. Fix all this by removing the hand-written parsing/mutation functions in favor of email.parser and email.generator.
|
Sure, done. |
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The PyPA Binary Distribution Format spec says the
WHEELmetadata file is "in the same basic key: value format" as theMETADATAfile.METADATAin turn is governed by the Core Metadata Specifications, which say:wheel.bdist_wheelaccordingly usesemail.generator.BytesGeneratorto generateWHEEL, but the CLIpackandtagscommands opt to hand-implementWHEELparsing and mutation. Their mutation functions,set_tags()andset_build_number(), append any new headers to the existingWHEELfile. SinceWHEELtends to have a trailing blank line (separating the headers from the nonexistent body), the new headers end up in the "body" and are ignored by any tool that parsesWHEELwithemail.parser.In addition, both functions assume that
WHEELuses CRLF line endings, whilebdist_wheel(andemail.generatorwith thecompat32policy) specifically always writes LF line endings. This turns out okay forset_tags(), which rewrites the whole file, butset_build_number()ends up appending a CRLF-terminated line to a file with LF-terminated lines.Fix all this by removing the hand-written parsing/mutation functions in favor of
email.parserandemail.generator.In addition, fix removing the build tag with
wheel pack --build-number "", and add support forwheel tags --build ""for symmetry. There's an existing test for thewheel packcase, which verifies that the original build tag is removed from the wheel filename and is not removed from theWHEELmetadata. Presumably that isn't what was intended.Finally, remove dead code for postprocessing
WHEELline endings inbdist_wheel.