Skip to content

make complex marker unions and intersections deterministic #843

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 28, 2025
Merged
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
12 changes: 6 additions & 6 deletions src/poetry/core/version/markers.py
Original file line number Diff line number Diff line change
Expand Up @@ -734,13 +734,13 @@ def union_simplify(self, other: BaseMarker) -> BaseMarker | None:
if not shared_markers:
return None

unique_markers = our_markers - their_markers
other_unique_markers = their_markers - our_markers
# Do not use sets to create MultiMarkers for deterministic order!
unique_markers = [m for m in self.markers if m not in their_markers]
other_unique_markers = [m for m in other.markers if m not in our_markers]
unique_union = MultiMarker(*unique_markers).union(
MultiMarker(*other_unique_markers)
)
if isinstance(unique_union, (SingleMarkerLike, AnyMarker)):
# Use list instead of set for deterministic order.
common_markers = [
marker for marker in self.markers if marker in shared_markers
]
Expand Down Expand Up @@ -908,13 +908,13 @@ def intersect_simplify(self, other: BaseMarker) -> BaseMarker | None:
if not shared_markers:
return None

unique_markers = our_markers - their_markers
other_unique_markers = their_markers - our_markers
# Do not use sets to create MarkerUnions for deterministic order!
unique_markers = [m for m in self.markers if m not in their_markers]
other_unique_markers = [m for m in other.markers if m not in our_markers]
unique_intersection = MarkerUnion(*unique_markers).intersect(
MarkerUnion(*other_unique_markers)
)
if isinstance(unique_intersection, (SingleMarkerLike, EmptyMarker)):
# Use list instead of set for deterministic order.
common_markers = [
marker for marker in self.markers if marker in shared_markers
]
Expand Down
29 changes: 29 additions & 0 deletions tests/version/test_markers.py
Original file line number Diff line number Diff line change
Expand Up @@ -2154,6 +2154,35 @@ def test_complex_intersection() -> None:
)


def test_complex_union_is_deterministic() -> None:
"""
This test might fail sporadically if marker operations are not deterministic!
"""
m1 = parse_marker(
'sys_platform != "darwin" and python_version >= "3.12"'
' and platform_system != "Emscripten" and (python_version < "4.0"'
' and sys_platform == "linux" and extra == "stretch"'
' or platform_system == "Windows" or extra == "test"'
' and sys_platform == "win32")'
)
m2 = parse_marker(
'sys_platform == "linux" and python_version >= "3.12"'
' and platform_system == "Emscripten" and python_version < "4.0"'
' and extra == "stretch" or sys_platform == "win32"'
' and python_version >= "3.12" and platform_system == "Emscripten"'
' and extra == "test"'
)
assert str(m1.union(m2)) == (
'python_version >= "3.12" and platform_system == "Windows"'
' and sys_platform != "darwin" or sys_platform == "linux"'
' and python_version >= "3.12" and python_version < "4.0"'
' and extra == "stretch" or python_version >= "3.12" and python_version < "4.0"'
' and extra == "stretch" and extra == "test" and (sys_platform == "linux"'
' or sys_platform == "win32") or sys_platform == "win32"'
' and python_version >= "3.12" and extra == "test"'
)


def test_union_avoids_combinatorial_explosion() -> None:
"""
combinatorial explosion without AtomicMultiMarker and AtomicMarkerUnion
Expand Down
Loading