Skip to content

Commit

Permalink
Get representations working
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Nov 4, 2024
1 parent 4dc1678 commit d082763
Showing 9 changed files with 281 additions and 98 deletions.
4 changes: 2 additions & 2 deletions Cargo.lock

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

4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -127,8 +127,8 @@ petgraph = { version = "0.6.5" }
platform-info = { version = "2.0.3" }
procfs = { version = "0.17.0" , default-features = false, features = ["flate2"] }
proc-macro2 = { version = "1.0.86" }
pubgrub = { git = "https://github.com/astral-sh/pubgrub", rev = "95e1390399cdddee986b658be19587eb1fdb2d79" }
version-ranges = { git = "https://github.com/astral-sh/pubgrub", rev = "95e1390399cdddee986b658be19587eb1fdb2d79" }
pubgrub = { git = "https://github.com/astral-sh/pubgrub", branch = "charlie/mut" }
version-ranges = { git = "https://github.com/astral-sh/pubgrub", branch = "charlie/mut" }
quote = { version = "1.0.37" }
rayon = { version = "1.10.0" }
reflink-copy = { version = "0.1.19" }
32 changes: 21 additions & 11 deletions crates/uv-pep440/src/version.rs
Original file line number Diff line number Diff line change
@@ -725,7 +725,12 @@ impl std::fmt::Display for Version {
let local = if self.local().is_empty() {
String::new()
} else {
format!("+{}", self.local().local_identifier_string())
match self.local() {
LocalVersionSlice::Actual(_) => {
format!("+{}", self.local().local_identifier_string())
}
LocalVersionSlice::Sentinel => String::new(),
}
};
write!(f, "{epoch}{release}{pre}{post}{dev}{local}")
}
@@ -1389,17 +1394,18 @@ impl std::fmt::Display for Prerelease {
}
}

/// Either a sequence of local segments or [`LocalVersion::Sentinel`], an internal-only value that compares greater than all other local versions.
/// Either a sequence of local segments or [`LocalVersion::Sentinel`], an internal-only value that
/// compares greater than all other local versions.
#[derive(Eq, PartialEq, Debug, Clone, Hash)]
#[cfg_attr(
feature = "rkyv",
derive(rkyv::Archive, rkyv::Deserialize, rkyv::Serialize)
)]
#[cfg_attr(feature = "rkyv", rkyv(derive(Debug, Eq, PartialEq, PartialOrd, Ord)))]
pub enum LocalVersion {
/// A sequence of local segments
/// A sequence of local segments.
Actual(Vec<LocalSegment>),
/// An internal-only value that compares greater to all other local versions
/// An internal-only value that compares greater to all other local versions.
Max,
}

@@ -1408,37 +1414,41 @@ pub enum LocalVersion {
pub enum LocalVersionSlice<'a> {
/// Like [`LocalVersion::Actual`]
Actual(&'a [LocalSegment]),
/// Like [`LocalVersion::Sentintel`]
/// Like [`LocalVersion::Sentinel`]
Sentinel,
}

impl LocalVersion {
/// convert into a local version slice
/// Convert the local version segments into a slice.
pub fn as_slice(&self) -> LocalVersionSlice<'_> {
match self {
LocalVersion::Actual(segments) => LocalVersionSlice::Actual(segments),
LocalVersion::Max => LocalVersionSlice::Sentinel,
}
}

/// clear the local version segments, if they exist
/// Clear the local version segments, if they exist.
pub fn clear(&mut self) {
if let Self::Actual(segments) = self {
segments.clear();
match self {
Self::Actual(segments) => segments.clear(),
Self::Max => *self = Self::Actual(Vec::new()),
}
}
}

impl LocalVersionSlice<'_> {
/// output the local version identifier string. [`LocalVersionSlice::Sentinel`] maps to `"[max-local-version]"` which is otherwise an illegal local version because `<` and `>` are not allowed
/// Output the local version identifier string.
///
/// [`LocalVersionSlice::Sentinel`] maps to `"[max]"` which is otherwise an illegal local
/// version because `[` and `]` are not allowed.
pub fn local_identifier_string(&self) -> String {
match self {
LocalVersionSlice::Actual(segments) => segments
.iter()
.map(ToString::to_string)
.collect::<Vec<String>>()
.join("."),
LocalVersionSlice::Sentinel => String::from("[max-local-version]"),
LocalVersionSlice::Sentinel => String::from("[max]"),
}
}
}
38 changes: 14 additions & 24 deletions crates/uv-pep440/src/version_ranges.rs
Original file line number Diff line number Diff line change
@@ -2,7 +2,10 @@
use version_ranges::Ranges;

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

