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

Better handling of unrecognized metadata versions #9195

Closed
jfly opened this issue Mar 22, 2024 · 9 comments · Fixed by #9203
Closed

Better handling of unrecognized metadata versions #9195

jfly opened this issue Mar 22, 2024 · 9 comments · Fixed by #9203
Labels
kind/feature Feature requests/implementations status/triage This issue needs to be triaged

Comments

@jfly
Copy link
Contributor

jfly commented Mar 22, 2024

Issue Kind

Change in current behaviour

Description

See https://github.com/jfly/2024-03-19-poetry-build-deps-issue for details of a saga I went through this week to figure out why poetry recently stopped installing the transitive dependencies of hatchling.

tl;dr: Hatchling now uses metadata version 2.3, and existing installations of poetry may or may not handle metadata version 2.3 well, depending on if they were installed before or after 2024-03-03 (when pkginfo 1.10.0 was released).

Impact

The next 1.8 release of poetry will be guaranteed to handle metadata 2.3 correctly thanks to #9131. However, upgrading to a fixed version of poetry isn't enough, as your poetry cache may already be poisoned with bad information.

In my option, it would be better if poetry just crashed clearly when presented with unrecognized metadata. I believe the current pkginfo api doesn't make this possible to do, but I've asked upstream if this is something they'd be willing to expand their API to support: https://bugs.launchpad.net/pkginfo/+bug/2058697.

Is this something poetry would be interested in handling differently? If so, please chime in on this bug report upstream with pkginfo.

Workarounds

N/A

@jfly jfly added kind/feature Feature requests/implementations status/triage This issue needs to be triaged labels Mar 22, 2024
@jfly
Copy link
Contributor Author

jfly commented Mar 22, 2024

For the record, the maintainer of pkginfo is open to expanding the existing Distribution api to allow tools like poetry to do something more nuanced here. See https://bugs.launchpad.net/pkginfo/+bug/2058697/comments/2

@dimbleby
Copy link
Contributor

Not sure that this issue is tracking anything useful in this repository. If the pkginfo API changes then I expect that poetry will either pick that up; or use something else (I see that a recent PR preferred packaging.metadata).

pkginfo throwing an exception on unrecognised metadata sounds fine, but for it to make a meaningful difference to end users someone will still have to write code here to turn that into a helpful error message.

You could do that today if you wanted to: where the code currently says "Unable to create a package with no name" you could just as well add a hint that it probably is because of faulty or unexpected metadata.

@jfly
Copy link
Contributor Author

jfly commented Mar 22, 2024

Not sure that this issue is tracking anything useful in this repository

That's fair. Perhaps I should have filed this as a discussion, but to try to state this as a concrete bug/user experience issue, I might rephrase this as "poetry silently ignores transitive dependencies of package A when package A uses an unrecognized metadata format".

where the code currently says "Unable to create a package with no name"

Sorry, I'm not familiar with this message. If you read through my description of the symptoms on https://github.com/jfly/2024-03-19-poetry-build-deps-issue, it's about transitive dependencies not getting added to the lockfile. From the user's perspective, there isn't even an error, poetry just seems to neglecting dependencies.

@jfly
Copy link
Contributor Author

jfly commented Mar 22, 2024

poetry silently ignores transitive dependencies of package A when package A uses an unrecognized metadata format

And for the record, I am aware that this won't happen anymore once a version of poetry is released with this change: 83d4a0e, but the fundamental architectural issue remains: when the next metadata version gets released, poetry will start behaving the same way again.

@dimbleby
Copy link
Contributor

dimbleby commented Mar 22, 2024

Sorry, I'm not familiar with this message

eg per #9191.

But same principle if there is some earlier point in the code where the failure to parse metadata could have been spotted. If you want to, you can identify that point and recognise that the metadata has missing mandatory fields, and raise a useful error message - and you can do all of that today.

@jfly
Copy link
Contributor Author

jfly commented Mar 22, 2024

you can identify that point and recognise that the metadata has missing mandatory fields, and raise a useful error message - and you can do all of that today.

That's totally fair, you are right that we could add a heuristic to detect this that works with existing versions of pkginfo.

If you want to

I want to! But I'm not a maintainer of poetry. Is this something poetry would be open to? I'd be happy to try to put together a PR if so.

@dimbleby
Copy link
Contributor

I do not have the power to merge or reject pull requests, but if you bring something sensible and with a testcase I would expect it has every chance of being accepted

@piercefreeman
Copy link

Thanks for this thread @jfly, wish my search terms dug this up sooner. I only stumbled onto this issue after I went down the same debugging rabbit hole myself. Posting in case this helps weary future travelers:

I ran into this bug with my packages that are bundled with Maturin, which recently bumped its metadata version to 2.3 in PyO3/maturin#1965. Notably this only seems to affect non-pypi hosted packages since poetry's pypi resolution schema uses a separate json metadata index. In my case poetry installs were able to pull the main wheel artifacts but they ignored any declared dependencies. As you note this ignoring happens silently so client observations were:

  • poetry add my-private-package
  • Poetry installs main dependency
  • Execution of package fails since no additional dependencies were resolved

My workaround at the moment before the new version of poetry is published (and indeed for awhile after as clients upgrade to a correct poetry version), I'm manually updating the METADATA in packages that don't require 2.3 support:

sed -i 's/Metadata-Version: 2.3/Metadata-Version: 2.1/g' unpackaed-pkg-*.dist-info/METADATA

Copy link

github-actions bot commented May 4, 2024

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/feature Feature requests/implementations status/triage This issue needs to be triaged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants