From f39b0b30bc1a018ae061825341ae967215a7976d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Randy=20D=C3=B6ring?= <30527984+radoering@users.noreply.github.com> Date: Sat, 18 Jan 2025 09:00:46 +0100 Subject: [PATCH] perf: add support for AtomicMultiMarker and AtomicMarkerUnion for extra markers --- src/poetry/core/packages/dependency.py | 9 +- src/poetry/core/version/markers.py | 50 ++++--- tests/packages/test_main.py | 10 ++ tests/version/test_markers.py | 192 +++++++++++++++++++++++++ 4 files changed, 240 insertions(+), 21 deletions(-) diff --git a/src/poetry/core/packages/dependency.py b/src/poetry/core/packages/dependency.py index f5670648a..0db71b06a 100644 --- a/src/poetry/core/packages/dependency.py +++ b/src/poetry/core/packages/dependency.py @@ -161,9 +161,12 @@ def marker(self, marker: str | BaseMarker) -> None: for op, extra in or_: if op == "==": new_in_extras.append(canonicalize_name(extra)) - elif op == "" and "||" in extra: - for _extra in extra.split(" || "): - new_in_extras.append(canonicalize_name(_extra)) + elif op == "" and ("||" in extra or "," in extra): + sep = "||" if "||" in extra else "," + extra_values = [e.strip() for e in extra.split(sep)] + for _extra in extra_values: + if not _extra.startswith("!="): + new_in_extras.append(canonicalize_name(_extra)) self._in_extras = [*self._in_extras, *new_in_extras] # Recalculate python versions. diff --git a/src/poetry/core/version/markers.py b/src/poetry/core/version/markers.py index b8bfb6d0c..67e370661 100644 --- a/src/poetry/core/version/markers.py +++ b/src/poetry/core/version/markers.py @@ -233,6 +233,7 @@ def __init__(self, name: str, constraint: SingleMarkerConstraint) -> None: from poetry.core.constraints.generic import ( parse_constraint as parse_generic_constraint, ) + from poetry.core.constraints.generic import parse_extra_constraint from poetry.core.constraints.version import parse_marker_version_constraint self._name = ALIASES.get(name, name) @@ -242,6 +243,8 @@ def __init__(self, name: str, constraint: SingleMarkerConstraint) -> None: self._parser = functools.partial( parse_marker_version_constraint, pep440=name != "platform_release" ) + elif name == "extra": + self._parser = parse_extra_constraint else: self._parser = parse_generic_constraint @@ -358,6 +361,7 @@ def __init__( from poetry.core.constraints.generic import ( parse_constraint as parse_generic_constraint, ) + from poetry.core.constraints.generic import parse_extra_constraint from poetry.core.constraints.version import parse_marker_version_constraint parsed_constraint: BaseConstraint | VersionConstraint @@ -381,7 +385,7 @@ def __init__( self._operator = "==" self._value = m.group("value") - parser = parse_generic_constraint + parser = parse_extra_constraint if name == "extra" else parse_generic_constraint if swapped_name_value and name not in PYTHON_VERSION_MARKERS: # Something like `"tegra" in platform_release` @@ -516,14 +520,21 @@ def __str__(self) -> str: class AtomicMultiMarker(SingleMarkerLike[MultiConstraint]): def __init__(self, name: str, constraint: MultiConstraint) -> None: - assert all(c.operator == "!=" for c in constraint.constraints) + assert all( + c.operator in ({"==", "!="} if name == "extra" else {"!="}) + for c in constraint.constraints + ) super().__init__(name, constraint) - self._values: list[str] = [] @property def complexity(self) -> tuple[int, int]: return len(self._constraint.constraints), 1 + def validate(self, environment: Mapping[str, Any] | None) -> bool: + if self._name == "extra": + return self.expand().validate(environment) + return super().validate(environment) + def invert(self) -> BaseMarker: return AtomicMarkerUnion(self._name, self._constraint.invert()) @@ -534,14 +545,16 @@ def expand(self) -> MultiMarker: def __str__(self) -> str: return " and ".join( - f'{self._name} != "{c.value}"' for c in self._constraint.constraints + f'{self._name} {c.operator} "{c.value}"' + for c in self._constraint.constraints ) class AtomicMarkerUnion(SingleMarkerLike[UnionConstraint]): def __init__(self, name: str, constraint: UnionConstraint) -> None: assert all( - isinstance(c, Constraint) and c.operator == "==" + isinstance(c, Constraint) + and c.operator in ({"==", "!="} if name == "extra" else {"=="}) for c in constraint.constraints ) super().__init__(name, constraint) @@ -550,6 +563,11 @@ def __init__(self, name: str, constraint: UnionConstraint) -> None: def complexity(self) -> tuple[int, int]: return len(self._constraint.constraints), 1 + def validate(self, environment: Mapping[str, Any] | None) -> bool: + if self._name == "extra": + return self.expand().validate(environment) + return super().validate(environment) + def invert(self) -> BaseMarker: return AtomicMultiMarker(self._name, self._constraint.invert()) @@ -563,7 +581,7 @@ def __str__(self) -> str: # contains only elements of type Constraint (instead of BaseConstraint) # but mypy can't see that. return " or ".join( - f'{self._name} == "{c.value}"' # type: ignore[attr-defined] + f'{self._name} {c.operator} "{c.value}"' # type: ignore[attr-defined] for c in self._constraint.constraints ) @@ -755,7 +773,7 @@ def __hash__(self) -> int: def __str__(self) -> str: elements = [] for m in self._markers: - if isinstance(m, (SingleMarker, MultiMarker)): + if isinstance(m, (SingleMarker, MultiMarker, AtomicMultiMarker)): elements.append(str(m)) else: elements.append(f"({m})") @@ -1009,7 +1027,9 @@ def _compact_markers( groups.append([]) # Combine the groups. - sub_markers = [MultiMarker(*group) for group in groups] + sub_markers = [ + group[0] if len(group) == 1 else MultiMarker(*group) for group in groups + ] # This function calls itself recursively. In the inner calls we don't perform any # simplification, instead doing it all only when we have the complete marker. @@ -1094,14 +1114,6 @@ def _merge_single_markers( if marker1.name != marker2.name: return None - # "extra" is special because it can have multiple values at the same time. - # That's why we can only merge two "extra" markers if they have the same value. - if marker1.name == "extra": - assert isinstance(marker1, SingleMarker) - assert isinstance(marker2, SingleMarker) - if marker1.value != marker2.value: # type: ignore[attr-defined] - return None - if merge_class == MultiMarker: merge_method = marker1.constraint.intersect else: @@ -1125,12 +1137,14 @@ def _merge_single_markers( ): result_marker = SingleMarker(marker1.name, result_constraint) elif isinstance(result_constraint, UnionConstraint) and all( - isinstance(c, Constraint) and c.operator == "==" + isinstance(c, Constraint) + and c.operator in ({"==", "!="} if marker1.name == "extra" else {"=="}) for c in result_constraint.constraints ): result_marker = AtomicMarkerUnion(marker1.name, result_constraint) elif isinstance(result_constraint, MultiConstraint) and all( - c.operator == "!=" for c in result_constraint.constraints + c.operator in ({"==", "!="} if marker1.name == "extra" else {"!="}) + for c in result_constraint.constraints ): result_marker = AtomicMultiMarker(marker1.name, result_constraint) return result_marker diff --git a/tests/packages/test_main.py b/tests/packages/test_main.py index 862f12631..9bb10d062 100644 --- a/tests/packages/test_main.py +++ b/tests/packages/test_main.py @@ -56,6 +56,16 @@ def test_dependency_from_pep_508_with_extras() -> None: assert str(dep.marker) == 'extra == "foo" or extra == "bar"' +def test_dependency_from_pep_508_with_extra_and_inverse_extra() -> None: + name = 'requests==2.18.0; extra != "foo" and extra == "bar"' + dep = Dependency.create_from_pep_508(name) + + assert dep.name == "requests" + assert str(dep.constraint) == "2.18.0" + assert dep.in_extras == ["bar"] + assert str(dep.marker) == 'extra != "foo" and extra == "bar"' + + def test_dependency_from_pep_508_with_python_version() -> None: name = 'requests (==2.18.0); python_version == "2.7" or python_version == "2.6"' dep = Dependency.create_from_pep_508(name) diff --git a/tests/version/test_markers.py b/tests/version/test_markers.py index f7e26b294..d2a8fd0fb 100644 --- a/tests/version/test_markers.py +++ b/tests/version/test_markers.py @@ -59,10 +59,12 @@ 'extra == "a" and extra != "b"', 'extra != "a" and extra == "b"', 'extra != "a" and extra != "b"', + 'extra == "a" and extra == "b" and extra != "c" and extra != "d"', 'extra == "a" or extra == "b"', 'extra == "a" or extra != "b"', 'extra != "a" or extra == "b"', 'extra != "a" or extra != "b"', + 'extra == "a" or extra == "b" or extra != "c" or extra != "d"', # String comparison markers '"tegra" in platform_release', '"tegra" not in platform_release', @@ -250,6 +252,98 @@ def test_single_marker_not_in_python_intersection() -> None: ('extra == "a"', 'extra != "b"', 'extra == "a" and extra != "b"'), ('extra != "a"', 'extra == "b"', 'extra != "a" and extra == "b"'), ('extra != "a"', 'extra != "b"', 'extra != "a" and extra != "b"'), + # AtomicMultiMarker + ( + 'extra == "a" and extra == "b"', + 'extra == "c"', + 'extra == "a" and extra == "b" and extra == "c"', + ), + ( + 'extra != "a" and extra != "b"', + 'extra != "c"', + 'extra != "a" and extra != "b" and extra != "c"', + ), + ( + 'extra != "a" and extra != "b"', + 'extra == "c"', + 'extra != "a" and extra != "b" and extra == "c"', + ), + ( + 'extra == "a" and extra == "b"', + 'extra != "c"', + 'extra == "a" and extra == "b" and extra != "c"', + ), + ( + 'extra == "a" and extra == "b"', + 'extra == "a" and extra == "b"', + 'extra == "a" and extra == "b"', + ), + ( + 'extra == "a" and extra == "b"', + 'extra == "b" and extra == "a"', + 'extra == "a" and extra == "b"', + ), + ( + 'extra == "a" and extra == "b"', + 'extra == "c" and extra != "d"', + 'extra == "a" and extra == "b" and extra == "c" and extra != "d"', + ), + ('extra != "a" and extra != "b"', 'extra == "a"', ""), + ('extra != "a" and extra == "b"', 'extra == "a" and extra == "c"', ""), + ( + 'extra != "a" and extra != "b"', + 'extra != "a"', + 'extra != "a" and extra != "b"', + ), + ( + 'extra == "a" and extra != "b"', + 'extra == "a"', + 'extra == "a" and extra != "b"', + ), + # AtomicMarkerUnion + ( + 'extra == "a" or extra == "b"', + 'extra == "c"', + '(extra == "a" or extra == "b") and extra == "c"', + ), + ( + 'extra == "a" or extra == "b"', + 'extra != "c"', + '(extra == "a" or extra == "b") and extra != "c"', + ), + ('extra == "a" or extra == "b"', 'extra == "a"', 'extra == "a"'), + ('extra != "a" or extra == "b"', 'extra != "a"', 'extra != "a"'), + ( + 'extra == "a" or extra == "b"', + 'extra != "a"', + 'extra == "b" and extra != "a"', + ), + ( + 'extra == "a" or extra == "b"', + 'extra == "a" or extra == "b"', + 'extra == "a" or extra == "b"', + ), + ( + 'extra == "a" or extra == "b"', + 'extra == "b" or extra == "a"', + 'extra == "a" or extra == "b"', + ), + ( + 'extra == "a" or extra == "b"', + 'extra == "a" or extra != "c"', + '(extra == "a" or extra == "b") and (extra == "a" or extra != "c")', + ), + # AtomicMultiMarker and AtomicMarkerUnion + ( + 'extra != "a" and extra != "b"', + 'extra == "a" or extra == "b"', + "", + ), + ( + 'extra != "a" and extra != "b"', + 'extra == "a" or extra == "c"', + 'extra == "c" and extra != "a" and extra != "b"', + ), ], ) def test_single_marker_intersect_extras( @@ -443,6 +537,104 @@ def test_single_marker_union_with_inverse() -> None: ('extra == "a"', 'extra != "b"', 'extra == "a" or extra != "b"'), ('extra != "a"', 'extra == "b"', 'extra != "a" or extra == "b"'), ('extra != "a"', 'extra != "b"', 'extra != "a" or extra != "b"'), + # AtomicMultiMarker + ( + 'extra == "a" and extra == "b"', + 'extra == "c"', + 'extra == "a" and extra == "b" or extra == "c"', + ), + ( + 'extra != "a" and extra != "b"', + 'extra != "c"', + 'extra != "a" and extra != "b" or extra != "c"', + ), + ( + 'extra != "a" and extra != "b"', + 'extra == "c"', + 'extra != "a" and extra != "b" or extra == "c"', + ), + ( + 'extra == "a" and extra == "b"', + 'extra != "c"', + 'extra == "a" and extra == "b" or extra != "c"', + ), + ( + 'extra == "a" and extra == "b"', + 'extra == "a" and extra == "b"', + 'extra == "a" and extra == "b"', + ), + ( + 'extra == "a" and extra == "b"', + 'extra == "c" and extra != "d"', + 'extra == "a" and extra == "b" or extra == "c" and extra != "d"', + ), + ( + 'extra != "a" and extra != "b"', + 'extra == "a"', + 'extra != "b" or extra == "a"', + ), + ( + 'extra != "a" and extra == "b"', + 'extra == "a" and extra == "c"', + 'extra != "a" and extra == "b" or extra == "a" and extra == "c"', + ), + ( + 'extra != "a" and extra != "b"', + 'extra != "a"', + 'extra != "a"', + ), + ( + 'extra == "a" and extra != "b"', + 'extra == "a"', + 'extra == "a"', + ), + # AtomicMarkerUnion + ( + 'extra == "a" or extra == "b"', + 'extra == "c"', + 'extra == "a" or extra == "b" or extra == "c"', + ), + ( + 'extra == "a" or extra == "b"', + 'extra != "c"', + 'extra == "a" or extra == "b" or extra != "c"', + ), + ( + 'extra == "a" or extra == "b"', + 'extra == "a"', + 'extra == "a" or extra == "b"', + ), + ( + 'extra != "a" or extra == "b"', + 'extra != "a"', + 'extra != "a" or extra == "b"', + ), + ( + 'extra == "a" or extra == "b"', + 'extra != "a"', + "", + ), + ( + 'extra == "a" or extra == "b"', + 'extra == "a" or extra == "b"', + 'extra == "a" or extra == "b"', + ), + ( + 'extra == "a" or extra == "b"', + 'extra == "a" or extra != "c"', + 'extra == "a" or extra == "b" or extra != "c"', + ), + # AtomicMultiMarker and AtomicMarkerUnion + ( + 'extra != "a" and extra != "b"', + 'extra == "a" or extra == "b"', + 'extra != "a" and extra != "b" or extra == "a" or extra == "b"', + ), + ( + 'extra != "a" and extra != "b"', + 'extra == "a" or extra == "c"', + 'extra != "a" and extra != "b" or extra == "a" or extra == "c"', + ), ], ) def test_single_marker_union_extras(marker1: str, marker2: str, expected: str) -> None: