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

twine check should guard against things not accepted by PyPI like version format #430

Open
webknjaz opened this issue Dec 7, 2018 · 14 comments
Labels
blocked Issues we can't or shouldn't get to yet feature request

Comments

@webknjaz
Copy link
Member

webknjaz commented Dec 7, 2018

Your Environment

  1. Your operating system:
N/A
  1. Version of python you are running:
N/A
  1. How did you install twine? Did you use your operating system's package manager or pip or something else?
pip
  1. Version of twine you have installed (include complete output of):
twine --version
twine==1.12.1
  1. Which package repository are you targeting?

https://test.pypi.org

Metadata:
Metadata-Version: 2.1                                                                                
Name: ansible-lint                                                                                   
Version: 3.5.2.dev35+gaa91980

The Issue

I'm involved with https://github.com/ansible/ansible-lint packaging.
We're using setuptools-scm plugin to identify the current version of the distribution, then used in its metadata.

I've configured Travis CI to have one job continuously deploying to Test PyPI. This helps us to keep this whole PyPI automation healthy at all times.

Except that it fails.
setuptools-scm generates PEP440-compliant version based on the previous tag and the current commit: 3.5.2.dev35+gaa91980 (where 35 is the distance to tag 3.5.1 and gaa91980 is sha1 of the current commit. +gaa91980 is a local version identifier.
Up until today I didn't know that public indexes are discouraged from accepting dists with a version containing local identifier:

As the Python Package Index is intended solely for indexing and hosting upstream projects, it MUST NOT allow the use of local version identifiers.

Warehouse respects this: https://github.com/pypa/warehouse/blob/27ed636/warehouse/forklift/legacy.py#L190.

And responds with HTTPError: 400 Client Error: '3.5.2.dev35+gaa91980' is an invalid value for Version. Error: Can't use PEP 440 local versions. See https://packaging.python.org/specifications/core-metadata for url: https://test.pypi.org/legacy/:
https://travis-ci.com/ansible/ansible-lint/jobs/163591117#L1095

So what twine has to do with all of this?

I think twine check should have some --strict option or so and do those same checks warehouse does in order to inform users about potential problems with metadata happening on the PyPI side and produce a more user-friendly explanation of what's happening.

@sigmavirus24
Copy link
Member

twine check only checks the README right now. We very clearly said we'd be happy to add other things to check, but we didn't add them to start. If you'd like to implement that, that'd be great.

@webknjaz
Copy link
Member Author

webknjaz commented Dec 7, 2018

Sure, I was thinking about tackling it.

@di
Copy link
Member

di commented Dec 7, 2018

FWIW, the right way to do this would be to move the validation logic from Warehouse into pypa/packaging, where it can be reused by both Warehouse and twine.

I've started doing this, but it's quite a bit more complicated than one might expect on first glance. I'm hoping to have this wrapped up this month, but I'd also be willing to hand off the work I've already done if there are folks interested in taking it over that could complete it more quickly.

@webknjaz
Copy link
Member Author

webknjaz commented Dec 7, 2018

the right way to do this would be to move

I was also thinking about this but decided that it might be too complicated to do at once, I think it could be easier to do such move in smaller pieces.

Just git push what you've got to somewhere and maybe I'll find some time to contribute some patches there.

@bhrutledge
Copy link
Contributor

@di I proposed a similar feature on pypa/setuptools-scm#365, before I remembered this conversation. Do you have a related issue and/or branch for your work thus far?

@di
Copy link
Member

di commented Oct 21, 2019

@bhrutledge
Copy link
Contributor

Thanks, @di. Is pypa/packaging#147 the related issue?

@ssbarnea
Copy link

Before the magic move of validation does materialize, can we just rely on wheelhouse to validate the packages without uploading them? I am personally not against using official pypi.org as SaaS way for validating if a package is ok, without endup up with it already uploaded. This is key for testing pull-requests (or even developers), so we know that the new changes are ok, even if the user building them may not have the rights to upload the package.

@di
Copy link
Member

di commented Oct 22, 2019

@bhrutledge Yep that's it. Are you thinking about picking it up?

@ssbarnea That seems like an unnecessary round trip and extra work for PyPI. Compared to the same logic existing locally in twine, what would be the advantage to that?

@bhrutledge
Copy link
Contributor

Are you thinking about picking it up?

@di Not any time soon; just connecting the dots in case other folks are interested. Thanks for sharing your work.

@brainwane
Copy link
Contributor

Thanks to pypi/warehouse#7582 , pypa/packaging#147 is now resolved, which may unblock progress on this.

@sigmavirus24
Copy link
Member

We can make incremental progress here in validating the trove classifiers. We can't address the original issue, however, of being entirely certain that the verison is correct. We can try to parse it and reflect any InvalidVersion exception we get, but I'm not 100% confident that's the only validation warehouse does.

@di
Copy link
Member

di commented Apr 5, 2020

Relevant update here: pypa/packaging#147 (comment), TL;DR: once I finish that we'll be able to do the exact same checks Warehouse does.

@henryiii
Copy link
Contributor

This would be great to have!

jschwartzentruber added a commit to jschwartzentruber/FuzzManager that referenced this issue Feb 23, 2022
fuzzing-decision (used by taskmanager) uses a direct url reference
(allowed by PEP 508). But PyPI cannot accept direct url references.

We don't need any of the extras anyways for the wheel (Collector/FTB only).

See pypa/twine#430
jschwartzentruber added a commit to MozillaSecurity/FuzzManager that referenced this issue Feb 25, 2022
fuzzing-decision (used by taskmanager) uses a direct url reference
(allowed by PEP 508). But PyPI cannot accept direct url references.

We don't need any of the extras anyways for the wheel (Collector/FTB only).

See pypa/twine#430
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Issues we can't or shouldn't get to yet feature request
Projects
None yet
Development

No branches or pull requests

7 participants