Consistency between Range::iter and Range::bounding_range

.github/workflows/permaref.yaml

@@ -0,0 +1,40 @@
+# Automatically creates a tag for each commit to `main` so when we rebase
+# changes on top of the upstream, we retain permanent references to each
+# previous commit so they are not orphaned and eventually deleted.
+name: Create permanent reference
+
+on:
+  push:
+    branches:
+      - "main"
+
+jobs:
+  create-permaref:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+
+      - name: Get the permanent ref number
+        id: get_version
+        run: |
+          # Enable pipefail so git command failures do not result in null versions downstream
+          set -x
+
+          echo "LAST_PERMA_NUMBER=$(\
+            git ls-remote --tags --refs --sort="v:refname" \
+            https://github.com/zanieb/pubgrub.git | grep "tags/perma-" | tail -n1 | sed 's/.*\/perma-//' \
+          )" >> $GITHUB_OUTPUT
+
+      - name: Configure Git
+        run: |
+          git config user.name "$GITHUB_ACTOR"
+          git config user.email "[email protected]"
+
+      - name: Create and push the new tag
+        run: |
+          TAG="perma-$((LAST_PERMA_NUMBER + 1))"
+          git tag -a "$TAG" -m 'Automatically created on push to `main`'
+          git push origin "$TAG"
+        env:
+          LAST_PERMA_NUMBER: ${{ steps.get_version.outputs.LAST_PERMA_NUMBER }}

src/internal/arena.rs

@@ -10,7 +10,7 @@ use std::ops::{Index, Range};
 /// that we actually don't need since it is phantom.
 ///
 /// <https://github.com/rust-lang/rust/issues/26925>
