Skip to content

Commit

Permalink
Simplify pep440 -> version ranges conversion
Browse files Browse the repository at this point in the history
With #8669 as preparation, we can remove what used to be the `PubGrubSpecifier` and instead implement a direct conversion from `VersionSpecifiers` to `Range<Version>`. Thanks to #8669 (comment), this conversion is infallible, removing a lot of fallibility downstream.

The conversion takes an owned value, to signal that we consume the value. Previously, the function had cloned the version internally.
  • Loading branch information
konstin committed Oct 30, 2024
1 parent 2b0e16c commit c30a666
Show file tree
Hide file tree
Showing 24 changed files with 286 additions and 415 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/uv-build-backend/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ spdx = { workspace = true }
thiserror = { workspace = true }
toml = { workspace = true }
tracing = { workspace = true }
version-ranges = { workspace = true }
walkdir = { workspace = true }
zip = { workspace = true }

Expand Down
9 changes: 5 additions & 4 deletions crates/uv-build-backend/src/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,11 @@ use std::str::FromStr;
use tracing::debug;
use uv_fs::Simplified;
use uv_normalize::{ExtraName, PackageName};
use uv_pep440::{Version, VersionRangesSpecifier, VersionSpecifiers};
use uv_pep440::{Version, VersionSpecifiers};
use uv_pep508::{Requirement, VersionOrUrl};
use uv_pypi_types::{Metadata23, VerbatimParsedUrl};
use uv_warnings::warn_user_once;
use version_ranges::Ranges;

#[derive(Debug, Error)]
pub enum ValidationError {
Expand Down Expand Up @@ -134,9 +135,9 @@ impl PyProjectToml {
);
passed = false;
}
VersionRangesSpecifier::from_pep440_specifiers(specifier)
.ok()
.and_then(|specifier| Some(specifier.bounding_range()?.1 != Bound::Unbounded))
Ranges::from(specifier.clone())
.bounding_range()
.map(|bounding_range| bounding_range.1 != Bound::Unbounded)
.unwrap_or(false)
}
};
Expand Down
1 change: 1 addition & 0 deletions crates/uv-pep440/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ serde = { workspace = true, features = ["derive"] }
tracing = { workspace = true, optional = true }
unicode-width = { workspace = true }
unscanny = { workspace = true }
# Adds conversions from [`VersionSpecifiers`] to [`version_ranges::Ranges`]
version-ranges = { workspace = true, optional = true }

[dev-dependencies]
Expand Down
4 changes: 2 additions & 2 deletions crates/uv-pep440/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
#![warn(missing_docs)]

