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

Feature Request: make twine check validate package dependency data #813

Closed
sirosen opened this issue Sep 16, 2021 · 5 comments
Closed

Feature Request: make twine check validate package dependency data #813

sirosen opened this issue Sep 16, 2021 · 5 comments

Comments

@sirosen
Copy link

sirosen commented Sep 16, 2021

I recently had a bug surface in a package I maintain, in which a python_version spec on a dependency was malformed. Specifically, we had typing-extensions; python_version<"3,8".

twine check passes on the package builds in this state. This led to a downstream failure when other tools (poetry) couldn't parse the dependency data.

There doesn't appear to be any relevant command which could be given to pip or setuptools. twine check is the closest thing we have.

My specific case traces down into setuptools, where there's support for legacy specifiers in the vendored copy of packaging.
I think it would be awesome if we could specify that the version format in the setuptools grammar be just the PEP440 versions, and then use that grammar in twine check to validate more strictly. e.g. Wrap the grammar in build_grammar() and set REQUIREMENT = build_grammar(strict=False).

I'm happy to work on getting changes accepted into packaging and setuptools if this seems like a good path.
But for any of that to be relevant, twine check would have to start parsing install_requires, python_requires, and extras, which I don't think it does today.

@sigmavirus24
Copy link
Member

which I don't think it does today.

It doesn't.

It's also not 100% clear what you're proposing we check here. Should we be checking just python_version? python_version plus other items like install_requires? How vigorous is the checking? Do we check PyPI to ensure those package names exist?

@sirosen
Copy link
Author

sirosen commented Sep 17, 2021

I wouldn't expect twine to be contacting pypi to check that packages exist. I want the check to parse all of install_requires and extra_requires and just make sure it's syntactically valid.

Today, if I wrote install_requires = requests<>3, that's not a "real" specifier. I think twine check should error or at least warn on that.

Ideally the check would also be looking at python_version and platform conditions in the specifiers too, to solve my original use-case, but I understand that can get super tricky.

@bhrutledge
Copy link
Contributor

bhrutledge commented Sep 18, 2021

I think the issue as described is out of scope for Twine. My understanding is that Twine is agnostic of the build backend, i.e., I don't think there's any code specific to setuptools, and I think Twine can upload packages from flit, poetry, etc.

install_requires, python_requires, and anything else in setup.cfg/setup.py is specific to setuptools, so it seems like validating those is a job for setuptools.

What Twine does read is the standardized package metadata, via the pkginfo library. So, the equivalent of python_version and install_requires is Requires-Python and Requires-Dist.

twine/twine/package.py

Lines 160 to 166 in 3040bf9

# Metadata 1.2
"project_urls": meta.project_urls,
"provides_dist": meta.provides_dist,
"obsoletes_dist": meta.obsoletes_dist,
"requires_dist": meta.requires_dist,
"requires_external": meta.requires_external,
"requires_python": meta.requires_python,

However, I think this issue is fundamentally a duplicate of #430, which suggests using packaging for metadata validation, and is currently blocked on pypa/packaging#147.

Aside: setuptools currently doesn't write Requires-Dist, but wheel does, which is probably one of the reasons why Twine uploads wheels first.

@sirosen
Copy link
Author

sirosen commented Sep 18, 2021

I hadn't found #430 before filing, and would be comfortable closing this as a duplicate.

Also, thanks for clearing up my confusion on setuptools vs packaging. I didn't mean to phrase this as something specific to any given build backend -- I want twine check to read the dependency data from the packaged dists, not setup.{py,cfg} or pyproject.toml.

The only thing not captured by #430 that I want is to do stricter validation than what is done by pypi/warehouse. I don't think there's any value in supporting the loose "legacy version" parsing behavior if this check is added.

@bhrutledge
Copy link
Contributor

The only thing not captured by #430 that I want is to do stricter validation than what is done by pypi/warehouse.

I think that might already be under discussion in packaging and/or warehouse in existing issues. IIRC one of the hopes with pypa/packaging#147 is that packaging will define reusable validation for all backends and repositories.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants