markers: Consistently fallback to Python comparison rules

In the dependency specifiers specification (originally PEP 508), when
there is no PEP 440 defined behaviour for evaluating a marker because
one or both sides are not valid versions [specifiers], a fallback to
Python comparison behaviour is mandated:

> The <marker_op> operators that are not in <version_cmp> perform the
> same as they do for strings in Python. The <version_cmp> operators use
> the version comparison rules of the Version specifier specification
> when those are defined (that is when both sides have a valid version
> specifier). If there is no defined behaviour of this specification and
> the operator exists in Python, then the operator falls back to the
> Python behaviour. Otherwise an error should be raised.

However, this fallback has been broken for a while. When the right
side has PEP 440 defined semantics (being a valid specifier), but the
left side is an invalid version, an InvalidVersion error is raised.

This patch suppresses said InvalidVersion error and fallbacks as
expected.

Author: ichard26

CHANGELOG.rst

@@ -4,7 +4,10 @@ Changelog
 *unreleased*
 ~~~~~~~~~~~~
 
-No unreleased changes.
+* Consistently fallback to Python string comparison behaviour when evaluating
+  a marker where there is no PEP 440 defined behaviour, notably when the right
+  side has PEP 440 defined semantics, but the left side is an invalid
+  version (:issue:`774`)
 
 24.1 - 2024-06-10
 ~~~~~~~~~~~~~~~~~

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
 
 __all__ = [
     "InvalidMarker",
@@ -178,9 +179,18 @@ def _eval_op(lhs: str, op: Op, rhs: str) -> bool:
     try:
         spec = Specifier("".join([op.serialize(), rhs]))
     except InvalidSpecifier:
+        # As per the specification, if there is no PEP 440 defined
+        # behaviour because the right side is not a valid version
+        # specifier, fallback to Python string comparision behaviour.
         pass
     else:
-        return spec.contains(lhs, prereleases=True)
+        try:
+            return spec.contains(lhs, prereleases=True)
+        except InvalidVersion:
+            # Even though there is PEP 440 defined behaviour for the
+            # right side, fallback as the left left is not a valid
+            # version.
+            pass
 
     oper: Operator | None = _operators.get(op.serialize())
     if oper is None:

tests/test_markers.py

@@ -20,7 +20,6 @@
     default_environment,
     format_full_version,
 )
-from packaging.version import InvalidVersion
 
 VARIABLES = [
     "extra",
@@ -388,12 +387,38 @@ def test_extra_str_normalization(self):
         assert str(Marker(rhs)) == f'extra == "{normalized_name}"'
 
     def test_python_full_version_untagged_user_provided(self):
-        """A user-provided python_full_version ending with a + fails to parse."""
-        with pytest.raises(InvalidVersion):
-            Marker("python_full_version < '3.12'").evaluate(
-                {"python_full_version": "3.11.1+"}
-            )
+        """A user-provided python_full_version ending with a + uses Python behaviour."""
+        env = {"python_full_version": "3.11.1+"}
+        assert Marker("python_full_version < '3.12'").evaluate(env)
 
     def test_python_full_version_untagged(self):
         with mock.patch("platform.python_version", return_value="3.11.1+"):
             assert Marker("python_full_version < '3.12'").evaluate()
+
+    @pytest.mark.parametrize(
+        ("marker_string", "environment", "expected"),
+        [
+            ("platform_release >= '20.0'", {"platform_release": "21-foobar"}, True),
+            ("platform_release >= '8'", {"platform_release": "6.7.0-gentoo"}, False),
+            ("platform_version == '27'", {"platform_version": "weird string"}, False),
+            (
+                "implementation_version == '3.*'",
+                {"implementation_version": "2_private"},
+                False,
+            ),
+            (
+                "implementation_version == '3.*'",
+                {"implementation_version": "3.*"},
+                True,
+            ),
+        ],
+    )
+    def test_valid_specifier_invalid_version_fallback_to_python(
+        self, marker_string: str, environment: dict, expected: bool
+    ):
+        """If the right operand is a valid version specifier, but the
+        left operand is not a valid version, fallback to Python string
+        comparison behaviour.
+        """
+        args = [] if environment is None else [environment]
+        assert Marker(marker_string).evaluate(*args) == expected