Skip to content

Commit d2041c1

Browse files
dimblebyCopilotradoering
authored
Fix union(range, public_version) to fill local-variant punctures (#950)
The constraint algebra treated '==X' as the literal point [X, X] when computing 'range.union(public_X)', returning the range unchanged whenever the range already allowed X. But per PEP 440 release-equality '==X' also matches every local-tagged variant 'X+local', so a range whose bound is such a variant (e.g. '>=X,<X+local' or '>X+local,<Y') must be broadened to include those variants in the union. Apply the broadening symmetrically in VersionRange.union(Version), handling both bounds in a single pass, and delegate Version.union(non Version) to the other operand so the rules apply regardless of operand order. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Randy Döring <30527984+radoering@users.noreply.github.com>
1 parent 19dfc92 commit d2041c1

3 files changed

Lines changed: 143 additions & 18 deletions

File tree

src/poetry/core/constraints/version/version.py

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -109,28 +109,16 @@ def intersect(self, other: VersionConstraint) -> VersionConstraint:
109109
return other.intersect(self)
110110

111111
def union(self, other: VersionConstraint) -> VersionConstraint:
112-
from poetry.core.constraints.version.version_range import VersionRange
112+
if not isinstance(other, Version):
113+
# Delegate to the other operand's ``union`` so the broadening
114+
# rules for local-tagged bounds are applied symmetrically
115+
# whether the union is written as ``range.union(point)`` or
116+
# ``point.union(range)``.
117+
return other.union(self)
113118

114119
if other.allows(self):
115120
return other
116121

117-
if isinstance(other, VersionRangeConstraint):
118-
if self.allows(other.min):
119-
return VersionRange(
120-
other.min,
121-
other.max,
122-
include_min=True,
123-
include_max=other.include_max,
124-
)
125-
126-
if self.allows(other.max):
127-
return VersionRange(
128-
other.min,
129-
other.max,
130-
include_min=other.include_min,
131-
include_max=True,
132-
)
133-
134122
return VersionUnion.of(self, other)
135123

136124
def difference(self, other: VersionConstraint) -> Version | EmptyConstraint:

src/poetry/core/constraints/version/version_range.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,9 @@ def intersect(self, other: VersionConstraint) -> VersionConstraint:
215215
and not other.is_local()
216216
and other.allows(self.min)
217217
):
218+
# Strictly speaking, next_patch() is not quite correct
219+
# because you cannot specify the upper bound. It only
220+
# works for versions with a precision of three or less.
218221
upper = other.stable.next_patch()
219222
return _range_or_empty(
220223
min=self.min,
@@ -269,6 +272,42 @@ def union(self, other: VersionConstraint) -> VersionConstraint:
269272
from poetry.core.constraints.version.version import Version
270273

271274
if isinstance(other, Version):
275+
# If ``other`` is a public version that covers any local-tagged
276+
# variants at our bounds (by PEP 440 release-equality), the
277+
# union absorbs those local variants and we can broaden the
278+
# bound(s). Handle both bounds at once so e.g. a range that is
279+
# bracketed by two local variants of ``other`` collapses to a
280+
# single release-spanning range.
281+
new_min: Version | None = self._min
282+
new_include_min = self._include_min
283+
new_max: Version | None = self._max
284+
new_include_max = self._include_max
285+
broadened = False
286+
if not other.is_local():
287+
if (
288+
self._min is not None
289+
and self._min.is_local()
290+
and other.allows(self._min)
291+
):
292+
new_min = other
293+
new_include_min = True
294+
broadened = True
295+
if (
296+
self._max is not None
297+
and self._max.is_local()
298+
and other.allows(self._max)
299+
):
300+
# Strictly speaking, next_patch() is not quite correct
301+
# because you cannot specify the upper bound. It only
302+
# works for versions with a precision of three or less.
303+
new_max = other.stable.next_patch()
304+
new_include_max = False
305+
broadened = True
306+
if broadened:
307+
return _range_or_empty(
308+
new_min, new_max, new_include_min, new_include_max
309+
)
310+
272311
if self.allows(other):
273312
return self
274313

tests/constraints/version/test_version_range.py

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -812,6 +812,104 @@ def test_intersect_punctured_range_with_excluded_point_is_empty() -> None:
812812
assert excluded_point.intersect(punctured).is_empty()
813813

814814

815+
def test_union_upper_bound_local_with_public_extends_to_next_patch() -> None:
816+
"""``>=X,<X+local union ==X`` broadens the upper bound to
817+
``X.next_patch()`` (exclusive). ``==X`` matches every ``X+local``
818+
variant by PEP 440 release-equality, so the union must cover them.
819+
"""
820+
range_below = VersionRange(
821+
Version.parse("0.21.0"),
822+
Version.parse("0.21.0+cpu"),
823+
include_min=True,
824+
include_max=False,
825+
)
826+
public = Version.parse("0.21.0")
827+
expected = VersionRange(
828+
Version.parse("0.21.0"),
829+
Version.parse("0.21.1"),
830+
include_min=True,
831+
include_max=False,
832+
)
833+
assert range_below.union(public) == expected
834+
assert public.union(range_below) == expected
835+
836+
837+
def test_union_local_bounds_with_public_collapses_to_next_patch() -> None:
838+
"""``>=X+local1,<=X+local2 union ==X`` collapses to ``[X, X.next_patch())``.
839+
Both bounds are local variants of the same public version ``X``.
840+
Since ``==X`` matches all of them by PEP 440 release-equality, the
841+
union must cover the entire public range up to ``X.next_patch()``.
842+
"""
843+
range_locals = VersionRange(
844+
Version.parse("0.21.0+cpu"),
845+
Version.parse("0.21.0+gpu"),
846+
include_min=True,
847+
include_max=True,
848+
)
849+
public = Version.parse("0.21.0")
850+
expected = VersionRange(
851+
Version.parse("0.21.0"),
852+
Version.parse("0.21.1"),
853+
include_min=True,
854+
include_max=False,
855+
)
856+
assert range_locals.union(public) == expected
857+
assert public.union(range_locals) == expected
858+
859+
860+
def test_union_lower_bound_local_with_public_extends_to_public() -> None:
861+
"""``>X+local,<Y union ==X`` extends the lower bound down to ``X``
862+
(inclusive). ``==X`` matches the literal point ``X`` and every
863+
``X+local`` variant, including the excluded ``X+local`` lower bound.
864+
"""
865+
range_above = VersionRange(
866+
Version.parse("0.21.0+cpu"),
867+
Version.parse("0.22.0"),
868+
include_min=False,
869+
include_max=False,
870+
)
871+
public = Version.parse("0.21.0")
872+
expected = VersionRange(
873+
Version.parse("0.21.0"),
874+
Version.parse("0.22.0"),
875+
include_min=True,
876+
include_max=False,
877+
)
878+
assert range_above.union(public) == expected
879+
assert public.union(range_above) == expected
880+
881+
882+
def test_union_punctured_versionunion_with_public_collapses_puncture() -> None:
883+
"""End-to-end on a punctured ``VersionUnion``: ``==X`` fills the
884+
interior puncture at ``X+local``, collapsing the two-range union
885+
back to a single unpunctured range.
886+
"""
887+
punctured = parse_constraint(">=0.21.0,!=0.21.0+cpu,<0.22.0")
888+
public = parse_constraint("==0.21.0")
889+
plain = parse_constraint(">=0.21.0,<0.22.0")
890+
assert punctured.union(public) == plain
891+
assert public.union(punctured) == plain
892+
893+
894+
def test_union_range_with_local_other_does_not_broaden() -> None:
895+
"""Negative control: a *local* ``==X+other`` matches only itself
896+
literally, never sibling local-tagged versions. Unioning it with a
897+
range that excludes a different ``X+local`` must preserve the
898+
puncture rather than broaden.
899+
"""
900+
range_below = VersionRange(
901+
Version.parse("0.21.0"),
902+
Version.parse("0.21.0+cpu"),
903+
include_min=True,
904+
include_max=False,
905+
)
906+
other_local = Version.parse("0.21.0+other")
907+
result = range_below.union(other_local)
908+
assert result.allows(Version.parse("0.21.0"))
909+
assert result.allows(other_local)
910+
assert not result.allows(Version.parse("0.21.0+cpu"))
911+
912+
815913
def test_parsed_strict_max_excludes_dev_releases_of_stable() -> None:
816914
"""PEP 440: ``<V`` for stable V MUST NOT allow pre-/dev-releases of V.
817915
The parser canonicalizes to ``<V.dev0`` so ``allows`` reports correctly."""

0 commit comments

Comments
 (0)