impl From<VersionSpecifiers> for Ranges<Version> {
/// Convert [`VersionSpecifiers`] to a PubGrub-compatible version range, using PEP 440
@@ -23,13 +26,13 @@ impl From<VersionSpecifier> for Ranges<Version> {
let VersionSpecifier { operator, version } = specifier;
match operator {
Operator::Equal => match version.local() {
crate::LocalVersionSlice::Actual(&[]) => {
LocalVersionSlice::Actual(&[]) => {
let low = version;
let high = low.clone().with_local(LocalVersion::Max);
Ranges::between(low, high)
}
crate::LocalVersionSlice::Actual(_) => Ranges::singleton(version),
crate::LocalVersionSlice::Sentinel => unreachable!(
LocalVersionSlice::Actual(_) => Ranges::singleton(version),
LocalVersionSlice::Sentinel => unreachable!(
"found `LocalVersionSlice::Sentinel`, which should be an internal-only value"
),
},
@@ -146,34 +149,21 @@ pub fn release_specifier_to_range(specifier: VersionSpecifier) -> Ranges<Version
match operator {
Operator::Equal => {
let version = version.only_release();
match version.local() {
crate::LocalVersionSlice::Actual(&[]) => {
let low = version;
let high = low.clone().with_local(LocalVersion::Max);
Ranges::between(low, high)
}
crate::LocalVersionSlice::Actual(_) => Ranges::singleton(version),
crate::LocalVersionSlice::Sentinel => unreachable!(
"found `LocalVersionSlice::Sentinel`, which should be an internal-only value"
),
}
Ranges::singleton(version)
}
Operator::ExactEqual => {
let version = version.only_release();
Ranges::singleton(version)
}
Operator::NotEqual => release_specifier_to_range(VersionSpecifier {
operator: Operator::Equal,
version,
})
.complement(),
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 =
// greater release includes local
Version::new(rest.iter().chain([&(last + 1)]));
let upper = Version::new(rest.iter().chain([&(last + 1)]));
let version = version.only_release();
Ranges::from_range_bounds(version..upper)
}
@@ -183,7 +173,7 @@ pub fn release_specifier_to_range(specifier: VersionSpecifier) -> Ranges<Version
}
Operator::LessThanEqual => {
let version = version.only_release();
Ranges::lower_than(version.with_local(LocalVersion::Max))
Ranges::lower_than(version)
}
Operator::GreaterThan => {
let version = version.only_release();
160 changes: 157 additions & 3 deletions crates/uv-resolver/src/error.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
use std::collections::{BTreeMap, BTreeSet};
use std::collections::{BTreeMap, BTreeSet, Bound};
use std::fmt::Formatter;
use std::sync::Arc;

use indexmap::IndexSet;
use pubgrub::{DefaultStringReporter, DerivationTree, Derived, External, Range, Reporter};
use pubgrub::{
DefaultStringReporter, DerivationTree, Derived, External, Range, Ranges, Reporter, Term,
};
use rustc_hash::FxHashMap;

use crate::candidate_selector::CandidateSelector;
@@ -19,7 +21,7 @@ use uv_distribution_types::{
BuiltDist, IndexCapabilities, IndexLocations, IndexUrl, InstalledDist, SourceDist,
};
use uv_normalize::PackageName;
use uv_pep440::Version;
use uv_pep440::{LocalVersionSlice, Version};
use uv_pep508::MarkerTree;
use uv_static::EnvVars;

@@ -210,6 +212,157 @@ impl NoSolutionError {
.expect("derivation tree should contain at least one external term")
}

/// Simplifies the version ranges on any incompatibilities to remove the `[max]` sentinel.
///
/// The `[max]` sentinel is used to represent the maximum local version of a package, to
/// implement PEP 440 semantics for local version equality. For example, `1.0.0+foo` needs to
/// satisfy `==1.0.0`.
pub(crate) fn simplify_local_version_segments(
mut derivation_tree: ErrorTree,
) -> Option<ErrorTree> {
/// Remove local versions sentinels (`+[max]`) from the given version ranges.
fn strip_sentinel(versions: &mut Ranges<Version>) {
versions.iter_mut().for_each(|(lower, upper)| {
match (&lower, &upper) {
(Bound::Unbounded, Bound::Unbounded) => {}
(Bound::Unbounded, Bound::Included(v)) => {
// `<=1.0.0+[max]` is equivalent to `<=1.0.0`
if v.local() == LocalVersionSlice::Sentinel {
*upper = Bound::Included(v.clone().without_local());
}
}
(Bound::Unbounded, Bound::Excluded(v)) => {
// `<1.0.0+[max]` is equivalent to `<1.0.0`
if v.local() == LocalVersionSlice::Sentinel {
*upper = Bound::Excluded(v.clone().without_local());
}
}
(Bound::Included(v), Bound::Unbounded) => {
// `>=1.0.0+[max]` is equivalent to `>1.0.0`
if v.local() == LocalVersionSlice::Sentinel {
*lower = Bound::Excluded(v.clone().without_local());
}
}
(Bound::Included(v), Bound::Included(b)) => {
// `>=1.0.0+[max]` is equivalent to `>1.0.0`
if v.local() == LocalVersionSlice::Sentinel {
*lower = Bound::Excluded(v.clone().without_local());
}
// `<=1.0.0+[max]` is equivalent to `<=1.0.0`
if b.local() == LocalVersionSlice::Sentinel {
*upper = Bound::Included(b.clone().without_local());
}
}
(Bound::Included(v), Bound::Excluded(b)) => {
// `>=1.0.0+[max]` is equivalent to `>1.0.0`
if v.local() == LocalVersionSlice::Sentinel {
*lower = Bound::Excluded(v.clone().without_local());
}
// `<1.0.0+[max]` is equivalent to `<1.0.0`
if b.local() == LocalVersionSlice::Sentinel {
*upper = Bound::Included(b.clone().without_local());
}
}
(Bound::Excluded(v), Bound::Unbounded) => {
// `>1.0.0+[max]` is equivalent to `>1.0.0`
if v.local() == LocalVersionSlice::Sentinel {
*lower = Bound::Excluded(v.clone().without_local());
}
}
(Bound::Excluded(v), Bound::Included(b)) => {
// `>1.0.0+[max]` is equivalent to `>1.0.0`
if v.local() == LocalVersionSlice::Sentinel {
*lower = Bound::Excluded(v.clone().without_local());
}
// `<=1.0.0+[max]` is equivalent to `<=1.0.0`
if b.local() == LocalVersionSlice::Sentinel {
*upper = Bound::Included(b.clone().without_local());
}
}
(Bound::Excluded(v), Bound::Excluded(b)) => {
// `>1.0.0+[max]` is equivalent to `>1.0.0`
if v.local() == LocalVersionSlice::Sentinel {
*lower = Bound::Excluded(v.clone().without_local());
}
// `<1.0.0+[max]` is equivalent to `<1.0.0`
if b.local() == LocalVersionSlice::Sentinel {
*upper = Bound::Excluded(b.clone().without_local());
}
}
}
});
}

/// Returns `true` if the range appears to be, e.g., `>1.0.0, <1.0.0+[max]`.
fn is_sentinel(versions: &mut Ranges<Version>) -> bool {
versions.iter().all(|(lower, upper)| {
let (Bound::Excluded(lower), Bound::Excluded(upper)) = (lower, upper) else {
return false;
};
if lower.local() == LocalVersionSlice::Sentinel {
return false;
}
if upper.local() != LocalVersionSlice::Sentinel {
return false;
}
*lower == upper.clone().without_local()
})
}

match derivation_tree {
DerivationTree::External(External::NotRoot(_, _)) => Some(derivation_tree),
DerivationTree::External(External::NoVersions(_, ref mut versions)) => {
if is_sentinel(versions) {
None
} else {
strip_sentinel(versions);
Some(derivation_tree)
}
}
DerivationTree::External(External::FromDependencyOf(
_,
ref mut versions1,
_,
ref mut versions2,
)) => {
strip_sentinel(versions1);
strip_sentinel(versions2);
Some(derivation_tree)
}
DerivationTree::External(External::Custom(_, ref mut versions, _)) => {
strip_sentinel(versions);
Some(derivation_tree)
}
DerivationTree::Derived(mut derived) => {
let cause1 = Self::simplify_local_version_segments((*derived.cause1).clone());
let cause2 = Self::simplify_local_version_segments((*derived.cause2).clone());
match (cause1, cause2) {
(Some(cause1), Some(cause2)) => Some(DerivationTree::Derived(Derived {
cause1: Arc::new(cause1),
cause2: Arc::new(cause2),
terms: std::mem::take(&mut derived.terms)
.into_iter()
.map(|(pkg, mut term)| {
match &mut term {
Term::Positive(versions) => {
strip_sentinel(versions);
}
Term::Negative(versions) => {
strip_sentinel(versions);
}
}
(pkg, term)
})
.collect(),
shared_id: derived.shared_id,
})),
(Some(cause), None) | (None, Some(cause)) => Some(cause),
_ => None,
}
}
}
}

/// Initialize a [`NoSolutionHeader`] for this error.
pub fn header(&self) -> NoSolutionHeader {
NoSolutionHeader::new(self.markers.clone())
@@ -238,6 +391,7 @@ impl std::fmt::Display for NoSolutionError {
display_tree(&tree, "Resolver derivation tree before reduction");
}

// simplify_local_version_segments(&mut tree);
collapse_no_versions_of_workspace_members(&mut tree, &self.workspace_members);

if self.workspace_members.len() == 1 {
10 changes: 10 additions & 0 deletions crates/uv-resolver/src/pubgrub/report.rs
Original file line number Diff line number Diff line change
@@ -496,6 +496,16 @@ impl PubGrubReportFormatter<'_> {
}
}

// fn simplify_locals<'a>(
// &self,
// set: &'a Range<Version>,
// ) -> Cow<'a, Range<Version>> {
// // for (lower, upper) in set.iter() {
// //
// // }
// Cow::Owned(Range::from((Bound::Included(Version::new([0])), Bound::Excluded(Version::new([1])))))
// }

/// Generate the [`PubGrubHints`] for a derivation tree.
///
/// The [`PubGrubHints`] help users resolve errors by providing additional context or modifying
Loading

0 comments on commit d082763

Please sign in to comment.