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

Compatible version specifier merge incorrectly strip trailing "0" components #421

Closed
uranusjr opened this issue Apr 17, 2021 · 7 comments · Fixed by #493
Closed

Compatible version specifier merge incorrectly strip trailing "0" components #421

uranusjr opened this issue Apr 17, 2021 · 7 comments · Fixed by #493

Comments

@uranusjr
Copy link
Member

uranusjr commented Apr 17, 2021

>>> from packaging.specifiers import SpecifierSet
>>> SpecifierSet("~=1.18.0") & SpecifierSet("~=1.18")
SpecifierSet("~=1.18")

This is wrong. According to PEP 440:

  • ~=1.18.0 means >=1.18.0, ==1.18.*
  • ~=1.18 means >=1.18, ==1.*

So merging them should give

  • Naive merge: >=1.18, >=1.18.0, ==1.*, ==1.18.*
  • Equivalent specifiers stripped: >=1.18, ==1.*, ==1.18.*
  • Removed strict supersets >=1.18, ==1.18.* ← This is where packaging currently gets wrong.

i.e. ~=1.18.0.

Originally reported in pypa/pip#9613.

@kasium
Copy link
Contributor

kasium commented Dec 13, 2021

Any update so far?

@pradyunsg
Copy link
Member

If there were any, they would've been posted on this issue. If you're interested in seeing this get resolved, a PR fixing this would be welcome.

@kasium
Copy link
Contributor

kasium commented Dec 13, 2021

@pradyunsg thanks for the info. I'll take a look if I might can provide a PR

@kasium
Copy link
Contributor

kasium commented Dec 13, 2021

So as far as I understand that, the following code should return always the more strict version, right?

from packaging.specifiers import SpecifierSet
SpecifierSet("~=1.18") & SpecifierSet("~=1.18.0")

Returns SpecifierSet("~=1.18", but should return SpecifierSet("~=1.18.0")

Currently, it returns the specs of the left side. Therefore my question, is there already a function which returns the more strict version? If not, would the Specifier class the right place?

See https://github.com/pypa/packaging/blob/main/packaging/specifiers.py#L667

@uranusjr
Copy link
Member Author

You don’t really need such a function (logically; cosmetic is another issue). A SpecifierSet is the sum of its parts, so instead of returning SpecifierSet("~=1.18.0"), you can just return SpecifierSet("~=1.18,~=1.18.0") to achieve the same purpose. The root problem here is that, the current implementation considers Specifier("~=1.18") and Specifier("~=1.18.0") to be equivalent, which is wrong.

So the solution would be to fix _IndividualSpecifier’s __hash__ and __eq__ to special-case ~=. Both of these rely on _canonical_spec, which in turn relies on canonicalize_version, which strips trailing zeros. This is fine most of the time, but not for ~=. So the solution is to allow canonicalize_version to not strip zeros (maybe with an optional flag, not sure about this part), and add an if-else in _canonical_spec to not strip zeros if the operator (_spec[0]) is ~=.

@kasium
Copy link
Contributor

kasium commented Dec 13, 2021

Great, thanks!

@kasium
Copy link
Contributor

kasium commented Jan 17, 2022

Can you please release a new version with the fix?

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

Successfully merging a pull request may close this issue.

4 participants