search

pypa / twine

Utilities for interacting with PyPI
https://twine.readthedocs.io/
Apache License 2.0
1.59k stars 305 forks source link

Restore support for pkginfo 1.11 #1123

Open jaraco opened 2 months ago

jaraco commented 2 months ago

For now, this approach retains the current behavior that an unrecognized metadata version will fail with an error.

This approach splits out the detection based on the version of pkginfo. Starting with pkginfo 1.11, it now properly detects when a metadata version is unrecognized and errors on that condition explicitly. For backward compatibility, until pkginfo 1.11 is the minimum, the check will still provide guidance to consider the metadata version when fields are missing.

Closes #1116

jaraco commented 2 months ago

Well, shoot. It seems that after allowing pkginfo 1.11, the type checks are failing. It seems that pkginfo.Distribution.name was str and now is str | None. Same for .version.

woodruffw commented 2 months ago

Random thought: is there a reason this can't constrain pkginfo ~= 1.11 in pyproject.toml? That would avoid the need for the run-time version check + some of the extra handling in this PR.

jaraco commented 2 months ago

Random thought: is there a reason this can't constrain pkginfo ~= 1.11 in pyproject.toml? That would avoid the need for the run-time version check + some of the extra handling in this PR.

My thought was that there are probably users still reliant on older versions, so it would be nice to have at least one release transition where users aren't forced to the newer version. I absolutely agree that after twine stabilizes, we might bump the dependency on pkginfo (and remove the compatibility logic). I coded it in such a way as to make it easy to remove the compatibility logic.

I also observed that there's already an open issue https://github.com/pypa/twine/issues/1070 tracking the move to pkginfo 1.10, so that issue would need to be addressed first (or simultaneously) with the move to 1.11.

jaraco commented 1 week ago

I don't understand why the coverage tests are failing. They weren't failing before. Maybe the upstream changes pushed the coverage marginally lower and now these changes push them even lower? Since the coverage failures are for total coverage and not diff coverage, it's difficult for me to see what changes in this PR have contributed to the failing check.

jaraco commented 1 week ago

Due to #1121, I'm unable to locally assess the coverage issues.

stefanor commented 1 hour ago

Applied this PR as a patch, in Debian's twine.