diff --git a/examples/caching_dependency_provider.rs b/examples/caching_dependency_provider.rs index 2ebe4150..5669b5ff 100644 --- a/examples/caching_dependency_provider.rs +++ b/examples/caching_dependency_provider.rs @@ -23,16 +23,16 @@ impl CachingDependencyProvider { } } -impl DependencyProvider for CachingDependencyProvider { +impl> DependencyProvider for CachingDependencyProvider { // Caches dependencies if they were already queried fn get_dependencies( &self, package: &DP::P, version: &DP::V, - ) -> Result, DP::Err> { + ) -> Result, DP::Err> { let mut cache = self.cached_dependencies.borrow_mut(); match cache.get_dependencies(package, version) { - Ok(Dependencies::Unknown) => { + Ok(Dependencies::Unknown(reason)) => { let dependencies = self.remote_dependencies.get_dependencies(package, version); match dependencies { Ok(Dependencies::Known(dependencies)) => { @@ -43,7 +43,7 @@ impl DependencyProvider for CachingDependencyProvider Ok(Dependencies::Unknown), + Ok(Dependencies::Unknown(reason)) => Ok(Dependencies::Unknown(reason)), error @ Err(_) => error, } } @@ -67,6 +67,7 @@ impl DependencyProvider for CachingDependencyProvider> for CustomReportFormatter { +impl ReportFormatter, String> for CustomReportFormatter { type Output = String; fn format_terms(&self, terms: &Map>>) -> String { @@ -49,10 +49,12 @@ impl ReportFormatter> for CustomReportFormatter format!("{package} {range} is mandatory") } [(p1, Term::Positive(r1)), (p2, Term::Negative(r2))] => { - External::FromDependencyOf(p1, r1.clone(), p2, r2.clone()).to_string() + External::<_, _, String>::FromDependencyOf(p1, r1.clone(), p2, r2.clone()) + .to_string() } [(p1, Term::Negative(r1)), (p2, Term::Positive(r2))] => { - External::FromDependencyOf(p2, r2.clone(), p1, r1.clone()).to_string() + External::<_, _, String>::FromDependencyOf(p2, r2.clone(), p1, r1.clone()) + .to_string() } slice => { let str_terms: Vec<_> = slice.iter().map(|(p, t)| format!("{p} {t}")).collect(); @@ -61,7 +63,10 @@ impl ReportFormatter> for CustomReportFormatter } } - fn format_external(&self, external: &External>) -> String { + fn format_external( + &self, + external: &External, String>, + ) -> String { match external { External::NotRoot(package, version) => { format!("we are solving dependencies of {package} {version}") @@ -73,11 +78,11 @@ impl ReportFormatter> for CustomReportFormatter format!("there is no version of {package} in {set}") } } - External::UnavailableDependencies(package, set) => { + External::Custom(package, set, reason) => { if set == &Range::full() { - format!("dependencies of {package} are unavailable") + format!("dependencies of {package} are unavailable because {reason}") } else { - format!("dependencies of {package} at version {set} are unavailable") + format!("dependencies of {package} at version {set} are unavailable because {reason}") } } External::FromDependencyOf(package, package_set, dependency, dependency_set) => { diff --git a/src/error.rs b/src/error.rs index 47636036..b4921f32 100644 --- a/src/error.rs +++ b/src/error.rs @@ -15,7 +15,7 @@ where { /// There is no solution for this set of dependencies. #[error("No solution")] - NoSolution(DerivationTree), + NoSolution(DerivationTree), /// Error arising when the implementer of /// [DependencyProvider] diff --git a/src/internal/core.rs b/src/internal/core.rs index 476a9c6c..c20a6dd8 100644 --- a/src/internal/core.rs +++ b/src/internal/core.rs @@ -43,7 +43,7 @@ pub struct State { 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 @@ -74,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); } @@ -297,7 +297,10 @@ impl State { // Error reporting ######################################################### - fn build_derivation_tree(&self, incompat: IncompDpId) -> DerivationTree { + 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]; diff --git a/src/internal/incompatibility.rs b/src/internal/incompatibility.rs index 864d6822..979dde6f 100644 --- a/src/internal/incompatibility.rs +++ b/src/internal/incompatibility.rs @@ -3,7 +3,7 @@ //! An incompatibility is a set of terms for different packages //! that should never be satisfied all together. -use std::fmt; +use std::fmt::{self, Debug, Display}; use std::sync::Arc; use crate::internal::arena::{Arena, Id}; @@ -32,26 +32,44 @@ use crate::version_set::VersionSet; /// 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 struct Incompatibility { +pub struct Incompatibility { package_terms: SmallMap>, - kind: Kind, + kind: Kind, } /// Type alias of unique identifiers for incompatibilities. -pub type IncompId = Id>; +pub type IncompId = Id>; #[derive(Debug, Clone)] -enum Kind { +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 + /// packages. NotRoot(P, VS::V), /// There are no versions in the given range for this package. + /// + /// This incompatibility is used when we tried all versions in a range and no version + /// worked, so we have to backtrack NoVersions(P, VS), - /// Dependencies of the package are unavailable for versions in that range. - UnavailableDependencies(P, VS), /// Incompatibility coming from the dependencies of a given package. + /// + /// If a@1 depends on b>=1,<2, we create an incompatibility with terms `{a 1, b <1,>=2}` with + /// kind `FromDependencyOf(a, 1, b, >=1,<2)`. + /// + /// We can merge multiple dependents with the same version. For example, if a@1 depends on b and + /// a@2 depends on b, we can say instead a@1||2 depends on b. FromDependencyOf(P, VS, P, VS), /// Derived from two causes. Stores cause ids. - DerivedFrom(IncompId, IncompId), + /// + /// For example, if a -> b and b -> c, we can derive a -> c. + DerivedFrom(IncompId, IncompId), + /// The package is unavailable for reasons outside pubgrub. + /// + /// Examples: + /// * The version would require building the package, but builds are disabled. + /// * The package is not available in the cache, but internet access has been disabled. + Custom(P, VS, M), } /// A Relation describes how a set of terms can be compared to an incompatibility. @@ -71,7 +89,7 @@ pub enum Relation { Inconclusive, } -impl Incompatibility { +impl Incompatibility { /// Create the initial "not Root" incompatibility. pub fn not_root(package: P, version: VS::V) -> Self { Self { @@ -83,8 +101,7 @@ impl Incompatibility { } } - /// Create an incompatibility to remember - /// that a given set does not contain any version. + /// Create an incompatibility to remember that a given set does not contain any version. pub fn no_versions(package: P, term: Term) -> Self { let set = match &term { Term::Positive(r) => r.clone(), @@ -96,14 +113,25 @@ impl Incompatibility { } } - /// Create an incompatibility to remember - /// that a package version is not selectable - /// because its list of dependencies is unavailable. - pub fn unavailable_dependencies(package: P, version: VS::V) -> Self { + /// Create an incompatibility for a reason outside pubgrub. + pub fn custom_term(package: P, term: Term, metadata: M) -> Self { + let set = match &term { + Term::Positive(r) => r.clone(), + Term::Negative(_) => panic!("No version should have a positive term"), + }; + Self { + package_terms: SmallMap::One([(package.clone(), term)]), + kind: Kind::Custom(package, set, metadata), + } + } + + /// Create an incompatibility for a reason outside pubgrub. + pub fn custom_version(package: P, version: VS::V, metadata: M) -> Self { let set = VS::singleton(version); + let term = Term::Positive(set.clone()); Self { - package_terms: SmallMap::One([(package.clone(), Term::Positive(set.clone()))]), - kind: Kind::UnavailableDependencies(package, set), + package_terms: SmallMap::One([(package.clone(), term)]), + kind: Kind::Custom(package, set, metadata), } } @@ -135,7 +163,7 @@ impl Incompatibility { /// When multiple versions of a package depend on the same range of another package, /// we can merge the two into a single incompatibility. /// For example, if a@1 depends on b and a@2 depends on b, we can say instead - /// a@1 and a@b depend on b. + /// a@1||2 depends on b. /// /// It is a special case of prior cause computation where the unified package /// is the common dependant in the two incompatibilities expressing dependencies. @@ -155,6 +183,11 @@ impl Incompatibility { if dep_term != other.get(p2) { return None; } + // The metadata must be identical to merge + let self_metadata = self.metadata(); + if self_metadata != other.metadata() { + return None; + } return Some(Self::from_dependency( p1.clone(), self.get(p1) @@ -231,8 +264,8 @@ impl Incompatibility { self_id: Id, shared_ids: &Set>, store: &Arena, - precomputed: &Map, Arc>>, - ) -> DerivationTree { + precomputed: &Map, Arc>>, + ) -> DerivationTree { match store[self_id].kind.clone() { Kind::DerivedFrom(id1, id2) => { let derived = Derived { @@ -253,19 +286,38 @@ impl Incompatibility { DerivationTree::External(External::NotRoot(package, version)) } Kind::NoVersions(package, set) => { - DerivationTree::External(External::NoVersions(package, set)) + DerivationTree::External(External::NoVersions(package.clone(), set.clone())) } - Kind::UnavailableDependencies(package, set) => { - DerivationTree::External(External::UnavailableDependencies(package, set)) + Kind::FromDependencyOf(package, set, dep_package, dep_set) => { + DerivationTree::External(External::FromDependencyOf( + package.clone(), + set.clone(), + dep_package.clone(), + dep_set.clone(), + )) } - Kind::FromDependencyOf(package, set, dep_package, dep_set) => DerivationTree::External( - External::FromDependencyOf(package, set, dep_package, dep_set), - ), + Kind::Custom(package, set, metadata) => DerivationTree::External(External::Custom( + package.clone(), + set.clone(), + metadata.clone(), + )), + } + } + + fn metadata(&self) -> Option<&M> { + match &self.kind { + Kind::NotRoot(_, _) + | Kind::NoVersions(_, _) + | Kind::FromDependencyOf(_, _, _, _) + | Kind::DerivedFrom(_, _) => None, + Kind::Custom(_, _, metadata) => Some(metadata), } } } -impl<'a, P: Package, VS: VersionSet + 'a> Incompatibility { +impl<'a, P: Package, VS: VersionSet + 'a, M: Eq + Clone + Debug + Display + 'a> + Incompatibility +{ /// CF definition of Relation enum. pub fn relation(&self, terms: impl Fn(&P) -> Option<&'a Term>) -> Relation

{ let mut relation = Relation::Satisfied; @@ -293,12 +345,17 @@ impl<'a, P: Package, VS: VersionSet + 'a> Incompatibility { } } -impl fmt::Display for Incompatibility { +impl fmt::Display + for Incompatibility +{ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!( f, "{}", - DefaultStringReportFormatter.format_terms(&self.package_terms.as_map()) + ReportFormatter::::format_terms( + &DefaultStringReportFormatter, + &self.package_terms.as_map() + ) ) } } @@ -326,12 +383,12 @@ pub mod tests { let mut store = Arena::new(); let i1 = store.alloc(Incompatibility { package_terms: SmallMap::Two([("p1", t1.clone()), ("p2", t2.negate())]), - kind: Kind::UnavailableDependencies("0", Range::full()) + kind: Kind::Custom("0", Range::full(), "foo".to_string()) }); let i2 = store.alloc(Incompatibility { package_terms: SmallMap::Two([("p2", t2), ("p3", t3.clone())]), - kind: Kind::UnavailableDependencies("0", Range::full()) + kind: Kind::Custom("0", Range::full(), "bar".to_string()) }); let mut i3 = Map::default(); diff --git a/src/internal/partial_solution.rs b/src/internal/partial_solution.rs index 631e386f..b69c2815 100644 --- a/src/internal/partial_solution.rs +++ b/src/internal/partial_solution.rs @@ -3,7 +3,7 @@ //! A Memory acts like a structured partial solution //! where terms are regrouped by package in a [Map](crate::type_aliases::Map). -use std::fmt::Display; +use std::fmt::{Debug, Display}; use std::hash::BuildHasherDefault; use priority_queue::PriorityQueue; @@ -47,7 +47,7 @@ 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: @@ -77,14 +77,16 @@ impl Display for PartialSolution { /// that have already been made for a given package, /// as well as the intersection of terms by all of these. #[derive(Clone, Debug)] -struct PackageAssignments { +struct PackageAssignments { smallest_decision_level: DecisionLevel, highest_decision_level: DecisionLevel, - dated_derivations: SmallVec>, + dated_derivations: SmallVec>, assignments_intersection: AssignmentsIntersection, } -impl Display for PackageAssignments { +impl Display + for PackageAssignments +{ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let derivations: Vec<_> = self .dated_derivations @@ -103,14 +105,16 @@ impl Display for PackageAssignments { } #[derive(Clone, Debug)] -pub struct DatedDerivation { +pub struct DatedDerivation { global_index: u32, decision_level: DecisionLevel, - cause: IncompId, + cause: IncompId, accumulated_intersection: Term, } -impl Display for DatedDerivation { +impl Display + for DatedDerivation +{ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "{:?}, cause: {:?}", self.decision_level, self.cause) } @@ -134,16 +138,16 @@ impl Display for AssignmentsIntersection { } #[derive(Clone, Debug)] -pub enum SatisfierSearch { +pub enum SatisfierSearch { DifferentDecisionLevels { previous_satisfier_level: DecisionLevel, }, SameDecisionLevels { - satisfier_cause: IncompId, + satisfier_cause: IncompId, }, } -type SatisfiedMap<'i, P, VS> = SmallMap<&'i P, (Option>, u32, DecisionLevel)>; +type SatisfiedMap<'i, P, VS, M> = SmallMap<&'i P, (Option>, u32, DecisionLevel)>; impl PartialSolution { /// Initialize an empty PartialSolution. @@ -207,7 +211,7 @@ impl PartialSolution { &mut self, package: DP::P, cause: IncompDpId, - store: &Arena>, + store: &Arena>, ) { use indexmap::map::Entry; let mut dated_derivation = DatedDerivation { @@ -351,11 +355,11 @@ impl PartialSolution { &mut self, package: DP::P, version: DP::V, - new_incompatibilities: std::ops::Range>, - store: &Arena>, + 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,7 +384,7 @@ impl PartialSolution { } /// Check if the terms in the partial solution satisfy the incompatibility. - pub fn relation(&self, incompat: &Incompatibility) -> Relation { + pub fn relation(&self, incompat: &Incompatibility) -> Relation { incompat.relation(|package| self.term_intersection_for_package(package)) } @@ -394,9 +398,9 @@ impl PartialSolution { /// Figure out if the satisfier and previous satisfier are of different decision levels. pub fn satisfier_search<'i>( &self, - incompat: &'i Incompatibility, - store: &Arena>, - ) -> (&'i DP::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 +435,9 @@ impl PartialSolution { /// It would be nice if we could get rid of it, but I don't know if then it will be possible /// to return a coherent previous_satisfier_level. fn find_satisfier<'i>( - incompat: &'i Incompatibility, - package_assignments: &FnvIndexMap>, - ) -> SatisfiedMap<'i, DP::P, DP::VS> { + incompat: &'i Incompatibility, + package_assignments: &FnvIndexMap>, + ) -> SatisfiedMap<'i, DP::P, DP::VS, DP::M> { let mut satisfied = SmallMap::Empty; for (package, incompat_term) in incompat.iter() { let pa = package_assignments.get(package).expect("Must exist"); @@ -446,11 +450,11 @@ impl PartialSolution { /// such that incompatibility is satisfied by the partial solution up to /// and including that assignment plus satisfier. fn find_previous_satisfier<'i>( - incompat: &Incompatibility, + incompat: &Incompatibility, satisfier_package: &'i DP::P, - mut satisfied_map: SatisfiedMap<'i, DP::P, DP::VS>, - package_assignments: &FnvIndexMap>, - store: &Arena>, + mut satisfied_map: SatisfiedMap<'i, DP::P, DP::VS, DP::M>, + 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(); @@ -490,12 +494,12 @@ impl PartialSolution { } } -impl PackageAssignments { +impl PackageAssignments { fn satisfier( &self, package: &P, start_term: &Term, - ) -> (Option>, u32, DecisionLevel) { + ) -> (Option>, u32, DecisionLevel) { let empty = Term::empty(); // Indicate if we found a satisfier in the list of derivations, otherwise it will be the decision. let idx = self diff --git a/src/lib.rs b/src/lib.rs index 640461b8..434d614c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -96,7 +96,7 @@ //! &self, //! package: &String, //! version: &SemanticVersion, -//! ) -> Result, Infallible> { +//! ) -> Result, Infallible> { //! unimplemented!() //! } //! @@ -104,6 +104,7 @@ //! type P = String; //! type V = SemanticVersion; //! type VS = SemVS; +//! type M = String; //! } //! ``` //! @@ -162,11 +163,12 @@ //! # use pubgrub::package::Package; //! # use pubgrub::version_set::VersionSet; //! # use pubgrub::report::DerivationTree; +//! # use std::fmt::{Debug, Display}; //! # -//! pub trait Reporter { +//! pub trait Reporter { //! type Output; //! -//! fn report(derivation_tree: &DerivationTree) -> Self::Output; +//! fn report(derivation_tree: &DerivationTree) -> Self::Output; //! } //! ``` //! Implementing a [Reporter](crate::report::Reporter) may involve a lot of heuristics diff --git a/src/report.rs b/src/report.rs index 9f354183..668126b2 100644 --- a/src/report.rs +++ b/src/report.rs @@ -3,7 +3,7 @@ //! Build a report as clear as possible as to why //! dependency solving failed. -use std::fmt; +use std::fmt::{self, Debug, Display}; use std::ops::Deref; use std::sync::Arc; @@ -13,49 +13,49 @@ use crate::type_aliases::Map; use crate::version_set::VersionSet; /// Reporter trait. -pub trait Reporter { +pub trait Reporter { /// Output type of the report. type Output; /// Generate a report from the derivation tree /// describing the resolution failure using the default formatter. - fn report(derivation_tree: &DerivationTree) -> Self::Output; + fn report(derivation_tree: &DerivationTree) -> Self::Output; /// Generate a report from the derivation tree /// describing the resolution failure using a custom formatter. fn report_with_formatter( - derivation_tree: &DerivationTree, - formatter: &impl ReportFormatter, + derivation_tree: &DerivationTree, + formatter: &impl ReportFormatter, ) -> Self::Output; } /// Derivation tree resulting in the impossibility /// to solve the dependencies of our root package. #[derive(Debug, Clone)] -pub enum DerivationTree { +pub enum DerivationTree { /// External incompatibility. - External(External), + External(External), /// Incompatibility derived from two others. - Derived(Derived), + Derived(Derived), } /// Incompatibilities that are not derived from others, /// they have their own reason. #[derive(Debug, Clone)] -pub enum External { +pub enum External { /// Initial incompatibility aiming at picking the root package for the first decision. NotRoot(P, VS::V), /// There are no versions in the given set for this package. NoVersions(P, VS), - /// Dependencies of the package are unavailable for versions in that set. - UnavailableDependencies(P, VS), /// Incompatibility coming from the dependencies of a given package. FromDependencyOf(P, VS, P, VS), + /// The package is unusable for reasons outside pubgrub. + Custom(P, VS, M), } /// Incompatibility derived from two others. #[derive(Debug, Clone)] -pub struct Derived { +pub struct Derived { /// Terms of the incompatibility. pub terms: Map>, /// Indicate if that incompatibility is present multiple times @@ -65,12 +65,12 @@ pub struct Derived { /// and refer to the explanation for the other times. pub shared_id: Option, /// First cause. - pub cause1: Arc>, + pub cause1: Arc>, /// Second cause. - pub cause2: Arc>, + pub cause2: Arc>, } -impl DerivationTree { +impl DerivationTree { /// Merge the [NoVersions](External::NoVersions) external incompatibilities /// with the other one they are matched with /// in a derived incompatibility. @@ -118,12 +118,9 @@ impl DerivationTree { DerivationTree::External(External::NotRoot(_, _)) => { panic!("How did we end up with a NoVersions merged with a NotRoot?") } - DerivationTree::External(External::NoVersions(_, r)) => Some(DerivationTree::External( - External::NoVersions(package, set.union(&r)), - )), - DerivationTree::External(External::UnavailableDependencies(_, r)) => Some( - DerivationTree::External(External::UnavailableDependencies(package, set.union(&r))), - ), + // + // Cannot be merged because the reason may not match + DerivationTree::External(External::NoVersions(_, _)) => None, DerivationTree::External(External::FromDependencyOf(p1, r1, p2, r2)) => { if p1 == package { Some(DerivationTree::External(External::FromDependencyOf( @@ -141,11 +138,15 @@ impl DerivationTree { ))) } } + // Cannot be merged because the reason may not match + DerivationTree::External(External::Custom(_, _, _)) => None, } } } -impl fmt::Display for External { +impl fmt::Display + for External +{ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { Self::NotRoot(package, version) => { @@ -158,14 +159,18 @@ impl fmt::Display for External { write!(f, "there is no version of {} in {}", package, set) } } - Self::UnavailableDependencies(package, set) => { + Self::Custom(package, set, metadata) => { if set == &VS::full() { - write!(f, "dependencies of {} are unavailable", package) + write!( + f, + "dependencies of {} are unavailable {}", + package, metadata + ) } else { write!( f, - "dependencies of {} at version {} are unavailable", - package, set + "dependencies of {} at version {} are unavailable {}", + package, set, metadata ) } } @@ -185,12 +190,12 @@ impl fmt::Display for External { } /// Trait for formatting outputs in the reporter. -pub trait ReportFormatter { +pub trait ReportFormatter { /// Output type of the report. type Output; /// Format an [External] incompatibility. - fn format_external(&self, external: &External) -> Self::Output; + fn format_external(&self, external: &External) -> Self::Output; /// Format terms of an incompatibility. fn format_terms(&self, terms: &Map>) -> Self::Output; @@ -200,10 +205,12 @@ pub trait ReportFormatter { #[derive(Default, Debug)] pub struct DefaultStringReportFormatter; -impl ReportFormatter for DefaultStringReportFormatter { +impl ReportFormatter + for DefaultStringReportFormatter +{ type Output = String; - fn format_external(&self, external: &External) -> String { + fn format_external(&self, external: &External) -> String { external.to_string() } @@ -214,12 +221,12 @@ impl ReportFormatter for DefaultStringReportF // TODO: special case when that unique package is root. [(package, Term::Positive(range))] => format!("{} {} is forbidden", package, range), [(package, Term::Negative(range))] => format!("{} {} is mandatory", package, range), - [(p1, Term::Positive(r1)), (p2, Term::Negative(r2))] => { - self.format_external(&External::FromDependencyOf(p1, r1.clone(), p2, r2.clone())) - } - [(p1, Term::Negative(r1)), (p2, Term::Positive(r2))] => { - self.format_external(&External::FromDependencyOf(p2, r2.clone(), p1, r1.clone())) - } + [(p1, Term::Positive(r1)), (p2, Term::Negative(r2))] => self.format_external( + &External::<_, _, M>::FromDependencyOf(p1, r1.clone(), p2, r2.clone()), + ), + [(p1, Term::Negative(r1)), (p2, Term::Positive(r2))] => self.format_external( + &External::<_, _, M>::FromDependencyOf(p2, r2.clone(), p1, r1.clone()), + ), slice => { let str_terms: Vec<_> = slice.iter().map(|(p, t)| format!("{} {}", p, t)).collect(); str_terms.join(", ") + " are incompatible" @@ -249,9 +256,14 @@ impl DefaultStringReporter { } } - fn build_recursive>( + fn build_recursive< + P: Package, + VS: VersionSet, + M: Eq + Clone + Debug + Display, + F: ReportFormatter, + >( &mut self, - derived: &Derived, + derived: &Derived, formatter: &F, ) { self.build_recursive_helper(derived, formatter); @@ -267,10 +279,11 @@ impl DefaultStringReporter { fn build_recursive_helper< P: Package, VS: VersionSet, - F: ReportFormatter, + M: Eq + Clone + Debug + Display, + F: ReportFormatter, >( &mut self, - current: &Derived, + current: &Derived, formatter: &F, ) { match (current.cause1.deref(), current.cause2.deref()) { @@ -363,10 +376,15 @@ impl DefaultStringReporter { /// /// The result will depend on the fact that the derived incompatibility /// has already been explained or not. - fn report_one_each>( + fn report_one_each< + P: Package, + VS: VersionSet, + M: Eq + Clone + Debug + Display, + F: ReportFormatter, + >( &mut self, - derived: &Derived, - external: &External, + derived: &Derived, + external: &External, current_terms: &Map>, formatter: &F, ) { @@ -386,11 +404,12 @@ impl DefaultStringReporter { fn report_recurse_one_each< P: Package, VS: VersionSet, - F: ReportFormatter, + M: Eq + Clone + Debug + Display, + F: ReportFormatter, >( &mut self, - derived: &Derived, - external: &External, + derived: &Derived, + external: &External, current_terms: &Map>, formatter: &F, ) { @@ -434,10 +453,11 @@ impl DefaultStringReporter { fn explain_both_external< P: Package, VS: VersionSet, - F: ReportFormatter, + M: Eq + Clone + Debug + Display, + F: ReportFormatter, >( - external1: &External, - external2: &External, + external1: &External, + external2: &External, current_terms: &Map>, formatter: &F, ) -> String { @@ -451,11 +471,16 @@ impl DefaultStringReporter { } /// Both causes have already been explained so we use their refs. - fn explain_both_ref>( + fn explain_both_ref< + P: Package, + VS: VersionSet, + M: Eq + Clone + Debug + Display, + F: ReportFormatter, + >( ref_id1: usize, - derived1: &Derived, + derived1: &Derived, ref_id2: usize, - derived2: &Derived, + derived2: &Derived, current_terms: &Map>, formatter: &F, ) -> String { @@ -476,11 +501,12 @@ impl DefaultStringReporter { fn explain_ref_and_external< P: Package, VS: VersionSet, - F: ReportFormatter, + M: Eq + Clone + Debug + Display, + F: ReportFormatter, >( ref_id: usize, - derived: &Derived, - external: &External, + derived: &Derived, + external: &External, current_terms: &Map>, formatter: &F, ) -> String { @@ -498,9 +524,10 @@ impl DefaultStringReporter { fn and_explain_external< P: Package, VS: VersionSet, - F: ReportFormatter, + M: Eq + Clone + Debug + Display, + F: ReportFormatter, >( - external: &External, + external: &External, current_terms: &Map>, formatter: &F, ) -> String { @@ -512,9 +539,14 @@ impl DefaultStringReporter { } /// Add an already explained incompat to the chain of explanations. - fn and_explain_ref>( + fn and_explain_ref< + P: Package, + VS: VersionSet, + M: Eq + Clone + Debug + Display, + F: ReportFormatter, + >( ref_id: usize, - derived: &Derived, + derived: &Derived, current_terms: &Map>, formatter: &F, ) -> String { @@ -530,10 +562,11 @@ impl DefaultStringReporter { fn and_explain_prior_and_external< P: Package, VS: VersionSet, - F: ReportFormatter, + M: Eq + Clone + Debug + Display, + F: ReportFormatter, >( - prior_external: &External, - external: &External, + prior_external: &External, + external: &External, current_terms: &Map>, formatter: &F, ) -> String { @@ -560,10 +593,12 @@ impl DefaultStringReporter { } } -impl Reporter for DefaultStringReporter { +impl Reporter + for DefaultStringReporter +{ type Output = String; - fn report(derivation_tree: &DerivationTree) -> Self::Output { + fn report(derivation_tree: &DerivationTree) -> Self::Output { let formatter = DefaultStringReportFormatter; match derivation_tree { DerivationTree::External(external) => formatter.format_external(external), @@ -576,8 +611,8 @@ impl Reporter for DefaultStringReporter { } fn report_with_formatter( - derivation_tree: &DerivationTree, - formatter: &impl ReportFormatter, + derivation_tree: &DerivationTree, + formatter: &impl ReportFormatter, ) -> Self::Output { match derivation_tree { DerivationTree::External(external) => formatter.format_external(external), diff --git a/src/solver.rs b/src/solver.rs index 588b46f9..82febc3f 100644 --- a/src/solver.rs +++ b/src/solver.rs @@ -150,10 +150,11 @@ pub fn resolve( })?; let known_dependencies = match dependencies { - Dependencies::Unknown => { - state.add_incompatibility(Incompatibility::unavailable_dependencies( + Dependencies::Unknown(reason) => { + state.add_incompatibility(Incompatibility::custom_version( p.clone(), v.clone(), + reason, )); continue; } @@ -191,9 +192,9 @@ pub fn resolve( /// An enum used by [DependencyProvider] that holds information about package dependencies. /// For each [Package] there is a set of versions allowed as a dependency. #[derive(Clone)] -pub enum Dependencies { - /// Package dependencies are unavailable. - Unknown, +pub enum Dependencies { + /// Package dependencies are unavailable with the reason why they are missing. + Unknown(M), /// Container for all available package versions. Known(DependencyConstraints), } @@ -211,6 +212,9 @@ pub trait DependencyProvider { /// The requirements must be able to process the same kind of version as this dependency provider. type VS: VersionSet; + /// How this provider stores metadata or additional context about incompatibilities + type M: Eq + Clone + Debug + Display; + /// [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. @@ -265,7 +269,7 @@ pub trait DependencyProvider { &self, package: &Self::P, version: &Self::V, - ) -> Result, Self::Err>; + ) -> Result, Self::Err>; /// This is called fairly regularly during the resolution, /// if it returns an Err then resolution will be terminated. @@ -353,6 +357,7 @@ impl DependencyProvider for OfflineDependencyProvide type P = P; type V = VS::V; type VS = VS; + type M = String; type Err = Infallible; @@ -377,9 +382,9 @@ impl DependencyProvider for OfflineDependencyProvide &self, package: &P, version: &VS::V, - ) -> Result, Infallible> { + ) -> Result, Infallible> { Ok(match self.dependencies(package, version) { - None => Dependencies::Unknown, + None => Dependencies::Unknown("its dependencies could not be determined".to_string()), Some(dependencies) => Dependencies::Known(dependencies), }) } diff --git a/src/type_aliases.rs b/src/type_aliases.rs index 1bee1a89..96aabbf4 100644 --- a/src/type_aliases.rs +++ b/src/type_aliases.rs @@ -22,5 +22,8 @@ pub type SelectedDependencies = /// while the latter means they could not be fetched by the [DependencyProvider]. pub type DependencyConstraints = Map; -pub(crate) type IncompDpId = - IncompId<::P, ::VS>; +pub(crate) type IncompDpId = IncompId< + ::P, + ::VS, + ::M, +>; diff --git a/tests/proptest.rs b/tests/proptest.rs index e35cd35a..238aebeb 100644 --- a/tests/proptest.rs +++ b/tests/proptest.rs @@ -4,6 +4,7 @@ use std::collections::BTreeSet as Set; use std::convert::Infallible; +use std::fmt::{Debug, Display}; use pubgrub::error::PubGrubError; use pubgrub::package::Package; @@ -31,7 +32,11 @@ struct OldestVersionsDependencyProvider( ); impl DependencyProvider for OldestVersionsDependencyProvider { - fn get_dependencies(&self, p: &P, v: &VS::V) -> Result, Infallible> { + fn get_dependencies( + &self, + p: &P, + v: &VS::V, + ) -> Result, Infallible> { self.0.get_dependencies(p, v) } @@ -56,6 +61,7 @@ impl DependencyProvider for OldestVersionsDependency type P = P; type V = VS::V; type VS = VS; + type M = String; } /// The same as DP but it has a timeout. @@ -83,7 +89,7 @@ impl DependencyProvider for TimeoutDependencyProvider Result, DP::Err> { + ) -> Result, DP::Err> { self.dp.get_dependencies(p, v) } @@ -110,6 +116,7 @@ impl DependencyProvider for TimeoutDependencyProvider::V; type VS = DP::VS; + type M = DP::M; } fn timeout_resolve( @@ -314,7 +321,7 @@ fn retain_versions( continue; } let deps = match dependency_provider.get_dependencies(n, v).unwrap() { - Dependencies::Unknown => panic!(), + Dependencies::Unknown(_) => panic!(), Dependencies::Known(deps) => deps, }; smaller_dependency_provider.add_dependencies(n.clone(), v.clone(), deps) @@ -338,7 +345,7 @@ fn retain_dependencies( for n in dependency_provider.packages() { for v in dependency_provider.versions(n).unwrap() { let deps = match dependency_provider.get_dependencies(n, v).unwrap() { - Dependencies::Unknown => panic!(), + Dependencies::Unknown(_) => panic!(), Dependencies::Known(deps) => deps, }; smaller_dependency_provider.add_dependencies( @@ -368,9 +375,9 @@ fn errors_the_same_with_only_report_dependencies( return; }; - fn recursive( + fn recursive( to_retain: &mut Vec<(N, VS, N)>, - tree: &DerivationTree, + tree: &DerivationTree, ) { match tree { DerivationTree::External(External::FromDependencyOf(n1, vs1, n2, _)) => { @@ -509,7 +516,7 @@ proptest! { .get_dependencies(package, version) .unwrap() { - Dependencies::Unknown => panic!(), + Dependencies::Unknown(_) => panic!(), Dependencies::Known(d) => d.into_iter().collect(), }; if !dependencies.is_empty() { diff --git a/tests/sat_dependency_provider.rs b/tests/sat_dependency_provider.rs index f74a2eab..36840f8e 100644 --- a/tests/sat_dependency_provider.rs +++ b/tests/sat_dependency_provider.rs @@ -66,7 +66,7 @@ impl SatResolve { // active packages need each of there `deps` to be satisfied for (p, v, var) in &all_versions { let deps = match dp.get_dependencies(p, v).unwrap() { - Dependencies::Unknown => panic!(), + Dependencies::Unknown(_) => panic!(), Dependencies::Known(d) => d, }; for (p1, range) in &deps {