From fe309ffb63b2f3ce9b35eb7746b2350cd704515e Mon Sep 17 00:00:00 2001 From: Jacob Finkelman Date: Sun, 5 Nov 2023 21:05:42 -0500 Subject: [PATCH] perf!: use a priority queue (#104) BREAKING CHANGE: Changes the API of DependencyProvider --- Cargo.toml | 2 + examples/caching_dependency_provider.rs | 21 ++- src/error.rs | 2 +- src/internal/core.rs | 6 +- src/internal/partial_solution.rs | 135 ++++++++++----- src/lib.rs | 26 +-- src/solver.rs | 212 +++++++++++------------- src/version.rs | 2 +- tests/proptest.rs | 56 ++++--- 9 files changed, 264 insertions(+), 198 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 1898eb96..a4c94e69 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,6 +20,8 @@ include = ["Cargo.toml", "LICENSE", "README.md", "src/**", "tests/**", "examples # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] +indexmap = "2.0.2" +priority-queue = "1.1.1" thiserror = "1.0" rustc-hash = "1.1.0" serde = { version = "1.0", features = ["derive"], optional = true } diff --git a/examples/caching_dependency_provider.rs b/examples/caching_dependency_provider.rs index bec7d2ab..6ef8e893 100644 --- a/examples/caching_dependency_provider.rs +++ b/examples/caching_dependency_provider.rs @@ -32,13 +32,6 @@ impl> impl> DependencyProvider for CachingDependencyProvider { - fn choose_package_version, U: std::borrow::Borrow>( - &self, - packages: impl Iterator, - ) -> Result<(T, Option), Box> { - self.remote_dependencies.choose_package_version(packages) - } - // Caches dependencies if they were already queried fn get_dependencies( &self, @@ -66,6 +59,20 @@ impl> DependencyProvid error @ Err(_) => error, } } + + fn choose_version( + &self, + package: &P, + range: &VS, + ) -> Result, Box> { + self.remote_dependencies.choose_version(package, range) + } + + type Priority = DP::Priority; + + fn prioritize(&self, package: &P, range: &VS) -> Self::Priority { + self.remote_dependencies.prioritize(package, range) + } } fn main() { diff --git a/src/error.rs b/src/error.rs index d27090e2..15a410e3 100644 --- a/src/error.rs +++ b/src/error.rs @@ -46,7 +46,7 @@ pub enum PubGrubError { /// Error arising when the implementer of /// [DependencyProvider](crate::solver::DependencyProvider) /// returned an error in the method - /// [choose_package_version](crate::solver::DependencyProvider::choose_package_version). + /// [choose_version](crate::solver::DependencyProvider::choose_version). #[error("Decision making failed")] ErrorChoosingPackageVersion(Box), diff --git a/src/internal/core.rs b/src/internal/core.rs index 973a9b8b..06e3ae21 100644 --- a/src/internal/core.rs +++ b/src/internal/core.rs @@ -20,7 +20,7 @@ use crate::version_set::VersionSet; /// Current state of the PubGrub algorithm. #[derive(Clone)] -pub struct State { +pub struct State { root_package: P, root_version: VS::V, @@ -32,7 +32,7 @@ pub struct State { /// Partial solution. /// TODO: remove pub. - pub partial_solution: PartialSolution, + pub partial_solution: PartialSolution, /// The store is the reference storage for all incompatibilities. pub incompatibility_store: Arena>, @@ -43,7 +43,7 @@ pub struct State { unit_propagation_buffer: SmallVec

, } -impl State { +impl State { /// Initialization of PubGrub state. pub fn init(root_package: P, root_version: VS::V) -> Self { let mut incompatibility_store = Arena::new(); diff --git a/src/internal/partial_solution.rs b/src/internal/partial_solution.rs index c2d88bed..057dea13 100644 --- a/src/internal/partial_solution.rs +++ b/src/internal/partial_solution.rs @@ -1,20 +1,26 @@ // SPDX-License-Identifier: MPL-2.0 //! A Memory acts like a structured partial solution -//! where terms are regrouped by package in a [Map]. +//! where terms are regrouped by package in a [Map](crate::type_aliases::Map). use std::fmt::Display; +use std::hash::BuildHasherDefault; + +use priority_queue::PriorityQueue; +use rustc_hash::FxHasher; use crate::internal::arena::Arena; use crate::internal::incompatibility::{IncompId, Incompatibility, Relation}; use crate::internal::small_map::SmallMap; use crate::package::Package; use crate::term::Term; -use crate::type_aliases::{Map, SelectedDependencies}; +use crate::type_aliases::SelectedDependencies; use crate::version_set::VersionSet; use super::small_vec::SmallVec; +type FnvIndexMap = indexmap::IndexMap>; + #[derive(Debug, Copy, Clone, Ord, PartialOrd, Eq, PartialEq)] pub struct DecisionLevel(pub u32); @@ -27,13 +33,29 @@ impl DecisionLevel { /// The partial solution contains all package assignments, /// organized by package and historically ordered. #[derive(Clone, Debug)] -pub struct PartialSolution { +pub struct PartialSolution { next_global_index: u32, current_decision_level: DecisionLevel, - package_assignments: Map>, + /// `package_assignments` is primarily a HashMap from a package to its + /// `PackageAssignments`. But it can also keep the items in an order. + /// We maintain three sections in this order: + /// 1. `[..current_decision_level]` Are packages that have had a decision made sorted by the `decision_level`. + /// This makes it very efficient to extract the solution, And to backtrack to a particular decision level. + /// 2. `[current_decision_level..changed_this_decision_level]` Are packages that have **not** had there assignments + /// changed since the last time `prioritize` has bean called. Within this range there is no sorting. + /// 3. `[changed_this_decision_level..]` Containes all packages that **have** had there assignments changed since + /// the last time `prioritize` has bean called. The inverse is not necessarily true, some packages in the range + /// did not have a change. Within this range there is no sorting. + package_assignments: FnvIndexMap>, + /// `prioritized_potential_packages` is primarily a HashMap from a package with no desition and a positive assignment + /// to its `Priority`. But, it also maintains a max heap of packages by `Priority` order. + prioritized_potential_packages: PriorityQueue>, + changed_this_decision_level: usize, } -impl Display for PartialSolution { +impl Display + for PartialSolution +{ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let mut assignments: Vec<_> = self .package_assignments @@ -120,13 +142,15 @@ pub enum SatisfierSearch { }, } -impl PartialSolution { +impl PartialSolution { /// Initialize an empty PartialSolution. pub fn empty() -> Self { Self { next_global_index: 0, current_decision_level: DecisionLevel(0), - package_assignments: Map::default(), + package_assignments: FnvIndexMap::default(), + prioritized_potential_packages: PriorityQueue::default(), + changed_this_decision_level: 0, } } @@ -151,11 +175,16 @@ impl PartialSolution { } }, } + assert_eq!( + self.changed_this_decision_level, + self.package_assignments.len() + ); } + let new_idx = self.current_decision_level.0 as usize; self.current_decision_level = self.current_decision_level.increment(); - let pa = self + let (old_idx, _, pa) = self .package_assignments - .get_mut(&package) + .get_full_mut(&package) .expect("Derivations must already exist"); pa.highest_decision_level = self.current_decision_level; pa.assignments_intersection = AssignmentsIntersection::Decision(( @@ -163,6 +192,10 @@ impl PartialSolution { version.clone(), Term::exact(version), )); + // Maintain that the beginning of the `package_assignments` Have all decisions in sorted order. + if new_idx != old_idx { + self.package_assignments.swap_indices(new_idx, old_idx); + } self.next_global_index += 1; } @@ -173,7 +206,7 @@ impl PartialSolution { cause: IncompId, store: &Arena>, ) { - use std::collections::hash_map::Entry; + use indexmap::map::Entry; let term = store[cause].get(&package).unwrap().negate(); let dated_derivation = DatedDerivation { global_index: self.next_global_index, @@ -181,8 +214,10 @@ impl PartialSolution { cause, }; self.next_global_index += 1; + let pa_last_index = self.package_assignments.len().saturating_sub(1); match self.package_assignments.entry(package) { Entry::Occupied(mut occupied) => { + let idx = occupied.index(); let pa = occupied.get_mut(); pa.highest_decision_level = self.current_decision_level; match &mut pa.assignments_intersection { @@ -192,11 +227,21 @@ impl PartialSolution { } AssignmentsIntersection::Derivations(t) => { *t = t.intersection(&term); + if t.is_positive() { + // we can use `swap_indices` to make `changed_this_decision_level` only go down by 1 + // but the copying is slower then the larger search + self.changed_this_decision_level = + std::cmp::min(self.changed_this_decision_level, idx); + } } } pa.dated_derivations.push(dated_derivation); } Entry::Vacant(v) => { + if term.is_positive() { + self.changed_this_decision_level = + std::cmp::min(self.changed_this_decision_level, pa_last_index); + } v.insert(PackageAssignments { smallest_decision_level: self.current_decision_level, highest_decision_level: self.current_decision_level, @@ -207,43 +252,48 @@ impl PartialSolution { } } - /// Extract potential packages for the next iteration of unit propagation. - /// Return `None` if there is no suitable package anymore, which stops the algorithm. - /// A package is a potential pick if there isn't an already - /// selected version (no "decision") - /// and if it contains at least one positive derivation term - /// in the partial solution. - pub fn potential_packages(&self) -> Option> { - let mut iter = self - .package_assignments + pub fn pick_highest_priority_pkg( + &mut self, + prioritizer: impl Fn(&P, &VS) -> Priority, + ) -> Option

{ + 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; + let prioritized_potential_packages = &mut self.prioritized_potential_packages; + self.package_assignments + .get_range(self.changed_this_decision_level..) + .unwrap() .iter() + .filter(|(_, 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)) - .peekable(); - if iter.peek().is_some() { - Some(iter) - } else { - None - } + .for_each(|(p, r)| { + let priority = prioritizer(p, r); + prioritized_potential_packages.push(p.clone(), priority); + }); + self.changed_this_decision_level = self.package_assignments.len(); + prioritized_potential_packages.pop().map(|(p, _)| p) } /// 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 fn extract_solution(&self) -> Option> { - let mut solution = Map::default(); - for (p, pa) in &self.package_assignments { - match &pa.assignments_intersection { - AssignmentsIntersection::Decision((_, v, _)) => { - solution.insert(p.clone(), v.clone()); - } - AssignmentsIntersection::Derivations(term) => { - if term.is_positive() { - return None; - } + pub fn extract_solution(&self) -> SelectedDependencies { + self.package_assignments + .iter() + .take(self.current_decision_level.0 as usize) + .map(|(p, pa)| match &pa.assignments_intersection { + AssignmentsIntersection::Decision((_, v, _)) => (p.clone(), v.clone()), + AssignmentsIntersection::Derivations(_) => { + panic!("Derivations in the Decision part") } - } - } - Some(solution) + }) + .collect() } /// Backtrack the partial solution to a given decision level. @@ -290,6 +340,9 @@ impl PartialSolution { true } }); + // Throw away all stored priority levels, And mark that they all need to be recomputed. + self.prioritized_potential_packages.clear(); + self.changed_this_decision_level = self.current_decision_level.0.saturating_sub(1) as usize; } /// We can add the version to the partial solution as a decision @@ -386,7 +439,7 @@ impl PartialSolution { /// to return a coherent previous_satisfier_level. fn find_satisfier( incompat: &Incompatibility, - package_assignments: &Map>, + package_assignments: &FnvIndexMap>, store: &Arena>, ) -> SmallMap { let mut satisfied = SmallMap::Empty; @@ -407,7 +460,7 @@ impl PartialSolution { incompat: &Incompatibility, satisfier_package: &P, mut satisfied_map: SmallMap, - package_assignments: &Map>, + package_assignments: &FnvIndexMap>, store: &Arena>, ) -> DecisionLevel { // First, let's retrieve the previous derivations and the initial accum_term. diff --git a/src/lib.rs b/src/lib.rs index 3934b8f3..5f61fb51 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -75,7 +75,7 @@ //! trait for our own type. //! Let's say that we will use [String] for packages, //! and [SemanticVersion](version::SemanticVersion) for versions. -//! This may be done quite easily by implementing the two following functions. +//! This may be done quite easily by implementing the three following functions. //! ``` //! # use pubgrub::solver::{DependencyProvider, Dependencies}; //! # use pubgrub::version::SemanticVersion; @@ -89,7 +89,12 @@ //! type SemVS = Range; //! //! impl DependencyProvider for MyDependencyProvider { -//! fn choose_package_version, U: Borrow>(&self,packages: impl Iterator) -> Result<(T, Option), Box> { +//! fn choose_version(&self, package: &String, range: &SemVS) -> Result, Box> { +//! unimplemented!() +//! } +//! +//! type Priority = usize; +//! fn prioritize(&self, package: &String, range: &SemVS) -> Self::Priority { //! unimplemented!() //! } //! @@ -104,18 +109,17 @@ //! ``` //! //! The first method -//! [choose_package_version](crate::solver::DependencyProvider::choose_package_version) -//! chooses a package and available version compatible with the provided options. -//! A helper function -//! [choose_package_with_fewest_versions](crate::solver::choose_package_with_fewest_versions) -//! is provided for convenience -//! in cases when lists of available versions for packages are easily obtained. -//! The strategy of that helper function consists in choosing the package -//! with the fewest number of compatible versions to speed up resolution. +//! [choose_version](crate::solver::DependencyProvider::choose_version) +//! chooses a version compatible with the provided range for a package. +//! The second method +//! [prioritize](crate::solver::DependencyProvider::prioritize) +//! in which order different packages should be chosen. +//! Usually prioritizing packages +//! with the fewest number of compatible versions speeds up resolution. //! But in general you are free to employ whatever strategy suits you best //! to pick a package and a version. //! -//! The second method [get_dependencies](crate::solver::DependencyProvider::get_dependencies) +//! The third method [get_dependencies](crate::solver::DependencyProvider::get_dependencies) //! aims at retrieving the dependencies of a given package at a given version. //! Returns [None] if dependencies are unknown. //! diff --git a/src/solver.rs b/src/solver.rs index 7354ff7d..a1e5e06c 100644 --- a/src/solver.rs +++ b/src/solver.rs @@ -68,7 +68,7 @@ //! to satisfy the dependencies of that package and version pair. //! If there is no solution, the reason will be provided as clear as possible. -use std::borrow::Borrow; +use std::cmp::Reverse; use std::collections::{BTreeMap, BTreeSet as Set}; use std::error::Error; @@ -103,30 +103,27 @@ pub fn resolve( state.partial_solution ); - let Some(potential_packages) = state.partial_solution.potential_packages() else { - return state.partial_solution.extract_solution().ok_or_else(|| { - PubGrubError::Failure( - "How did we end up with no package to choose but no solution?".into(), - ) - }); + let Some(highest_priority_pkg) = state + .partial_solution + .pick_highest_priority_pkg(|p, r| dependency_provider.prioritize(p, r)) + else { + return Ok(state.partial_solution.extract_solution()); }; + next = highest_priority_pkg; - let decision = dependency_provider - .choose_package_version(potential_packages) - .map_err(PubGrubError::ErrorChoosingPackageVersion)?; - info!("DP chose: {} @ {:?}", decision.0, decision.1); - - next = decision.0.clone(); - - // Pick the next compatible version. let term_intersection = state .partial_solution .term_intersection_for_package(&next) .ok_or_else(|| { PubGrubError::Failure("a package was chosen but we don't have a term.".into()) })?; + let decision = dependency_provider + .choose_version(&next, term_intersection.unwrap_positive()) + .map_err(PubGrubError::ErrorChoosingPackageVersion)?; + info!("DP chose: {} @ {:?}", next, decision); - let v = match decision.1 { + // Pick the next compatible version. + let v = match decision { None => { let inc = Incompatibility::no_versions(next.clone(), term_intersection.clone()); state.add_incompatibility(inc); @@ -146,51 +143,53 @@ pub fn resolve( .or_default() .insert(v.clone()); - if !is_new_dependency { + if is_new_dependency { + // Retrieve that package dependencies. + let p = &next; + let dependencies = dependency_provider.get_dependencies(p, &v).map_err(|err| { + PubGrubError::ErrorRetrievingDependencies { + package: p.clone(), + version: v.clone(), + source: err, + } + })?; + + let known_dependencies = match dependencies { + Dependencies::Unknown => { + state.add_incompatibility(Incompatibility::unavailable_dependencies( + p.clone(), + v.clone(), + )); + continue; + } + Dependencies::Known(x) if x.contains_key(p) => { + return Err(PubGrubError::SelfDependency { + package: p.clone(), + version: v, + }); + } + Dependencies::Known(x) => x, + }; + + // Add that package and version if the dependencies are not problematic. + let dep_incompats = state.add_incompatibility_from_dependencies( + p.clone(), + v.clone(), + &known_dependencies, + ); + + state.partial_solution.add_version( + p.clone(), + v, + dep_incompats, + &state.incompatibility_store, + ); + } else { // `dep_incompats` are already in `incompatibilities` so we know there are not satisfied // terms and can add the decision directly. info!("add_decision (not first time): {} @ {}", &next, v); state.partial_solution.add_decision(next.clone(), v); - continue; } - - // Retrieve that package dependencies. - let p = &next; - let dependencies = dependency_provider.get_dependencies(p, &v).map_err(|err| { - PubGrubError::ErrorRetrievingDependencies { - package: p.clone(), - version: v.clone(), - source: err, - } - })?; - - let known_dependencies = match dependencies { - Dependencies::Unknown => { - state.add_incompatibility(Incompatibility::unavailable_dependencies( - p.clone(), - v.clone(), - )); - continue; - } - Dependencies::Known(x) if x.contains_key(p) => { - return Err(PubGrubError::SelfDependency { - package: p.clone(), - version: v, - }); - } - Dependencies::Known(x) => x, - }; - - // Add that package and version if the dependencies are not problematic. - let dep_incompats = - state.add_incompatibility_from_dependencies(p.clone(), v.clone(), &known_dependencies); - - state.partial_solution.add_version( - p.clone(), - v, - dep_incompats, - &state.incompatibility_store, - ); } } @@ -210,11 +209,15 @@ pub trait DependencyProvider { /// [Decision making](https://github.com/dart-lang/pub/blob/master/doc/solver.md#decision-making) /// is the process of choosing the next package /// and version that will be appended to the partial solution. - /// Every time such a decision must be made, - /// potential valid packages and sets of versions are preselected by the resolver, - /// and the dependency provider must choose. /// - /// The strategy employed to choose such package and version + /// Every time such a decision must be made, the resolver looks at all the potential valid + /// packages that have changed, and a asks the dependency provider how important each one is. + /// For each one it calls `prioritize` with the name of the package and the current set of + /// acceptable versions. + /// The resolver will then pick the package with the highes priority from all the potential valid + /// packages. + /// + /// The strategy employed to prioritize packages /// cannot change the existence of a solution or not, /// but can drastically change the performances of the solver, /// or the properties of the solution. @@ -227,16 +230,24 @@ pub trait DependencyProvider { /// > since these packages will run out of versions to try more quickly. /// > But there's likely room for improvement in these heuristics. /// - /// A helper function [choose_package_with_fewest_versions] is provided to ease - /// implementations of this method if you can produce an iterator - /// of the available versions in preference order for any package. + /// Note: the resolver may call this even when the range has not change, + /// if it is more efficient for the resolveres internal data structures. + fn prioritize(&self, package: &P, range: &VS) -> Self::Priority; + /// The type returned from `prioritize`. The resolver does not care what type this is + /// as long as it can pick a largest one and clone it. /// - /// Note: the type `T` ensures that this returns an item from the `packages` argument. - #[allow(clippy::type_complexity)] - fn choose_package_version, U: Borrow>( + /// [std::cmp::Reverse] can be useful if you want to pick the package with + /// the fewest versions that match the outstanding constraint. + type Priority: Ord + Clone; + + /// Once the resolver has found the highest `Priority` package from all potential valid + /// packages, it needs to know what vertion of that package to use. The most common pattern + /// is to select the largest vertion that the range contains. + fn choose_version( &self, - potential_packages: impl Iterator, - ) -> Result<(T, Option), Box>; + package: &P, + range: &VS, + ) -> Result, Box>; /// Retrieves the package dependencies. /// Return [Dependencies::Unknown] if its dependencies are unknown. @@ -256,35 +267,6 @@ pub trait DependencyProvider { } } -/// This is a helper function to make it easy to implement -/// [DependencyProvider::choose_package_version]. -/// It takes a function `list_available_versions` that takes a package and returns an iterator -/// of the available versions in preference order. -/// The helper finds the package from the `packages` argument with the fewest versions from -/// `list_available_versions` contained in the constraints. Then takes that package and finds the -/// first version contained in the constraints. -pub fn choose_package_with_fewest_versions( - list_available_versions: F, - potential_packages: impl Iterator, -) -> (T, Option) -where - T: Borrow

, - U: Borrow, - I: Iterator, - F: Fn(&P) -> I, -{ - let count_valid = |(p, set): &(T, U)| { - list_available_versions(p.borrow()) - .filter(|v| set.borrow().contains(v)) - .count() - }; - let (pkg, set) = potential_packages - .min_by_key(count_valid) - .expect("potential_packages gave us an empty iterator"); - let version = list_available_versions(pkg.borrow()).find(|v| set.borrow().contains(v)); - (pkg, version) -} - /// A basic implementation of [DependencyProvider]. #[derive(Debug, Clone, Default)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] @@ -354,25 +336,29 @@ impl OfflineDependencyProvider { /// An implementation of [DependencyProvider] that /// contains all dependency information available in memory. -/// Packages are picked with the fewest versions contained in the constraints first. +/// Currently packages are picked with the fewest versions contained in the constraints first. +/// But, that may change in new versions if better heuristics are found. /// Versions are picked with the newest versions first. impl DependencyProvider for OfflineDependencyProvider { - #[allow(clippy::type_complexity)] - fn choose_package_version, U: Borrow>( + fn choose_version( &self, - potential_packages: impl Iterator, - ) -> Result<(T, Option), Box> { - Ok(choose_package_with_fewest_versions( - |p| { - self.dependencies - .get(p) - .into_iter() - .flat_map(|k| k.keys()) - .rev() - .cloned() - }, - potential_packages, - )) + package: &P, + range: &VS, + ) -> Result, Box> { + Ok(self + .dependencies + .get(package) + .and_then(|versions| versions.keys().rev().find(|v| range.contains(v)).cloned())) + } + + type Priority = Reverse; + fn prioritize(&self, package: &P, range: &VS) -> Self::Priority { + Reverse( + self.dependencies + .get(package) + .map(|versions| versions.keys().filter(|v| range.contains(v)).count()) + .unwrap_or(0), + ) } fn get_dependencies( diff --git a/src/version.rs b/src/version.rs index bf0524b4..6f67c7de 100644 --- a/src/version.rs +++ b/src/version.rs @@ -121,7 +121,7 @@ impl SemanticVersion { } /// Error creating [SemanticVersion] from [String]. -#[derive(Error, Debug, PartialEq)] +#[derive(Error, Debug, PartialEq, Eq)] pub enum VersionParseError { /// [SemanticVersion] must contain major, minor, patch versions. #[error("version {full_version} must contain 3 numbers separated by dot")] diff --git a/tests/proptest.rs b/tests/proptest.rs index adea37b2..017efed0 100644 --- a/tests/proptest.rs +++ b/tests/proptest.rs @@ -6,10 +6,7 @@ use pubgrub::error::PubGrubError; use pubgrub::package::Package; use pubgrub::range::Range; use pubgrub::report::{DefaultStringReporter, DerivationTree, External, Reporter}; -use pubgrub::solver::{ - choose_package_with_fewest_versions, resolve, Dependencies, DependencyProvider, - OfflineDependencyProvider, -}; +use pubgrub::solver::{resolve, Dependencies, DependencyProvider, OfflineDependencyProvider}; use pubgrub::type_aliases::SelectedDependencies; use pubgrub::version::{NumberVersion, SemanticVersion}; use pubgrub::version_set::VersionSet; @@ -33,16 +30,6 @@ struct OldestVersionsDependencyProvider( impl DependencyProvider for OldestVersionsDependencyProvider { - fn choose_package_version, U: std::borrow::Borrow>( - &self, - potential_packages: impl Iterator, - ) -> Result<(T, Option), Box> { - Ok(choose_package_with_fewest_versions( - |p| self.0.versions(p).into_iter().flatten().cloned(), - potential_packages, - )) - } - fn get_dependencies( &self, p: &P, @@ -50,6 +37,26 @@ impl DependencyProvider ) -> Result, Box> { self.0.get_dependencies(p, v) } + + fn choose_version( + &self, + package: &P, + range: &VS, + ) -> Result, Box> { + Ok(self + .0 + .versions(package) + .into_iter() + .flatten() + .find(|&v| range.contains(v)) + .cloned()) + } + + type Priority = as DependencyProvider>::Priority; + + fn prioritize(&self, package: &P, range: &VS) -> Self::Priority { + self.0.prioritize(package, range) + } } /// The same as DP but it has a timeout. @@ -75,13 +82,6 @@ impl TimeoutDependencyProvider { impl> DependencyProvider for TimeoutDependencyProvider { - fn choose_package_version, U: std::borrow::Borrow>( - &self, - potential_packages: impl Iterator, - ) -> Result<(T, Option), Box> { - self.dp.choose_package_version(potential_packages) - } - fn get_dependencies( &self, p: &P, @@ -97,6 +97,20 @@ impl> DependencyProvid self.call_count.set(calls + 1); Ok(()) } + + fn choose_version( + &self, + package: &P, + range: &VS, + ) -> Result, Box> { + self.dp.choose_version(package, range) + } + + type Priority = DP::Priority; + + fn prioritize(&self, package: &P, range: &VS) -> Self::Priority { + self.dp.prioritize(package, range) + } } fn timeout_resolve>(