-pub(crate) struct Id<T> {
+pub struct Id<T> {
     raw: u32,
     _ty: PhantomData<fn() -> T>,
 }
@@ -71,7 +71,7 @@ impl<T> Id<T> {
 /// to have references between those items.
 /// They are all dropped at once when the arena is dropped.
 #[derive(Clone, PartialEq, Eq)]
-pub(crate) struct Arena<T> {
+pub struct Arena<T> {
     data: Vec<T>,
 }
 

src/internal/core.rs

@@ -14,12 +14,13 @@ use crate::{DependencyProvider, DerivationTree, Map, NoSolutionError, VersionSet
 
 /// Current state of the PubGrub algorithm.
 #[derive(Clone)]
-pub(crate) struct State<DP: DependencyProvider> {
+pub struct State<DP: DependencyProvider> {
     root_package: DP::P,
     root_version: DP::V,
 
+    /// All incompatibilities indexed by package.
     #[allow(clippy::type_complexity)]
-    incompatibilities: Map<DP::P, Vec<IncompDpId<DP>>>,
+    pub incompatibilities: Map<DP::P, Vec<IncompDpId<DP>>>,
 
     /// Store the ids of incompatibilities that are already contradicted.
     /// For each one keep track of the decision level when it was found to be contradicted.
@@ -32,11 +33,10 @@ pub(crate) struct State<DP: DependencyProvider> {
     merged_dependencies: Map<(DP::P, DP::P), SmallVec<IncompDpId<DP>>>,
 
     /// Partial solution.
-    /// TODO: remove pub.
-    pub(crate) partial_solution: PartialSolution<DP>,
+    pub partial_solution: PartialSolution<DP>,
 
     /// The store is the reference storage for all incompatibilities.
-    pub(crate) incompatibility_store: Arena<Incompatibility<DP::P, DP::VS, DP::M>>,
+    pub incompatibility_store: Arena<Incompatibility<DP::P, DP::VS, DP::M>>,
 
     /// This is a stack of work to be done in `unit_propagation`.
     /// It can definitely be a local variable to that method, but
@@ -46,7 +46,7 @@ pub(crate) struct State<DP: DependencyProvider> {
 
 impl<DP: DependencyProvider> State<DP> {
     /// Initialization of PubGrub state.
-    pub(crate) fn init(root_package: DP::P, root_version: DP::V) -> Self {
+    pub fn init(root_package: DP::P, root_version: DP::V) -> Self {
         let mut incompatibility_store = Arena::new();
         let not_root_id = incompatibility_store.alloc(Incompatibility::not_root(
             root_package.clone(),
@@ -66,8 +66,28 @@ impl<DP: DependencyProvider> State<DP> {
         }
     }
 
+    /// Add the dependencies for the current version of the current package as incompatibilities.
+    pub fn add_package_version_dependencies(
+        &mut self,
+        package: DP::P,
+        version: DP::V,
+        dependencies: impl IntoIterator<Item = (DP::P, DP::VS)>,
+    ) {
+        let dep_incompats = self.add_incompatibility_from_dependencies(
+            package.clone(),
+            version.clone(),
+            dependencies,
+        );
+        self.partial_solution.add_package_version_incompatibilities(
+            package.clone(),
+            version.clone(),
+            dep_incompats,
+            &self.incompatibility_store,
+        )
+    }
+
     /// Add an incompatibility to the state.
-    pub(crate) fn add_incompatibility(&mut self, incompat: Incompatibility<DP::P, DP::VS, DP::M>) {
+    pub fn add_incompatibility(&mut self, incompat: Incompatibility<DP::P, DP::VS, DP::M>) {
         let id = self.incompatibility_store.alloc(incompat);
         self.merge_incompatibility(id);
     }
@@ -98,7 +118,7 @@ impl<DP: DependencyProvider> State<DP> {
 
     /// Unit propagation is the core mechanism of the solving algorithm.
     /// CF <https://github.com/dart-lang/pub/blob/master/doc/solver.md#unit-propagation>
-    pub(crate) fn unit_propagation(&mut self, package: DP::P) -> Result<(), NoSolutionError<DP>> {
+    pub fn unit_propagation(&mut self, package: DP::P) -> Result<(), NoSolutionError<DP>> {
         self.unit_propagation_buffer.clear();
         self.unit_propagation_buffer.push(package);
         while let Some(current_package) = self.unit_propagation_buffer.pop() {

src/internal/incompatibility.rs

@@ -28,9 +28,10 @@ use crate::{
 /// during conflict resolution. More about all this in
 /// [PubGrub documentation](https://github.com/dart-lang/pub/blob/master/doc/solver.md#incompatibility).
 #[derive(Debug, Clone)]
-pub(crate) struct Incompatibility<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> {
+pub struct Incompatibility<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> {
     package_terms: SmallMap<P, Term<VS>>,
-    kind: Kind<P, VS, M>,
+    /// The reason for the incompatibility.
+    pub kind: Kind<P, VS, M>,
 }
 
 /// Type alias of unique identifiers for incompatibilities.
@@ -42,8 +43,9 @@ pub(crate) type IncompDpId<DP> = IncompId<
     <DP as DependencyProvider>::M,
 >;
 
+/// The reason for the incompatibility.
 #[derive(Debug, Clone)]
-enum Kind<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> {
+pub enum Kind<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> {
     /// Initial incompatibility aiming at picking the root package for the first decision.
     ///
     /// This incompatibility drives the resolution, it requires that we pick the (virtual) root
@@ -104,7 +106,7 @@ impl<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> Incompatibilit
     }
 
     /// Create an incompatibility to remember that a given set does not contain any version.
-    pub(crate) fn no_versions(package: P, term: Term<VS>) -> Self {
+    pub fn no_versions(package: P, term: Term<VS>) -> Self {
         let set = match &term {
             Term::Positive(r) => r.clone(),
             Term::Negative(_) => panic!("No version should have a positive term"),
@@ -117,7 +119,7 @@ impl<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> Incompatibilit
 
     /// Create an incompatibility for a reason outside pubgrub.
     #[allow(dead_code)] // Used by uv
-    pub(crate) fn custom_term(package: P, term: Term<VS>, metadata: M) -> Self {
+    pub fn custom_term(package: P, term: Term<VS>, metadata: M) -> Self {
         let set = match &term {
             Term::Positive(r) => r.clone(),
             Term::Negative(_) => panic!("No version should have a positive term"),
@@ -129,7 +131,7 @@ impl<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> Incompatibilit
     }
 
     /// Create an incompatibility for a reason outside pubgrub.
-    pub(crate) fn custom_version(package: P, version: VS::V, metadata: M) -> Self {
+    pub fn custom_version(package: P, version: VS::V, metadata: M) -> Self {
         let set = VS::singleton(version);
         let term = Term::Positive(set.clone());
         Self {
@@ -139,7 +141,7 @@ impl<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> Incompatibilit
     }
 
     /// Build an incompatibility from a given dependency.
-    pub(crate) fn from_dependency(package: P, versions: VS, dep: (P, VS)) -> Self {
+    pub fn from_dependency(package: P, versions: VS, dep: (P, VS)) -> Self {
         let (p2, set2) = dep;
         Self {
             package_terms: if set2 == VS::empty() {

src/internal/mod.rs

@@ -10,8 +10,11 @@ mod small_map;
 mod small_vec;
 
 pub(crate) use arena::{Arena, Id};
-pub(crate) use core::State;
-pub(crate) use incompatibility::{IncompDpId, IncompId, Incompatibility, Relation};
+pub(crate) use incompatibility::{IncompDpId, IncompId, Relation};
 pub(crate) use partial_solution::{DecisionLevel, PartialSolution, SatisfierSearch};
 pub(crate) use small_map::SmallMap;
 pub(crate) use small_vec::SmallVec;
+
+// uv-specific additions
+pub use core::State;
+pub use incompatibility::{Incompatibility, Kind};

src/internal/partial_solution.rs

@@ -9,8 +9,7 @@ use std::hash::BuildHasherDefault;
 use priority_queue::PriorityQueue;
 use rustc_hash::FxHasher;
 
-use super::small_vec::SmallVec;
-use crate::internal::{Arena, IncompDpId, IncompId, Incompatibility, Relation, SmallMap};
+use crate::internal::{Arena, IncompDpId, IncompId, Incompatibility, Relation, SmallMap, SmallVec};
 use crate::{DependencyProvider, Package, SelectedDependencies, Term, VersionSet};
 
 type FnvIndexMap<K, V> = indexmap::IndexMap<K, V, BuildHasherDefault<FxHasher>>;
@@ -27,7 +26,7 @@ impl DecisionLevel {
 /// The partial solution contains all package assignments,
 /// organized by package and historically ordered.
 #[derive(Clone, Debug)]
-pub(crate) struct PartialSolution<DP: DependencyProvider> {
+pub struct PartialSolution<DP: DependencyProvider> {
     next_global_index: u32,
     current_decision_level: DecisionLevel,
     /// `package_assignments` is primarily a HashMap from a package to its
@@ -156,7 +155,7 @@ impl<DP: DependencyProvider> PartialSolution<DP> {
     }
 
     /// Add a decision.
-    pub(crate) fn add_decision(&mut self, package: DP::P, version: DP::V) {
+    pub fn add_decision(&mut self, package: DP::P, version: DP::V) {
         // Check that add_decision is never used in the wrong context.
         if cfg!(debug_assertions) {
             match self.package_assignments.get_mut(&package) {
@@ -255,7 +254,25 @@ impl<DP: DependencyProvider> PartialSolution<DP> {
         }
     }
 
-    pub(crate) fn pick_highest_priority_pkg(
+    pub fn prioritized_packages(&self) -> impl Iterator<Item = (&DP::P, &DP::VS)> {
+        let check_all = self.changed_this_decision_level
+            == self.current_decision_level.0.saturating_sub(1) as usize;
+        let current_decision_level = self.current_decision_level;
+        self.package_assignments
+            .get_range(self.changed_this_decision_level..)
+            .unwrap()
+            .iter()
+            .filter(move |(_, pa)| {
+                // We only actually need to update the package if its Been changed
+                // since the last time we called prioritize.
+                // Which means it's highest decision level is the current decision level,
+                // or if we backtracked in the mean time.
+                check_all || pa.highest_decision_level == current_decision_level
+            })
+            .filter_map(|(p, pa)| pa.assignments_intersection.potential_package_filter(p))
+    }
+
+    pub fn pick_highest_priority_pkg(
         &mut self,
         prioritizer: impl Fn(&DP::P, &DP::VS) -> DP::Priority,
     ) -> Option<DP::P> {
@@ -286,7 +303,7 @@ impl<DP: DependencyProvider> PartialSolution<DP> {
     /// If a partial solution has, for every positive derivation,
     /// a corresponding decision that satisfies that assignment,
     /// it's a total solution and version solving has succeeded.
-    pub(crate) fn extract_solution(&self) -> SelectedDependencies<DP> {
+    pub fn extract_solution(&self) -> SelectedDependencies<DP> {
         self.package_assignments
             .iter()
             .take(self.current_decision_level.0 as usize)
@@ -345,7 +362,7 @@ impl<DP: DependencyProvider> PartialSolution<DP> {
     /// In practice I think it can only produce a conflict if one of the dependencies
     /// (which are used to make the new incompatibilities)
     /// is already in the partial solution with an incompatible version.
-    pub(crate) fn add_version(
+    pub(crate) fn add_package_version_incompatibilities(
         &mut self,
         package: DP::P,
         version: DP::V,
@@ -386,7 +403,7 @@ impl<DP: DependencyProvider> PartialSolution<DP> {
     }
 
     /// Retrieve intersection of terms related to package.
-    pub(crate) fn term_intersection_for_package(&self, package: &DP::P) -> Option<&Term<DP::VS>> {
+    pub fn term_intersection_for_package(&self, package: &DP::P) -> Option<&Term<DP::VS>> {
         self.package_assignments
             .get(package)
             .map(|pa| pa.assignments_intersection.term())

src/lib.rs

@@ -233,4 +233,7 @@ pub use type_aliases::{DependencyConstraints, Map, SelectedDependencies, Set};
 pub use version::{SemanticVersion, VersionParseError};
 pub use version_set::VersionSet;
 
+// uv-specific additions
+pub use internal::{Incompatibility, Kind, State};
+
 mod internal;

src/range.rs

@@ -685,8 +685,10 @@ impl<V: Ord + Clone> Range<V> {
     }
 
     /// Iterate over the parts of the range.
-    pub fn iter(&self) -> impl Iterator<Item = (&Bound<V>, &Bound<V>)> {
-        self.segments.iter().map(|(start, end)| (start, end))
+    pub fn iter(&self) -> impl Iterator<Item = (Bound<&V>, Bound<&V>)> {
+        self.segments
+            .iter()
+            .map(|(start, end)| (start.as_ref(), end.as_ref()))
     }
 }
 
@@ -748,7 +750,7 @@ impl<V: Display + Eq> Display for Range<V> {
                     (Included(v), Unbounded) => write!(f, ">={v}")?,
                     (Included(v), Included(b)) => {
                         if v == b {
-                            write!(f, "{v}")?
+                            write!(f, "=={v}")?
                         } else {
                             write!(f, ">={v}, <={b}")?
                         }

src/report.rs

@@ -144,6 +144,9 @@ impl<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> DerivationTree
             //
             // Cannot be merged because the reason may not match
             DerivationTree::External(External::NoVersions(_, _)) => None,
+            DerivationTree::External(External::Custom(_, r, reason)) => Some(
+                DerivationTree::External(External::Custom(package, set.union(&r), reason)),
+            ),
             DerivationTree::External(External::FromDependencyOf(p1, r1, p2, r2)) => {
                 if p1 == package {
                     Some(DerivationTree::External(External::FromDependencyOf(
@@ -161,8 +164,6 @@ impl<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> DerivationTree
                     )))
                 }
             }
-            // Cannot be merged because the reason may not match
-            DerivationTree::External(External::Custom(_, _, _)) => None,
         }
     }
 }

src/solver.rs

@@ -163,15 +163,7 @@ pub fn resolve<DP: DependencyProvider>(
             };
 
             // Add that package and version if the dependencies are not problematic.
-            let dep_incompats =
-                state.add_incompatibility_from_dependencies(p.clone(), v.clone(), dependencies);
-
-            state.partial_solution.add_version(
-                p.clone(),
-                v.clone(),
-                dep_incompats,
-                &state.incompatibility_store,
-            );
+            state.add_package_version_dependencies(p.clone(), v.clone(), dependencies);
         } else {
             // `dep_incompats` are already in `incompatibilities` so we know there are not satisfied
             // terms and can add the decision directly.

src/term.rs

@@ -72,7 +72,7 @@ impl<VS: VersionSet> Term<VS> {
 
     /// Unwrap the set contained in a positive term.
     /// Will panic if used on a negative set.
-    pub(crate) fn unwrap_positive(&self) -> &VS {
+    pub fn unwrap_positive(&self) -> &VS {
         match self {
             Self::Positive(set) => set,
             _ => panic!("Negative term cannot unwrap positive set"),

tests/examples.rs

@@ -237,13 +237,13 @@ fn confusing_with_lots_of_holes() {
     };
     assert_eq!(
         &DefaultStringReporter::report(&derivation_tree),
-        r#"Because there is no available version for bar and foo 1 | 2 | 3 | 4 | 5 depends on bar, foo 1 | 2 | 3 | 4 | 5 is forbidden.
-And because there is no version of foo in <1 | >1, <2 | >2, <3 | >3, <4 | >4, <5 | >5 and root 1 depends on foo, root 1 is forbidden."#
+        r#"Because there is no available version for bar and foo ==1 | ==2 | ==3 | ==4 | ==5 depends on bar, foo ==1 | ==2 | ==3 | ==4 | ==5 is forbidden.
+And because there is no version of foo in <1 | >1, <2 | >2, <3 | >3, <4 | >4, <5 | >5 and root ==1 depends on foo, root ==1 is forbidden."#
     );
     derivation_tree.collapse_no_versions();
     assert_eq!(
         &DefaultStringReporter::report(&derivation_tree),
-        "Because foo depends on bar and root 1 depends on foo, root 1 is forbidden."
+        "Because foo depends on bar and root ==1 depends on foo, root ==1 is forbidden."
     );
     assert_eq!(
         derivation_tree.packages(),