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

Specifiers with .* and epoch changed behaviours #673

Closed
abravalheri opened this issue Jan 30, 2023 · 9 comments · Fixed by #674
Closed

Specifiers with .* and epoch changed behaviours #673

abravalheri opened this issue Jan 30, 2023 · 9 comments · Fixed by #674

Comments

@abravalheri
Copy link
Contributor

Hello, it seems that SpecifierSet.contains works differently between versions 21 and 23, when the == operator is used with a version that contains both an epoch and .*:

> docker run --rm -it python:3.11 /bin/bash

python3 -m venv /tmp/venv21

/tmp/venv21/bin/python3 -m pip install 'packaging==21.3'

cat <<EOF | /tmp/venv21/bin/python3 -
from packaging.requirements import Requirement

req = Requirement('metomi-isodatetime==1!3.0.*')

print(f"{req.specifier.contains('1!3.0.0', prereleases=True)=}")
EOF
# => req.specifier.contains('1!3.0.0', prereleases=True)=True

# ---

python3 -m venv /tmp/venv23

/tmp/venv23/bin/python3 -m pip install 'packaging==23.0'

cat <<EOF | /tmp/venv23/bin/python3 -
from packaging.requirements import Requirement

req = Requirement('metomi-isodatetime==1!3.0.*')

print(f"{req.specifier.contains('1!3.0.0', prereleases=True)=}")
EOF
# => req.specifier.contains('1!3.0.0', prereleases=True)=False

Is this change intended?

I went quickly trough PEP 440 and PEP 508 and I have the impression that this kind of requirement/comparison is allowed, but I might be wrong or missing something.


Note that when we remove the version epoch, the return value of contains is the same for both 21.3 and 23:

cat <<EOF | /tmp/venv21/bin/python3 -
from packaging.requirements import Requirement

req = Requirement('metomi-isodatetime==3.0.*')

print(f"{req.specifier.contains('3.0.0', prereleases=True)=}")
EOF
# => req.specifier.contains('3.0.0', prereleases=True)=True

cat <<EOF | /tmp/venv23/bin/python3 -
from packaging.requirements import Requirement

req = Requirement('metomi-isodatetime==3.0.*')

print(f"{req.specifier.contains('3.0.0', prereleases=True)=}")
EOF
# => req.specifier.contains('3.0.0', prereleases=True)=True

