From 43745d2ecfd34c07e17d952d982e8ef6a3056bcb Mon Sep 17 00:00:00 2001 From: konsti Date: Tue, 1 Jul 2025 17:48:48 +0200 Subject: [PATCH] Fix equals-star and tilde-equals with `python_version` and `python_full_version` (#14271) The marker display code assumes that all versions are normalized, in that all trailing zeroes are stripped. This is not the case for tilde-equals and equals-star versions, where the trailing zeroes (before the `.*`) are semantically relevant. This would cause path dependent-behavior where we would get a different marker string depending on whether a version with or without a trailing zero was added to the cache first. To handle both equals-star and tilde-equals when converting `python_version` to `python_full_version` markers, we have to merge the version normalization (i.e. trimming the trailing zeroes) and the conversion both to `python_full_version` and to `Ranges`, while special casing equals-star and tilde-equals. To avoid churn in lockfiles, we only trim in the conversion to `Ranges` for markers, but keep using untrimmed versions for requires-python. (Note that this behavior is technically also path dependent, as versions with and without trailing zeroes have the same Hash and Eq. E.q., `requires-python == ">= 3.10.0"` and `requires-python == ">= 3.10"` in the same workspace could lead to either value in `uv.lock`, and which one it is could change if we make unrelated (performance) changes. Always trimming however definitely changes lockfiles, a churn I wouldn't do outside another breaking or lockfile-changing change.) Nevertheless, there is a change for users who have `requires-python = "~= 3.12.0"` in their `pyproject.toml`, as this now hits the correct normalization path. Fixes #14231 Fixes #14270 --- .../src/requires_python.rs | 6 +- crates/uv-pep440/src/version.rs | 18 +++ crates/uv-pep440/src/version_ranges.rs | 81 ++++++------ crates/uv-pep440/src/version_specifier.rs | 92 ++++++++++--- crates/uv-pep508/src/lib.rs | 1 + crates/uv-pep508/src/marker/algebra.rs | 122 ++++++++++-------- crates/uv-pep508/src/marker/simplify.rs | 42 +++--- crates/uv-pep508/src/marker/tree.rs | 117 +++++++++++++++-- crates/uv-pypi-types/src/conflicts.rs | 4 +- crates/uv-pypi-types/src/identifier.rs | 1 + ...__line-endings-poetry-with-hashes.txt.snap | 11 +- ...t__test__parse-poetry-with-hashes.txt.snap | 11 +- crates/uv/tests/it/lock.rs | 39 +++++- 13 files changed, 382 insertions(+), 163 deletions(-) diff --git a/crates/uv-distribution-types/src/requires_python.rs b/crates/uv-distribution-types/src/requires_python.rs index 786aed83a..cedfff843 100644 --- a/crates/uv-distribution-types/src/requires_python.rs +++ b/crates/uv-distribution-types/src/requires_python.rs @@ -71,7 +71,7 @@ impl RequiresPython { // Warn if there’s exactly one `~=` specifier without a patch. if let [spec] = &specs[..] { if spec.is_tilde_without_patch() { - if let Some((lo_b, hi_b)) = release_specifier_to_range(spec.clone()) + if let Some((lo_b, hi_b)) = release_specifier_to_range(spec.clone(), false) .bounding_range() .map(|(l, u)| (l.cloned(), u.cloned())) { @@ -80,8 +80,8 @@ impl RequiresPython { warn_user_once!( "The release specifier (`{spec}`) contains a compatible release \ match without a patch version. This will be interpreted as \ - `{lo_spec}, {hi_spec}`. Did you mean `{spec}.0` to freeze the minor \ - version?" + `{lo_spec}, {hi_spec}`. Did you mean `{spec}.0` to freeze the \ + minor version?" ); } } diff --git a/crates/uv-pep440/src/version.rs b/crates/uv-pep440/src/version.rs index 1ef0badf2..a496f95a2 100644 --- a/crates/uv-pep440/src/version.rs +++ b/crates/uv-pep440/src/version.rs @@ -610,6 +610,24 @@ impl Version { Self::new(self.release().iter().copied()) } + /// Return the version with any segments apart from the release removed, with trailing zeroes + /// trimmed. + #[inline] + #[must_use] + pub fn only_release_trimmed(&self) -> Self { + if let Some(last_non_zero) = self.release().iter().rposition(|segment| *segment != 0) { + if last_non_zero == self.release().len() { + // Already trimmed. + self.clone() + } else { + Self::new(self.release().iter().take(last_non_zero + 1).copied()) + } + } else { + // `0` is a valid version. + Self::new([0]) + } + } + /// Return the version with trailing `.0` release segments removed. /// /// # Panics diff --git a/crates/uv-pep440/src/version_ranges.rs b/crates/uv-pep440/src/version_ranges.rs index 26cd048d3..38038ffcf 100644 --- a/crates/uv-pep440/src/version_ranges.rs +++ b/crates/uv-pep440/src/version_ranges.rs @@ -130,10 +130,11 @@ impl From for Ranges { /// /// See: pub fn release_specifiers_to_ranges(specifiers: VersionSpecifiers) -> Ranges { - specifiers - .into_iter() - .map(release_specifier_to_range) - .fold(Ranges::full(), |acc, range| acc.intersection(&range)) + let mut range = Ranges::full(); + for specifier in specifiers { + range = range.intersection(&release_specifier_to_range(specifier, false)); + } + range } /// Convert the [`VersionSpecifier`] to a PubGrub-compatible version range, using release-only @@ -147,67 +148,57 @@ pub fn release_specifiers_to_ranges(specifiers: VersionSpecifiers) -> Ranges -pub fn release_specifier_to_range(specifier: VersionSpecifier) -> Ranges { +pub fn release_specifier_to_range(specifier: VersionSpecifier, trim: bool) -> Ranges { let VersionSpecifier { operator, version } = specifier; + // Note(konsti): We switched strategies to trimmed for the markers, but we don't want to cause + // churn in lockfile requires-python, so we only trim for markers. + let version_trimmed = if trim { + version.only_release_trimmed() + } else { + version.only_release() + }; match operator { - Operator::Equal => { - let version = version.only_release(); - Ranges::singleton(version) - } - Operator::ExactEqual => { - let version = version.only_release(); - Ranges::singleton(version) - } - Operator::NotEqual => { - let version = version.only_release(); - Ranges::singleton(version).complement() - } + // Trailing zeroes are not semantically relevant. + Operator::Equal => Ranges::singleton(version_trimmed), + Operator::ExactEqual => Ranges::singleton(version_trimmed), + Operator::NotEqual => Ranges::singleton(version_trimmed).complement(), + Operator::LessThan => Ranges::strictly_lower_than(version_trimmed), + Operator::LessThanEqual => Ranges::lower_than(version_trimmed), + Operator::GreaterThan => Ranges::strictly_higher_than(version_trimmed), + Operator::GreaterThanEqual => Ranges::higher_than(version_trimmed), + + // Trailing zeroes are semantically relevant. Operator::TildeEqual => { let release = version.release(); let [rest @ .., last, _] = &*release else { unreachable!("~= must have at least two segments"); }; let upper = Version::new(rest.iter().chain([&(last + 1)])); - let version = version.only_release(); - Ranges::from_range_bounds(version..upper) - } - Operator::LessThan => { - let version = version.only_release(); - Ranges::strictly_lower_than(version) - } - Operator::LessThanEqual => { - let version = version.only_release(); - Ranges::lower_than(version) - } - Operator::GreaterThan => { - let version = version.only_release(); - Ranges::strictly_higher_than(version) - } - Operator::GreaterThanEqual => { - let version = version.only_release(); - Ranges::higher_than(version) + Ranges::from_range_bounds(version_trimmed..upper) } Operator::EqualStar => { - let low = version.only_release(); + // For (not-)equal-star, trailing zeroes are still before the star. + let low_full = version.only_release(); let high = { - let mut high = low.clone(); + let mut high = low_full.clone(); let mut release = high.release().to_vec(); *release.last_mut().unwrap() += 1; high = high.with_release(release); high }; - Ranges::from_range_bounds(low..high) + Ranges::from_range_bounds(version..high) } Operator::NotEqualStar => { - let low = version.only_release(); + // For (not-)equal-star, trailing zeroes are still before the star. + let low_full = version.only_release(); let high = { - let mut high = low.clone(); + let mut high = low_full.clone(); let mut release = high.release().to_vec(); *release.last_mut().unwrap() += 1; high = high.with_release(release); high }; - Ranges::from_range_bounds(low..high).complement() + Ranges::from_range_bounds(version..high).complement() } } } @@ -222,8 +213,8 @@ impl LowerBound { /// These bounds use release-only semantics when comparing versions. pub fn new(bound: Bound) -> Self { Self(match bound { - Bound::Included(version) => Bound::Included(version.only_release()), - Bound::Excluded(version) => Bound::Excluded(version.only_release()), + Bound::Included(version) => Bound::Included(version.only_release_trimmed()), + Bound::Excluded(version) => Bound::Excluded(version.only_release_trimmed()), Bound::Unbounded => Bound::Unbounded, }) } @@ -357,8 +348,8 @@ impl UpperBound { /// These bounds use release-only semantics when comparing versions. pub fn new(bound: Bound) -> Self { Self(match bound { - Bound::Included(version) => Bound::Included(version.only_release()), - Bound::Excluded(version) => Bound::Excluded(version.only_release()), + Bound::Included(version) => Bound::Included(version.only_release_trimmed()), + Bound::Excluded(version) => Bound::Excluded(version.only_release_trimmed()), Bound::Unbounded => Bound::Unbounded, }) } diff --git a/crates/uv-pep440/src/version_specifier.rs b/crates/uv-pep440/src/version_specifier.rs index 19acff2eb..bbbe440bf 100644 --- a/crates/uv-pep440/src/version_specifier.rs +++ b/crates/uv-pep440/src/version_specifier.rs @@ -80,24 +80,38 @@ impl VersionSpecifiers { // Add specifiers for the holes between the bounds. for (lower, upper) in bounds { - match (next, lower) { + let specifier = match (next, lower) { // Ex) [3.7, 3.8.5), (3.8.5, 3.9] -> >=3.7,!=3.8.5,<=3.9 (Bound::Excluded(prev), Bound::Excluded(lower)) if prev == lower => { - specifiers.push(VersionSpecifier::not_equals_version(prev.clone())); + Some(VersionSpecifier::not_equals_version(prev.clone())) } // Ex) [3.7, 3.8), (3.8, 3.9] -> >=3.7,!=3.8.*,<=3.9 - (Bound::Excluded(prev), Bound::Included(lower)) - if prev.release().len() == 2 - && *lower.release() == [prev.release()[0], prev.release()[1] + 1] => - { - specifiers.push(VersionSpecifier::not_equals_star_version(prev.clone())); - } - _ => { - #[cfg(feature = "tracing")] - warn!( - "Ignoring unsupported gap in `requires-python` version: {next:?} -> {lower:?}" - ); + (Bound::Excluded(prev), Bound::Included(lower)) => { + match *prev.only_release_trimmed().release() { + [major] if *lower.only_release_trimmed().release() == [major, 1] => { + Some(VersionSpecifier::not_equals_star_version(Version::new([ + major, 0, + ]))) + } + [major, minor] + if *lower.only_release_trimmed().release() == [major, minor + 1] => + { + Some(VersionSpecifier::not_equals_star_version(Version::new([ + major, minor, + ]))) + } + _ => None, + } } + _ => None, + }; + if let Some(specifier) = specifier { + specifiers.push(specifier); + } else { + #[cfg(feature = "tracing")] + warn!( + "Ignoring unsupported gap in `requires-python` version: {next:?} -> {lower:?}" + ); } next = upper; } @@ -348,6 +362,33 @@ impl VersionSpecifier { Ok(Self { operator, version }) } + /// Remove all non-release parts of the version. + /// + /// The marker decision diagram relies on the assumption that the negation of a marker tree is + /// the complement of the marker space. However, pre-release versions violate this assumption. + /// + /// For example, the marker `python_full_version > '3.9' or python_full_version <= '3.9'` + /// does not match `python_full_version == 3.9.0a0` and so cannot simplify to `true`. However, + /// its negation, `python_full_version > '3.9' and python_full_version <= '3.9'`, also does not + /// match `3.9.0a0` and simplifies to `false`, which violates the algebra decision diagrams + /// rely on. For this reason we ignore pre-release versions entirely when evaluating markers. + /// + /// Note that `python_version` cannot take on pre-release values as it is truncated to just the + /// major and minor version segments. Thus using release-only specifiers is definitely necessary + /// for `python_version` to fully simplify any ranges, such as + /// `python_version > '3.9' or python_version <= '3.9'`, which is always `true` for + /// `python_version`. For `python_full_version` however, this decision is a semantic change. + /// + /// For Python versions, the major.minor is considered the API version, so unlike the rules + /// for package versions in PEP 440, we Python `3.9.0a0` is acceptable for `>= "3.9"`. + #[must_use] + pub fn only_release(self) -> Self { + Self { + operator: self.operator, + version: self.version.only_release(), + } + } + /// `==` pub fn equals_version(version: Version) -> Self { Self { @@ -442,14 +483,23 @@ impl VersionSpecifier { (Some(VersionSpecifier::equals_version(v1.clone())), None) } // `v >= 3.7 && v < 3.8` is equivalent to `v == 3.7.*` - (Bound::Included(v1), Bound::Excluded(v2)) - if v1.release().len() == 2 - && *v2.release() == [v1.release()[0], v1.release()[1] + 1] => - { - ( - Some(VersionSpecifier::equals_star_version(v1.clone())), - None, - ) + (Bound::Included(v1), Bound::Excluded(v2)) => { + match *v1.only_release_trimmed().release() { + [major] if *v2.only_release_trimmed().release() == [major, 1] => { + let version = Version::new([major, 0]); + (Some(VersionSpecifier::equals_star_version(version)), None) + } + [major, minor] + if *v2.only_release_trimmed().release() == [major, minor + 1] => + { + let version = Version::new([major, minor]); + (Some(VersionSpecifier::equals_star_version(version)), None) + } + _ => ( + VersionSpecifier::from_lower_bound(&Bound::Included(v1.clone())), + VersionSpecifier::from_upper_bound(&Bound::Excluded(v2.clone())), + ), + } } (lower, upper) => ( VersionSpecifier::from_lower_bound(lower), diff --git a/crates/uv-pep508/src/lib.rs b/crates/uv-pep508/src/lib.rs index 46e2f3039..e2945743b 100644 --- a/crates/uv-pep508/src/lib.rs +++ b/crates/uv-pep508/src/lib.rs @@ -16,6 +16,7 @@ #![warn(missing_docs)] +#[cfg(feature = "schemars")] use std::borrow::Cow; use std::error::Error; use std::fmt::{Debug, Display, Formatter}; diff --git a/crates/uv-pep508/src/marker/algebra.rs b/crates/uv-pep508/src/marker/algebra.rs index f421a8fa3..2a3f82f27 100644 --- a/crates/uv-pep508/src/marker/algebra.rs +++ b/crates/uv-pep508/src/marker/algebra.rs @@ -172,7 +172,7 @@ impl InternerGuard<'_> { ), // Normalize `python_version` markers to `python_full_version` nodes. MarkerValueVersion::PythonVersion => { - match python_version_to_full_version(normalize_specifier(specifier)) { + match python_version_to_full_version(specifier.only_release()) { Ok(specifier) => ( Variable::Version(CanonicalMarkerValueVersion::PythonFullVersion), Edges::from_specifier(specifier), @@ -1214,7 +1214,7 @@ impl Edges { /// Returns the [`Edges`] for a version specifier. fn from_specifier(specifier: VersionSpecifier) -> Edges { - let specifier = release_specifier_to_range(normalize_specifier(specifier)); + let specifier = release_specifier_to_range(specifier.only_release(), true); Edges::Version { edges: Edges::from_range(&specifier), } @@ -1227,9 +1227,9 @@ impl Edges { let mut range: Ranges = versions .into_iter() .map(|version| { - let specifier = VersionSpecifier::equals_version(version.clone()); + let specifier = VersionSpecifier::equals_version(version.only_release()); let specifier = python_version_to_full_version(specifier)?; - Ok(release_specifier_to_range(normalize_specifier(specifier))) + Ok(release_specifier_to_range(specifier, true)) }) .flatten_ok() .collect::, NodeId>>()?; @@ -1526,57 +1526,62 @@ impl Edges { } } -// Normalize a [`VersionSpecifier`] before adding it to the tree. -fn normalize_specifier(specifier: VersionSpecifier) -> VersionSpecifier { - let (operator, version) = specifier.into_parts(); - - // The decision diagram relies on the assumption that the negation of a marker tree is - // the complement of the marker space. However, pre-release versions violate this assumption. - // - // For example, the marker `python_full_version > '3.9' or python_full_version <= '3.9'` - // does not match `python_full_version == 3.9.0a0` and so cannot simplify to `true`. However, - // its negation, `python_full_version > '3.9' and python_full_version <= '3.9'`, also does not - // match `3.9.0a0` and simplifies to `false`, which violates the algebra decision diagrams - // rely on. For this reason we ignore pre-release versions entirely when evaluating markers. - // - // Note that `python_version` cannot take on pre-release values as it is truncated to just the - // major and minor version segments. Thus using release-only specifiers is definitely necessary - // for `python_version` to fully simplify any ranges, such as `python_version > '3.9' or python_version <= '3.9'`, - // which is always `true` for `python_version`. For `python_full_version` however, this decision - // is a semantic change. - let mut release = &*version.release(); - - // Strip any trailing `0`s. - // - // The [`Version`] type ignores trailing `0`s for equality, but still preserves them in its - // [`Display`] output. We must normalize all versions by stripping trailing `0`s to remove the - // distinction between versions like `3.9` and `3.9.0`. Otherwise, their output would depend on - // which form was added to the global marker interner first. - // - // Note that we cannot strip trailing `0`s for star equality, as `==3.0.*` is different from `==3.*`. - if !operator.is_star() { - if let Some(end) = release.iter().rposition(|segment| *segment != 0) { - if end > 0 { - release = &release[..=end]; - } - } - } - - VersionSpecifier::from_version(operator, Version::new(release)).unwrap() -} - /// Returns the equivalent `python_full_version` specifier for a `python_version` specifier. /// /// Returns `Err` with a constant node if the equivalent comparison is always `true` or `false`. fn python_version_to_full_version(specifier: VersionSpecifier) -> Result { + // Trailing zeroes matter only for (not-)equals-star and tilde-equals. This means that below + // the next two blocks, we can use the trimmed release as the release. + if specifier.operator().is_star() { + // Input python_version python_full_version + // ==3.* 3.* 3.* + // ==3.0.* 3.0 3.0.* + // ==3.0.0.* 3.0 3.0.* + // ==3.9.* 3.9 3.9.* + // ==3.9.0.* 3.9 3.9.* + // ==3.9.0.0.* 3.9 3.9.* + // ==3.9.1.* FALSE FALSE + // ==3.9.1.0.* FALSE FALSE + // ==3.9.1.0.0.* FALSE FALSE + return match &*specifier.version().release() { + // `3.*` + [_major] => Ok(specifier), + // Ex) `3.9.*`, `3.9.0.*`, or `3.9.0.0.*` + [major, minor, rest @ ..] if rest.iter().all(|x| *x == 0) => { + let python_version = Version::new([major, minor]); + // Unwrap safety: A star operator with two version segments is always valid. + Ok(VersionSpecifier::from_version(*specifier.operator(), python_version).unwrap()) + } + // Ex) `3.9.1.*` or `3.9.0.1.*` + _ => Err(NodeId::FALSE), + }; + } + + if *specifier.operator() == Operator::TildeEqual { + // python_version python_full_version + // ~=3 (not possible) + // ~= 3.0 >= 3.0, < 4.0 + // ~= 3.9 >= 3.9, < 4.0 + // ~= 3.9.0 == 3.9.* + // ~= 3.9.1 FALSE + // ~= 3.9.0.0 == 3.9.* + // ~= 3.9.0.1 FALSE + return match &*specifier.version().release() { + // Ex) `3.0`, `3.7` + [_major, _minor] => Ok(specifier), + // Ex) `3.9`, `3.9.0`, or `3.9.0.0` + [major, minor, rest @ ..] if rest.iter().all(|x| *x == 0) => { + let python_version = Version::new([major, minor]); + Ok(VersionSpecifier::equals_star_version(python_version)) + } + // Ex) `3.9.1` or `3.9.0.1` + _ => Err(NodeId::FALSE), + }; + } + // Extract the major and minor version segments if the specifier contains exactly // those segments, or if it contains a major segment with an implied minor segment of `0`. - let major_minor = match *specifier.version().release() { - // For star operators, we cannot add a trailing `0`. - // - // `python_version == 3.*` is equivalent to `python_full_version == 3.*`. Adding a - // trailing `0` would result in `python_version == 3.0.*`, which is incorrect. - [_major] if specifier.operator().is_star() => return Ok(specifier), + let major_minor = match *specifier.version().only_release_trimmed().release() { // Add a trailing `0` for the minor version, which is implied. // For example, `python_version == 3` matches `3.0.1`, `3.0.2`, etc. [major] => Some((major, 0)), @@ -1614,9 +1619,10 @@ fn python_version_to_full_version(specifier: VersionSpecifier) -> Result specifier, + Operator::EqualStar | Operator::NotEqualStar | Operator::TildeEqual => { + // Handled above. + unreachable!() + } }) } else { let [major, minor, ..] = *specifier.version().release() else { @@ -1624,13 +1630,14 @@ fn python_version_to_full_version(specifier: VersionSpecifier) -> Result { + // `python_version` cannot have more than two release segments, and we know + // that the following release segments aren't purely zeroes so equality is impossible. + Operator::Equal | Operator::ExactEqual => { return Err(NodeId::FALSE); } // Similarly, inequalities are always `true`. - Operator::NotEqual | Operator::NotEqualStar => return Err(NodeId::TRUE), + Operator::NotEqual => return Err(NodeId::TRUE), // `python_version {<,<=} 3.7.8` is equivalent to `python_full_version < 3.8`. Operator::LessThan | Operator::LessThanEqual => { @@ -1641,6 +1648,11 @@ fn python_version_to_full_version(specifier: VersionSpecifier) -> Result { VersionSpecifier::greater_than_equal_version(Version::new([major, minor + 1])) } + + Operator::EqualStar | Operator::NotEqualStar | Operator::TildeEqual => { + // Handled above. + unreachable!() + } }) } } diff --git a/crates/uv-pep508/src/marker/simplify.rs b/crates/uv-pep508/src/marker/simplify.rs index 34c095b09..3dc03693a 100644 --- a/crates/uv-pep508/src/marker/simplify.rs +++ b/crates/uv-pep508/src/marker/simplify.rs @@ -64,8 +64,8 @@ fn collect_dnf( continue; } - // Detect whether the range for this edge can be simplified as a star inequality. - if let Some(specifier) = star_range_inequality(&range) { + // Detect whether the range for this edge can be simplified as a star specifier. + if let Some(specifier) = star_range_specifier(&range) { path.push(MarkerExpression::Version { key: marker.key().into(), specifier, @@ -343,22 +343,34 @@ where Some(excluded) } -/// Returns `Some` if the version expression can be simplified as a star inequality with the given -/// specifier. +/// Returns `Some` if the version range can be simplified as a star specifier. /// -/// For example, `python_full_version < '3.8' or python_full_version >= '3.9'` can be simplified to -/// `python_full_version != '3.8.*'`. -fn star_range_inequality(range: &Ranges) -> Option { +/// Only for the two bounds case not covered by [`VersionSpecifier::from_release_only_bounds`]. +/// +/// For negative ranges like `python_full_version < '3.8' or python_full_version >= '3.9'`, +/// returns `!= '3.8.*'`. +fn star_range_specifier(range: &Ranges) -> Option { + if range.iter().count() != 2 { + return None; + } + // Check for negative star range: two segments [(Unbounded, Excluded(v1)), (Included(v2), Unbounded)] let (b1, b2) = range.iter().collect_tuple()?; - - match (b1, b2) { - ((Bound::Unbounded, Bound::Excluded(v1)), (Bound::Included(v2), Bound::Unbounded)) - if v1.release().len() == 2 - && *v2.release() == [v1.release()[0], v1.release()[1] + 1] => - { - Some(VersionSpecifier::not_equals_star_version(v1.clone())) + if let ((Bound::Unbounded, Bound::Excluded(v1)), (Bound::Included(v2), Bound::Unbounded)) = + (b1, b2) + { + match *v1.only_release_trimmed().release() { + [major] if *v2.release() == [major, 1] => { + Some(VersionSpecifier::not_equals_star_version(Version::new([ + major, 0, + ]))) + } + [major, minor] if *v2.release() == [major, minor + 1] => { + Some(VersionSpecifier::not_equals_star_version(v1.clone())) + } + _ => None, } - _ => None, + } else { + None } } diff --git a/crates/uv-pep508/src/marker/tree.rs b/crates/uv-pep508/src/marker/tree.rs index 975d4ff10..5739d7c98 100644 --- a/crates/uv-pep508/src/marker/tree.rs +++ b/crates/uv-pep508/src/marker/tree.rs @@ -2271,13 +2271,13 @@ mod test { #[test] fn test_marker_simplification() { assert_false("python_version == '3.9.1'"); - assert_false("python_version == '3.9.0.*'"); assert_true("python_version != '3.9.1'"); - // Technically these is are valid substring comparison, but we do not allow them. - // e.g., using a version with patch components with `python_version` is considered - // impossible to satisfy since the value it is truncated at the minor version - assert_false("python_version in '3.9.0'"); + // This is an edge case that happens to be supported, but is not critical to support. + assert_simplifies( + "python_version in '3.9.0'", + "python_full_version == '3.9.*'", + ); // e.g., using a version that is not PEP 440 compliant is considered arbitrary assert_true("python_version in 'foo'"); // e.g., including `*` versions, which would require tracking a version specifier @@ -2287,16 +2287,25 @@ mod test { assert_true("python_version in '3.9,3.10'"); assert_true("python_version in '3.9 or 3.10'"); - // e.g, when one of the values cannot be true - // TODO(zanieb): This seems like a quirk of the `python_full_version` normalization, this - // should just act as though the patch version isn't present - assert_false("python_version in '3.9 3.10.0 3.11'"); + // This is an edge case that happens to be supported, but is not critical to support. + assert_simplifies( + "python_version in '3.9 3.10.0 3.11'", + "python_full_version >= '3.9' and python_full_version < '3.12'", + ); assert_simplifies("python_version == '3.9'", "python_full_version == '3.9.*'"); assert_simplifies( "python_version == '3.9.0'", "python_full_version == '3.9.*'", ); + assert_simplifies( + "python_version == '3.9.0.*'", + "python_full_version == '3.9.*'", + ); + assert_simplifies( + "python_version == '3.*'", + "python_full_version >= '3' and python_full_version < '4'", + ); // ` in` // e.g., when the range is not contiguous @@ -2528,6 +2537,68 @@ mod test { ); } + #[test] + fn test_python_version_equal_star() { + // Input, equivalent with python_version, equivalent with python_full_version + let cases = [ + ("3.*", "3.*", "3.*"), + ("3.0.*", "3.0", "3.0.*"), + ("3.0.0.*", "3.0", "3.0.*"), + ("3.9.*", "3.9", "3.9.*"), + ("3.9.0.*", "3.9", "3.9.*"), + ("3.9.0.0.*", "3.9", "3.9.*"), + ]; + for (input, equal_python_version, equal_python_full_version) in cases { + assert_eq!( + m(&format!("python_version == '{input}'")), + m(&format!("python_version == '{equal_python_version}'")), + "{input} {equal_python_version}" + ); + assert_eq!( + m(&format!("python_version == '{input}'")), + m(&format!( + "python_full_version == '{equal_python_full_version}'" + )), + "{input} {equal_python_full_version}" + ); + } + + let cases_false = ["3.9.1.*", "3.9.1.0.*", "3.9.1.0.0.*"]; + for input in cases_false { + assert!( + m(&format!("python_version == '{input}'")).is_false(), + "{input}" + ); + } + } + + #[test] + fn test_tilde_equal_normalization() { + assert_eq!( + m("python_version ~= '3.10.0'"), + m("python_version >= '3.10.0' and python_version < '3.11.0'") + ); + + // Two digit versions such as `python_version` get padded with a zero, so they can never + // match + assert_eq!(m("python_version ~= '3.10.1'"), MarkerTree::FALSE); + + assert_eq!( + m("python_version ~= '3.10'"), + m("python_version >= '3.10' and python_version < '4.0'") + ); + + assert_eq!( + m("python_full_version ~= '3.10.0'"), + m("python_full_version >= '3.10.0' and python_full_version < '3.11.0'") + ); + + assert_eq!( + m("python_full_version ~= '3.10'"), + m("python_full_version >= '3.10' and python_full_version < '4.0'") + ); + } + /// This tests marker implication. /// /// Specifically, these test cases come from a [bug] where `foo` and `bar` @@ -3324,4 +3395,32 @@ mod test { ] ); } + + /// Case a: There is no version `3` (no trailing zero) in the interner yet. + #[test] + fn marker_normalization_a() { + let left_tree = m("python_version == '3.0.*'"); + let left = left_tree.try_to_string().unwrap(); + let right = "python_full_version == '3.0.*'"; + assert_eq!(left, right, "{left} != {right}"); + } + + /// Case b: There is already a version `3` (no trailing zero) in the interner. + #[test] + fn marker_normalization_b() { + m("python_version >= '3' and python_version <= '3.0'"); + + let left_tree = m("python_version == '3.0.*'"); + let left = left_tree.try_to_string().unwrap(); + let right = "python_full_version == '3.0.*'"; + assert_eq!(left, right, "{left} != {right}"); + } + + #[test] + fn marker_normalization_c() { + let left_tree = MarkerTree::from_str("python_version == '3.10.0.*'").unwrap(); + let left = left_tree.try_to_string().unwrap(); + let right = "python_full_version == '3.10.*'"; + assert_eq!(left, right, "{left} != {right}"); + } } diff --git a/crates/uv-pypi-types/src/conflicts.rs b/crates/uv-pypi-types/src/conflicts.rs index dd1e96b77..81064955a 100644 --- a/crates/uv-pypi-types/src/conflicts.rs +++ b/crates/uv-pypi-types/src/conflicts.rs @@ -3,7 +3,9 @@ use petgraph::{ graph::{DiGraph, NodeIndex}, }; use rustc_hash::{FxHashMap, FxHashSet}; -use std::{borrow::Cow, collections::BTreeSet, hash::Hash, rc::Rc}; +#[cfg(feature = "schemars")] +use std::borrow::Cow; +use std::{collections::BTreeSet, hash::Hash, rc::Rc}; use uv_normalize::{ExtraName, GroupName, PackageName}; use crate::dependency_groups::{DependencyGroupSpecifier, DependencyGroups}; diff --git a/crates/uv-pypi-types/src/identifier.rs b/crates/uv-pypi-types/src/identifier.rs index 972a327ae..47439f2c9 100644 --- a/crates/uv-pypi-types/src/identifier.rs +++ b/crates/uv-pypi-types/src/identifier.rs @@ -1,4 +1,5 @@ use serde::{Serialize, Serializer}; +#[cfg(feature = "schemars")] use std::borrow::Cow; use std::fmt::Display; use std::str::FromStr; diff --git a/crates/uv-requirements-txt/src/snapshots/uv_requirements_txt__test__line-endings-poetry-with-hashes.txt.snap b/crates/uv-requirements-txt/src/snapshots/uv_requirements_txt__test__line-endings-poetry-with-hashes.txt.snap index ad5e2a0e6..e13ab75b7 100644 --- a/crates/uv-requirements-txt/src/snapshots/uv_requirements_txt__test__line-endings-poetry-with-hashes.txt.snap +++ b/crates/uv-requirements-txt/src/snapshots/uv_requirements_txt__test__line-endings-poetry-with-hashes.txt.snap @@ -1,6 +1,7 @@ --- source: crates/uv-requirements-txt/src/lib.rs expression: actual +snapshot_kind: text --- RequirementsTxt { requirements: [ @@ -23,7 +24,7 @@ RequirementsTxt { ), ), ), - marker: python_full_version >= '3.8' and python_full_version < '4.0', + marker: python_full_version >= '3.8' and python_full_version < '4', origin: Some( File( "/poetry-with-hashes.txt", @@ -54,7 +55,7 @@ RequirementsTxt { ), ), ), - marker: python_full_version >= '3.8' and python_full_version < '4.0', + marker: python_full_version >= '3.8' and python_full_version < '4', origin: Some( File( "/poetry-with-hashes.txt", @@ -85,7 +86,7 @@ RequirementsTxt { ), ), ), - marker: python_full_version >= '3.8' and python_full_version < '4.0' and sys_platform == 'win32', + marker: python_full_version >= '3.8' and python_full_version < '4' and sys_platform == 'win32', origin: Some( File( "/poetry-with-hashes.txt", @@ -116,7 +117,7 @@ RequirementsTxt { ), ), ), - marker: python_full_version >= '3.8' and python_full_version < '4.0', + marker: python_full_version >= '3.8' and python_full_version < '4', origin: Some( File( "/poetry-with-hashes.txt", @@ -148,7 +149,7 @@ RequirementsTxt { ), ), ), - marker: python_full_version >= '3.8' and python_full_version < '4.0', + marker: python_full_version >= '3.8' and python_full_version < '4', origin: Some( File( "/poetry-with-hashes.txt", diff --git a/crates/uv-requirements-txt/src/snapshots/uv_requirements_txt__test__parse-poetry-with-hashes.txt.snap b/crates/uv-requirements-txt/src/snapshots/uv_requirements_txt__test__parse-poetry-with-hashes.txt.snap index ad5e2a0e6..e13ab75b7 100644 --- a/crates/uv-requirements-txt/src/snapshots/uv_requirements_txt__test__parse-poetry-with-hashes.txt.snap +++ b/crates/uv-requirements-txt/src/snapshots/uv_requirements_txt__test__parse-poetry-with-hashes.txt.snap @@ -1,6 +1,7 @@ --- source: crates/uv-requirements-txt/src/lib.rs expression: actual +snapshot_kind: text --- RequirementsTxt { requirements: [ @@ -23,7 +24,7 @@ RequirementsTxt { ), ), ), - marker: python_full_version >= '3.8' and python_full_version < '4.0', + marker: python_full_version >= '3.8' and python_full_version < '4', origin: Some( File( "/poetry-with-hashes.txt", @@ -54,7 +55,7 @@ RequirementsTxt { ), ), ), - marker: python_full_version >= '3.8' and python_full_version < '4.0', + marker: python_full_version >= '3.8' and python_full_version < '4', origin: Some( File( "/poetry-with-hashes.txt", @@ -85,7 +86,7 @@ RequirementsTxt { ), ), ), - marker: python_full_version >= '3.8' and python_full_version < '4.0' and sys_platform == 'win32', + marker: python_full_version >= '3.8' and python_full_version < '4' and sys_platform == 'win32', origin: Some( File( "/poetry-with-hashes.txt", @@ -116,7 +117,7 @@ RequirementsTxt { ), ), ), - marker: python_full_version >= '3.8' and python_full_version < '4.0', + marker: python_full_version >= '3.8' and python_full_version < '4', origin: Some( File( "/poetry-with-hashes.txt", @@ -148,7 +149,7 @@ RequirementsTxt { ), ), ), - marker: python_full_version >= '3.8' and python_full_version < '4.0', + marker: python_full_version >= '3.8' and python_full_version < '4', origin: Some( File( "/poetry-with-hashes.txt", diff --git a/crates/uv/tests/it/lock.rs b/crates/uv/tests/it/lock.rs index ff5465042..1c375b2e3 100644 --- a/crates/uv/tests/it/lock.rs +++ b/crates/uv/tests/it/lock.rs @@ -5013,14 +5013,14 @@ fn lock_requires_python_not_equal() -> Result<()> { "#, )?; - uv_snapshot!(context.filters(), context.lock(), @r###" + uv_snapshot!(context.filters(), context.lock(), @r" success: true exit_code: 0 ----- stdout ----- ----- stderr ----- Resolved 2 packages in [TIME] - "###); + "); let lock = fs_err::read_to_string(&lockfile).unwrap(); @@ -27522,7 +27522,7 @@ fn windows_arm() -> Result<()> { lock, @r#" version = 1 revision = 2 - requires-python = ">=3.12.[X], <3.13" + requires-python = "==3.12.*" resolution-markers = [ "platform_machine == 'x86_64' and sys_platform == 'linux'", "platform_machine == 'AMD64' and sys_platform == 'win32'", @@ -27599,7 +27599,7 @@ fn windows_amd64_required() -> Result<()> { lock, @r#" version = 1 revision = 2 - requires-python = ">=3.12.[X], <3.13" + requires-python = "==3.12.*" required-markers = [ "platform_machine == 'x86' and sys_platform == 'win32'", "platform_machine == 'AMD64' and sys_platform == 'win32'", @@ -28725,3 +28725,34 @@ fn lock_prefix_match() -> Result<()> { Ok(()) } + +/// Regression test for . +#[test] +fn test_tilde_equals_python_version() -> Result<()> { + let context = TestContext::new("3.12"); + + let pyproject_toml = context.temp_dir.child("pyproject.toml"); + pyproject_toml.write_str( + r#" + [project] + name = "debug" + version = "0.1.0" + requires-python = ">=3.9" + dependencies = [ + "anyio==4.2.0; python_full_version >= '3.11'", + "anyio==4.3.0; python_full_version ~= '3.10.0'", + ] + "#, + )?; + + uv_snapshot!(context.filters(), context.lock(), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 7 packages in [TIME] + "); + + Ok(()) +}