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

utils.canonicalize_version produces an abnormal version #792

Open
jaraco opened this issue Apr 12, 2024 · 7 comments
Open

utils.canonicalize_version produces an abnormal version #792

jaraco opened this issue Apr 12, 2024 · 7 comments

Comments

@jaraco
Copy link
Member

jaraco commented Apr 12, 2024

In pypa/setuptools#4302, we learned that using packaging.utils.canonicalize_version to canonicalize a version produces an aberrant version.

 @ pip-run packaging -- -c 'import packaging.utils; print(packaging.utils.canonicalize_version("69.3.0"))'
69.3

Instead of matching the convention of wheels, which will produce a wheel with a version of "69.3.0", canonicalize_version strips the trailing zero and produces 69.3.

image

Moreover, semver indicates that the version number should include a trailing zero, so it's not possible to honor both semver and the canonicalized_version.

In pypa/setuptools#3593 (comment), @pfmoore says that wheels and sdists should use the same format.

What's the correct format?

Also, why does a function called canonicalize_version produce both canonical and non-canonical versions depending on a flag?

if strip_trailing_zero:
# NB: This strips trailing '.0's to normalize
release_segment = re.sub(r"(\.0)+$", "", release_segment)

I see that wheel implements its own safer_version function.

If packaging.utils.canonicalize_version isn't the right implementation for a PEP 625 canonicalized version, what is?

@jaraco
Copy link
Member Author

jaraco commented Apr 13, 2024

In pypa/setuptools#3593 (comment), I confirmed that the spec actually considers 1.0 to be a canonical version. Therefore, the fact that canonicalize_version transforms it is operating against the spec.

@pradyunsg
Copy link
Member

If packaging.utils.canonicalize_version isn't the right implementation for a PEP 625 canonicalized version, what is?

canonicalize_version(..., strip_trailing_zeros=False) does.

@pradyunsg
Copy link
Member

It might be worthwhile to make that the default on this helper function.

@Laurent-Dx
Copy link
Contributor

This was missing from the API doc, I made a PR #801.

@jaraco
Copy link
Member Author

jaraco commented May 18, 2024

I feel like there should be two functions - one for users to use when normalizing a version and another to use when comparing two versions, so it's clear that there are two purposes here and can give the user clear guidance as to which form to use for a given circumstance. It's an oxymoron for a version to have multiple canonical forms.

@uranusjr
Copy link
Member

For comparison I think Version should be the preferred API.

@brettcannon
Copy link
Member

It might be worthwhile to make that the default on this helper function.

How would you want to handle the transition on this? The parameter is 2.5 years old, so you know there are people out there using this as-is. We could remove the default for a release or two and re-introduce it flipped.

Or we can document it and not worry about it.

It's an oxymoron for a version to have multiple canonical forms.

Not necessarily. If you think there's One True Representation For All Cases, then I would agree. But versions get used in so many cases for so many reasons, I think the canonical version for comparison and canonical version for display can be different. I can see how that would then make sense to have two functions, but we also already have the one function w/ a toggle, so I'm not if it's worth the hassle of adding two more functions for this.

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

5 participants