From d0237b8ed8a97fbc3c00582839c37a8807e93118 Mon Sep 17 00:00:00 2001 From: konstin Date: Thu, 19 Dec 2024 10:13:22 +0100 Subject: [PATCH 01/15] Fix `clippy::new-without-default` for `HashArena` --- src/internal/arena.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/internal/arena.rs b/src/internal/arena.rs index e044bc37..1010be74 100644 --- a/src/internal/arena.rs +++ b/src/internal/arena.rs @@ -150,9 +150,7 @@ impl fmt::Debug for HashArena { impl HashArena { pub fn new() -> Self { - HashArena { - data: FnvIndexSet::default(), - } + Self::default() } pub fn alloc(&mut self, value: T) -> Id { @@ -161,6 +159,14 @@ impl HashArena { } } +impl Default for HashArena { + fn default() -> Self { + Self { + data: FnvIndexSet::default(), + } + } +} + impl Index> for HashArena { type Output = T; fn index(&self, id: Id) -> &T { From cb429d448ff2127f3625c6b972918f9fa15134a5 Mon Sep 17 00:00:00 2001 From: konstin Date: Wed, 31 Jul 2024 14:26:05 +0200 Subject: [PATCH 02/15] Expose internal interfaces used by uv This wrapper avoids accessing the `incompatibility_store` directly in uv code. Before: ```rust let dep_incompats = self.pubgrub.add_version( package.clone(), version.clone(), dependencies, ); self.pubgrub.partial_solution.add_version( package.clone(), version.clone(), dep_incompats, &self.pubgrub.incompatibility_store, ); ``` After: ```rust self.pubgrub.add_incompatibility_from_dependencies(package.clone(), version.clone(), dependencies); ``` `add_incompatibility_from_dependencies` is one of the main methods for the custom interface to pubgrub. --- src/internal/arena.rs | 4 ++-- src/internal/core.rs | 21 +++++++++++---------- src/internal/incompatibility.rs | 17 ++++++++++------- src/internal/mod.rs | 10 +++++++--- src/internal/partial_solution.rs | 13 +++++-------- src/lib.rs | 3 +++ src/term.rs | 2 +- 7 files changed, 39 insertions(+), 31 deletions(-) diff --git a/src/internal/arena.rs b/src/internal/arena.rs index 1010be74..426852a6 100644 --- a/src/internal/arena.rs +++ b/src/internal/arena.rs @@ -12,7 +12,7 @@ type FnvIndexSet = indexmap::IndexSet; /// that we actually don't need since it is phantom. /// /// -pub(crate) struct Id { +pub struct Id { raw: u32, _ty: PhantomData T>, } @@ -73,7 +73,7 @@ impl Id { /// 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 { +pub struct Arena { data: Vec, } diff --git a/src/internal/core.rs b/src/internal/core.rs index a61eba0d..7d3cd303 100644 --- a/src/internal/core.rs +++ b/src/internal/core.rs @@ -14,12 +14,14 @@ use crate::{DependencyProvider, DerivationTree, Map, NoSolutionError, VersionSet /// Current state of the PubGrub algorithm. #[derive(Clone)] -pub(crate) struct State { +pub struct State { + /// The root package and version. pub root_package: Id, root_version: DP::V, + /// All incompatibilities indexed by package. #[allow(clippy::type_complexity)] - incompatibilities: Map, Vec>>, + pub incompatibilities: Map, Vec>>, /// As an optimization, store the ids of incompatibilities that are already contradicted. /// @@ -33,14 +35,13 @@ pub(crate) struct State { merged_dependencies: Map<(Id, Id), SmallVec>>, /// Partial solution. - /// TODO: remove pub. - pub(crate) partial_solution: PartialSolution, + pub partial_solution: PartialSolution, /// The store is the reference storage for all incompatibilities. - pub(crate) incompatibility_store: Arena>, + pub incompatibility_store: Arena>, /// The store is the reference storage for all packages. - pub(crate) package_store: HashArena, + pub package_store: HashArena, /// This is a stack of work to be done in `unit_propagation`. /// It can definitely be a local variable to that method, but @@ -50,7 +51,7 @@ pub(crate) struct State { impl State { /// 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 mut package_store = HashArena::new(); let root_package = package_store.alloc(root_package); @@ -74,7 +75,7 @@ impl State { } /// Add the dependencies for the current version of the current package as incompatibilities. - pub(crate) fn add_package_version_dependencies( + pub fn add_package_version_dependencies( &mut self, package: Id, version: DP::V, @@ -91,7 +92,7 @@ impl State { } /// Add an incompatibility to the state. - pub(crate) fn add_incompatibility(&mut self, incompat: Incompatibility) { + pub fn add_incompatibility(&mut self, incompat: Incompatibility) { let id = self.incompatibility_store.alloc(incompat); self.merge_incompatibility(id); } @@ -129,7 +130,7 @@ impl State { /// incompatibility. #[cold] #[allow(clippy::type_complexity)] // Type definitions don't support impl trait. - pub(crate) fn unit_propagation( + pub fn unit_propagation( &mut self, package: Id, ) -> Result, IncompDpId)>, NoSolutionError> { diff --git a/src/internal/incompatibility.rs b/src/internal/incompatibility.rs index 5ad1f66b..a41c4da0 100644 --- a/src/internal/incompatibility.rs +++ b/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 { +pub struct Incompatibility { package_terms: SmallMap, Term>, - kind: Kind, + /// The reason for the incompatibility. + pub kind: Kind, } /// Type alias of unique identifiers for incompatibilities. @@ -42,8 +43,9 @@ pub(crate) type IncompDpId = IncompId< ::M, >; +/// The reason for the incompatibility. #[derive(Debug, Clone)] -enum Kind { +pub enum Kind { /// 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 Incompatibilit } /// Create an incompatibility to remember that a given set does not contain any version. - pub(crate) fn no_versions(package: Id

, term: Term) -> Self { + pub fn no_versions(package: Id

, term: Term) -> 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 Incompatibilit /// Create an incompatibility for a reason outside pubgrub. #[allow(dead_code)] // Used by uv - pub(crate) fn custom_term(package: Id

, term: Term, metadata: M) -> Self { + pub fn custom_term(package: Id

, term: Term, 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 Incompatibilit } /// Create an incompatibility for a reason outside pubgrub. - pub(crate) fn custom_version(package: Id

, version: VS::V, metadata: M) -> Self { + pub fn custom_version(package: Id

, version: VS::V, metadata: M) -> Self { let set = VS::singleton(version); let term = Term::Positive(set.clone()); Self { @@ -139,7 +141,7 @@ impl Incompatibilit } /// Build an incompatibility from a given dependency. - pub(crate) fn from_dependency(package: Id

, versions: VS, dep: (Id

, VS)) -> Self { + pub fn from_dependency(package: Id

, versions: VS, dep: (Id

, VS)) -> Self { let (p2, set2) = dep; Self { package_terms: if set2 == VS::empty() { @@ -353,6 +355,7 @@ impl<'a, P: Package, VS: VersionSet + 'a, M: Eq + Clone + Debug + Display + 'a> } impl Incompatibility { + /// Display the incompatibility. pub fn display<'a>(&'a self, package_store: &'a HashArena

) -> impl Display + 'a { match self.iter().collect::>().as_slice() { [] => "version solving failed".into(), diff --git a/src/internal/mod.rs b/src/internal/mod.rs index e10770d4..eb94bf3f 100644 --- a/src/internal/mod.rs +++ b/src/internal/mod.rs @@ -9,9 +9,13 @@ mod partial_solution; mod small_map; mod small_vec; -pub(crate) use arena::{Arena, HashArena, Id}; -pub(crate) use core::State; -pub(crate) use incompatibility::{IncompDpId, IncompId, Incompatibility, Relation}; +pub(crate) use arena::{Arena, HashArena}; +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 arena::Id; +pub use core::State; +pub use incompatibility::{Incompatibility, Kind}; diff --git a/src/internal/partial_solution.rs b/src/internal/partial_solution.rs index af40795f..72cf5bdb 100644 --- a/src/internal/partial_solution.rs +++ b/src/internal/partial_solution.rs @@ -30,7 +30,7 @@ impl DecisionLevel { /// The partial solution contains all package assignments, /// organized by package and historically ordered. #[derive(Clone, Debug)] -pub(crate) struct PartialSolution { +pub struct PartialSolution { next_global_index: u32, /// The number of decisions that have been made, equal to the number of packages with decisions. current_decision_level: DecisionLevel, @@ -202,7 +202,7 @@ impl PartialSolution { } /// Add a decision. - pub(crate) fn add_decision(&mut self, package: Id, version: DP::V) { + pub fn add_decision(&mut self, package: Id, 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) { @@ -291,7 +291,7 @@ impl PartialSolution { } #[cold] - pub(crate) fn pick_highest_priority_pkg( + pub fn pick_highest_priority_pkg( &mut self, mut prioritizer: impl FnMut(Id, &DP::VS) -> DP::Priority, ) -> Option<(Id, &DP::VS)> { @@ -320,7 +320,7 @@ impl PartialSolution { /// 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) -> impl Iterator, DP::V)> + '_ { + pub fn extract_solution(&self) -> impl Iterator, DP::V)> + '_ { self.package_assignments .iter() .take(self.current_decision_level.0 as usize) @@ -462,10 +462,7 @@ impl PartialSolution { } /// Retrieve intersection of terms related to package. - pub(crate) fn term_intersection_for_package( - &self, - package: Id, - ) -> Option<&Term> { + pub fn term_intersection_for_package(&self, package: Id) -> Option<&Term> { self.package_assignments .get(&package) .map(|pa| pa.assignments_intersection.term()) diff --git a/src/lib.rs b/src/lib.rs index 9eb76bf2..6d7a69b1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -238,4 +238,7 @@ pub use version_ranges::Ranges; pub use version_ranges::Ranges as Range; pub use version_set::VersionSet; +// uv-specific additions +pub use internal::{Id, Incompatibility, Kind, State}; + mod internal; diff --git a/src/term.rs b/src/term.rs index 5e800874..e2b56b20 100644 --- a/src/term.rs +++ b/src/term.rs @@ -75,7 +75,7 @@ impl Term { /// Unwrap the set contained in a positive term. /// /// Panics 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, Self::Negative(set) => panic!("Negative term cannot unwrap positive set: {set:?}"), From dfd8de8cc452e7951c026969787e951ffa710ed5 Mon Sep 17 00:00:00 2001 From: konstin Date: Wed, 31 Jul 2024 14:34:05 +0200 Subject: [PATCH 03/15] Add `PartialSolution::prioritized_packages` --- src/internal/partial_solution.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/internal/partial_solution.rs b/src/internal/partial_solution.rs index 72cf5bdb..68b81d8c 100644 --- a/src/internal/partial_solution.rs +++ b/src/internal/partial_solution.rs @@ -290,6 +290,20 @@ impl PartialSolution { } } + #[cold] + pub fn prioritized_packages(&self) -> impl Iterator, &DP::VS)> { + // TODO(konsti): Should we use `self.outdated_priorities` instead? + let current_decision_level = self.current_decision_level; + self.package_assignments + .get_range(self.current_decision_level.0 as usize..) + .unwrap() + .iter() + .filter(move |(_, pa)| pa.highest_decision_level == current_decision_level) + .filter_map(|(&p, pa)| { + Some((p, pa.assignments_intersection.potential_package_filter()?)) + }) + } + #[cold] pub fn pick_highest_priority_pkg( &mut self, From e3546fbbe7a5e5ae13d3ef6fb2b16514f2412150 Mon Sep 17 00:00:00 2001 From: konstin Date: Wed, 31 Jul 2024 14:34:35 +0200 Subject: [PATCH 04/15] Use PEP440 style for range formatting --- tests/examples.rs | 6 +++--- version-ranges/src/lib.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/examples.rs b/tests/examples.rs index fcc237c1..00547f8d 100644 --- a/tests/examples.rs +++ b/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(), diff --git a/version-ranges/src/lib.rs b/version-ranges/src/lib.rs index 72de0d73..aac7c942 100644 --- a/version-ranges/src/lib.rs +++ b/version-ranges/src/lib.rs @@ -1085,7 +1085,7 @@ impl Display for Ranges { (Included(v), Unbounded) => write!(f, ">={v}")?, (Included(v), Included(b)) => { if v == b { - write!(f, "{v}")? + write!(f, "=={v}")? } else { write!(f, ">={v}, <={b}")? } From 392da448fb76f0515aba54b038c4159653d0379f Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Thu, 16 Nov 2023 13:19:40 -0600 Subject: [PATCH 05/15] Add GitHub workflow to automatically tag each commit on `main` (#2) --- .github/workflows/permaref.yaml | 40 +++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 .github/workflows/permaref.yaml diff --git a/.github/workflows/permaref.yaml b/.github/workflows/permaref.yaml new file mode 100644 index 00000000..29b5dbef --- /dev/null +++ b/.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: + release: + 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 ::set-output name=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-//' \ + ) + + - name: Configure Git + run: | + git config user.name "$GITHUB_ACTOR" + git config user.email "$GITHUB_ACTOR@users.noreply.github.com" + + - 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 }} From 4124f5d489dfca5661cf943d5659bbc40a5536f9 Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Thu, 16 Nov 2023 13:36:19 -0600 Subject: [PATCH 06/15] Fix-ups to tag generating action (#3) * Use new GitHub output syntax * Fix tag message --- .github/workflows/permaref.yaml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/permaref.yaml b/.github/workflows/permaref.yaml index 29b5dbef..eee99e6f 100644 --- a/.github/workflows/permaref.yaml +++ b/.github/workflows/permaref.yaml @@ -9,7 +9,7 @@ on: - "main" jobs: - release: + create-permaref: runs-on: ubuntu-latest steps: - name: Checkout @@ -21,10 +21,10 @@ jobs: # Enable pipefail so git command failures do not result in null versions downstream set -x - echo ::set-output name=LAST_PERMA_NUMBER::$(\ + 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: | @@ -34,7 +34,7 @@ jobs: - 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 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 }} From 0e17c6e71b37c0eadb7180d9c2016676aa729e13 Mon Sep 17 00:00:00 2001 From: konstin Date: Sat, 1 Jun 2024 17:04:00 +0200 Subject: [PATCH 07/15] Merge external custom reasons --- src/report.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/report.rs b/src/report.rs index 9beedecb..e137ab41 100644 --- a/src/report.rs +++ b/src/report.rs @@ -141,6 +141,9 @@ impl 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( @@ -158,8 +161,6 @@ impl DerivationTree ))) } } - // Cannot be merged because the reason may not match - DerivationTree::External(External::Custom(_, _, _)) => None, } } } From 7f680c24317bd0cbec8f60d0a4290eb1a26d3a13 Mon Sep 17 00:00:00 2001 From: konsti Date: Thu, 17 Oct 2024 11:43:51 +0200 Subject: [PATCH 08/15] Add a method to retrieve unchanging terms for a package (#32) --- src/internal/partial_solution.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/internal/partial_solution.rs b/src/internal/partial_solution.rs index 68b81d8c..1696170c 100644 --- a/src/internal/partial_solution.rs +++ b/src/internal/partial_solution.rs @@ -586,6 +586,18 @@ impl PartialSolution { pub(crate) fn current_decision_level(&self) -> DecisionLevel { self.current_decision_level } + + /// Retrieve the constraints on a package that will not change. + pub fn unchanging_term_for_package(&self, package: Id) -> Option<&Term> { + let pa = self.package_assignments.get(&package)?; + + let idx_newer = pa + .dated_derivations + .as_slice() + .partition_point(|dd| dd.decision_level <= DecisionLevel(1)); + let idx = idx_newer.checked_sub(1)?; + Some(&pa.dated_derivations[idx].accumulated_intersection) + } } impl PackageAssignments { From eea4528cb57fd5683fdcdf1c0c6b0fb7c96a82f4 Mon Sep 17 00:00:00 2001 From: konsti Date: Mon, 16 Dec 2024 09:46:49 +0100 Subject: [PATCH 09/15] Add method to return undecided packages (#37) This is used in uv for logging --- src/internal/partial_solution.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/internal/partial_solution.rs b/src/internal/partial_solution.rs index 1696170c..e7e4efd0 100644 --- a/src/internal/partial_solution.rs +++ b/src/internal/partial_solution.rs @@ -241,6 +241,21 @@ impl PartialSolution { self.next_global_index += 1; } + /// The list of package that have not been selected after the last prioritization. + /// + /// This list gets updated by [`Self::pick_highest_priority_pkg`] and cleared by backtracking. + #[allow(clippy::type_complexity)] + pub fn undecided_packages( + &self, + ) -> impl Iterator< + Item = ( + &Id, + &(::Priority, Reverse), + ), + > { + self.prioritized_potential_packages.iter() + } + /// Add a derivation. pub(crate) fn add_derivation( &mut self, From a8e565e421f84059177a43ebefde14d884c8dd3a Mon Sep 17 00:00:00 2001 From: konsti Date: Mon, 16 Dec 2024 09:59:23 +0100 Subject: [PATCH 10/15] Allow backtracking to before a specific package (#38) This allows discarding a previously made decision if it turned out to be a bad decision, even if all options with this decision have not yet been rejected. We allow attempting to backtrack on packages that were not decided yet to avoid the caller from making the duplicative check. https://github.com/astral-sh/uv/issues/8157 --- src/internal/core.rs | 18 +++++++++++++++++- src/internal/partial_solution.rs | 23 +++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/src/internal/core.rs b/src/internal/core.rs index 7d3cd303..0fe2b206 100644 --- a/src/internal/core.rs +++ b/src/internal/core.rs @@ -289,7 +289,8 @@ impl State { } } - /// Backtracking. + /// After a conflict occurred, backtrack the partial solution to a given decision level, and add + /// the incompatibility if it was new. fn backtrack( &mut self, incompat: IncompDpId, @@ -305,6 +306,21 @@ impl State { } } + /// Manually backtrack before the given package was selected. + /// + /// This can be used to switch the order of packages if the previous prioritization was bad. + /// + /// Returns the number of the decisions that were backtracked, or `None` if the package was not + /// decided on yet. + pub fn backtrack_package(&mut self, package: Id) -> Option { + let base_decision_level = self.partial_solution.current_decision_level(); + let new_decision_level = self.partial_solution.backtrack_package(package).ok()?; + // Remove contradicted incompatibilities that depend on decisions we just backtracked away. + self.contradicted_incompatibilities + .retain(|_, dl| *dl <= new_decision_level); + Some(base_decision_level.0 - new_decision_level.0) + } + /// Add this incompatibility into the set of all incompatibilities. /// /// PubGrub collapses identical dependencies from adjacent package versions diff --git a/src/internal/partial_solution.rs b/src/internal/partial_solution.rs index e7e4efd0..ba815fe8 100644 --- a/src/internal/partial_solution.rs +++ b/src/internal/partial_solution.rs @@ -7,6 +7,7 @@ use std::cmp::Reverse; use std::fmt::{Debug, Display}; use std::hash::BuildHasherDefault; +use log::debug; use priority_queue::PriorityQueue; use rustc_hash::FxHasher; @@ -432,6 +433,28 @@ impl PartialSolution { self.has_ever_backtracked = true; } + /// Backtrack the partial solution before a particular package was selected. + /// + /// This can be used to switch the order of packages if the previous prioritization was bad. + /// + /// Returns the new decision level on success and an error if the package was not decided on + /// yet. + pub(crate) fn backtrack_package(&mut self, package: Id) -> Result { + let Some(decision_level) = self.package_assignments.get_index_of(&package) else { + return Err(()); + }; + let decision_level = DecisionLevel(decision_level as u32); + if decision_level > self.current_decision_level { + return Err(()); + } + debug!( + "Package backtracking ot decision level {}", + decision_level.0 + ); + self.backtrack(decision_level); + Ok(decision_level) + } + /// Add a package version as decision if none of its dependencies conflicts with the partial /// solution. /// From 03ed3ac2ffbe4b6e8565370488f52ffe7f5dd70f Mon Sep 17 00:00:00 2001 From: konstin Date: Thu, 9 Jan 2025 10:48:47 +0100 Subject: [PATCH 11/15] Make `IncompId` public --- src/internal/incompatibility.rs | 2 +- src/internal/mod.rs | 4 ++-- src/lib.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/internal/incompatibility.rs b/src/internal/incompatibility.rs index a41c4da0..c344520d 100644 --- a/src/internal/incompatibility.rs +++ b/src/internal/incompatibility.rs @@ -35,7 +35,7 @@ pub struct Incompatibility = Id>; +pub type IncompId = Id>; pub(crate) type IncompDpId = IncompId< ::P, diff --git a/src/internal/mod.rs b/src/internal/mod.rs index eb94bf3f..b4bc2eec 100644 --- a/src/internal/mod.rs +++ b/src/internal/mod.rs @@ -10,7 +10,7 @@ mod small_map; mod small_vec; pub(crate) use arena::{Arena, HashArena}; -pub(crate) use incompatibility::{IncompDpId, IncompId, Relation}; +pub(crate) use incompatibility::{IncompDpId, Relation}; pub(crate) use partial_solution::{DecisionLevel, PartialSolution, SatisfierSearch}; pub(crate) use small_map::SmallMap; pub(crate) use small_vec::SmallVec; @@ -18,4 +18,4 @@ pub(crate) use small_vec::SmallVec; // uv-specific additions pub use arena::Id; pub use core::State; -pub use incompatibility::{Incompatibility, Kind}; +pub use incompatibility::{IncompId, Incompatibility, Kind}; diff --git a/src/lib.rs b/src/lib.rs index 6d7a69b1..ed0cb16d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -239,6 +239,6 @@ pub use version_ranges::Ranges as Range; pub use version_set::VersionSet; // uv-specific additions -pub use internal::{Id, Incompatibility, Kind, State}; +pub use internal::{Id, IncompId, Incompatibility, Kind, State}; mod internal; From a63e448d36ada53284bdfdb883b1d48672df4e80 Mon Sep 17 00:00:00 2001 From: konsti Date: Mon, 16 Dec 2024 10:18:52 +0100 Subject: [PATCH 12/15] Make `Incompatibility::iter` public (#41) --- src/internal/incompatibility.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/internal/incompatibility.rs b/src/internal/incompatibility.rs index c344520d..9f7247d7 100644 --- a/src/internal/incompatibility.rs +++ b/src/internal/incompatibility.rs @@ -256,7 +256,7 @@ impl Incompatibilit } /// Iterate over packages. - pub(crate) fn iter(&self) -> impl Iterator, &Term)> { + pub fn iter(&self) -> impl Iterator, &Term)> { self.package_terms .iter() .map(|(package, term)| (*package, term)) From aa04f121a1f0a7b7ba390bb646e498f24fad7055 Mon Sep 17 00:00:00 2001 From: konsti Date: Wed, 23 Apr 2025 13:43:48 +0200 Subject: [PATCH 13/15] Give the GitHub token write permissions (#43) Without the default write permissions the job otherwise fails, see https://github.com/astral-sh/pubgrub/actions/runs/14617176327/job/41007947575 --- .github/workflows/permaref.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/permaref.yaml b/.github/workflows/permaref.yaml index eee99e6f..f49dfb30 100644 --- a/.github/workflows/permaref.yaml +++ b/.github/workflows/permaref.yaml @@ -11,6 +11,8 @@ on: jobs: create-permaref: runs-on: ubuntu-latest + permissions: + contents: "write" steps: - name: Checkout uses: actions/checkout@v4 @@ -23,7 +25,7 @@ jobs: 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-//' \ + https://github.com/astral-sh/pubgrub.git | grep "tags/perma-" | tail -n1 | sed 's/.*\/perma-//' \ )" >> $GITHUB_OUTPUT - name: Configure Git From 3b3a66ee87f7088e52d5dca9186e10c70387b992 Mon Sep 17 00:00:00 2001 From: konsti Date: Fri, 9 May 2025 18:25:01 +0200 Subject: [PATCH 14/15] Allow multiple self-dependencies (#44) In uv, we don't use the `DependencyConstraints` map, but pass in the dependencies through an iterator. This means there can be duplicate dependencies in the input. This would previously make `merge_dependents` panic if a package dependent on itself twice with the same range: ```toml [package] name = "foo" version = "0.1.0" dependencies = ["foo", "foo"] ``` The fix is to ignore self-dependencies when merging dependents, given that they are always trivially true or trivially false. From a73b321e67bc158d7a56987911151b6c8ab4caea Mon Sep 17 00:00:00 2001 From: konstin Date: Sun, 10 Aug 2025 13:18:27 +0200 Subject: [PATCH 15/15] Add method for incompatibilities between virtual and base packages Add a new method to add a single custom incompatibility that requires the base package and the proxy package share the same version range. This intended for cases where proxy packages (also known as virtual packages) are used. Without this information, pubgrub does not know that these packages have to be at the same version. In cases where the base package is already to an incompatible version, this avoids going through all versions of the proxy package. In cases where there are two incompatible proxy packages, it avoids trying versions for both of them. Both improve performance (we don't need to check all versions when there is a conflict) and error messages (report a conflict of version ranges instead of enumerating the conflicting versions). There's several usage patterns for this method. The basic one is upon encountering a dependency on a proxy package with a range, using this method with its base package and that range. --- src/internal/core.rs | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/internal/core.rs b/src/internal/core.rs index 0fe2b206..74c1cfc4 100644 --- a/src/internal/core.rs +++ b/src/internal/core.rs @@ -97,6 +97,38 @@ impl State { self.merge_incompatibility(id); } + /// Add a single custom incompatibility that requires the base package and the proxy package + /// share the same version range. + /// + /// This intended for cases where proxy packages (also known as virtual packages) are used. + /// Without this information, pubgrub does not know that these packages have to be at the same + /// version. In cases where the base package is already to an incompatible version, this avoids + /// going through all versions of the proxy package. In cases where there are two incompatible + /// proxy packages, it avoids trying versions for both of them. Both improve performance (we + /// don't need to check all versions when there is a conflict) and error messages (report a + /// conflict of version ranges instead of enumerating the conflicting versions). + /// + /// Using this method requires that each version of the proxy package depends on the exact + /// version of the base package. + pub fn add_proxy_package( + &mut self, + proxy_package: Id, + base_package: Id, + versions: DP::VS, + ) { + let incompat = Incompatibility::from_dependency( + proxy_package, + versions.clone(), + (base_package, versions), + ); + let id = self + .incompatibility_store + .alloc_iter([incompat].into_iter()); + for id in IncompDpId::::range_to_iter(id) { + self.merge_incompatibility(id); + } + } + /// Add an incompatibility to the state. #[cold] pub(crate) fn add_incompatibility_from_dependencies(