-
-
Notifications
You must be signed in to change notification settings - Fork 591
fix: parsing metadata with inline licenses #2806
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
fix: parsing metadata with inline licenses #2806
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! CHANGELOG will need to be updated, but I can also do this.
EDIT: decided to not update the changelog since there have been no stable release yet.
Requires-Dist: bar; extra == "all" | ||
Provides-Extra: all | ||
|
||
Requires-Dist: this will be ignored |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that it is probably more sensible to not ignore this line when parsing METADATA. I think the number of packages that will have Requires-Dist:
in their description will be relatively small or non-existent, so it is better to go with that assumption at first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, let's not do that for now since, the two packages referenced in the issue will be handled by the fix here, e.g.: https://pypi-browser.org/package/mlflow/mlflow-2.22.0rc0-py3-none-any.whl/mlflow-2.22.0rc0.dist-info/METADATA
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yea i was thinking that too, also as evidence by the previous parsing implementation parsed the whole file
The wheel `METADATA` parsing implemented in 1.4 missed the fact that whitespace is significant and sometimes License is included inline in the `METADATA` file itself. This change ensures that we stop parsing the `METADATA` file only on first completely empty line. Fixes #2796 --------- Co-authored-by: Ignas Anikevicius <240938+aignas@users.noreply.github.com> (cherry picked from commit 1d69ad6)
Thanks for the quick fix @keith the issue looks resolved on my side! |
The wheel
METADATA
parsing implemented in 1.4 missed the factthat whitespace is significant and sometimes License is included
inline in the
METADATA
file itself.This change ensures that we stop parsing the
METADATA
file onlyon first completely empty line.
Fixes #2796