Skip to content

Commit 7ccace1

Browse files
committed
Edge case tests, <V.postN interval fix
1 parent d780841 commit 7ccace1

3 files changed

Lines changed: 183 additions & 5 deletions

File tree

PR.md

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
Before merging I wanted to do another pass of finding edge cases, and I found a particularly problematic one. `<V.postN` (where V is not a pre-release) excludes pre-releases sharing V's base release via `_compare_less_than`. The interval model represents `<1.0.post1` as `(-inf, 1.0.post1.dev0)`, which is correct for non-pre-release versions but too wide for pre-releases: it includes `1.0.dev0`, `1.0a1`, `1.0.post0.dev0`, etc. that `_compare_less_than` actually rejects.
2+
3+
This caused my current approach to miss cases like `==1.0.dev0,<1.0.post1`. As `1.0.dev0` falls inside the interval `(-inf, 1.0.post1.dev0)` so the intersection with `==1.0.dev0` looks non-empty. But `filter()` returns nothing because `_compare_less_than` rejects `1.0.dev0` (it is a pre-release of base `1.0`, same base as `1.0.post1`).
4+
5+
Or another case `>=1.0.dev0,<1.0.post1,!=1.0,!=1.0.post0`. After `!=` removes the two non-pre-release versions (`1.0` and `1.0.post0`), only pre-releases remain. All of them are excluded by `<1.0.post1`, but the interval still looks non-empty.
6+
7+
The non-pre-release versions that `<1.0.post1` accepts (`1.0`, `1.0.post0`) are interleaved with pre-releases it rejects (`1.0.dev0`, `1.0.post0.dev0`) on the version number line:
8+
```
9+
1.0.dev0 < 1.0a1 < ... < 1.0 < 1.0.post0.dev0 < 1.0.post0 < 1.0.post1.dev0
10+
^^^^^^^^ ^^^^^ ^^^ ^^^^^^^^^^^^^^^ ^^^^^^^^^^
11+
rejected rejected OK rejected OK
12+
```
13+
14+
No single interval boundary can separate them because they alternate.
15+
16+
One option is to split `<1.0.post1` into separate intervals for each non-pre-release "island":
17+
18+
```
19+
(-inf, 1.0.dev0) [1.0, AFTER_LOCALS(1.0)] [1.0.post0, AFTER_LOCALS(1.0.post0)]
20+
```
21+
22+
This is correct but produces N+2 intervals for `<V.postN`, and also requires fixing `_BoundaryVersion` comparison for `AFTER_POSTS` vs `AFTER_LOCALS` with different base versions.
23+
24+
Instead I went with keeping the single interval `(-inf, 1.0.post1.dev0)` but tagging the upper bound with the excluded base release. During intersection, `_interval_is_empty` uses this tag to check whether any non-pre-release version of the base survives in the interval. The check derives the relevant post number from the lower bound in O(1) instead of iterating. Concretely: `_UpperBound` gets an `_excl_base` field set by `<V.postN`, `_interval_is_empty` checks if the interval contains a surviving non-pre-release version when the tag is present, and `_intersect_intervals` propagates the tag through `with_excl()`.

src/packaging/specifiers.py

Lines changed: 98 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -167,11 +167,27 @@ class _UpperBound:
167167
ends earlier.
168168
"""
169169

170-
__slots__ = ("inclusive", "version")
170+
__slots__ = ("_excl_base", "inclusive", "version")
171171

172-
def __init__(self, version: _VersionOrBoundary, inclusive: bool) -> None:
172+
def __init__(
173+
self,
174+
version: _VersionOrBoundary,
175+
inclusive: bool,
176+
excl_base: Version | None = None,
177+
) -> None:
173178
self.version = version
174179
self.inclusive = inclusive
180+
# Set by ``<V.postN`` (non-prerelease) to record that
181+
# ``_compare_less_than`` excludes pre-releases of this base
182+
# release. Used by ``_interval_is_empty`` to discard intervals
183+
# containing only excluded pre-releases.
184+
self._excl_base = excl_base
185+
186+
def with_excl(self, other: _UpperBound) -> _UpperBound:
187+
"""Return a copy carrying *other*'s exclusion metadata, if needed."""
188+
if self._excl_base is not None or other._excl_base is None:
189+
return self
190+
return _UpperBound(self.version, self.inclusive, other._excl_base)
175191

