pypa / twine

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

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

Open webknjaz opened 5 years ago

webknjaz commented 5 years ago

Your Environment

1) Your operating system:

N/A

2) Version of python you are running:

N/A

3) How did you install twine? Did you use your operating system's package manager or pip or something else?

pip

4) Version of twine you have installed (include complete output of):

twine --version
twine==1.12.1

5) 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 commented 5 years ago

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 commented 5 years ago

Sure, I was thinking about tackling it.

di commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

@di I proposed a similar feature on https://github.com/pypa/setuptools_scm/issues/365, before I remembered this conversation. Do you have a related issue and/or branch for your work thus far?

di commented 5 years ago

@bhrutledge Yes, it's here: https://github.com/di/packaging/tree/metadata-validation

Edit: Diff is here: https://github.com/pypa/packaging/compare/master...di:metadata-validation?expand=1

bhrutledge commented 5 years ago

Thanks, @di. Is https://github.com/pypa/packaging/issues/147 the related issue?

ssbarnea commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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 commented 4 years ago

Thanks to https://github.com/pypa/warehouse/pull/7582 , https://github.com/pypa/packaging/issues/147 is now resolved, which may unblock progress on this.

sigmavirus24 commented 4 years ago

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 commented 4 years ago

Relevant update here: https://github.com/pypa/packaging/issues/147#issuecomment-602928562, TL;DR: once I finish that we'll be able to do the exact same checks Warehouse does.

henryiii commented 4 years ago

This would be great to have!