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

runtime: bump pkginfo to ~1.12 #311

Closed
wants to merge 2 commits into from

Conversation

woodruffw
Copy link
Member

@woodruffw woodruffw commented Dec 5, 2024

This follows #309 and should fix the metadata issues more thoroughly, since pkginfo 1.12 includes support for metadata 2.4.

See #310

CC @webknjaz

Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
Copy link
Member

@webknjaz webknjaz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for looking into this sooner than me! Before rushing to merge, I'd like to see if it'd be possible to integrate dists w/ different metadata versions into CI…

Comment on lines +3 to +6
# NOTE: 1.12.0 and later enable support for metadata 2.4
# NOTE: This can be dropped once twine stops using pkginfo
# Ref: https://github.com/pypa/twine/pull/1180
pkginfo ~= 1.12.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer indirect deps to go into a dedicated constraint file… Though, since there are precedents of having them here, this request is out of the scope of this PR. It's something to address separately.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I feel bad about this being here. The "good" news is that I can revert it pretty much immediately once it's locked, since pip won't downgrade it. But I don't know if that's better or worse 😅

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@woodruffw here's a pattern I came up with and started using in many places: https://github.com/ansible/awx-plugins/blob/e7d5733/dependencies/direct/py.in / https://github.com/ansible/awx-plugins/blob/e7d5733/dependencies/direct/py-constraints.in. Basically, any limitations, including transitive deps, are maintained in a separate file that does not contribute its entries to a list of "user install requests" but to "restrict if met during dependency resolution".

@webknjaz webknjaz linked an issue Dec 6, 2024 that may be closed by this pull request
@webknjaz
Copy link
Member

webknjaz commented Dec 6, 2024

Before rushing to merge, I'd like to see if it'd be possible to integrate dists w/ different metadata versions into CI…

So I experimented with Maturin in #312, and it reproduces the issue and shows that this PR fixes it. I'm going to pull that into this PR soon, cleaning up the commits. Hold tight!

P.S. I was hoping to find something more lightweight, but it appears that the version of Hatchling producing metadata 2.4 is unreleased (perhaps, waiting for us? — there's no public explanation and I filed pypa/hatch#1842). Poetry is unclear on what it produces, the change log doesn't mention it. I looked into the test suites for pip and twine but decided to postpone porting the wheel creation here, maybe I'll do so later. That's why I'm keeping Maturin for now, it adds 18 seconds to the CI job.

@webknjaz webknjaz self-assigned this Dec 6, 2024
@webknjaz webknjaz added bug Something isn't working enhancement New feature or request labels Dec 6, 2024
@webknjaz webknjaz marked this pull request as draft December 6, 2024 18:22
@webknjaz
Copy link
Member

webknjaz commented Dec 6, 2024

Since I can't push to this branch, I'm going to complete this in #313.

@webknjaz webknjaz closed this in f371c3d Dec 6, 2024
@webknjaz webknjaz closed this in #313 Dec 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

InvalidDistribution error for whl from pyo3/maturin 0.23.3
2 participants