From ee226c389e58c6e4825c4e369abb5df8693652d6 Mon Sep 17 00:00:00 2001 From: Jacob Finkelman Date: Tue, 12 Mar 2024 18:16:29 +0000 Subject: [PATCH] feat!: associated types instead of generics for P and VS --- benches/large_case.rs | 2 +- examples/caching_dependency_provider.rs | 29 +++++----- src/error.rs | 71 +++++++++++++++++++------ src/internal/core.rs | 62 ++++++++++----------- src/internal/incompatibility.rs | 3 +- src/internal/partial_solution.rs | 66 +++++++++++------------ src/internal/small_map.rs | 2 - src/lib.rs | 4 +- src/report.rs | 3 +- src/solver.rs | 49 ++++++++++------- src/type_aliases.rs | 14 ++++- tests/proptest.rs | 45 ++++++++++------ tests/sat_dependency_provider.rs | 11 ++-- 13 files changed, 217 insertions(+), 144 deletions(-) diff --git a/benches/large_case.rs b/benches/large_case.rs index 8efdaa9e..a9a8d8e3 100644 --- a/benches/large_case.rs +++ b/benches/large_case.rs @@ -17,7 +17,7 @@ fn bench<'a, P: Package + Deserialize<'a>, VS: VersionSet + Deserialize<'a>>( ) where ::V: Deserialize<'a>, { - let dependency_provider: OfflineDependencyProvider = ron::de::from_str(&case).unwrap(); + let dependency_provider: OfflineDependencyProvider = ron::de::from_str(case).unwrap(); b.iter(|| { for p in dependency_provider.packages() { diff --git a/examples/caching_dependency_provider.rs b/examples/caching_dependency_provider.rs index c4d82e20..9332a21b 100644 --- a/examples/caching_dependency_provider.rs +++ b/examples/caching_dependency_provider.rs @@ -2,24 +2,21 @@ use std::cell::RefCell; -use pubgrub::package::Package; use pubgrub::range::Range; use pubgrub::solver::{resolve, Dependencies, DependencyProvider, OfflineDependencyProvider}; +use pubgrub::type_aliases::V; use pubgrub::version::NumberVersion; -use pubgrub::version_set::VersionSet; type NumVS = Range; // An example implementing caching dependency provider that will // store queried dependencies in memory and check them before querying more from remote. -struct CachingDependencyProvider> { +struct CachingDependencyProvider { remote_dependencies: DP, - cached_dependencies: RefCell>, + cached_dependencies: RefCell>, } -impl> - CachingDependencyProvider -{ +impl CachingDependencyProvider { pub fn new(remote_dependencies_provider: DP) -> Self { CachingDependencyProvider { remote_dependencies: remote_dependencies_provider, @@ -28,15 +25,13 @@ impl> } } -impl> DependencyProvider - for CachingDependencyProvider -{ +impl DependencyProvider for CachingDependencyProvider { // Caches dependencies if they were already queried fn get_dependencies( &self, - package: &P, - version: &VS::V, - ) -> Result, DP::Err> { + package: &DP::P, + version: &V, + ) -> Result, DP::Err> { let mut cache = self.cached_dependencies.borrow_mut(); match cache.get_dependencies(package, version) { Ok(Dependencies::Unknown) => { @@ -59,17 +54,21 @@ impl> DependencyProvid } } - fn choose_version(&self, package: &P, range: &VS) -> Result, DP::Err> { + fn choose_version(&self, package: &DP::P, range: &DP::VS) -> Result>, DP::Err> { self.remote_dependencies.choose_version(package, range) } type Priority = DP::Priority; - fn prioritize(&self, package: &P, range: &VS) -> Self::Priority { + fn prioritize(&self, package: &DP::P, range: &DP::VS) -> Self::Priority { self.remote_dependencies.prioritize(package, range) } type Err = DP::Err; + + type P = DP::P; + + type VS = DP::VS; } fn main() { diff --git a/src/error.rs b/src/error.rs index 82e07419..18c951a0 100644 --- a/src/error.rs +++ b/src/error.rs @@ -4,58 +4,95 @@ use thiserror::Error; -use crate::package::Package; use crate::report::DerivationTree; -use crate::version_set::VersionSet; +use crate::solver::DependencyProvider; +use crate::type_aliases::V; /// Errors that may occur while solving dependencies. -#[derive(Error, Debug)] -pub enum PubGrubError { +#[derive(Error)] +pub enum PubGrubError +where + DP: DependencyProvider, +{ /// There is no solution for this set of dependencies. #[error("No solution")] - NoSolution(DerivationTree), + NoSolution(DerivationTree), /// Error arising when the implementer of - /// [DependencyProvider](crate::solver::DependencyProvider) + /// [DependencyProvider] /// returned an error in the method /// [get_dependencies](crate::solver::DependencyProvider::get_dependencies). #[error("Retrieving dependencies of {package} {version} failed")] ErrorRetrievingDependencies { /// Package whose dependencies we want. - package: P, + package: DP::P, /// Version of the package for which we want the dependencies. - version: VS::V, + version: V, /// Error raised by the implementer of - /// [DependencyProvider](crate::solver::DependencyProvider). - source: E, + /// [DependencyProvider]. + source: DP::Err, }, /// Error arising when the implementer of - /// [DependencyProvider](crate::solver::DependencyProvider) + /// [DependencyProvider] /// returned a dependency on the requested package. /// This technically means that the package directly depends on itself, /// and is clearly some kind of mistake. #[error("{package} {version} depends on itself")] SelfDependency { /// Package whose dependencies we want. - package: P, + package: DP::P, /// Version of the package for which we want the dependencies. - version: VS::V, + version: V, }, /// Error arising when the implementer of - /// [DependencyProvider](crate::solver::DependencyProvider) + /// [DependencyProvider] /// returned an error in the method /// [choose_version](crate::solver::DependencyProvider::choose_version). #[error("Decision making failed")] - ErrorChoosingPackageVersion(E), + ErrorChoosingPackageVersion(DP::Err), - /// Error arising when the implementer of [DependencyProvider](crate::solver::DependencyProvider) + /// Error arising when the implementer of [DependencyProvider] /// returned an error in the method [should_cancel](crate::solver::DependencyProvider::should_cancel). #[error("We should cancel")] - ErrorInShouldCancel(E), + ErrorInShouldCancel(DP::Err), /// Something unexpected happened. #[error("{0}")] Failure(String), } + +impl std::fmt::Debug for PubGrubError +where + DP: DependencyProvider, +{ + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::NoSolution(arg0) => f.debug_tuple("NoSolution").field(arg0).finish(), + Self::ErrorRetrievingDependencies { + package, + version, + source, + } => f + .debug_struct("ErrorRetrievingDependencies") + .field("package", package) + .field("version", version) + .field("source", source) + .finish(), + Self::SelfDependency { package, version } => f + .debug_struct("SelfDependency") + .field("package", package) + .field("version", version) + .finish(), + Self::ErrorChoosingPackageVersion(arg0) => f + .debug_tuple("ErrorChoosingPackageVersion") + .field(arg0) + .finish(), + Self::ErrorInShouldCancel(arg0) => { + f.debug_tuple("ErrorInShouldCancel").field(arg0).finish() + } + Self::Failure(arg0) => f.debug_tuple("Failure").field(arg0).finish(), + } + } +} diff --git a/src/internal/core.rs b/src/internal/core.rs index ca598564..c12bd0cb 100644 --- a/src/internal/core.rs +++ b/src/internal/core.rs @@ -3,55 +3,57 @@ //! Core model and functions //! to write a functional PubGrub algorithm. -use std::error::Error; +use std::collections::HashSet as Set; use std::sync::Arc; use crate::error::PubGrubError; use crate::internal::arena::Arena; -use crate::internal::incompatibility::{IncompId, Incompatibility, Relation}; +use crate::internal::incompatibility::{Incompatibility, Relation}; use crate::internal::partial_solution::SatisfierSearch::{ DifferentDecisionLevels, SameDecisionLevels, }; use crate::internal::partial_solution::{DecisionLevel, PartialSolution}; use crate::internal::small_vec::SmallVec; -use crate::package::Package; use crate::report::DerivationTree; -use crate::type_aliases::{DependencyConstraints, Map, Set}; +use crate::solver::DependencyProvider; +use crate::type_aliases::{DependencyConstraints, IncompDpId, Map, V}; use crate::version_set::VersionSet; /// Current state of the PubGrub algorithm. #[derive(Clone)] -pub struct State { - root_package: P, - root_version: VS::V, +pub struct State { + root_package: DP::P, + root_version: V, - incompatibilities: Map>>, + #[allow(clippy::type_complexity)] + incompatibilities: Map>>, /// 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. /// These will stay contradicted until we have backtracked beyond its associated decision level. - contradicted_incompatibilities: Map, DecisionLevel>, + contradicted_incompatibilities: Map, DecisionLevel>, /// All incompatibilities expressing dependencies, /// with common dependents merged. - merged_dependencies: Map<(P, P), SmallVec>>, + #[allow(clippy::type_complexity)] + merged_dependencies: Map<(DP::P, DP::P), SmallVec>>, /// 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>, + pub incompatibility_store: Arena>, /// This is a stack of work to be done in `unit_propagation`. /// It can definitely be a local variable to that method, but /// this way we can reuse the same allocation for better performance. - unit_propagation_buffer: SmallVec

, + unit_propagation_buffer: SmallVec, } -impl State { +impl State { /// Initialization of PubGrub state. - pub fn init(root_package: P, root_version: VS::V) -> Self { + pub fn init(root_package: DP::P, root_version: V) -> Self { let mut incompatibility_store = Arena::new(); let not_root_id = incompatibility_store.alloc(Incompatibility::not_root( root_package.clone(), @@ -72,7 +74,7 @@ impl State { } /// Add an incompatibility to the state. - pub 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); } @@ -80,22 +82,22 @@ impl State { /// Add an incompatibility to the state. pub fn add_incompatibility_from_dependencies( &mut self, - package: P, - version: VS::V, - deps: &DependencyConstraints, - ) -> std::ops::Range> { + package: DP::P, + version: V, + deps: &DependencyConstraints, + ) -> std::ops::Range> { // Create incompatibilities and allocate them in the store. let new_incompats_id_range = self.incompatibility_store .alloc_iter(deps.iter().map(|dep| { Incompatibility::from_dependency( package.clone(), - VS::singleton(version.clone()), + ::singleton(version.clone()), dep, ) })); // Merge the newly created incompatibilities with the older ones. - for id in IncompId::range_to_iter(new_incompats_id_range.clone()) { + for id in IncompDpId::::range_to_iter(new_incompats_id_range.clone()) { self.merge_incompatibility(id); } new_incompats_id_range @@ -103,7 +105,7 @@ impl State { /// Unit propagation is the core mechanism of the solving algorithm. /// CF - pub fn unit_propagation(&mut self, package: P) -> Result<(), PubGrubError> { + pub fn unit_propagation(&mut self, package: DP::P) -> Result<(), PubGrubError> { self.unit_propagation_buffer.clear(); self.unit_propagation_buffer.push(package); while let Some(current_package) = self.unit_propagation_buffer.pop() { @@ -179,10 +181,10 @@ impl State { /// Return the root cause and the backtracked model. /// CF #[allow(clippy::type_complexity)] - fn conflict_resolution( + fn conflict_resolution( &mut self, - incompatibility: IncompId, - ) -> Result<(P, IncompId), PubGrubError> { + incompatibility: IncompDpId, + ) -> Result<(DP::P, IncompDpId), PubGrubError> { let mut current_incompat_id = incompatibility; let mut current_incompat_changed = false; loop { @@ -229,7 +231,7 @@ impl State { /// Backtracking. fn backtrack( &mut self, - incompat: IncompId, + incompat: IncompDpId, incompat_changed: bool, decision_level: DecisionLevel, ) { @@ -257,7 +259,7 @@ impl State { /// (provided that no other version of foo exists between 1.0.0 and 2.0.0). /// We could collapse them into { foo (1.0.0 ∪ 1.1.0), not bar ^1.0.0 } /// without having to check the existence of other versions though. - fn merge_incompatibility(&mut self, mut id: IncompId) { + fn merge_incompatibility(&mut self, mut id: IncompDpId) { if let Some((p1, p2)) = self.incompatibility_store[id].as_dependency() { // If we are a dependency, there's a good chance we can be merged with a previous dependency let deps_lookup = self @@ -295,8 +297,8 @@ impl State { // Error reporting ######################################################### - fn build_derivation_tree(&self, incompat: IncompId) -> DerivationTree { - let mut all_ids = Set::default(); + fn build_derivation_tree(&self, incompat: IncompDpId) -> DerivationTree { + let mut all_ids: Set> = Set::default(); let mut shared_ids = Set::default(); let mut stack = vec![incompat]; while let Some(i) = stack.pop() { diff --git a/src/internal/incompatibility.rs b/src/internal/incompatibility.rs index 2dc6fbde..754cbf17 100644 --- a/src/internal/incompatibility.rs +++ b/src/internal/incompatibility.rs @@ -161,7 +161,7 @@ impl Incompatibility { .unwrap() .unwrap_positive() .union(other.get(p1).unwrap().unwrap_positive()), // It is safe to `simplify` here - (&p2, dep_term.map_or(&VS::empty(), |v| v.unwrap_negative())), + (p2, dep_term.map_or(&VS::empty(), |v| v.unwrap_negative())), )); } @@ -307,7 +307,6 @@ pub mod tests { use super::*; use crate::range::Range; use crate::term::tests::strategy as term_strat; - use crate::type_aliases::Map; use proptest::prelude::*; proptest! { diff --git a/src/internal/partial_solution.rs b/src/internal/partial_solution.rs index cf1d24db..77beade7 100644 --- a/src/internal/partial_solution.rs +++ b/src/internal/partial_solution.rs @@ -13,8 +13,9 @@ use crate::internal::arena::Arena; use crate::internal::incompatibility::{IncompId, Incompatibility, Relation}; use crate::internal::small_map::SmallMap; use crate::package::Package; +use crate::solver::DependencyProvider; use crate::term::Term; -use crate::type_aliases::SelectedDependencies; +use crate::type_aliases::{IncompDpId, SelectedDependencies, V}; use crate::version_set::VersionSet; use super::small_vec::SmallVec; @@ -33,7 +34,7 @@ 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` is primarily a HashMap from a package to its @@ -46,16 +47,15 @@ pub struct PartialSolution { /// 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>, + 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>, + 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 @@ -145,7 +145,7 @@ pub enum SatisfierSearch { type SatisfiedMap<'i, P, VS> = SmallMap<&'i P, (Option>, u32, DecisionLevel)>; -impl PartialSolution { +impl PartialSolution { /// Initialize an empty PartialSolution. pub fn empty() -> Self { Self { @@ -158,7 +158,7 @@ impl PartialSolution) { // Check that add_decision is never used in the wrong context. if cfg!(debug_assertions) { match self.package_assignments.get_mut(&package) { @@ -205,9 +205,9 @@ impl PartialSolution, - store: &Arena>, + package: DP::P, + cause: IncompDpId, + store: &Arena>, ) { use indexmap::map::Entry; let mut dated_derivation = DatedDerivation { @@ -259,8 +259,8 @@ impl PartialSolution Priority, - ) -> Option

{ + prioritizer: impl Fn(&DP::P, &DP::VS) -> DP::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; @@ -288,7 +288,7 @@ impl PartialSolution SelectedDependencies { + pub fn extract_solution(&self) -> SelectedDependencies { self.package_assignments .iter() .take(self.current_decision_level.0 as usize) @@ -349,13 +349,13 @@ impl PartialSolution>, - store: &Arena>, + package: DP::P, + version: V, + new_incompatibilities: std::ops::Range>, + store: &Arena>, ) { let exact = Term::exact(version.clone()); - let not_satisfied = |incompat: &Incompatibility| { + let not_satisfied = |incompat: &Incompatibility| { incompat.relation(|p| { if p == &package { Some(&exact) @@ -380,12 +380,12 @@ impl PartialSolution) -> Relation

{ + pub fn relation(&self, incompat: &Incompatibility) -> Relation { incompat.relation(|package| self.term_intersection_for_package(package)) } /// Retrieve intersection of terms related to package. - pub fn term_intersection_for_package(&self, package: &P) -> Option<&Term> { + pub fn term_intersection_for_package(&self, package: &DP::P) -> Option<&Term> { self.package_assignments .get(package) .map(|pa| pa.assignments_intersection.term()) @@ -394,9 +394,9 @@ impl PartialSolution( &self, - incompat: &'i Incompatibility, - store: &Arena>, - ) -> (&'i P, SatisfierSearch) { + incompat: &'i Incompatibility, + store: &Arena>, + ) -> (&'i DP::P, SatisfierSearch) { let satisfied_map = Self::find_satisfier(incompat, &self.package_assignments); let (&satisfier_package, &(satisfier_cause, _, satisfier_decision_level)) = satisfied_map .iter() @@ -431,9 +431,9 @@ impl PartialSolution( - incompat: &'i Incompatibility, - package_assignments: &FnvIndexMap>, - ) -> SatisfiedMap<'i, P, VS> { + incompat: &'i Incompatibility, + package_assignments: &FnvIndexMap>, + ) -> SatisfiedMap<'i, DP::P, DP::VS> { let mut satisfied = SmallMap::Empty; for (package, incompat_term) in incompat.iter() { let pa = package_assignments.get(package).expect("Must exist"); @@ -446,11 +446,11 @@ impl PartialSolution( - incompat: &Incompatibility, - satisfier_package: &'i P, - mut satisfied_map: SatisfiedMap<'i, P, VS>, - package_assignments: &FnvIndexMap>, - store: &Arena>, + incompat: &Incompatibility, + satisfier_package: &'i DP::P, + mut satisfied_map: SatisfiedMap<'i, DP::P, DP::VS>, + package_assignments: &FnvIndexMap>, + store: &Arena>, ) -> DecisionLevel { // First, let's retrieve the previous derivations and the initial accum_term. let satisfier_pa = package_assignments.get(satisfier_package).unwrap(); diff --git a/src/internal/small_map.rs b/src/internal/small_map.rs index 8fc16a06..a1fe5f9e 100644 --- a/src/internal/small_map.rs +++ b/src/internal/small_map.rs @@ -177,8 +177,6 @@ impl<'a, K: 'a, V: 'a> Iterator for IterSmallMap<'a, K, V> { fn next(&mut self) -> Option { match self { - // False-positive, remove when stable is >=1.76 February 24 - #[allow(clippy::map_identity)] IterSmallMap::Inline(inner) => inner.next().map(|(k, v)| (k, v)), IterSmallMap::Map(inner) => inner.next(), } diff --git a/src/lib.rs b/src/lib.rs index 3389e250..bdd28429 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -89,7 +89,7 @@ //! # //! type SemVS = Range; //! -//! impl DependencyProvider for MyDependencyProvider { +//! impl DependencyProvider for MyDependencyProvider { //! fn choose_version(&self, package: &String, range: &SemVS) -> Result, Infallible> { //! unimplemented!() //! } @@ -108,6 +108,8 @@ //! } //! //! type Err = Infallible; +//! type P = String; +//! type VS = SemVS; //! } //! ``` //! diff --git a/src/report.rs b/src/report.rs index 481d06e9..9f354183 100644 --- a/src/report.rs +++ b/src/report.rs @@ -256,7 +256,8 @@ impl DefaultStringReporter { ) { self.build_recursive_helper(derived, formatter); if let Some(id) = derived.shared_id { - if self.shared_with_ref.get(&id).is_none() { + #[allow(clippy::map_entry)] // `add_line_ref` not compatible with proposed fix. + if !self.shared_with_ref.contains_key(&id) { self.add_line_ref(); self.shared_with_ref.insert(id, self.ref_count); } diff --git a/src/solver.rs b/src/solver.rs index 9ac2edfe..7197c052 100644 --- a/src/solver.rs +++ b/src/solver.rs @@ -49,7 +49,7 @@ //! # //! # type NumVS = Range; //! # -//! # fn try_main() -> Result<(), PubGrubError<&'static str, NumVS, Infallible>> { +//! # fn try_main() -> Result<(), PubGrubError>> { //! # let dependency_provider = OfflineDependencyProvider::<&str, NumVS>::new(); //! # let package = "root"; //! # let version = 1; @@ -78,25 +78,24 @@ use crate::error::PubGrubError; use crate::internal::core::State; use crate::internal::incompatibility::Incompatibility; use crate::package::Package; -use crate::type_aliases::{DependencyConstraints, Map, SelectedDependencies}; +use crate::type_aliases::{DependencyConstraints, Map, SelectedDependencies, V}; use crate::version_set::VersionSet; use log::{debug, info}; /// Main function of the library. /// Finds a set of packages satisfying dependency bounds for a given package + version pair. -#[allow(clippy::type_complexity)] -pub fn resolve>( +pub fn resolve( dependency_provider: &DP, - package: P, - version: impl Into, -) -> Result, PubGrubError> { - let mut state = State::init(package.clone(), version.into()); - let mut added_dependencies: Map> = Map::default(); + package: DP::P, + version: impl Into>, +) -> Result, PubGrubError> { + let mut state: State = State::init(package.clone(), version.into()); + let mut added_dependencies: Map>> = Map::default(); let mut next = package; loop { dependency_provider .should_cancel() - .map_err(|err| PubGrubError::ErrorInShouldCancel(err))?; + .map_err(PubGrubError::ErrorInShouldCancel)?; info!("unit_propagation: {}", &next); state.unit_propagation(next)?; @@ -208,7 +207,13 @@ pub enum Dependencies { /// Trait that allows the algorithm to retrieve available packages and their dependencies. /// An implementor needs to be supplied to the [resolve] function. -pub trait DependencyProvider { +pub trait DependencyProvider { + /// How this provider stores the name of the packages. + type P: Package; + /// How this provider stores the version requirements for the packages. + /// This also specifies the representation of a version. + type VS: VersionSet; + /// [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. @@ -235,7 +240,7 @@ pub trait DependencyProvider { /// /// 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; + fn prioritize(&self, package: &Self::P, range: &Self::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. /// @@ -246,20 +251,24 @@ pub trait DependencyProvider { /// The kind of error returned from these methods. /// /// Returning this signals that resolution should fail with this error. - type Err: Error; + type Err: Error + 'static; /// 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, package: &P, range: &VS) -> Result, Self::Err>; + fn choose_version( + &self, + package: &Self::P, + range: &Self::VS, + ) -> Result>, Self::Err>; /// Retrieves the package dependencies. /// Return [Dependencies::Unknown] if its dependencies are unknown. fn get_dependencies( &self, - package: &P, - version: &VS::V, - ) -> Result, Self::Err>; + package: &Self::P, + version: &V, + ) -> Result, Self::Err>; /// This is called fairly regularly during the resolution, /// if it returns an Err then resolution will be terminated. @@ -343,7 +352,11 @@ impl OfflineDependencyProvider { /// 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 { +impl DependencyProvider for OfflineDependencyProvider { + type P = P; + + type VS = VS; + type Err = Infallible; fn choose_version(&self, package: &P, range: &VS) -> Result, Infallible> { diff --git a/src/type_aliases.rs b/src/type_aliases.rs index ba57be06..86846178 100644 --- a/src/type_aliases.rs +++ b/src/type_aliases.rs @@ -2,19 +2,29 @@ //! Publicly exported type aliases. +use crate::{ + internal::incompatibility::IncompId, solver::DependencyProvider, version_set::VersionSet, +}; + /// Map implementation used by the library. pub type Map = rustc_hash::FxHashMap; /// Set implementation used by the library. pub type Set = rustc_hash::FxHashSet; +/// The version type associated with the version set associated with the dependency provider. +pub type V = <::VS as VersionSet>::V; + /// Concrete dependencies picked by the library during [resolve](crate::solver::resolve) /// from [DependencyConstraints]. -pub type SelectedDependencies = Map; +pub type SelectedDependencies = Map<::P, V>; /// Holds information about all possible versions a given package can accept. /// There is a difference in semantics between an empty map /// inside [DependencyConstraints] and [Dependencies::Unknown](crate::solver::Dependencies::Unknown): /// the former means the package has no dependency and it is a known fact, -/// while the latter means they could not be fetched by the [DependencyProvider](crate::solver::DependencyProvider). +/// while the latter means they could not be fetched by the [DependencyProvider]. pub type DependencyConstraints = Map; + +pub(crate) type IncompDpId = + IncompId<::P, ::VS>; diff --git a/tests/proptest.rs b/tests/proptest.rs index 24dd64a4..3043e23c 100644 --- a/tests/proptest.rs +++ b/tests/proptest.rs @@ -10,7 +10,7 @@ use pubgrub::package::Package; use pubgrub::range::Range; use pubgrub::report::{DefaultStringReporter, DerivationTree, External, Reporter}; use pubgrub::solver::{resolve, Dependencies, DependencyProvider, OfflineDependencyProvider}; -use pubgrub::type_aliases::SelectedDependencies; +use pubgrub::type_aliases::{SelectedDependencies, V}; use pubgrub::version::NumberVersion; #[cfg(feature = "serde")] use pubgrub::version::SemanticVersion; @@ -32,9 +32,7 @@ struct OldestVersionsDependencyProvider( OfflineDependencyProvider, ); -impl DependencyProvider - for OldestVersionsDependencyProvider -{ +impl DependencyProvider for OldestVersionsDependencyProvider { fn get_dependencies(&self, p: &P, v: &VS::V) -> Result, Infallible> { self.0.get_dependencies(p, v) } @@ -49,13 +47,17 @@ impl DependencyProvider .cloned()) } - type Priority = as DependencyProvider>::Priority; + type Priority = as DependencyProvider>::Priority; fn prioritize(&self, package: &P, range: &VS) -> Self::Priority { self.0.prioritize(package, range) } type Err = Infallible; + + type P = P; + + type VS = VS; } /// The same as DP but it has a timeout. @@ -78,10 +80,12 @@ impl TimeoutDependencyProvider { } } -impl> DependencyProvider - for TimeoutDependencyProvider -{ - fn get_dependencies(&self, p: &P, v: &VS::V) -> Result, DP::Err> { +impl DependencyProvider for TimeoutDependencyProvider { + fn get_dependencies( + &self, + p: &DP::P, + v: &V, + ) -> Result, DP::Err> { self.dp.get_dependencies(p, v) } @@ -93,24 +97,31 @@ impl> DependencyProvid Ok(()) } - fn choose_version(&self, package: &P, range: &VS) -> Result, DP::Err> { + fn choose_version(&self, package: &DP::P, range: &DP::VS) -> Result>, DP::Err> { self.dp.choose_version(package, range) } type Priority = DP::Priority; - fn prioritize(&self, package: &P, range: &VS) -> Self::Priority { + fn prioritize(&self, package: &DP::P, range: &DP::VS) -> Self::Priority { self.dp.prioritize(package, range) } type Err = DP::Err; + + type P = DP::P; + + type VS = DP::VS; } -fn timeout_resolve>( +fn timeout_resolve( dependency_provider: DP, - name: P, - version: impl Into, -) -> Result, PubGrubError> { + name: DP::P, + version: impl Into>, +) -> Result< + SelectedDependencies>, + PubGrubError>, +> { resolve( &TimeoutDependencyProvider::new(dependency_provider, 50_000), name, @@ -549,7 +560,7 @@ proptest! { // that was not selected should not change that. let smaller_dependency_provider = retain_versions(&dependency_provider, |n, v| { used.get(n) == Some(v) // it was used - || to_remove.get(&(*n, *v)).is_none() // or it is not one to be removed + || !to_remove.contains(&(*n, *v)) // or it is not one to be removed }); prop_assert!( timeout_resolve(smaller_dependency_provider.clone(), name, ver).is_ok(), @@ -563,7 +574,7 @@ proptest! { // If resolution was unsuccessful, then it should stay unsuccessful // even if any version of a crate is unpublished. let smaller_dependency_provider = retain_versions(&dependency_provider, |n, v| { - to_remove.get(&(*n, *v)).is_some() // it is one to be removed + to_remove.contains(&(*n, *v)) // it is one to be removed }); prop_assert!( timeout_resolve(smaller_dependency_provider.clone(), name, ver).is_err(), diff --git a/tests/sat_dependency_provider.rs b/tests/sat_dependency_provider.rs index 8aecefe9..1b76c852 100644 --- a/tests/sat_dependency_provider.rs +++ b/tests/sat_dependency_provider.rs @@ -1,7 +1,5 @@ // SPDX-License-Identifier: MPL-2.0 -use std::convert::Infallible; - use pubgrub::error::PubGrubError; use pubgrub::package::Package; use pubgrub::solver::{Dependencies, DependencyProvider, OfflineDependencyProvider}; @@ -118,7 +116,10 @@ impl SatResolve { } } - pub fn is_valid_solution(&mut self, pids: &SelectedDependencies) -> bool { + pub fn is_valid_solution( + &mut self, + pids: &SelectedDependencies>, + ) -> bool { let mut assumption = vec![]; for (p, vs) in &self.all_versions_by_p { @@ -135,9 +136,9 @@ impl SatResolve { .expect("docs say it can't error in default config") } - pub fn check_resolve( + pub fn check_resolve>( &mut self, - res: &Result, PubGrubError>, + res: &Result, PubGrubError>, p: &P, v: &VS::V, ) {