Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Lazily evaluate markers to mitigate non-PEP 440 version errors #834

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 33 additions & 4 deletions src/packaging/markers.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from ._tokenizer import ParserSyntaxError
from .specifiers import InvalidSpecifier, Specifier
from .utils import canonicalize_name
from .version import InvalidVersion

__all__ = [
"InvalidMarker",
Expand Down Expand Up @@ -202,7 +203,7 @@ def _normalize(*values: str, key: str) -> tuple[str, ...]:


def _evaluate_markers(markers: MarkerList, environment: dict[str, str]) -> bool:
groups: list[list[bool]] = [[]]
groups: list[list[bool | Exception]] = [[]]

for marker in markers:
assert isinstance(marker, (list, tuple, str))
Expand All @@ -222,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(_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(item) for item 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}"
Expand Down
21 changes: 20 additions & 1 deletion tests/test_markers.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
default_environment,
format_full_version,
)
from packaging.version import InvalidVersion

VARIABLES = [
"extra",
Expand Down Expand Up @@ -317,11 +318,29 @@ 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,
),
(
"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",
Expand Down