From 60d693bb4f89c8c2ac29f9f3fbf7564a1e765a01 Mon Sep 17 00:00:00 2001 From: Hal Blackburn Date: Thu, 27 Mar 2025 02:47:35 +0000 Subject: [PATCH 1/3] Add failing test for marker eval on versions This commit adds test cases to cover version-to-version and non-version-to-version marker evaluation, according to PEP 508, Environment Markers. https://peps.python.org/pep-0508/#environment-markers Marker evaluation currently throws an InvalidVersion error when the rhs is a valid version but the lhs is not. For example, consider the dependency: > Requires-Dist: typing-extensions (>=4,<5) ; extra == "v8" The extra name "v8" is both a valid version and extra name. During evaluation of this requirement, the LHS of the marker can take on the value "", or some other extra name, which are not valid versions. When this situation occurs, the comparison fails by throwing InvalidVersion, rather then returning False. This is causing pip to crash when attempting to install a package with such a requirement. Currently, just one of these added cases is failing - the "quux" == "v8" case. I've also added two cases to document the (perhaps surprising) behaviour that is a result of the PEP 508 spec requiring normalized version comparison to apply to extras when both sides are valid versions. --- tests/test_markers.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/test_markers.py b/tests/test_markers.py index 5106427db..a729869e2 100644 --- a/tests/test_markers.py +++ b/tests/test_markers.py @@ -314,6 +314,14 @@ def test_environment_with_extra_none(self): {"extra": "different__punctuation_is_EQUAL"}, True, ), + # extra name that is also a valid version - version comparison applies + # if both sides are valid versions + ("extra == 'v8'", {"extra": "quux"}, False), + ("extra == 'v8'", {"extra": "v8"}, True), + # LHS and RHS are valid versions, so equality uses normalized version + ("extra == 'v8-dev'", {"extra": "v8-dev.0"}, True), + # LHS and RHS are not valid versions, so equality is str comparison + ("extra == 'v8-foo'", {"extra": "v8-foo.0"}, False), ], ) def test_evaluates(self, marker_string, environment, expected): From 69a86c68ac9cbaeeb9c3fb97a29167b39d006bfa Mon Sep 17 00:00:00 2001 From: Hal Blackburn Date: Thu, 27 Mar 2025 03:06:12 +0000 Subject: [PATCH 2/3] Fix Marker non-version-to-version eval failing As described in my previous commit to add test cases covering this behaviour, PEP 508 - Environment Markers section specifies that markers are evaluated using version operator rules when both sides are valid versions, and otherwise regular python operator rules apply. However, the implementation was failing to handle the case where the RHS was a valid version specifier, but the LHS was not a valid version. In this case, PEP 508 requires the comparison falls back to python rules, but the implementation was throwing InvalidVersion. The implementation now matches the behaviour in the spec, as we now check that both the LHS and the RHS are versions before attempting version comparison (not just the RHS as before). --- src/packaging/markers.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/packaging/markers.py b/src/packaging/markers.py index a4889cddd..a262f9618 100644 --- a/src/packaging/markers.py +++ b/src/packaging/markers.py @@ -15,6 +15,7 @@ from ._tokenizer import ParserSyntaxError from .specifiers import InvalidSpecifier, Specifier from .utils import canonicalize_name +from .version import InvalidVersion, Version __all__ = [ "EvaluateContext", @@ -179,12 +180,20 @@ def _format_marker( def _eval_op(lhs: str, op: Op, rhs: str | AbstractSet[str]) -> bool: if isinstance(rhs, str): + # PEP 508 - Environment Markers + # https://peps.python.org/pep-0508/#environment-markers + # > The operators use the PEP 440 version comparison rules + # > when those are defined (that is when both sides have a valid version + # > specifier). If there is no defined PEP 440 behaviour and the operator + # > exists in Python, then the operator falls back to the Python behaviour. + # > Otherwise an error should be raised. try: spec = Specifier("".join([op.serialize(), rhs])) - except InvalidSpecifier: + lhs_ver = Version(lhs) + except (InvalidSpecifier, InvalidVersion): pass else: - return spec.contains(lhs, prereleases=True) + return spec.contains(lhs_ver, prereleases=True) oper: Operator | None = _operators.get(op.serialize()) if oper is None: From aff2962e4a9716051388903cf5ad0e62052b1b6e Mon Sep 17 00:00:00 2001 From: Hal Blackburn Date: Sat, 26 Jul 2025 07:05:02 +0000 Subject: [PATCH 3/3] Add tests to document counter-intuitive version comparison As discussed in https://github.com/pypa/packaging/issues/774, falling back to lexicographic string comparison when using greater/less-than operators can result in counter-intuitive behaviour on version-like numeric strings. --- tests/test_markers.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/test_markers.py b/tests/test_markers.py index a729869e2..d75d3f057 100644 --- a/tests/test_markers.py +++ b/tests/test_markers.py @@ -322,6 +322,15 @@ def test_environment_with_extra_none(self): ("extra == 'v8-dev'", {"extra": "v8-dev.0"}, True), # LHS and RHS are not valid versions, so equality is str comparison ("extra == 'v8-foo'", {"extra": "v8-foo.0"}, False), + # + # When using order comparisons, fallback to str when one or both are + # not valid versions can have counter-intuitive results. + # + # The gentoo value is not a valid version, lexicographic str + # comparison applies. This is True because '6' >= '2'. + ("platform_release >= '20'", {"platform_release": "6.7.0-gentoo"}, True), + # Whereas this is False because the gentoo value IS a valid version + ("platform_release >= '20'", {"platform_release": "6.7.0+gentoo"}, False), ], ) def test_evaluates(self, marker_string, environment, expected):