From 9167919b958268ef602dc623a59ac49a51455380 Mon Sep 17 00:00:00 2001 From: Edward Peek Date: Thu, 19 Sep 2024 16:18:52 +1200 Subject: [PATCH 1/2] Lazily evaluate markers to mitigate non-PEP 440 version errors --- src/packaging/markers.py | 10 ++++++---- tests/test_markers.py | 5 +++++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/packaging/markers.py b/src/packaging/markers.py index fb7f49cf..311d66aa 100644 --- a/src/packaging/markers.py +++ b/src/packaging/markers.py @@ -4,6 +4,7 @@ from __future__ import annotations +import functools import operator import os import platform @@ -202,13 +203,14 @@ def _normalize(*values: str, key: str) -> tuple[str, ...]: def _evaluate_markers(markers: MarkerList, environment: dict[str, str]) -> bool: - groups: list[list[bool]] = [[]] + # Lazy evaluation to mitigate https://github.com/pypa/packaging/issues/774 + groups: list[list[Callable[[], bool]]] = [[]] for marker in markers: assert isinstance(marker, (list, tuple, str)) if isinstance(marker, list): - groups[-1].append(_evaluate_markers(marker, environment)) + groups[-1].append(functools.partial(_evaluate_markers, marker, environment)) elif isinstance(marker, tuple): lhs, op, rhs = marker @@ -222,13 +224,13 @@ def _evaluate_markers(markers: MarkerList, environment: dict[str, str]) -> bool: rhs_value = environment[environment_key] lhs_value, rhs_value = _normalize(lhs_value, rhs_value, key=environment_key) - groups[-1].append(_eval_op(lhs_value, op, rhs_value)) + groups[-1].append(functools.partial(_eval_op, lhs_value, op, rhs_value)) else: assert marker in ["and", "or"] if marker == "or": groups.append([]) - return any(all(item) for item in groups) + return any(all(expr() for expr in group) for group in groups) def format_full_version(info: sys._version_info) -> str: diff --git a/tests/test_markers.py b/tests/test_markers.py index cf08b99e..cdfd9d52 100644 --- a/tests/test_markers.py +++ b/tests/test_markers.py @@ -317,6 +317,11 @@ def test_environment_with_extra_none(self): {"extra": "different__punctuation_is_EQUAL"}, True, ), + ( + "sys_platform == 'foo_os' and platform_release >= '4.5.6'", + {"sys_platform": "bar_os", "platform_release": "1.2.3-invalid"}, + False, + ) ], ) def test_evaluates(self, marker_string, environment, expected): From 8a82eb267f1dc66061c65850891e938738355899 Mon Sep 17 00:00:00 2001 From: Edward Peek Date: Tue, 12 Nov 2024 14:40:54 +1300 Subject: [PATCH 2/2] Ignore certain exceptions during marker evaluation --- src/packaging/markers.py | 41 +++++++++++++++++++++++++++++++++------- tests/test_markers.py | 18 ++++++++++++++++-- 2 files changed, 50 insertions(+), 9 deletions(-) diff --git a/src/packaging/markers.py b/src/packaging/markers.py index 311d66aa..18811415 100644 --- a/src/packaging/markers.py +++ b/src/packaging/markers.py @@ -4,7 +4,6 @@ from __future__ import annotations -import functools import operator import os import platform @@ -16,6 +15,7 @@ from ._tokenizer import ParserSyntaxError from .specifiers import InvalidSpecifier, Specifier from .utils import canonicalize_name +from .version import InvalidVersion __all__ = [ "InvalidMarker", @@ -203,14 +203,13 @@ def _normalize(*values: str, key: str) -> tuple[str, ...]: def _evaluate_markers(markers: MarkerList, environment: dict[str, str]) -> bool: - # Lazy evaluation to mitigate https://github.com/pypa/packaging/issues/774 - groups: list[list[Callable[[], bool]]] = [[]] + groups: list[list[bool | Exception]] = [[]] for marker in markers: assert isinstance(marker, (list, tuple, str)) if isinstance(marker, list): - groups[-1].append(functools.partial(_evaluate_markers, marker, environment)) + groups[-1].append(_evaluate_markers(marker, environment)) elif isinstance(marker, tuple): lhs, op, rhs = marker @@ -224,14 +223,42 @@ def _evaluate_markers(markers: MarkerList, environment: dict[str, str]) -> bool: rhs_value = environment[environment_key] lhs_value, rhs_value = _normalize(lhs_value, rhs_value, key=environment_key) - groups[-1].append(functools.partial(_eval_op, lhs_value, op, rhs_value)) + + # Defer handling certain exceptions for cases where the marker expression is + # otherwise well formed and they do not end up affecting the overall result. + op_result: bool | Exception + try: + op_result = _eval_op(lhs_value, op, rhs_value) + # Version comparisons may be overly strict despite being guarded against. + # https://github.com/pypa/packaging/issues/774 + except InvalidVersion as e: + op_result = e + + groups[-1].append(op_result) else: assert marker in ["and", "or"] if marker == "or": groups.append([]) - return any(all(expr() for expr in group) for group in groups) - + # The below is almost equivalent to `any(all(group) for group in groups)` except + # that exceptions are treated as an indeterminate logic level between true and false + any_result: bool | Exception = False + for group in groups: + all_result: bool | Exception = True + for op_result in group: + # Value precedence for `all()` is: `False`, `Exception()`, `True` + if (all_result is True) or (op_result is False): + all_result = op_result + + # Value precedence for `any()` is: `True`, `Exception()`, `False` + if (any_result is False) or (all_result is True): + any_result = all_result + + # Raise if the overall result is indeterminate due to a expression that errored out + if isinstance(any_result, Exception): + raise any_result + + return any_result def format_full_version(info: sys._version_info) -> str: version = f"{info.major}.{info.minor}.{info.micro}" diff --git a/tests/test_markers.py b/tests/test_markers.py index cdfd9d52..4571d175 100644 --- a/tests/test_markers.py +++ b/tests/test_markers.py @@ -20,6 +20,7 @@ default_environment, format_full_version, ) +from packaging.version import InvalidVersion VARIABLES = [ "extra", @@ -321,12 +322,25 @@ def test_environment_with_extra_none(self): "sys_platform == 'foo_os' and platform_release >= '4.5.6'", {"sys_platform": "bar_os", "platform_release": "1.2.3-invalid"}, False, - ) + ), + ( + "platform_release >= '4.5.6' and sys_platform == 'foo_os'", + {"sys_platform": "bar_os", "platform_release": "1.2.3-invalid"}, + False, + ), + ( + "platform_release >= '4.5.6'", + {"platform_release": "1.2.3-invalid"}, + InvalidVersion, + ), ], ) def test_evaluates(self, marker_string, environment, expected): args = [] if environment is None else [environment] - assert Marker(marker_string).evaluate(*args) == expected + try: + assert Marker(marker_string).evaluate(*args) == expected + except Exception as e: + assert isinstance(e, expected) @pytest.mark.parametrize( "marker_string",