#[cfg(feature = "version-ranges")]
pub use version_ranges_specifier::{VersionRangesSpecifier, VersionRangesSpecifierError};
pub use version_ranges::{release_specifier_to_range, release_specifiers_to_ranges};
pub use {
version::{
LocalSegment, Operator, OperatorParseError, Prerelease, PrereleaseKind, Version,
Expand All @@ -42,4 +42,4 @@ mod version_specifier;
#[cfg(test)]
mod tests;
#[cfg(feature = "version-ranges")]
mod version_ranges_specifier;
mod version_ranges;
2 changes: 2 additions & 0 deletions crates/uv-pep440/src/version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ pub enum Operator {
/// `!= 1.2.*`
NotEqualStar,
/// `~=`
///
/// Invariant: With `~=`, there are always at least 2 release segments.
TildeEqual,
/// `<`
LessThan,
Expand Down
192 changes: 192 additions & 0 deletions crates/uv-pep440/src/version_ranges.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,192 @@
//! Convert [`VersionSpecifiers`] to [`version_ranges::Ranges`].
use version_ranges::Ranges;

use crate::{Operator, Prerelease, Version, VersionSpecifier, VersionSpecifiers};

impl From<VersionSpecifiers> for Ranges<Version> {
/// Convert [`VersionSpecifiers`] to a PubGrub-compatible version range, using PEP 440
/// semantics.
fn from(specifiers: VersionSpecifiers) -> Self {
let mut range = Ranges::full();
for specifier in specifiers {
range = range.intersection(&Self::from(specifier));
}
range
}
}

impl From<VersionSpecifier> for Ranges<Version> {
/// Convert the [`VersionSpecifier`] to a PubGrub-compatible version range, using PEP 440
/// semantics.
fn from(specifier: VersionSpecifier) -> Self {
let VersionSpecifier { operator, version } = specifier;
match operator {
Operator::Equal => Ranges::singleton(version),
Operator::ExactEqual => Ranges::singleton(version),
Operator::NotEqual => Ranges::singleton(version).complement(),
Operator::TildeEqual => {
let [rest @ .., last, _] = version.release() else {
unreachable!("~= must have at least two segments");
};
let upper = Version::new(rest.iter().chain([&(last + 1)]))
.with_epoch(version.epoch())
.with_dev(Some(0));

Ranges::from_range_bounds(version..upper)
}
Operator::LessThan => {
if version.any_prerelease() {
Ranges::strictly_lower_than(version)
} else {
// Per PEP 440: "The exclusive ordered comparison <V MUST NOT allow a
// pre-release of the specified version unless the specified version is itself a
// pre-release."
Ranges::strictly_lower_than(version.with_min(Some(0)))
}
}
Operator::LessThanEqual => Ranges::lower_than(version),
Operator::GreaterThan => {
// Per PEP 440: "The exclusive ordered comparison >V MUST NOT allow a post-release of
// the given version unless V itself is a post release."

if let Some(dev) = version.dev() {
Ranges::higher_than(version.with_dev(Some(dev + 1)))
} else if let Some(post) = version.post() {
Ranges::higher_than(version.with_post(Some(post + 1)))
} else {
Ranges::strictly_higher_than(version.with_max(Some(0)))
}
}
Operator::GreaterThanEqual => Ranges::higher_than(version),
Operator::EqualStar => {
let low = version.with_dev(Some(0));
let mut high = low.clone();
if let Some(post) = high.post() {
high = high.with_post(Some(post + 1));
} else if let Some(pre) = high.pre() {
high = high.with_pre(Some(Prerelease {
kind: pre.kind,
number: pre.number + 1,
}));
} else {
let mut release = high.release().to_vec();
*release.last_mut().unwrap() += 1;
high = high.with_release(release);
}
Ranges::from_range_bounds(low..high)
}
Operator::NotEqualStar => {
let low = version.with_dev(Some(0));
let mut high = low.clone();
if let Some(post) = high.post() {
high = high.with_post(Some(post + 1));
} else if let Some(pre) = high.pre() {
high = high.with_pre(Some(Prerelease {
kind: pre.kind,
number: pre.number + 1,
}));
} else {
let mut release = high.release().to_vec();
*release.last_mut().unwrap() += 1;
high = high.with_release(release);
}
Ranges::from_range_bounds(low..high).complement()
}
}
}
}

/// Convert the [`VersionSpecifiers`] to a PubGrub-compatible version range, using release-only
/// semantics.
///
/// Assumes that the range will only be tested against versions that consist solely of release
/// segments (e.g., `3.12.0`, but not `3.12.0b1`).
///
/// These semantics are used for testing Python compatibility (e.g., `requires-python` against
/// the user's installed Python version). In that context, it's more intuitive that `3.13.0b0`
/// is allowed for projects that declare `requires-python = ">3.13"`.
///
/// See: <https://github.com/pypa/pip/blob/a432c7f4170b9ef798a15f035f5dfdb4cc939f35/src/pip/_internal/resolution/resolvelib/candidates.py#L540>
pub fn release_specifiers_to_ranges(specifiers: VersionSpecifiers) -> Ranges<Version> {
let mut range = Ranges::full();
for specifier in specifiers {
range = range.intersection(&release_specifier_to_range(specifier));
}
range
}

/// Convert the [`VersionSpecifier`] to a PubGrub-compatible version range, using release-only
/// semantics.
///
/// Assumes that the range will only be tested against versions that consist solely of release
/// segments (e.g., `3.12.0`, but not `3.12.0b1`).
///
/// These semantics are used for testing Python compatibility (e.g., `requires-python` against
/// the user's installed Python version). In that context, it's more intuitive that `3.13.0b0`
/// is allowed for projects that declare `requires-python = ">3.13"`.
///
/// See: <https://github.com/pypa/pip/blob/a432c7f4170b9ef798a15f035f5dfdb4cc939f35/src/pip/_internal/resolution/resolvelib/candidates.py#L540>
pub fn release_specifier_to_range(specifier: VersionSpecifier) -> Ranges<Version> {
let VersionSpecifier { operator, version } = specifier;
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()
}
Operator::TildeEqual => {
let [rest @ .., last, _] = version.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)
}
Operator::EqualStar => {
let low = version.only_release();
let high = {
let mut high = low.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)
}
Operator::NotEqualStar => {
let low = version.only_release();
let high = {
let mut high = low.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()
}
}
}
Loading

0 comments on commit c30a666

Please sign in to comment.