Support Metadata v2.5#1315
Conversation
| ] | ||
| if hasattr(metadata, "_VALID_METADATA_VERSIONS"): | ||
| if "2.0" not in metadata._VALID_METADATA_VERSIONS: | ||
| metadata._VALID_METADATA_VERSIONS[3:3] = ["2.0"] |
There was a problem hiding this comment.
What about
| metadata._VALID_METADATA_VERSIONS[3:3] = ["2.0"] | |
| metadata._VALID_METADATA_VERSIONS = sorted(metadata._VALID_METADATA_VERSIONS + ["2.0"]) |
So we don't need magic numbers/indices
There was a problem hiding this comment.
Straightforward sorting will go wrong if we reach metadata 2.10 with this monkeypatch still in place - hopefully that's unlikely, but it's not that hard to imagine. How about:
| metadata._VALID_METADATA_VERSIONS[3:3] = ["2.0"] | |
| i = metadata._VALID_METADATA_VERSIONS.index("2.1") | |
| metadata._VALID_METADATA_VERSIONS[i:i] = ["2.0"] |
That would fail if 2.1 is ever removed from packaging, but that's unlikely, it would fail in a very obvious way, and in that scenario the monkeypatch would probably need to change anyway.
There was a problem hiding this comment.
Yeah that works too. Could do this a few ways. Different question: I haven't been able to review where this is used, but does it need to be sorted out is it sorted this way purely because it's easiest for humans to update the list?
In other words, does whatever uses this list need it to be in a specific order?
There was a problem hiding this comment.
Also sorted with a key could create temporary tuples that convert the major/minor pieces to ints to get a decent sort, no?
There was a problem hiding this comment.
The list is just used for a membership check, and ordering isn’t necessary. Even just appending would probably be fine. I considered something like bisect.insort with a Version key to be overkill.
I used a slice assignment because I wanted to mutate the existing list rather than replace it, and used the index of the original position for “2.0” out of an abundance of caution.
There was a problem hiding this comment.
packaging's code does use the order of this list. I haven't checked to see if this use matters to twine, though.
There was a problem hiding this comment.
Oh, Thomas is right. Okay then I guess an index/insert is more self-documenting and avoids the magic number.
| metadata._VALID_METADATA_VERSIONS[3:3] = ["2.0"] | |
| i = metadata._VALID_METADATA_VERSIONS.index("2.1") | |
| metadata._VALID_METADATA_VERSIONS.insert(i, "2.0") |
There was a problem hiding this comment.
Index will raise a value error though, right? Not opposed but might be gnarly if things change for end users if we don't wrap it first
There was a problem hiding this comment.
Yeah, it would raise. If packaging were ever to remove "2.1" metadata version I'd think that would warrant a closer look at this monkeypatch, so perhaps that's a feature?
There are no old releases of packaging I can see which have the list but don't have "2.1" yet, it was there since the start (in pypa/packaging#686).
|
Has anyone thought about simply dropping support for metadata version 2.0? AFAIK it is included out of abundance of caution and it is not even clear whether there are tools in the wild that produce it. |
|
@dnicolodi It was discussed a bit in Dec 2024 (ref - now I see you and Ian are in the discussion too). I suppose that reasoning is still valid, i.e. twine could be used to upload old pkgs to non-PyPI indices. |
|
I went digging into Warehouse BigQuery data and I found that only 2546 distinct projects ever uploaded a release with metadata version 2.0 between 2024-03-18 07:07:13.042837 UTC and 2020-07-20 23:34:32.432750 UTC, for a total of 17093 artifacts. It seems that setuptools produced these artifacts. |
|
Dropping the monkeypatch sounds like a good idea to me. If anyone is still creating packages with a never-standardised metadata version from a decade ago and uploading them to private archives, and for whatever reason they can't fix that, they could pin a version of twine that works for that purpose. |
|
I opened #1317 removing the monkeypatch, as an alternative way to fix the same problem. |
|
I guess this can be closed :) |
Twine currently fails to upload a dist with
Metadata-Version: 2.5, despite packaging supporting that, due to a monkeypatch introduced in 0605ef0.This is a bandaid fix..