(Obs: related to pypa/setuptools#3802)

@abravalheri
Copy link
Contributor Author

The change in behaviour seem to have been introduced in version 22:

python3 -m venv /tmp/venv21
/tmp/venv21/bin/python3 -m pip install 'packaging==21.3'
cat <<EOF | /tmp/venv21/bin/python3 -
from packaging.specifiers import SpecifierSet
print(f'{SpecifierSet("==1!3.0.*").contains("1!3.0.0", prereleases=True)=}')
EOF
# => SpecifierSet("==1!3.0.*").contains("1!3.0.0", prereleases=True)=True

python3 -m venv /tmp/venv22
/tmp/venv22/bin/python3 -m pip install 'packaging==22.0'
cat <<EOF | /tmp/venv22/bin/python3 -
from packaging.specifiers import SpecifierSet
print(f'{SpecifierSet("==1!3.0.*").contains("1!3.0.0", prereleases=True)=}')
EOF
# => SpecifierSet("==1!3.0.*").contains("1!3.0.0", prereleases=True)=False

@pradyunsg
Copy link
Member

pradyunsg commented Jan 30, 2023

This was changed in #564

pip install -e .                  cat /tmp/test.py     
from packaging.specifiers import SpecifierSet

assert SpecifierSet("==1!3.0.*").contains("1!3.0.0", prereleases=True)git bisect good 21.3                 
status: waiting for bad commit, 1 good commit knowngit bisect bad 22.0 
Bisecting: 39 revisions left to test after this (roughly 5 steps)
[12800518fc4045ae2112c98f039a1a065cda24d0] Better output on linter failure in CI (#478)git bisect run python3 /tmp/test.py                                         
running  'python3' '/tmp/test.py'
Bisecting: 19 revisions left to test after this (roughly 4 steps)
[e91b50eef06d6f1bc475caf7da006facbec684a6] Rename `req` to `parsed` in `Requirement.__init__`
running  'python3' '/tmp/test.py'
Traceback (most recent call last):
  File "/tmp/test.py", line 3, in <module>
    assert SpecifierSet("==1!3.0.*").contains("1!3.0.0", prereleases=True)
AssertionError
Bisecting: 9 revisions left to test after this (roughly 3 steps)
[184fde738ebd3d639b10383cc7b2213fd5b4a03b] Update changelog (#595)
running  'python3' '/tmp/test.py'
Traceback (most recent call last):
  File "/tmp/test.py", line 3, in <module>
    assert SpecifierSet("==1!3.0.*").contains("1!3.0.0", prereleases=True)
AssertionError
Bisecting: 4 revisions left to test after this (roughly 2 steps)
[3223f8ca4ed2b2aa431c7ff9e526f6e543dea2f4] Update coverage to `>=5.0.0` (#586)
running  'python3' '/tmp/test.py'
Bisecting: 2 revisions left to test after this (roughly 1 step)
[8c6a45e20857db912d397afb489832d9528a84c8] Add python 3.11 to CI (#587)
running  'python3' '/tmp/test.py'
Bisecting: 0 revisions left to test after this (roughly 1 step)
[5f46d1532f45ae32cd7831c6f45212f7066c1ed0] Remove duplicate `namedtuple` (#589)
running  'python3' '/tmp/test.py'
Traceback (most recent call last):
  File "/tmp/test.py", line 3, in <module>
    assert SpecifierSet("==1!3.0.*").contains("1!3.0.0", prereleases=True)
AssertionError
Bisecting: 0 revisions left to test after this (roughly 0 steps)
[e184feef1a28a5c574ec41f5c263a3a573861f5a] Fix prefix version matching (#564)
running  'python3' '/tmp/test.py'
Traceback (most recent call last):
  File "/tmp/test.py", line 3, in <module>
    assert SpecifierSet("==1!3.0.*").contains("1!3.0.0", prereleases=True)
AssertionError
e184feef1a28a5c574ec41f5c263a3a573861f5a is the first bad commit
commit e184feef1a28a5c574ec41f5c263a3a573861f5a
Author: Matthieu Darbois <mayeut@users.noreply.github.com>
Date:   Fri Sep 23 23:26:36 2022 +0200

    Fix prefix version matching (#564)
    
    0-padding shall only be applied on the prospective version, before shortening,
    in order to get the correct shortened prospective version.

 packaging/specifiers.py  | 18 ++++++++----------
 tests/test_specifiers.py |  6 ++++++
 2 files changed, 14 insertions(+), 10 deletions(-)
bisect found first bad commit

@pradyunsg pradyunsg changed the title SpecifierSet.contains: differences between version 21 and 23 for == operator with .* and epoch Specifiers with .* and epoch changed behaviours Jan 30, 2023
@pradyunsg
Copy link
Member

pradyunsg commented Jan 30, 2023

return shortened_prospective == split_spec

The locals at this point are:

self = <Specifier('==2!1.0.*', prereleases=True)>
prospective = <Version('2!1.0.0')>
spec = '2!1.0.*'
normalized_prospective = '2!1'
normalized_spec = '2!1.0'
split_spec = ['2!1', '0']
split_prospective = ['2!1']
padded_prospective = ['2!1']
shortened_prospective = ['2!1']

@pradyunsg
Copy link
Member

pradyunsg commented Jan 30, 2023

diff --git a/src/packaging/specifiers.py b/src/packaging/specifiers.py
index e715ecc..5f33045 100644
--- a/src/packaging/specifiers.py
+++ b/src/packaging/specifiers.py
@@ -398,7 +398,9 @@ def _compare_equal(self, prospective: Version, spec: str) -> bool:
         # We need special logic to handle prefix matching
         if spec.endswith(".*"):
             # In the case of prefix matching we want to ignore local segment.
-            normalized_prospective = canonicalize_version(prospective.public)
+            normalized_prospective = canonicalize_version(
+                prospective.public, strip_trailing_zero=False
+            )
             # Get the normalized version string ignoring the trailing .*
             normalized_spec = canonicalize_version(spec[:-2], strip_trailing_zero=False)
             # Split the spec out by dots, and pretend that there is an implicit
diff --git a/tests/test_specifiers.py b/tests/test_specifiers.py
index 104fac9..799f873 100644
--- a/tests/test_specifiers.py
+++ b/tests/test_specifiers.py
@@ -369,6 +369,8 @@ def test_comparison_non_specifier(self):
                 ("2!1.0", "==2!1.*"),
                 ("2!1.0", "==2!1.0"),
                 ("2!1.0", "!=1.0"),
+                ("2!1.0.0", "==2!1.0.*"),
+                ("2!1.0.0", "==2!1.*"),
                 ("1.0", "!=2!1.0"),
                 ("1.0", "<=2!0.1"),
                 ("2!1.0", ">=2.0"),

Seems like an easy fix.

self = <Specifier('==1!3.0.*')>
prospective = <Version('1!3.0.0')>
spec = '1!3.0.*'
normalized_prospective = '1!3.0.0'
normalized_spec = '1!3.0'
split_spec = ['1!3', '0']
split_prospective = ['1!3', '0', '0']
padded_prospective = ['1!3', '0', '0']
shortened_prospective = ['1!3', '0']

Lawouach added a commit to chaostoolkit-incubator/chaostoolkit-opentracing that referenced this issue Feb 1, 2023
Signed-off-by: Sylvain Hellegouarch <sh@defuze.org>
@Lawouach
Copy link

Lawouach commented Feb 1, 2023

Hello,

Any chance for a rapid release with this fix? As it stands, this breaks any installation which is using up to date packaging.

@brettcannon
Copy link
Member

Any chance for a rapid release with this fix?

#674 should fix this, so we can get it fixed in main pretty quickly. But we have not discussed if a 23.1 release for this specific issue is necessary yet. If you want to avoid this problematic release for you, you could update your requirements to ignore 23.0 so that you still get the newest version while skipping this bug: packaging!=23.0.

@pradyunsg
Copy link
Member

pradyunsg commented Feb 1, 2023

@Lawouach Based on the commit cross-referenced, your issue isn't related to this but rather the fact that you're using .* with an operator other than == or != -- that is not valid. See #657 (comment)

@Lawouach
Copy link

Lawouach commented Feb 2, 2023

Hello @pradyunsg, be sure I assume you have much more knowledge of the spec than I do :)

But, when I read PEP 440 inclusive ordered comparison section, I can't see a clear indication that ">=X.Y.*" is forbidden.

@pradyunsg
Copy link
Member

pradyunsg commented Feb 2, 2023

It's the other way around: it's only defined as usable with the operators mentioned. You'll notice wildcard and prefix are only mentioned with matching operators. :)

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