setuptools-rust icon indicating copy to clipboard operation
setuptools-rust copied to clipboard

Replace semantic_version with packaging

Open tiran opened this issue 4 years ago • 6 comments

Use the "packaging" package instead of semantic_version package to parse Rust version and spec. Packaging is used by setuptools to parse version strings and specs.

This also solves a deprecation warning with semamtic_version. The partial argument to Version() has been deprecated.

Signed-off-by: Christian Heimes [email protected]

tiran avatar Jul 26 '21 10:07 tiran

Thanks for this. I'm definitely in favour of updating dependencies.

Using packaging to parse Rust versions makes me slightly uneasy. e.g. https://www.python.org/dev/peps/pep-0440/#semantic-versioning explicity says that the PEP 440 "Version" is not compatible with semantic version prereleases. Does this mean therefore that this PR would break compatibility with Rust nightly and beta releases?

I wonder if instead we can fix usage of the semantic_version package. Or is it totally deprecated?

davidhewitt avatar Jul 27 '21 21:07 davidhewitt

semantic_version is still supported, just the partial argument was deprecated in 2.7, https://github.com/rbarrois/python-semanticversion/blob/master/ChangeLog#270-2019-08-28

I'll investigate support for pre-releases with packaging and get back to you.

tiran avatar Jul 28 '21 10:07 tiran

Packaging's version parser parses Rust's beta versions correctly, e.g. rustc 1.55.0-beta.1 (739f8f0a8 2021-07-28) is recognized as 1.55.0b1.

>>> packaging.version.Version("1.55.0-beta.1") < packaging.version.Version("1.55")
True
>>> packaging.version.Version("1.55.0-beta.1") < packaging.version.Version("1.55.0-alpha.1")
False
>>> packaging.version.Version("1.55.0-beta.1") < packaging.version.Version("1.55.0-rc.1")
True

However it does not understand -nightly suffix:

>>> packaging.version.Version("1.56.0-nightly")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3.9/site-packages/packaging/version.py", line 298, in __init__
    raise InvalidVersion("Invalid version: '{0}'".format(version))
packaging.version.InvalidVersion: Invalid version: '1.56.0-nightly'

tiran avatar Jul 29 '21 10:07 tiran

The error message raised from get_rust_version if the compiler is not suitable will now use packaging syntax when it formats min_version on utils.py line 73. I think it'd potentially be confusing for users to see Rust versions which don't follow the actual Rust release syntax, so we should map the .bx back to -beta.x and .dev0 back to -nightly in the error.

min_version is provided as an argument to get_rust_version. It doesn't use the representation of packaging.version.Version.

tiran avatar Aug 03 '21 11:08 tiran

min_version is provided as an argument to get_rust_version. It doesn't use the representation of packaging.version.Version.

Not sure I follow - I think it's a packaging.version.SpecifierSet now, if I follow the code correctly? (If you're willing to add type annotations to any changed code, that's both welcome and appreciated)

We map the rustc specs to SpecifierSet now, so the error message will have changed?

Also, I think I've spotted another potential issue - it looks like we currently depend on ordering semantic_version.Version objects to determine the "minimum" version:

https://github.com/PyO3/setuptools-rust/blob/fab42d48cdce6081bfa57a1add39d1ef2b9414fb/setuptools_rust/command.py#L30-L36

It looks like packaging.SpecifierSet doesn't implement ordering operators, so this code will break?

davidhewitt avatar Aug 05 '21 22:08 davidhewitt

Note that I've now removed the deprecated partial flag in #174

In general I'm now leaning towards the opinion that we should stick to semantic_version instead of using packaging. There are enough differences between the Python package versions packaging wants and the semver-compliant Rust compiler versions; as we've proceeded with this PR we have hit a few kinks which have made me think that switching dependency won't make things simpler.

Unless you would like to make a further case for packaging, I propose to close this PR. Thanks for all your work on it.

davidhewitt avatar Sep 22 '21 07:09 davidhewitt