diff --git a/.github/workflows/permaref.yaml b/.github/workflows/permaref.yaml new file mode 100644 index 00000000..f49dfb30 --- /dev/null +++ b/.github/workflows/permaref.yaml @@ -0,0 +1,42 @@ +# 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 + permissions: + contents: "write" + 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/astral-sh/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 "$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 }} diff --git a/src/internal/arena.rs b/src/internal/arena.rs index e044bc37..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, } @@ -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 { diff --git a/src/internal/core.rs b/src/internal/core.rs index a61eba0d..74c1cfc4 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,11 +92,43 @@ 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); } + /// 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( @@ -129,7 +162,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> { @@ -288,7 +321,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, @@ -304,6 +338,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/incompatibility.rs b/src/internal/incompatibility.rs index 5ad1f66b..9f7247d7 100644 --- a/src/internal/incompatibility.rs +++ b/src/internal/incompatibility.rs @@ -28,13 +28,14 @@ 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. -pub(crate) type IncompId = Id>; +pub type IncompId = Id>; pub(crate) type IncompDpId = IncompId< ::P, @@ -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() { @@ -254,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)) @@ -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..b4bc2eec 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, 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::{IncompId, Incompatibility, Kind}; diff --git a/src/internal/partial_solution.rs b/src/internal/partial_solution.rs index af40795f..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; @@ -30,7 +31,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 +203,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) { @@ -241,6 +242,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, @@ -291,7 +307,21 @@ impl PartialSolution { } #[cold] - pub(crate) fn pick_highest_priority_pkg( + 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, mut prioritizer: impl FnMut(Id, &DP::VS) -> DP::Priority, ) -> Option<(Id, &DP::VS)> { @@ -320,7 +350,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) @@ -403,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. /// @@ -462,10 +514,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()) @@ -575,6 +624,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 { diff --git a/src/lib.rs b/src/lib.rs index 9eb76bf2..ed0cb16d 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, IncompId, Incompatibility, Kind, State}; + mod internal; 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, } } } 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:?}"), 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}")? }