176192
def __eq__(self, other: object) -> bool:
177193
if not isinstance(other, _UpperBound):
@@ -212,6 +228,64 @@ def _interval_is_empty(lower: _LowerBound, upper: _UpperBound) -> bool:
212228
"""Is the interval [lower, upper] empty?"""
213229
if lower.version is None or upper.version is None:
214230
return False
231+
if lower.version == upper.version:
232+
return not (lower.inclusive and upper.inclusive)
233+
if lower.version > upper.version:
234+
return True
235+
# ``<V.postN`` excludes pre-releases sharing V's base release, but
236+
# the interval model cannot split those out (pre-releases and
237+
# non-pre-releases are interleaved on the number line). When the
238+
# upper bound carries exclusion metadata and the interval is within
239+
# the base's range, check whether any non-pre-release version of
240+
# that base (the final release and each post-release below the
241+
# bound) still falls inside.
242+
if upper._excl_base is not None:
243+
base = upper._excl_base
244+
if not lower.version < base.__replace__(dev=0, local=None):
245+
return not _has_surviving_version(lower, upper, base)
246+
return False
247+
248+
249+
def _has_surviving_version(
250+
lower: _LowerBound, upper: _UpperBound, base: Version
251+
) -> bool:
252+
"""Is there a non-pre-release version of *base* inside [lower, upper]?
253+
254+
The non-pre-release versions are ``base, base.post0, base.post1, ...``.
255+
They are sorted, so we only need to find the first one at or above
256+
*lower* and check whether it is also at or below *upper*.
257+
"""
258+
if _interval_contains(lower, upper, base):
259+
return True
260+
# base is below lower. Derive which post-release is nearest.
261+
v = lower.version
262+
if isinstance(v, _BoundaryVersion):
263+
# AFTER_LOCALS(base.postK) -> first candidate is post(K+1).
264+
k = (v.version.post + 1) if v.version.post is not None else 0
265+
elif isinstance(v, Version) and v.post is not None:
266+
k = v.post
267+
else:
268+
k = 0
269+
candidate = base.__replace__(post=k, local=None)
270+
if _interval_contains(lower, upper, candidate):
271+
return True
272+
# If candidate was at the exclusive lower bound, try the next one.
273+
next_candidate = base.__replace__(post=k + 1, local=None)
274+
return _interval_contains(lower, upper, next_candidate)
275+
276+
277+
def _interval_contains(
278+
lower: _LowerBound, upper: _UpperBound, version: Version
279+
) -> bool:
280+
"""Is *version* inside the interval [lower, upper]?"""
281+
point_lo = _LowerBound(version, True)
282+
point_hi = _UpperBound(version, True)
283+
# version >= lower AND version <= upper
284+
return not (_bounds_empty(lower, point_hi) or _bounds_empty(point_lo, upper))
285+
286+
287+
def _bounds_empty(lower: _LowerBound, upper: _UpperBound) -> bool:
288+
"""Basic emptiness check (no survivor logic, avoids recursion)."""
215289
if lower.version == upper.version:
216290
return not (lower.inclusive and upper.inclusive)
217291
return lower.version > upper.version
@@ -230,6 +304,8 @@ def _intersect_intervals(
230304

231305
lower = max(left_lower, right_lower)
232306
upper = min(left_upper, right_upper)
307+
# Propagate <V.postN exclusion metadata to the winning bound.
308+
upper = upper.with_excl(left_upper).with_excl(right_upper)
233309

234310
if not _interval_is_empty(lower, upper):
235311
result.append((lower, upper))
@@ -343,6 +419,18 @@ def _base_dev0(version: Version) -> Version:
343419
return Version.from_parts(epoch=version.epoch, release=version.release, dev=0)
344420

345421

422+
def _base_version(version: Version) -> Version:
423+
"""Strip pre/post/dev/local, keeping only epoch and release."""
424+
if (
425+
version.pre is None
426+
and version.post is None
427+
and version.dev is None
428+
and version.local is None
429+
):
430+
return version
431+
return version.__replace__(pre=None, post=None, dev=None, local=None)
432+
433+
346434
class InvalidSpecifier(ValueError):
347435
"""
348436
Raised when attempting to create a :class:`Specifier` with a specifier
@@ -674,12 +762,17 @@ def _standard_intervals(self, op: str, ver_str: str) -> list[_SpecifierInterval]
674762

675763
if op == "<":
676764
# <V excludes prereleases of V when V is not a prerelease.
677-
# V.dev0 is the earliest prerelease of V regardless of
678-
# whether V is a final, post, or pre-of-pre release.
765+
# V.dev0 is the earliest prerelease of V (final, post, etc.).
679766
bound = v if v.is_prerelease else v.__replace__(dev=0, local=None)
680767
if bound <= _MIN_VERSION:
681768
return []
682-
return [(_NEG_INF, _UpperBound(bound, False))]
769+
# For <V.postN, tag the bound with the base release so
770+
# _interval_is_empty can detect intervals containing only
771+
# excluded pre-releases.
772+
excl_base = (
773+
_base_version(v) if not v.is_prerelease and v.post is not None else None
774+
)
775+
return [(_NEG_INF, _UpperBound(bound, False, excl_base))]
683776

684777
if op == "==":
685778
# ==V (no local) matches V+local; ==V+local matches exactly.

tests/test_specifiers.py

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2320,6 +2320,7 @@ def _build_sample_versions(
23202320
"not-a-version",
23212321
"1.01",
23222322
"1.0-1",
2323+
"v1.0",
23232324
]
23242325

23252326

@@ -2412,6 +2413,37 @@ class TestIsUnsatisfiable:
24122413
"===1.0,!=1.0",
24132414
"===1.0,>1.0",
24142415
"===1.01,==1.0",
2416+
# Non-overlapping wildcards (adjacent boundaries, upper exclusive)
2417+
"==1.0.*,==1.1.*",
2418+
"==1.*,==2.*",
2419+
# Conflicting compatible releases
2420+
"~=1.0,~=2.0",
2421+
"~=1.4.2,~=1.5.0",
2422+
# Local excluded by non-local != (locals ignored per spec)
2423+
"==1.0+local1,!=1.0",
2424+
# Wildcard exhaustion (single and multiple)
2425+
">=1.0,<1.1,!=1.0.*",
2426+
"!=1.*,!=2.*,>=1.0,<3.0",
2427+
"~=1.0,!=1.*",
2428+
# >V excludes posts of pre-release V
2429+
">1.0a1,<1.0a1.post2",
2430+
# Adjacent dev of zero: no version between devN and dev(N+1)
2431+
">0.dev0,<0.dev1",
2432+
# Compatible release with pre/dev suffix vs <base
2433+
"~=1.0a1,<1.0",
2434+
"~=1.0.dev5,<1.0",
2435+
# Between post dev and post: >V.postK.devN leaves no room
2436+
">1.0.post0.dev0,<1.0.post0",
2437+
# Deep release crossing compatible release boundary
2438+
"~=1.2.3.4.5,>=1.2.3.5",
2439+
# <V.postN excludes pre-releases with same base release
2440+
"==1.0.dev0,<1.0.post1",
2441+
"==1.0a1,<1.0.post0",
2442+
"==1.0rc1,<1.0.post0",
2443+
"==1.0.post0.dev0,<1.0.post1",
2444+
">=1.0.dev0,<1.0.post1,!=1.0,!=1.0.post0",
2445+
# Different base but release is above the <V.postN bound
2446+
"==1.1.dev0,<1.0.post1",
24152447
]
24162448

24172449
# Specifier sets that must NOT be detected as unsatisfiable.
@@ -2501,6 +2533,35 @@ class TestIsUnsatisfiable:
25012533
"===1.01,>=1.0",
25022534
# === with unnormalized version that parses to a matching version
25032535
"===1.01,==1.1",
2536+
# === case-insensitive identity
2537+
"===FOOBAR,===foobar",
2538+
"===FooBar,===FOOBAR",
2539+
# Final version sits below its own post-releases
2540+
">=1.0,<1.0.post0",
2541+
">=1.0,<1.0.post1",
2542+
# Lower bound below base.dev0: other-base versions survive
2543+
">=0,<1.0.post0,!=1.0",
2544+
# === with normalization variants
2545+
"===v1.0,>=1.0",
2546+
"===1.0-1,>=1.0",
2547+
# Zero-padding equivalence
2548+
"==1.0,==1.0.0",
2549+
# Compatible release with post suffix
2550+
"~=1.0.post1,>=1.0",
2551+
# Range below post dev
2552+
">=1.0,<1.0.post0.dev1",
2553+
# Post of alpha satisfies (1.0a1.post0 exists)
2554+
">=1.0a1,<1.0a2,!=1.0a1",
2555+
# Alpha-to-beta range has room (1.0a2.dev0, etc.)
2556+
">1.0a1,<1.0b1",
2557+
# Overlapping compatible releases
2558+
"~=1.0,~=1.1",
2559+
"~=1.0,~=1.0.1",
2560+
# Partial wildcard exclusion doesn't exhaust range
2561+
">=1.0,<2.0,!=1.0.*,!=1.1.*",
2562+
# ~= with pre/dev lower bound still accepts the final release
2563+
"~=1.0.dev0,<1.0.post0",
2564+
"~=1.0a1,<1.0.post0",
25042565
]
25052566

25062567
@pytest.mark.parametrize("spec_str", UNSATISFIABLE)

0 commit comments

Comments
 (0)