Get representations working

Author: charliermarsh

Cargo.lock

@@ -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" }

Cargo.toml

@@ -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]"),
         }
     }
 }

crates/uv-pep440/src/version.rs

@@ -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();

crates/uv-pep440/src/version_ranges.rs

@@ -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 {

crates/uv-resolver/src/error.rs

@@ -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