diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bf7df723..49526756 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -40,7 +40,7 @@ jobs: run: cargo build --verbose - name: Run tests - run: cargo test --features=serde --verbose + run: cargo test --verbose clippy: name: No warnings from Clippy diff --git a/Cargo.toml b/Cargo.toml index 1f6b32fb..b1994096 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,12 +23,14 @@ include = ["Cargo.toml", "LICENSE", "README.md", "src/**", "tests/**", "examples thiserror = "1.0" rustc-hash = "1.1.0" serde = { version = "1.0", features = ["derive"], optional = true } +log = { version = "0.4.14", default-features = false } # for debug logs in tests [dev-dependencies] proptest = "0.10.1" ron = "0.6" varisat = "0.2.2" criterion = "0.3" +env_logger = "0.9.0" [[bench]] name = "large_case" diff --git a/examples/branching_error_reporting.rs b/examples/branching_error_reporting.rs index c1daa1bf..64f431c4 100644 --- a/examples/branching_error_reporting.rs +++ b/examples/branching_error_reporting.rs @@ -1,14 +1,15 @@ // SPDX-License-Identifier: MPL-2.0 use pubgrub::error::PubGrubError; -use pubgrub::range::Range; +use pubgrub::range_trait::Range; use pubgrub::report::{DefaultStringReporter, Reporter}; use pubgrub::solver::{resolve, OfflineDependencyProvider}; -use pubgrub::version::SemanticVersion; +use pubgrub::version_trait::{SemanticInterval, SemanticVersion}; // https://github.com/dart-lang/pub/blob/master/doc/solver.md#branching-error-reporting fn main() { - let mut dependency_provider = OfflineDependencyProvider::<&str, SemanticVersion>::new(); + let mut dependency_provider = + OfflineDependencyProvider::<&str, SemanticInterval, SemanticVersion>::new(); #[rustfmt::skip] // root 1.0.0 depends on foo ^1.0.0 dependency_provider.add_dependencies( diff --git a/examples/caching_dependency_provider.rs b/examples/caching_dependency_provider.rs index bac730ea..b3efbc31 100644 --- a/examples/caching_dependency_provider.rs +++ b/examples/caching_dependency_provider.rs @@ -4,18 +4,25 @@ use std::cell::RefCell; use std::error::Error; use pubgrub::package::Package; -use pubgrub::range::Range; +use pubgrub::range_trait::Range; use pubgrub::solver::{resolve, Dependencies, DependencyProvider, OfflineDependencyProvider}; -use pubgrub::version::{NumberVersion, Version}; +use pubgrub::version_trait::{Interval, NumberInterval, NumberVersion, Version}; // 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< + P: Package, + I: Interval, + V: Version, + DP: DependencyProvider, +> { remote_dependencies: DP, - cached_dependencies: RefCell>, + cached_dependencies: RefCell>, } -impl> CachingDependencyProvider { +impl, V: Version, DP: DependencyProvider> + CachingDependencyProvider +{ pub fn new(remote_dependencies_provider: DP) -> Self { CachingDependencyProvider { remote_dependencies: remote_dependencies_provider, @@ -24,10 +31,10 @@ impl> CachingDependencyProv } } -impl> DependencyProvider - for CachingDependencyProvider +impl, V: Version, DP: DependencyProvider> + DependencyProvider for CachingDependencyProvider { - fn choose_package_version, U: std::borrow::Borrow>>( + fn choose_package_version, U: std::borrow::Borrow>>( &self, packages: impl Iterator, ) -> Result<(T, Option), Box> { @@ -39,7 +46,7 @@ impl> DependencyProvider Result, Box> { + ) -> Result, Box> { let mut cache = self.cached_dependencies.borrow_mut(); match cache.get_dependencies(package, version) { Ok(Dependencies::Unknown) => { @@ -65,7 +72,8 @@ impl> DependencyProvider::new(); + let mut remote_dependencies_provider = + OfflineDependencyProvider::<&str, NumberInterval, NumberVersion>::new(); // Add dependencies as needed. Here only root package is added. remote_dependencies_provider.add_dependencies("root", 1, Vec::new()); diff --git a/examples/doc_interface.rs b/examples/doc_interface.rs index b3e37838..3aeea43a 100644 --- a/examples/doc_interface.rs +++ b/examples/doc_interface.rs @@ -1,8 +1,8 @@ // SPDX-License-Identifier: MPL-2.0 -use pubgrub::range::Range; +use pubgrub::range_trait::Range; use pubgrub::solver::{resolve, OfflineDependencyProvider}; -use pubgrub::version::NumberVersion; +use pubgrub::version_trait::{NumberInterval, NumberVersion}; // `root` depends on `menu` and `icons` // `menu` depends on `dropdown` @@ -10,12 +10,12 @@ use pubgrub::version::NumberVersion; // `icons` has no dependency #[rustfmt::skip] fn main() { - let mut dependency_provider = OfflineDependencyProvider::<&str, NumberVersion>::new(); + let mut dependency_provider = OfflineDependencyProvider::<&str, NumberInterval, NumberVersion>::new(); dependency_provider.add_dependencies( - "root", 1, vec![("menu", Range::any()), ("icons", Range::any())], + "root", 1, vec![("menu", Range::full()), ("icons", Range::full())], ); - dependency_provider.add_dependencies("menu", 1, vec![("dropdown", Range::any())]); - dependency_provider.add_dependencies("dropdown", 1, vec![("icons", Range::any())]); + dependency_provider.add_dependencies("menu", 1, vec![("dropdown", Range::full())]); + dependency_provider.add_dependencies("dropdown", 1, vec![("icons", Range::full())]); dependency_provider.add_dependencies("icons", 1, vec![]); // Run the algorithm. diff --git a/examples/doc_interface_error.rs b/examples/doc_interface_error.rs index 0ef0f1ec..ebfa214e 100644 --- a/examples/doc_interface_error.rs +++ b/examples/doc_interface_error.rs @@ -1,10 +1,10 @@ // SPDX-License-Identifier: MPL-2.0 use pubgrub::error::PubGrubError; -use pubgrub::range::Range; +use pubgrub::range_trait::Range; use pubgrub::report::{DefaultStringReporter, Reporter}; use pubgrub::solver::{resolve, OfflineDependencyProvider}; -use pubgrub::version::SemanticVersion; +use pubgrub::version_trait::{SemanticInterval, SemanticVersion}; // `root` depends on `menu`, `icons 1.0.0` and `intl 5.0.0` // `menu 1.0.0` depends on `dropdown < 2.0.0` @@ -15,12 +15,12 @@ use pubgrub::version::SemanticVersion; // `intl` has no dependency #[rustfmt::skip] fn main() { - let mut dependency_provider = OfflineDependencyProvider::<&str, SemanticVersion>::new(); + let mut dependency_provider = OfflineDependencyProvider::<&str, SemanticInterval, SemanticVersion>::new(); // Direct dependencies: menu and icons. dependency_provider.add_dependencies("root", (1, 0, 0), vec![ - ("menu", Range::any()), - ("icons", Range::exact((1, 0, 0))), - ("intl", Range::exact((5, 0, 0))), + ("menu", Range::full()), + ("icons", Range::singleton((1, 0, 0))), + ("intl", Range::singleton((5, 0, 0))), ]); // Dependencies of the menu lib. @@ -45,19 +45,19 @@ fn main() { // Dependencies of the dropdown lib. dependency_provider.add_dependencies("dropdown", (1, 8, 0), vec![ - ("intl", Range::exact((3, 0, 0))), + ("intl", Range::singleton((3, 0, 0))), ]); dependency_provider.add_dependencies("dropdown", (2, 0, 0), vec![ - ("icons", Range::exact((2, 0, 0))), + ("icons", Range::singleton((2, 0, 0))), ]); dependency_provider.add_dependencies("dropdown", (2, 1, 0), vec![ - ("icons", Range::exact((2, 0, 0))), + ("icons", Range::singleton((2, 0, 0))), ]); dependency_provider.add_dependencies("dropdown", (2, 2, 0), vec![ - ("icons", Range::exact((2, 0, 0))), + ("icons", Range::singleton((2, 0, 0))), ]); dependency_provider.add_dependencies("dropdown", (2, 3, 0), vec![ - ("icons", Range::exact((2, 0, 0))), + ("icons", Range::singleton((2, 0, 0))), ]); // Icons have no dependencies. diff --git a/examples/doc_interface_semantic.rs b/examples/doc_interface_semantic.rs index b4c352e1..4825bb16 100644 --- a/examples/doc_interface_semantic.rs +++ b/examples/doc_interface_semantic.rs @@ -1,10 +1,10 @@ // SPDX-License-Identifier: MPL-2.0 use pubgrub::error::PubGrubError; -use pubgrub::range::Range; +use pubgrub::range_trait::Range; use pubgrub::report::{DefaultStringReporter, Reporter}; use pubgrub::solver::{resolve, OfflineDependencyProvider}; -use pubgrub::version::SemanticVersion; +use pubgrub::version_trait::{SemanticInterval, SemanticVersion}; // `root` depends on `menu` and `icons 1.0.0` // `menu 1.0.0` depends on `dropdown < 2.0.0` @@ -14,11 +14,11 @@ use pubgrub::version::SemanticVersion; // `icons` has no dependency #[rustfmt::skip] fn main() { - let mut dependency_provider = OfflineDependencyProvider::<&str, SemanticVersion>::new(); + let mut dependency_provider = OfflineDependencyProvider::<&str, SemanticInterval, SemanticVersion>::new(); // Direct dependencies: menu and icons. dependency_provider.add_dependencies("root", (1, 0, 0), vec![ - ("menu", Range::any()), - ("icons", Range::exact((1, 0, 0))), + ("menu", Range::full()), + ("icons", Range::singleton((1, 0, 0))), ]); // Dependencies of the menu lib. @@ -44,16 +44,16 @@ fn main() { // Dependencies of the dropdown lib. dependency_provider.add_dependencies("dropdown", (1, 8, 0), vec![]); dependency_provider.add_dependencies("dropdown", (2, 0, 0), vec![ - ("icons", Range::exact((2, 0, 0))), + ("icons", Range::singleton((2, 0, 0))), ]); dependency_provider.add_dependencies("dropdown", (2, 1, 0), vec![ - ("icons", Range::exact((2, 0, 0))), + ("icons", Range::singleton((2, 0, 0))), ]); dependency_provider.add_dependencies("dropdown", (2, 2, 0), vec![ - ("icons", Range::exact((2, 0, 0))), + ("icons", Range::singleton((2, 0, 0))), ]); dependency_provider.add_dependencies("dropdown", (2, 3, 0), vec![ - ("icons", Range::exact((2, 0, 0))), + ("icons", Range::singleton((2, 0, 0))), ]); // Icons has no dependency. diff --git a/examples/linear_error_reporting.rs b/examples/linear_error_reporting.rs index 0673fe36..2b2a3173 100644 --- a/examples/linear_error_reporting.rs +++ b/examples/linear_error_reporting.rs @@ -1,14 +1,15 @@ // SPDX-License-Identifier: MPL-2.0 use pubgrub::error::PubGrubError; -use pubgrub::range::Range; +use pubgrub::range_trait::Range; use pubgrub::report::{DefaultStringReporter, Reporter}; use pubgrub::solver::{resolve, OfflineDependencyProvider}; -use pubgrub::version::SemanticVersion; +use pubgrub::version_trait::{SemanticInterval, SemanticVersion}; // https://github.com/dart-lang/pub/blob/master/doc/solver.md#linear-error-reporting fn main() { - let mut dependency_provider = OfflineDependencyProvider::<&str, SemanticVersion>::new(); + let mut dependency_provider = + OfflineDependencyProvider::<&str, SemanticInterval, SemanticVersion>::new(); #[rustfmt::skip] // root 1.0.0 depends on foo ^1.0.0 and baz ^1.0.0 dependency_provider.add_dependencies( diff --git a/src/error.rs b/src/error.rs index 0553d8de..74e79fa8 100644 --- a/src/error.rs +++ b/src/error.rs @@ -6,14 +6,15 @@ use thiserror::Error; use crate::package::Package; use crate::report::DerivationTree; -use crate::version::Version; +use crate::version_trait::{Interval, Version}; +use std::fmt::Debug; /// Errors that may occur while solving dependencies. #[derive(Error, Debug)] -pub enum PubGrubError { +pub enum PubGrubError + Debug, V: Version> { /// 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) diff --git a/src/internal/core.rs b/src/internal/core.rs index f923850a..57c708eb 100644 --- a/src/internal/core.rs +++ b/src/internal/core.rs @@ -4,6 +4,7 @@ //! to write a functional PubGrub algorithm. use std::collections::HashSet as Set; +use std::fmt::Debug; use crate::error::PubGrubError; use crate::internal::arena::Arena; @@ -17,26 +18,26 @@ use crate::package::Package; use crate::report::DerivationTree; use crate::solver::DependencyConstraints; use crate::type_aliases::Map; -use crate::version::Version; +use crate::version_trait::{Interval, Version}; /// Current state of the PubGrub algorithm. #[derive(Clone)] -pub struct State { +pub struct State, V: Version> { root_package: P, root_version: V, - incompatibilities: Map>>, + incompatibilities: Map>>, /// Store the ids of incompatibilities that are already contradicted /// and will stay that way until the next conflict and backtrack is operated. - contradicted_incompatibilities: rustc_hash::FxHashSet>, + contradicted_incompatibilities: rustc_hash::FxHashSet>, /// 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 @@ -44,7 +45,7 @@ pub struct State { unit_propagation_buffer: SmallVec

, } -impl State { +impl + Debug, V: Version> State { /// Initialization of PubGrub state. pub fn init(root_package: P, root_version: V) -> Self { let mut incompatibility_store = Arena::new(); @@ -66,7 +67,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); } @@ -76,8 +77,8 @@ impl State { &mut self, package: P, version: V, - deps: &DependencyConstraints, - ) -> std::ops::Range> { + deps: &DependencyConstraints, + ) -> std::ops::Range> { // Create incompatibilities and allocate them in the store. let new_incompats_id_range = self .incompatibility_store @@ -92,13 +93,13 @@ impl State { } /// Check if an incompatibility is terminal. - pub fn is_terminal(&self, incompatibility: &Incompatibility) -> bool { + pub fn is_terminal(&self, incompatibility: &Incompatibility) -> bool { incompatibility.is_terminal(&self.root_package, &self.root_version) } /// 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: P) -> Result<(), PubGrubError> { self.unit_propagation_buffer.clear(); self.unit_propagation_buffer.push(package); while let Some(current_package) = self.unit_propagation_buffer.pop() { @@ -115,6 +116,10 @@ impl State { // If the partial solution satisfies the incompatibility // we must perform conflict resolution. Relation::Satisfied => { + log::info!( + "Start conflict resolution because incompat satisfied:\n {}", + current_incompat + ); conflict_id = Some(incompat_id); break; } @@ -156,10 +161,11 @@ impl State { /// Return the root cause and the backtracked model. /// CF + #[allow(clippy::type_complexity)] fn conflict_resolution( &mut self, - incompatibility: IncompId, - ) -> Result<(P, IncompId), PubGrubError> { + incompatibility: IncompId, + ) -> Result<(P, IncompId), PubGrubError> { let mut current_incompat_id = incompatibility; let mut current_incompat_changed = false; loop { @@ -183,6 +189,7 @@ impl State { current_incompat_changed, previous_satisfier_level, ); + log::info!("backtrack to {:?}", previous_satisfier_level); return Ok((package, current_incompat_id)); } SameDecisionLevels { satisfier_cause } => { @@ -192,6 +199,7 @@ impl State { &package, &self.incompatibility_store, ); + log::info!("prior cause: {}", prior_cause); current_incompat_id = self.incompatibility_store.alloc(prior_cause); current_incompat_changed = true; } @@ -203,7 +211,7 @@ impl State { /// Backtracking. fn backtrack( &mut self, - incompat: IncompId, + incompat: IncompId, incompat_changed: bool, decision_level: DecisionLevel, ) { @@ -234,7 +242,7 @@ impl State { /// Here we do the simple stupid thing of just growing the Vec. /// It may not be trivial since those incompatibilities /// may already have derived others. - fn merge_incompatibility(&mut self, id: IncompId) { + fn merge_incompatibility(&mut self, id: IncompId) { for (pkg, _term) in self.incompatibility_store[id].iter() { self.incompatibilities .entry(pkg.clone()) @@ -245,12 +253,12 @@ impl State { // Error reporting ######################################################### - fn build_derivation_tree(&self, incompat: IncompId) -> DerivationTree { + fn build_derivation_tree(&self, incompat: IncompId) -> DerivationTree { let shared_ids = self.find_shared_ids(incompat); Incompatibility::build_derivation_tree(incompat, &shared_ids, &self.incompatibility_store) } - fn find_shared_ids(&self, incompat: IncompId) -> Set> { + fn find_shared_ids(&self, incompat: IncompId) -> Set> { let mut all_ids = Set::new(); let mut shared_ids = Set::new(); let mut stack = vec![incompat]; diff --git a/src/internal/incompatibility.rs b/src/internal/incompatibility.rs index acf900b8..a1bce222 100644 --- a/src/internal/incompatibility.rs +++ b/src/internal/incompatibility.rs @@ -9,10 +9,10 @@ use std::fmt; use crate::internal::arena::{Arena, Id}; use crate::internal::small_map::SmallMap; use crate::package::Package; -use crate::range::Range; +use crate::range_trait::Range; use crate::report::{DefaultStringReporter, DerivationTree, Derived, External}; use crate::term::{self, Term}; -use crate::version::Version; +use crate::version_trait::{Interval, Version}; /// An incompatibility is a set of terms for different packages /// that should never be satisfied all together. @@ -30,26 +30,26 @@ use crate::version::Version; /// 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 { - package_terms: SmallMap>, - kind: Kind, +pub struct Incompatibility, V: Version> { + package_terms: SmallMap>, + kind: Kind, } /// Type alias of unique identifiers for incompatibilities. -pub type IncompId = Id>; +pub type IncompId = Id>; #[derive(Debug, Clone)] -enum Kind { +enum Kind, V: Version> { /// Initial incompatibility aiming at picking the root package for the first decision. NotRoot(P, V), /// There are no versions in the given range for this package. - NoVersions(P, Range), + NoVersions(P, Range), /// Dependencies of the package are unavailable for versions in that range. - UnavailableDependencies(P, Range), + UnavailableDependencies(P, Range), /// Incompatibility coming from the dependencies of a given package. - FromDependencyOf(P, Range, P, Range), + FromDependencyOf(P, Range, P, Range), /// Derived from two causes. Stores cause ids. - DerivedFrom(IncompId, IncompId), + DerivedFrom(IncompId, IncompId), } /// A Relation describes how a set of terms can be compared to an incompatibility. @@ -69,13 +69,13 @@ pub enum Relation { Inconclusive, } -impl Incompatibility { +impl, V: Version> Incompatibility { /// Create the initial "not Root" incompatibility. pub fn not_root(package: P, version: V) -> Self { Self { package_terms: SmallMap::One([( package.clone(), - Term::Negative(Range::exact(version.clone())), + Term::Negative(Range::singleton(version.clone())), )]), kind: Kind::NotRoot(package, version), } @@ -83,7 +83,7 @@ impl Incompatibility { /// Create an incompatibility to remember /// that a given range does not contain any version. - pub fn no_versions(package: P, term: Term) -> Self { + pub fn no_versions(package: P, term: Term) -> Self { let range = match &term { Term::Positive(r) => r.clone(), Term::Negative(_) => panic!("No version should have a positive term"), @@ -98,7 +98,7 @@ impl Incompatibility { /// that a package version is not selectable /// because its list of dependencies is unavailable. pub fn unavailable_dependencies(package: P, version: V) -> Self { - let range = Range::exact(version); + let range = Range::singleton(version); Self { package_terms: SmallMap::One([(package.clone(), Term::Positive(range.clone()))]), kind: Kind::UnavailableDependencies(package, range), @@ -106,8 +106,8 @@ impl Incompatibility { } /// Build an incompatibility from a given dependency. - pub fn from_dependency(package: P, version: V, dep: (&P, &Range)) -> Self { - let range1 = Range::exact(version); + pub fn from_dependency(package: P, version: V, dep: (&P, &Range)) -> Self { + let range1 = Range::singleton(version); let (p2, range2) = dep; Self { package_terms: SmallMap::Two([ @@ -152,17 +152,17 @@ impl Incompatibility { false } else { let (package, term) = self.package_terms.iter().next().unwrap(); - (package == root_package) && term.contains(&root_version) + (package == root_package) && term.contains(root_version) } } /// Get the term related to a given package (if it exists). - pub fn get(&self, package: &P) -> Option<&Term> { + pub fn get(&self, package: &P) -> Option<&Term> { self.package_terms.get(package) } /// Iterate over packages. - pub fn iter(&self) -> impl Iterator)> { + pub fn iter(&self) -> impl Iterator)> { self.package_terms.iter() } @@ -181,7 +181,7 @@ impl Incompatibility { self_id: Id, shared_ids: &Set>, store: &Arena, - ) -> DerivationTree { + ) -> DerivationTree { match &store[self_id].kind { Kind::DerivedFrom(id1, id2) => { let cause1 = Self::build_derivation_tree(*id1, shared_ids, store); @@ -215,12 +215,12 @@ impl Incompatibility { } } -impl<'a, P: Package, V: Version + 'a> Incompatibility { +impl<'a, P: Package, I: Interval + 'a, V: Version + 'a> Incompatibility { /// CF definition of Relation enum. - pub fn relation(&self, terms: impl Fn(&P) -> Option<&'a Term>) -> Relation

{ + pub fn relation(&self, terms: impl Fn(&P) -> Option<&'a Term>) -> Relation

{ let mut relation = Relation::Satisfied; for (package, incompat_term) in self.package_terms.iter() { - match terms(package).map(|term| incompat_term.relation_with(&term)) { + match terms(package).map(|term| incompat_term.relation_with(term)) { Some(term::Relation::Satisfied) => {} Some(term::Relation::Contradicted) => { return Relation::Contradicted(package.clone()); @@ -243,7 +243,7 @@ impl<'a, P: Package, V: Version + 'a> Incompatibility { } } -impl fmt::Display for Incompatibility { +impl, V: Version> fmt::Display for Incompatibility { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!( f, @@ -276,12 +276,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::any()) + kind: Kind::UnavailableDependencies("0", Range::full()) }); let i2 = store.alloc(Incompatibility { package_terms: SmallMap::Two([("p2", t2), ("p3", t3.clone())]), - kind: Kind::UnavailableDependencies("0", Range::any()) + kind: Kind::UnavailableDependencies("0", Range::full()) }); let mut i3 = Map::default(); diff --git a/src/internal/partial_solution.rs b/src/internal/partial_solution.rs index 84bfc5f8..f863011e 100644 --- a/src/internal/partial_solution.rs +++ b/src/internal/partial_solution.rs @@ -3,14 +3,17 @@ //! A Memory acts like a structured partial solution //! where terms are regrouped by package in a [Map](crate::type_aliases::Map). +use std::collections::BTreeSet; +use std::fmt::Display; + use crate::internal::arena::Arena; use crate::internal::incompatibility::{IncompId, Incompatibility, Relation}; use crate::internal::small_map::SmallMap; use crate::package::Package; -use crate::range::Range; +use crate::range_trait::Range; use crate::term::Term; use crate::type_aliases::{Map, SelectedDependencies}; -use crate::version::Version; +use crate::version_trait::{Interval, Version}; use super::small_vec::SmallVec; @@ -26,47 +29,100 @@ impl DecisionLevel { /// The partial solution contains all package assignments, /// organized by package and historically ordered. #[derive(Clone, Debug)] -pub struct PartialSolution { +pub struct PartialSolution, V: Version> { next_global_index: u32, current_decision_level: DecisionLevel, - package_assignments: Map>, + package_assignments: Map>, +} + +impl, V: Version> Display for PartialSolution { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let assignments: BTreeSet = self + .package_assignments + .iter() + .map(|(p, pa)| format!("{}: {}", p, pa)) + .collect(); + let assignments: Vec<_> = assignments.into_iter().collect(); + write!( + f, + "next_global_index: {}\ncurrent_decision_level: {:?}\npackage_assignements:\n{}", + self.next_global_index, + self.current_decision_level, + assignments.join("\n") + ) + } } /// Package assignments contain the potential decision and derivations /// 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, V: Version> { smallest_decision_level: DecisionLevel, highest_decision_level: DecisionLevel, - dated_derivations: SmallVec>, - assignments_intersection: AssignmentsIntersection, + dated_derivations: SmallVec>, + assignments_intersection: AssignmentsIntersection, +} + +impl, V: Version> Display for PackageAssignments { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let derivations: Vec<_> = self + .dated_derivations + .iter() + .map(|dd| dd.to_string()) + .collect(); + write!( + f, + "decision range: {:?}..{:?}\nderivations:\n {}\n,assignments_intersection: {}", + self.smallest_decision_level, + self.highest_decision_level, + derivations.join("\n "), + self.assignments_intersection + ) + } } #[derive(Clone, Debug)] -pub struct DatedDerivation { +pub struct DatedDerivation, V: Version> { global_index: u32, decision_level: DecisionLevel, - cause: IncompId, + cause: IncompId, +} + +impl, V: Version> Display for DatedDerivation { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{:?}, cause: {:?}", self.decision_level, self.cause) + } } #[derive(Clone, Debug)] -enum AssignmentsIntersection { - Decision((u32, V, Term)), - Derivations(Term), +enum AssignmentsIntersection, V: Version> { + Decision((u32, V, Term)), + Derivations(Term), +} + +impl, V: Version> Display for AssignmentsIntersection { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Decision((lvl, version, _)) => { + write!(f, "Decision: level {}, v = {}", lvl, version) + } + Self::Derivations(term) => write!(f, "Derivations term: {}", term), + } + } } #[derive(Clone, Debug)] -pub enum SatisfierSearch { +pub enum SatisfierSearch, V: Version> { DifferentDecisionLevels { previous_satisfier_level: DecisionLevel, }, SameDecisionLevels { - satisfier_cause: IncompId, + satisfier_cause: IncompId, }, } -impl PartialSolution { +impl, V: Version> PartialSolution { /// Initialize an empty PartialSolution. pub fn empty() -> Self { Self { @@ -110,11 +166,12 @@ impl PartialSolution { pub fn add_derivation( &mut self, package: P, - cause: IncompId, - store: &Arena>, + cause: IncompId, + store: &Arena>, ) { use std::collections::hash_map::Entry; let term = store[cause].get(&package).unwrap().negate(); + log::debug!("Add for {} the derivation term {}", package, term); let dated_derivation = DatedDerivation { global_index: self.next_global_index, decision_level: self.current_decision_level, @@ -125,6 +182,7 @@ impl PartialSolution { Entry::Occupied(mut occupied) => { let mut pa = occupied.get_mut(); pa.highest_decision_level = self.current_decision_level; + log::debug!("Terms intersection before: {}", pa.assignments_intersection); match &mut pa.assignments_intersection { // Check that add_derivation is never called in the wrong context. AssignmentsIntersection::Decision(_) => { @@ -134,6 +192,7 @@ impl PartialSolution { *t = t.intersection(&term); } } + log::debug!("Terms intersection after: {}", pa.assignments_intersection); pa.dated_derivations.push(dated_derivation); } Entry::Vacant(v) => { @@ -153,7 +212,7 @@ impl PartialSolution { /// selected version (no "decision") /// and if it contains at least one positive derivation term /// in the partial solution. - pub fn potential_packages(&self) -> Option)>> { + pub fn potential_packages(&self) -> Option)>> { let mut iter = self .package_assignments .iter() @@ -190,7 +249,7 @@ impl PartialSolution { pub fn backtrack( &mut self, decision_level: DecisionLevel, - store: &Arena>, + store: &Arena>, ) { self.current_decision_level = decision_level; self.package_assignments.retain(|p, pa| { @@ -223,7 +282,7 @@ impl PartialSolution { pa.dated_derivations .iter() .fold(Term::any(), |acc, dated_derivation| { - let term = store[dated_derivation.cause].get(&p).unwrap().negate(); + let term = store[dated_derivation.cause].get(p).unwrap().negate(); acc.intersection(&term) }), ); @@ -241,11 +300,11 @@ impl PartialSolution { &mut self, package: P, version: 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) @@ -258,17 +317,24 @@ impl PartialSolution { // Check none of the dependencies (new_incompatibilities) // would create a conflict (be satisfied). if store[new_incompatibilities].iter().all(not_satisfied) { + log::info!("add_decision: {} @ {}", package, version); self.add_decision(package, version); + } else { + log::info!( + "not adding {} @ {} because of its dependencies", + package, + version + ); } } /// 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)) } /// 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: &P) -> Option<&Term> { self.package_assignments .get(package) .map(|pa| pa.assignments_intersection.term()) @@ -277,9 +343,9 @@ impl PartialSolution { /// Figure out if the satisfier and previous satisfier are of different decision levels. pub fn satisfier_search( &self, - incompat: &Incompatibility, - store: &Arena>, - ) -> (P, SatisfierSearch) { + incompat: &Incompatibility, + store: &Arena>, + ) -> (P, SatisfierSearch) { let satisfied_map = Self::find_satisfier(incompat, &self.package_assignments, store); let (satisfier_package, &(satisfier_index, _, satisfier_decision_level)) = satisfied_map .iter() @@ -318,9 +384,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( - incompat: &Incompatibility, - package_assignments: &Map>, - store: &Arena>, + incompat: &Incompatibility, + package_assignments: &Map>, + store: &Arena>, ) -> SmallMap { let mut satisfied = SmallMap::Empty; for (package, incompat_term) in incompat.iter() { @@ -337,11 +403,11 @@ impl PartialSolution { /// such that incompatibility is satisfied by the partial solution up to /// and including that assignment plus satisfier. fn find_previous_satisfier( - incompat: &Incompatibility, + incompat: &Incompatibility, satisfier_package: &P, mut satisfied_map: SmallMap, - package_assignments: &Map>, - store: &Arena>, + package_assignments: &Map>, + store: &Arena>, ) -> DecisionLevel { // First, let's retrieve the previous derivations and the initial accum_term. let satisfier_pa = package_assignments.get(satisfier_package).unwrap(); @@ -375,13 +441,13 @@ impl PartialSolution { } } -impl PackageAssignments { +impl, V: Version> PackageAssignments { fn satisfier( &self, package: &P, - incompat_term: &Term, - start_term: Term, - store: &Arena>, + incompat_term: &Term, + start_term: Term, + store: &Arena>, ) -> (usize, u32, DecisionLevel) { // Term where we accumulate intersections until incompat_term is satisfied. let mut accum_term = start_term; @@ -422,9 +488,9 @@ impl PackageAssignments { } } -impl AssignmentsIntersection { +impl, V: Version> AssignmentsIntersection { /// Returns the term intersection of all assignments (decision included). - fn term(&self) -> &Term { + fn term(&self) -> &Term { match self { Self::Decision((_, _, term)) => term, Self::Derivations(term) => term, @@ -438,7 +504,7 @@ impl AssignmentsIntersection { fn potential_package_filter<'a, P: Package>( &'a self, package: &'a P, - ) -> Option<(&'a P, &'a Range)> { + ) -> Option<(&'a P, &'a Range)> { match self { Self::Decision(_) => None, Self::Derivations(term_intersection) => { diff --git a/src/lib.rs b/src/lib.rs index 7a6e1737..573e1469 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -47,16 +47,16 @@ //! We can model that scenario with this library as follows //! ``` //! # use pubgrub::solver::{OfflineDependencyProvider, resolve}; -//! # use pubgrub::version::NumberVersion; -//! # use pubgrub::range::Range; +//! # use pubgrub::version_trait::{NumberInterval, NumberVersion}; +//! # use pubgrub::range_trait::Range; //! # -//! let mut dependency_provider = OfflineDependencyProvider::<&str, NumberVersion>::new(); +//! let mut dependency_provider = OfflineDependencyProvider::<&str, NumberInterval, NumberVersion>::new(); //! //! dependency_provider.add_dependencies( -//! "root", 1, vec![("menu", Range::any()), ("icons", Range::any())], +//! "root", 1, vec![("menu", Range::full()), ("icons", Range::full())], //! ); -//! dependency_provider.add_dependencies("menu", 1, vec![("dropdown", Range::any())]); -//! dependency_provider.add_dependencies("dropdown", 1, vec![("icons", Range::any())]); +//! dependency_provider.add_dependencies("menu", 1, vec![("dropdown", Range::full())]); +//! dependency_provider.add_dependencies("dropdown", 1, vec![("icons", Range::full())]); //! dependency_provider.add_dependencies("icons", 1, vec![]); //! //! // Run the algorithm. @@ -76,16 +76,16 @@ //! This may be done quite easily by implementing the two following functions. //! ``` //! # use pubgrub::solver::{DependencyProvider, Dependencies}; -//! # use pubgrub::version::SemanticVersion; -//! # use pubgrub::range::Range; +//! # use pubgrub::version_trait::{SemanticInterval, SemanticVersion}; +//! # use pubgrub::range_trait::Range; //! # use pubgrub::type_aliases::Map; //! # use std::error::Error; //! # use std::borrow::Borrow; //! # //! # struct MyDependencyProvider; //! # -//! impl DependencyProvider for MyDependencyProvider { -//! fn choose_package_version, U: Borrow>>(&self,packages: impl Iterator) -> Result<(T, Option), Box> { +//! impl DependencyProvider for MyDependencyProvider { +//! fn choose_package_version, U: Borrow>>(&self,packages: impl Iterator) -> Result<(T, Option), Box> { //! unimplemented!() //! } //! @@ -93,7 +93,7 @@ //! &self, //! package: &String, //! version: &SemanticVersion, -//! ) -> Result, Box> { +//! ) -> Result, Box> { //! unimplemented!() //! } //! } @@ -153,13 +153,13 @@ //! [Output](crate::report::Reporter::Output) type and a single method. //! ``` //! # use pubgrub::package::Package; -//! # use pubgrub::version::Version; +//! # use pubgrub::version_trait::{Version, Interval}; //! # use pubgrub::report::DerivationTree; //! # -//! pub trait Reporter { +//! pub trait Reporter, V: Version> { //! 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 @@ -172,9 +172,9 @@ //! # use pubgrub::solver::{resolve, OfflineDependencyProvider}; //! # use pubgrub::report::{DefaultStringReporter, Reporter}; //! # use pubgrub::error::PubGrubError; -//! # use pubgrub::version::NumberVersion; +//! # use pubgrub::version_trait::{NumberInterval, NumberVersion}; //! # -//! # let dependency_provider = OfflineDependencyProvider::<&str, NumberVersion>::new(); +//! # let dependency_provider = OfflineDependencyProvider::<&str, NumberInterval, NumberVersion>::new(); //! # let root_package = "root"; //! # let root_version = 1; //! # @@ -212,10 +212,12 @@ pub mod error; pub mod package; pub mod range; +pub mod range_trait; pub mod report; pub mod solver; pub mod term; pub mod type_aliases; pub mod version; +pub mod version_trait; mod internal; diff --git a/src/range_trait.rs b/src/range_trait.rs new file mode 100644 index 00000000..179e5a9c --- /dev/null +++ b/src/range_trait.rs @@ -0,0 +1,618 @@ +// SPDX-License-Identifier: MPL-2.0 + +//! Ranges are constraints defining sets of versions. +//! +//! Concretely, those constraints correspond to any set of versions +//! representable as the concatenation, union, and complement +//! of the ranges building blocks. +//! +//! Those building blocks are: +//! - [empty()](Range::empty): the empty set +//! - [full()](Range::full): the set of all possible versions +//! - [singleton(v)](Range::singleton): the set containing only the version v +//! - [higher_than(v)](Range::higher_than): the set defined by `v <= versions` +//! - [strictly_lower_than(v)](Range::strictly_lower_than): the set defined by `versions < v` +//! - [between(v1, v2)](Range::between): the set defined by `v1 <= versions < v2` + +use std::cmp::Ordering; +use std::fmt; +use std::marker::PhantomData; +use std::ops::Bound; + +use crate::internal::small_vec::SmallVec; +use crate::version_trait::{flip_bound, owned_bound, ref_bound}; +use crate::version_trait::{Interval, Version}; + +#[derive(Debug, Clone, Eq, PartialEq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +#[cfg_attr(feature = "serde", serde(transparent))] +/// New range with intervals. +pub struct Range { + segments: SmallVec, + #[cfg_attr(feature = "serde", serde(skip_serializing))] + phantom: PhantomData, +} + +#[derive(Debug, Clone, Eq, PartialEq)] +enum SidedBound { + Left(Bound), + Right(Bound), +} + +impl PartialOrd for SidedBound { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for SidedBound { + fn cmp(&self, other: &Self) -> Ordering { + self.compare(other) + } +} + +impl SidedBound { + fn compare(&self, other: &Self) -> Ordering { + match (&self, other) { + // Handling of both left bounds. + (SidedBound::Left(Bound::Unbounded), SidedBound::Left(Bound::Unbounded)) => { + Ordering::Equal + } + (SidedBound::Left(Bound::Unbounded), SidedBound::Left(_)) => Ordering::Less, + (SidedBound::Left(Bound::Excluded(l1)), SidedBound::Left(Bound::Excluded(l2))) => { + l1.cmp(l2) + } + (SidedBound::Left(Bound::Excluded(l1)), SidedBound::Left(Bound::Included(l2))) => { + // An open left bound is greater than a closed left bound. + l1.cmp(l2).then(Ordering::Greater) + } + (SidedBound::Left(Bound::Included(l1)), SidedBound::Left(Bound::Included(l2))) => { + l1.cmp(l2) + } + (SidedBound::Left(_), SidedBound::Left(_)) => other.compare(self).reverse(), + + // Handling of both right bounds. + (SidedBound::Right(Bound::Unbounded), SidedBound::Right(Bound::Unbounded)) => { + Ordering::Equal + } + (SidedBound::Right(Bound::Unbounded), SidedBound::Right(_)) => Ordering::Greater, + (SidedBound::Right(Bound::Excluded(r1)), SidedBound::Right(Bound::Excluded(r2))) => { + r1.cmp(r2) + } + (SidedBound::Right(Bound::Excluded(r1)), SidedBound::Right(Bound::Included(r2))) => { + // An open Right bound is smaller than a closed Right bound. + r1.cmp(r2).then(Ordering::Less) + } + (SidedBound::Right(Bound::Included(r1)), SidedBound::Right(Bound::Included(r2))) => { + r1.cmp(r2) + } + (SidedBound::Right(_), SidedBound::Right(_)) => other.compare(self).reverse(), + + // Handling of left and right bounds. + (SidedBound::Left(Bound::Unbounded), SidedBound::Right(_)) => Ordering::Less, + (SidedBound::Left(_), SidedBound::Right(Bound::Unbounded)) => Ordering::Less, + (SidedBound::Left(Bound::Excluded(l)), SidedBound::Right(Bound::Excluded(r))) => { + // An open left bound is after an open right bound. + l.cmp(r).then(Ordering::Greater) + } + (SidedBound::Left(Bound::Excluded(l)), SidedBound::Right(Bound::Included(r))) => { + l.cmp(r) + } + (SidedBound::Left(Bound::Included(l)), SidedBound::Right(Bound::Excluded(r))) => { + l.cmp(r) + } + (SidedBound::Left(Bound::Included(l)), SidedBound::Right(Bound::Included(r))) => { + l.cmp(r).then(Ordering::Less) + } + + // Handling of right and left bounds. + (SidedBound::Right(_), SidedBound::Left(_)) => other.compare(self).reverse(), + } + } +} + +// Ranges building blocks. +impl, V: Version> Range { + /// Empty set of versions. + pub fn empty() -> Self { + Self { + segments: SmallVec::empty(), + phantom: PhantomData, + } + } + + /// Set of all possible versions. + pub fn full() -> Self { + Self { + segments: SmallVec::one(I::new(V::minimum(), V::maximum())), + phantom: PhantomData, + } + } + + /// Set containing exactly one version. + pub fn singleton(v: impl Into) -> Self { + let v = v.into(); + let start_bound = Bound::Included(v.clone()); + let end_bound = Bound::Included(v); + Self { + segments: SmallVec::one(I::new(start_bound, end_bound)), + phantom: PhantomData, + } + } + + /// Set of all versions higher or equal to some version. + pub fn higher_than(v: impl Into) -> Self { + Self { + segments: SmallVec::one(I::new(Bound::Included(v.into()), V::maximum())), + phantom: PhantomData, + } + } + + /// Set of all versions strictly lower than some version. + pub fn strictly_lower_than(v: impl Into) -> Self { + Self { + segments: SmallVec::one(I::new(V::minimum(), Bound::Excluded(v.into()))), + phantom: PhantomData, + } + } + + /// Set of all versions strictly lower than some version. + pub fn strictly_higher_than(v: impl Into) -> Self { + Self { + segments: SmallVec::one(I::new(Bound::Excluded(v.into()), V::maximum())), + phantom: PhantomData, + } + } + + /// Set of all versions comprised between two given versions. + /// The lower bound is included and the higher bound excluded. + /// `v1 <= v < v2`. + pub fn between(v1: impl Into, v2: impl Into) -> Self { + let start_bound = Bound::Included(v1.into()); + let end_bound = Bound::Excluded(v2.into()); + Self { + segments: SmallVec::one(I::new(start_bound, end_bound)), + phantom: PhantomData, + } + } +} + +// Set operations. +impl, V: Version> Range { + // Negate ################################################################## + + /// Compute the complement set of versions. + pub fn complement(&self) -> Self { + match self.segments.first() { + None => Self::full(), // Complement of ∅ is * + Some(seg) => { + if seg.start_bound() == ref_bound(&V::minimum()) { + Self::complement_segments( + owned_bound(flip_bound(seg.end_bound())), + &self.segments[1..], + ) + } else { + Self::complement_segments(V::minimum(), &self.segments) + } + } + } + } + + /// Helper function performing the negation of intervals in segments. + /// For example: + /// [ (v1, None) ] => [ (start, Some(v1)) ] + /// [ (v1, Some(v2)) ] => [ (start, Some(v1)), (v2, None) ] + fn complement_segments(start: Bound, segments: &[I]) -> Self { + let mut complemented_segments = SmallVec::empty(); + let mut start = start; + for seg in segments { + complemented_segments.push(I::new(start, owned_bound(flip_bound(seg.start_bound())))); + start = owned_bound(flip_bound(seg.end_bound())); + } + if start != V::maximum() { + complemented_segments.push(I::new(start, V::maximum())); + } + + Self { + segments: complemented_segments, + phantom: PhantomData, + } + } + + // Union and intersection ################################################## + + /// Compute the union of two sets of versions. + pub fn union(&self, other: &Self) -> Self { + self.complement() + .intersection(&other.complement()) + .complement() + } + + /// Compute the intersection of two sets of versions. + pub fn intersection(&self, other: &Self) -> Self { + let mut segments = SmallVec::empty(); + let mut self_iter = self.segments.iter(); + let mut other_iter = other.segments.iter(); + let mut first = self_iter.next(); + let mut second = other_iter.next(); + while let (Some(first_seg), Some(second_seg)) = (first, second) { + let fs = first_seg.start_bound(); + let fe = first_seg.end_bound(); + let ss = second_seg.start_bound(); + let se = second_seg.end_bound(); + match SidedBound::Right(fe).compare(&SidedBound::Left(ss)) { + // Disjoint intervals with left < right. + Ordering::Less => first = self_iter.next(), + Ordering::Equal => first = self_iter.next(), + // Possible intersection with left >= right. + Ordering::Greater => { + match SidedBound::Right(se).compare(&SidedBound::Left(fs)) { + // Disjoint intervals with left < right. + Ordering::Less => second = other_iter.next(), + Ordering::Equal => second = other_iter.next(), + // Intersection for sure. + Ordering::Greater => { + let start = match SidedBound::Left(fs).compare(&SidedBound::Left(ss)) { + Ordering::Less => ss, + _ => fs, + }; + match SidedBound::Right(fe).compare(&SidedBound::Right(se)) { + Ordering::Less => { + segments.push(I::new(owned_bound(start), owned_bound(fe))); + first = self_iter.next(); + } + Ordering::Equal => { + segments.push(I::new(owned_bound(start), owned_bound(fe))); + first = self_iter.next(); + second = other_iter.next(); + } + Ordering::Greater => { + segments.push(I::new(owned_bound(start), owned_bound(se))); + second = other_iter.next(); + } + } + } + } + } + } + } + + Self { + segments, + phantom: PhantomData, + } + } + + // Contains ################################################################ + + /// Check if ranges contain a given version. + pub fn contains(&self, version: &V) -> bool { + for seg in &self.segments { + match (seg.start_bound(), seg.end_bound()) { + (Bound::Unbounded, Bound::Unbounded) => return seg.contains(version), + (Bound::Unbounded, Bound::Excluded(r)) => match version.cmp(r) { + Ordering::Less => return seg.contains(version), + Ordering::Equal => return false, + Ordering::Greater => {} + }, + (Bound::Unbounded, Bound::Included(r)) => match version.cmp(r) { + Ordering::Greater => {} + _ => return seg.contains(version), + }, + (Bound::Excluded(l), Bound::Unbounded) => match version.cmp(l) { + Ordering::Greater => return seg.contains(version), + _ => return false, + }, + (Bound::Excluded(l), Bound::Excluded(r)) => match version.cmp(l) { + Ordering::Less => return false, + Ordering::Equal => return false, + Ordering::Greater => match version.cmp(r) { + Ordering::Less => return seg.contains(version), + Ordering::Equal => return false, + Ordering::Greater => {} + }, + }, + (Bound::Excluded(l), Bound::Included(r)) => match version.cmp(l) { + Ordering::Less => return false, + Ordering::Equal => return false, + Ordering::Greater => match version.cmp(r) { + Ordering::Greater => {} + _ => return seg.contains(version), + }, + }, + (Bound::Included(l), Bound::Unbounded) => match version.cmp(l) { + Ordering::Less => return false, + _ => return seg.contains(version), + }, + (Bound::Included(l), Bound::Excluded(r)) => match version.cmp(l) { + Ordering::Less => return false, + Ordering::Equal => return seg.contains(version), + Ordering::Greater => match version.cmp(r) { + Ordering::Less => return seg.contains(version), + Ordering::Equal => return false, + Ordering::Greater => {} + }, + }, + (Bound::Included(l), Bound::Included(r)) => match version.cmp(l) { + Ordering::Less => return false, + Ordering::Equal => return seg.contains(version), + Ordering::Greater => match version.cmp(r) { + Ordering::Greater => {} + _ => return seg.contains(version), + }, + }, + } + } + false + } +} + +// REPORT ###################################################################### + +impl, V: Version> fmt::Display for Range { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self.segments.as_slice() { + [] => write!(f, "∅"), + [seg] => { + write!(f, "{}", interval_to_string(seg)) + } + more_than_one_interval => { + let string_intervals: Vec<_> = more_than_one_interval + .iter() + .map(interval_to_string) + .collect(); + write!(f, "{}", string_intervals.join(" OR ")) + } + } + } +} + +fn interval_to_string, V: Version>(seg: &I) -> String { + let start = seg.start_bound(); + let end = seg.end_bound(); + if start == ref_bound(&V::minimum()) { + display_end_bound(end) + } else if end == ref_bound(&V::maximum()) { + display_start_bound(start) + } else { + format!("{}, {}", display_start_bound(start), display_end_bound(end)) + } +} + +fn display_start_bound(start: Bound<&V>) -> String { + match start { + Bound::Unbounded => "∗".to_string(), + Bound::Excluded(v) => format!("> {}", v), + Bound::Included(v) => format!(">= {}", v), + } +} + +fn display_end_bound(end: Bound<&V>) -> String { + match end { + Bound::Unbounded => "∗".to_string(), + Bound::Excluded(v) => format!("< {}", v), + Bound::Included(v) => format!("<= {}", v), + } +} + +// TESTS ####################################################################### + +#[cfg(test)] +pub mod tests { + use proptest::prelude::*; + + use super::*; + use crate::version_trait::{NumberInterval, NumberVersion}; + + // SidedBound tests. + + use Bound::{Excluded, Included, Unbounded}; + use SidedBound::{Left, Right}; + + fn ref_sided_bound(sb: &SidedBound) -> SidedBound<&V> { + match sb { + SidedBound::Left(bound) => SidedBound::Left(ref_bound(bound)), + SidedBound::Right(bound) => SidedBound::Right(ref_bound(bound)), + } + } + + fn sided_bound_strategy() -> impl Strategy> { + prop_oneof![ + bound_strategy().prop_map(Left), + bound_strategy().prop_map(Right), + ] + } + + fn bound_strategy() -> impl Strategy> { + prop_oneof![ + Just(Unbounded), + any::().prop_map(|n| Excluded(n.into())), + any::().prop_map(|n| Included(n.into())), + ] + } + + fn no_unbound_strategy() -> impl Strategy> { + prop_oneof![ + any::().prop_map(|n| Excluded(n.into())), + any::().prop_map(|n| Included(n.into())), + ] + } + + proptest! { + + #[test] + fn reverse_bounds_reverse_order(sb1 in sided_bound_strategy(), sb2 in sided_bound_strategy()) { + let s1 = ref_sided_bound(&sb1); + let s2 = ref_sided_bound(&sb2); + assert_eq!(s1.compare(&s2), s2.compare(&s1).reverse()); + } + + } + + // Ranges tests. + + #[test] + fn failing_symmetric() { + let s1: Range = Range::strictly_higher_than(0); + let s2 = Range::higher_than(0); + eprintln!("first: {}", s1); + eprintln!("second: {}", s2); + assert_eq!(s1.intersection(&s2), s2.intersection(&s1)); + } + + pub fn strategy() -> impl Strategy> { + // TODO: find a better strategy so that the "intersection_is_symmetric" test + // triggers the same failure than the negate_singleton above. + prop::collection::vec(no_unbound_strategy(), 0..20).prop_map(|mut vec| { + let extract = |b: &Bound| match b { + Unbounded => panic!("cannot happen"), + Excluded(v) => v.0, + Included(v) => v.0, + }; + vec.dedup(); + vec.sort_unstable_by_key(extract); + let mut segments: SmallVec = SmallVec::empty(); + let mut last_right: i64 = -1; + let mut pair_iter = vec.chunks_exact(2); + while let Some([b1, b2]) = pair_iter.next() { + let v1 = extract(b1); + let v2 = extract(b2); + if v1 as i64 == last_right { + segments.push(NumberInterval::new(Excluded(v1.into()), b2.clone())); + } else if v1 == v2 { + // segments.push((v1..=v1).into()); + segments.push(NumberInterval::new( + Included(v1.into()), + Included(v2.into()), + )); + } else { + segments.push(NumberInterval::new(b1.clone(), b2.clone())); + } + last_right = v2 as i64; + } + if let [b1] = pair_iter.remainder() { + let v1 = extract(b1); + if v1 as i64 == last_right { + segments.push(NumberInterval::new(Excluded(v1.into()), Unbounded)); + } else { + segments.push((v1..).into()); + } + } + eprintln!("generated segments length: {}", segments.len()); + Range { + // segments: merge_juxtaposed_segments(segments), + segments, + phantom: PhantomData, + } + }) + } + + fn version_strat() -> impl Strategy { + any::().prop_map(NumberVersion) + } + + proptest! { + + // #[test] + // fn long_ranges(ranges in strategy()) { + // if ranges.segments.len() > 2 { + // assert_eq!(ranges, Range::empty()); + // } + // } + + // Testing negate ---------------------------------- + + #[test] + fn negate_is_different(ranges in strategy()) { + assert_ne!(ranges.complement(), ranges); + } + + #[test] + fn double_negate_is_identity(ranges in strategy()) { + assert_eq!(ranges.complement().complement(), ranges); + } + + #[test] + fn negate_contains_opposite(ranges in strategy(), version in version_strat()) { + assert_ne!(ranges.contains(&version), ranges.complement().contains(&version)); + } + + // Testing intersection ---------------------------- + + #[test] + fn intersection_is_symmetric(r1 in strategy(), r2 in strategy()) { + assert_eq!(r1.intersection(&r2), r2.intersection(&r1)); + } + + #[test] + fn intersection_with_any_is_identity(ranges in strategy()) { + assert_eq!(Range::full().intersection(&ranges), ranges); + } + + #[test] + fn intersection_with_none_is_none(ranges in strategy()) { + assert_eq!(Range::empty().intersection(&ranges), Range::empty()); + } + + #[test] + fn intersection_is_idempotent(r1 in strategy(), r2 in strategy()) { + assert_eq!(r1.intersection(&r2).intersection(&r2), r1.intersection(&r2)); + } + + #[test] + fn intersection_is_associative(r1 in strategy(), r2 in strategy(), r3 in strategy()) { + assert_eq!(r1.intersection(&r2).intersection(&r3), r1.intersection(&r2.intersection(&r3))); + } + + #[test] + fn intesection_of_complements_is_none(ranges in strategy()) { + assert_eq!(ranges.complement().intersection(&ranges), Range::empty()); + } + + #[test] + fn intesection_contains_both(r1 in strategy(), r2 in strategy(), version in version_strat()) { + assert_eq!(r1.intersection(&r2).contains(&version), r1.contains(&version) && r2.contains(&version)); + } + + // Testing union ----------------------------------- + + #[test] + fn union_of_complements_is_any(ranges in strategy()) { + assert_eq!(ranges.complement().union(&ranges), Range::full()); + } + + #[test] + fn union_contains_either(r1 in strategy(), r2 in strategy(), version in version_strat()) { + assert_eq!(r1.union(&r2).contains(&version), r1.contains(&version) || r2.contains(&version)); + } + + // Testing contains -------------------------------- + + #[test] + fn always_contains_exact(version in version_strat()) { + assert!(Range::::singleton(version).contains(&version)); + } + + #[test] + fn contains_negation(ranges in strategy(), version in version_strat()) { + assert_ne!(ranges.contains(&version), ranges.complement().contains(&version)); + } + + #[test] + fn contains_intersection(ranges in strategy(), version in version_strat()) { + assert_eq!(ranges.contains(&version), ranges.intersection(&Range::singleton(version)) != Range::empty()); + } + } + + // #[test] + // fn contains_negation_0() { + // let full: Range = Range::full(); + // println!("full: {:?}", full); + // let full_comp = full.complement(); + // println!("full.complement(): {:?}", full_comp); + // let v0 = NumberVersion::from(0); + // assert_eq!(&full_comp, &Range::empty()); + // assert_ne!(full.contains(&v0), full_comp.contains(&v0)); + // panic!("force fail"); + // } +} diff --git a/src/report.rs b/src/report.rs index 07dec364..feb2e8f0 100644 --- a/src/report.rs +++ b/src/report.rs @@ -7,50 +7,50 @@ use std::fmt; use std::ops::{Deref, DerefMut}; use crate::package::Package; -use crate::range::Range; +use crate::range_trait::Range; use crate::term::Term; use crate::type_aliases::Map; -use crate::version::Version; +use crate::version_trait::{Interval, Version}; /// Reporter trait. -pub trait Reporter { +pub trait Reporter, V: Version> { /// Output type of the report. type Output; /// Generate a report from the derivation tree /// describing the resolution failure. - fn report(derivation_tree: &DerivationTree) -> Self::Output; + fn report(derivation_tree: &DerivationTree) -> 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, V: Version> { /// 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, V: Version> { /// Initial incompatibility aiming at picking the root package for the first decision. NotRoot(P, V), /// There are no versions in the given range for this package. - NoVersions(P, Range), + NoVersions(P, Range), /// Dependencies of the package are unavailable for versions in that range. - UnavailableDependencies(P, Range), + UnavailableDependencies(P, Range), /// Incompatibility coming from the dependencies of a given package. - FromDependencyOf(P, Range, P, Range), + FromDependencyOf(P, Range, P, Range), } /// Incompatibility derived from two others. #[derive(Debug, Clone)] -pub struct Derived { +pub struct Derived, V: Version> { /// Terms of the incompatibility. - pub terms: Map>, + pub terms: Map>, /// Indicate if that incompatibility is present multiple times /// in the derivation tree. /// If that is the case, it has a unique id, provided in that option. @@ -58,12 +58,12 @@ pub struct Derived { /// and refer to the explanation for the other times. pub shared_id: Option, /// First cause. - pub cause1: Box>, + pub cause1: Box>, /// Second cause. - pub cause2: Box>, + pub cause2: Box>, } -impl DerivationTree { +impl, V: Version> DerivationTree { /// Merge the [NoVersions](External::NoVersions) external incompatibilities /// with the other one they are matched with /// in a derived incompatibility. @@ -100,7 +100,7 @@ impl DerivationTree { } } - fn merge_no_versions(self, package: P, range: Range) -> Option { + fn merge_no_versions(self, package: P, range: Range) -> Option { match self { // TODO: take care of the Derived case. // Once done, we can remove the Option. @@ -138,21 +138,21 @@ impl DerivationTree { } } -impl fmt::Display for External { +impl, V: Version> fmt::Display for External { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { Self::NotRoot(package, version) => { write!(f, "we are solving dependencies of {} {}", package, version) } Self::NoVersions(package, range) => { - if range == &Range::any() { + if range == &Range::full() { write!(f, "there is no available version for {}", package) } else { write!(f, "there is no version of {} in {}", package, range) } } Self::UnavailableDependencies(package, range) => { - if range == &Range::any() { + if range == &Range::full() { write!(f, "dependencies of {} are unavailable", package) } else { write!( @@ -163,11 +163,11 @@ impl fmt::Display for External { } } Self::FromDependencyOf(p, range_p, dep, range_dep) => { - if range_p == &Range::any() && range_dep == &Range::any() { + if range_p == &Range::full() && range_dep == &Range::full() { write!(f, "{} depends on {}", p, dep) - } else if range_p == &Range::any() { + } else if range_p == &Range::full() { write!(f, "{} depends on {} {}", p, dep, range_dep) - } else if range_dep == &Range::any() { + } else if range_dep == &Range::full() { write!(f, "{} {} depends on {}", p, range_p, dep) } else { write!(f, "{} {} depends on {} {}", p, range_p, dep, range_dep) @@ -198,7 +198,10 @@ impl DefaultStringReporter { } } - fn build_recursive(&mut self, derived: &Derived) { + fn build_recursive, V: Version>( + &mut self, + derived: &Derived, + ) { self.build_recursive_helper(derived); if let Some(id) = derived.shared_id { if self.shared_with_ref.get(&id) == None { @@ -208,7 +211,10 @@ impl DefaultStringReporter { }; } - fn build_recursive_helper(&mut self, current: &Derived) { + fn build_recursive_helper, V: Version>( + &mut self, + current: &Derived, + ) { match (current.cause1.deref(), current.cause2.deref()) { (DerivationTree::External(external1), DerivationTree::External(external2)) => { // Simplest case, we just combine two external incompatibilities. @@ -285,11 +291,11 @@ 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, V: Version>( &mut self, - derived: &Derived, - external: &External, - current_terms: &Map>, + derived: &Derived, + external: &External, + current_terms: &Map>, ) { match self.line_ref_of(derived.shared_id) { Some(ref_id) => self.lines.push(Self::explain_ref_and_external( @@ -303,11 +309,11 @@ impl DefaultStringReporter { } /// Report one derived (without a line ref yet) and one external. - fn report_recurse_one_each( + fn report_recurse_one_each, V: Version>( &mut self, - derived: &Derived, - external: &External, - current_terms: &Map>, + derived: &Derived, + external: &External, + current_terms: &Map>, ) { match (derived.cause1.deref(), derived.cause2.deref()) { // If the derived cause has itself one external prior cause, @@ -341,10 +347,10 @@ impl DefaultStringReporter { // String explanations ##################################################### /// Simplest case, we just combine two external incompatibilities. - fn explain_both_external( - external1: &External, - external2: &External, - current_terms: &Map>, + fn explain_both_external, V: Version>( + external1: &External, + external2: &External, + current_terms: &Map>, ) -> String { // TODO: order should be chosen to make it more logical. format!( @@ -356,12 +362,12 @@ impl DefaultStringReporter { } /// Both causes have already been explained so we use their refs. - fn explain_both_ref( + fn explain_both_ref, V: Version>( ref_id1: usize, - derived1: &Derived, + derived1: &Derived, ref_id2: usize, - derived2: &Derived, - current_terms: &Map>, + derived2: &Derived, + current_terms: &Map>, ) -> String { // TODO: order should be chosen to make it more logical. format!( @@ -377,11 +383,11 @@ impl DefaultStringReporter { /// One cause is derived (already explained so one-line), /// the other is a one-line external cause, /// and finally we conclude with the current incompatibility. - fn explain_ref_and_external( + fn explain_ref_and_external, V: Version>( ref_id: usize, - derived: &Derived, - external: &External, - current_terms: &Map>, + derived: &Derived, + external: &External, + current_terms: &Map>, ) -> String { // TODO: order should be chosen to make it more logical. format!( @@ -394,9 +400,9 @@ impl DefaultStringReporter { } /// Add an external cause to the chain of explanations. - fn and_explain_external( - external: &External, - current_terms: &Map>, + fn and_explain_external, V: Version>( + external: &External, + current_terms: &Map>, ) -> String { format!( "And because {}, {}.", @@ -406,10 +412,10 @@ impl DefaultStringReporter { } /// Add an already explained incompat to the chain of explanations. - fn and_explain_ref( + fn and_explain_ref, V: Version>( ref_id: usize, - derived: &Derived, - current_terms: &Map>, + derived: &Derived, + current_terms: &Map>, ) -> String { format!( "And because {} ({}), {}.", @@ -420,10 +426,10 @@ impl DefaultStringReporter { } /// Add an already explained incompat to the chain of explanations. - fn and_explain_prior_and_external( - prior_external: &External, - external: &External, - current_terms: &Map>, + fn and_explain_prior_and_external, V: Version>( + prior_external: &External, + external: &External, + current_terms: &Map>, ) -> String { format!( "And because {} and {}, {}.", @@ -434,7 +440,9 @@ impl DefaultStringReporter { } /// Try to print terms of an incompatibility in a human-readable way. - pub fn string_terms(terms: &Map>) -> String { + pub fn string_terms, V: Version>( + terms: &Map>, + ) -> String { let terms_vec: Vec<_> = terms.iter().collect(); match terms_vec.as_slice() { [] => "version solving failed".into(), @@ -469,10 +477,10 @@ impl DefaultStringReporter { } } -impl Reporter for DefaultStringReporter { +impl, V: Version> Reporter for DefaultStringReporter { type Output = String; - fn report(derivation_tree: &DerivationTree) -> Self::Output { + fn report(derivation_tree: &DerivationTree) -> Self::Output { match derivation_tree { DerivationTree::External(external) => external.to_string(), DerivationTree::Derived(derived) => { diff --git a/src/solver.rs b/src/solver.rs index 62015911..014f8702 100644 --- a/src/solver.rs +++ b/src/solver.rs @@ -42,11 +42,11 @@ //! //! ``` //! # use pubgrub::solver::{resolve, OfflineDependencyProvider}; -//! # use pubgrub::version::NumberVersion; +//! # use pubgrub::version_trait::{NumberInterval, NumberVersion}; //! # use pubgrub::error::PubGrubError; //! # -//! # fn try_main() -> Result<(), PubGrubError<&'static str, NumberVersion>> { -//! # let dependency_provider = OfflineDependencyProvider::<&str, NumberVersion>::new(); +//! # fn try_main() -> Result<(), PubGrubError<&'static str, NumberInterval, NumberVersion>> { +//! # let dependency_provider = OfflineDependencyProvider::<&str, NumberInterval, NumberVersion>::new(); //! # let package = "root"; //! # let version = 1; //! let solution = resolve(&dependency_provider, package, version)?; @@ -68,22 +68,23 @@ use std::borrow::Borrow; use std::collections::{BTreeMap, BTreeSet as Set}; use std::error::Error; +use std::fmt::Debug; use crate::error::PubGrubError; use crate::internal::core::State; use crate::internal::incompatibility::Incompatibility; use crate::package::Package; -use crate::range::Range; +use crate::range_trait::Range; use crate::type_aliases::{Map, SelectedDependencies}; -use crate::version::Version; +use crate::version_trait::{Interval, Version}; /// Main function of the library. /// Finds a set of packages satisfying dependency bounds for a given package + version pair. -pub fn resolve( - dependency_provider: &impl DependencyProvider, +pub fn resolve + Debug, V: Version>( + dependency_provider: &impl DependencyProvider, package: P, version: impl Into, -) -> Result, PubGrubError> { +) -> Result, PubGrubError> { let mut state = State::init(package.clone(), version.into()); let mut added_dependencies: Map> = Map::default(); let mut next = package; @@ -92,7 +93,12 @@ pub fn resolve( .should_cancel() .map_err(|err| PubGrubError::ErrorInShouldCancel(err))?; + log::info!("unit_propagation: {}", &next); state.unit_propagation(next)?; + log::debug!( + "Partial solution after unit propagation: {}", + state.partial_solution + ); let potential_packages = state.partial_solution.potential_packages(); if potential_packages.is_none() { @@ -100,6 +106,7 @@ pub fn resolve( // The borrow checker did not like using a match on potential_packages. // This `if ... is_none ... drop` is a workaround. // I believe this is a case where Polonius could help, when and if it lands in rustc. + log::debug!("Incompatibility store: {:?}", state.incompatibility_store); 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(), @@ -109,6 +116,7 @@ pub fn resolve( let decision = dependency_provider .choose_package_version(potential_packages.unwrap()) .map_err(PubGrubError::ErrorChoosingPackageVersion)?; + log::info!("DP chose: {} @ {:?}", decision.0, decision.1); next = decision.0.clone(); // Pick the next compatible version. @@ -137,38 +145,37 @@ pub fn resolve( { // Retrieve that package dependencies. let p = &next; - let dependencies = - match dependency_provider - .get_dependencies(&p, &v) - .map_err(|err| PubGrubError::ErrorRetrievingDependencies { - package: p.clone(), - version: v.clone(), - source: err, - })? { - Dependencies::Unknown => { - state.add_incompatibility(Incompatibility::unavailable_dependencies( - p.clone(), - v.clone(), - )); - continue; + let dependencies = match dependency_provider.get_dependencies(p, &v).map_err(|err| { + PubGrubError::ErrorRetrievingDependencies { + package: p.clone(), + version: v.clone(), + source: err, + } + })? { + 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.clone(), + }); } - Dependencies::Known(x) => { - if x.contains_key(&p) { - return Err(PubGrubError::SelfDependency { - package: p.clone(), - version: v.clone(), - }); - } - if let Some((dependent, _)) = x.iter().find(|(_, r)| r == &&Range::none()) { - return Err(PubGrubError::DependencyOnTheEmptySet { - package: p.clone(), - version: v.clone(), - dependent: dependent.clone(), - }); - } - x + if let Some((dependent, _)) = x.iter().find(|(_, r)| r == &&Range::empty()) { + return Err(PubGrubError::DependencyOnTheEmptySet { + package: p.clone(), + version: v.clone(), + dependent: dependent.clone(), + }); } - }; + x + } + }; // Add that package and version if the dependencies are not problematic. let dep_incompats = @@ -195,6 +202,7 @@ pub fn resolve( } else { // `dep_incompats` are already in `incompatibilities` so we know there are not satisfied // terms and can add the decision directly. + log::info!("add_decision (not first time): {} @ {}", &next, v); state.partial_solution.add_decision(next.clone(), v); } } @@ -203,11 +211,11 @@ pub fn resolve( /// An enum used by [DependencyProvider] that holds information about package dependencies. /// For each [Package] there is a [Range] of concrete versions it allows as a dependency. #[derive(Clone)] -pub enum Dependencies { +pub enum Dependencies, V: Version> { /// Package dependencies are unavailable. Unknown, /// Container for all available package versions. - Known(DependencyConstraints), + Known(DependencyConstraints), } /// Subtype of [Dependencies] which holds information about @@ -216,11 +224,11 @@ pub enum Dependencies { /// inside [DependencyConstraints] and [Dependencies::Unknown]: /// the former means the package has no dependencies and it is a known fact, /// while the latter means they could not be fetched by [DependencyProvider]. -pub type DependencyConstraints = Map>; +pub type DependencyConstraints = Map>; /// 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, V: Version> { /// [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. @@ -246,7 +254,7 @@ pub trait DependencyProvider { /// of the available versions in preference order for any package. /// /// Note: the type `T` ensures that this returns an item from the `packages` argument. - fn choose_package_version, U: Borrow>>( + fn choose_package_version, U: Borrow>>( &self, potential_packages: impl Iterator, ) -> Result<(T, Option), Box>; @@ -257,7 +265,7 @@ pub trait DependencyProvider { &self, package: &P, version: &V, - ) -> Result, Box>; + ) -> Result, Box>; /// This is called fairly regularly during the resolution, /// if it returns an Err then resolution will be terminated. @@ -276,15 +284,15 @@ pub trait DependencyProvider { /// 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( +pub fn choose_package_with_fewest_versions, V: Version, T, U, It, F>( list_available_versions: F, potential_packages: impl Iterator, ) -> (T, Option) where T: Borrow

, - U: Borrow>, - I: Iterator, - F: Fn(&P) -> I, + U: Borrow>, + It: Iterator, + F: Fn(&P) -> It, { let count_valid = |(p, range): &(T, U)| { list_available_versions(p.borrow()) @@ -303,11 +311,11 @@ where #[derive(Debug, Clone, Default)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[cfg_attr(feature = "serde", serde(transparent))] -pub struct OfflineDependencyProvider { - dependencies: Map>>, +pub struct OfflineDependencyProvider, V: Version> { + dependencies: Map>>, } -impl OfflineDependencyProvider { +impl, V: Version> OfflineDependencyProvider { /// Creates an empty OfflineDependencyProvider with no dependencies. pub fn new() -> Self { Self { @@ -325,11 +333,11 @@ impl OfflineDependencyProvider { /// The API does not allow to add dependencies one at a time to uphold an assumption that /// [OfflineDependencyProvider.get_dependencies(p, v)](OfflineDependencyProvider::get_dependencies) /// provides all dependencies of a given package (p) and version (v) pair. - pub fn add_dependencies)>>( + pub fn add_dependencies)>>( &mut self, package: P, version: impl Into, - dependencies: I, + dependencies: It, ) { let package_deps = dependencies.into_iter().collect(); let v = version.into(); @@ -354,7 +362,7 @@ impl OfflineDependencyProvider { /// Lists dependencies of a given package and version. /// Returns [None] if no information is available regarding that package and version pair. - fn dependencies(&self, package: &P, version: &V) -> Option> { + fn dependencies(&self, package: &P, version: &V) -> Option> { self.dependencies.get(package)?.get(version).cloned() } } @@ -363,8 +371,10 @@ impl OfflineDependencyProvider { /// contains all dependency information available in memory. /// Packages are picked with the fewest versions contained in the constraints first. /// Versions are picked with the newest versions first. -impl DependencyProvider for OfflineDependencyProvider { - fn choose_package_version, U: Borrow>>( +impl, V: Version> DependencyProvider + for OfflineDependencyProvider +{ + fn choose_package_version, U: Borrow>>( &self, potential_packages: impl Iterator, ) -> Result<(T, Option), Box> { @@ -385,7 +395,7 @@ impl DependencyProvider for OfflineDependencyProvi &self, package: &P, version: &V, - ) -> Result, Box> { + ) -> Result, Box> { Ok(match self.dependencies(package, version) { None => Dependencies::Unknown, Some(dependencies) => Dependencies::Known(dependencies), diff --git a/src/term.rs b/src/term.rs index bc038acf..82ad293e 100644 --- a/src/term.rs +++ b/src/term.rs @@ -3,38 +3,38 @@ //! A term is the fundamental unit of operation of the PubGrub algorithm. //! It is a positive or negative expression regarding a set of versions. -use crate::range::Range; -use crate::version::Version; +use crate::range_trait::Range; +use crate::version_trait::{Interval, Version}; use std::fmt; /// A positive or negative expression regarding a set of versions. #[derive(Debug, Clone, Eq, PartialEq)] -pub enum Term { +pub enum Term, V: Version> { /// For example, "1.0.0 <= v < 2.0.0" is a positive expression /// that is evaluated true if a version is selected /// and comprised between version 1.0.0 and version 2.0.0. - Positive(Range), + Positive(Range), /// The term "not v < 3.0.0" is a negative expression /// that is evaluated true if a version is selected >= 3.0.0 /// or if no version is selected at all. - Negative(Range), + Negative(Range), } /// Base methods. -impl Term { +impl, V: Version> Term { /// A term that is always true. pub(crate) fn any() -> Self { - Self::Negative(Range::none()) + Self::Negative(Range::empty()) } /// A term that is never true. pub(crate) fn empty() -> Self { - Self::Positive(Range::none()) + Self::Positive(Range::empty()) } /// A positive term containing exactly that version. pub(crate) fn exact(version: V) -> Self { - Self::Positive(Range::exact(version)) + Self::Positive(Range::singleton(version)) } /// Simply check if a term is positive. @@ -65,7 +65,7 @@ impl Term { /// Unwrap the range contains in a positive term. /// Will panic if used on a negative range. - pub(crate) fn unwrap_positive(&self) -> &Range { + pub(crate) fn unwrap_positive(&self) -> &Range { match self { Self::Positive(range) => range, _ => panic!("Negative term cannot unwrap positive range"), @@ -74,17 +74,17 @@ impl Term { } /// Set operations with terms. -impl Term { +impl, V: Version> Term { /// Compute the intersection of two terms. /// If at least one term is positive, the intersection is also positive. - pub(crate) fn intersection(&self, other: &Term) -> Term { + pub(crate) fn intersection(&self, other: &Term) -> Term { match (self, other) { (Self::Positive(r1), Self::Positive(r2)) => Self::Positive(r1.intersection(r2)), (Self::Positive(r1), Self::Negative(r2)) => { - Self::Positive(r1.intersection(&r2.negate())) + Self::Positive(r1.intersection(&r2.complement())) } (Self::Negative(r1), Self::Positive(r2)) => { - Self::Positive(r1.negate().intersection(r2)) + Self::Positive(r1.complement().intersection(r2)) } (Self::Negative(r1), Self::Negative(r2)) => Self::Negative(r1.union(r2)), } @@ -92,14 +92,14 @@ impl Term { /// Compute the union of two terms. /// If at least one term is negative, the union is also negative. - pub(crate) fn union(&self, other: &Term) -> Term { + pub(crate) fn union(&self, other: &Term) -> Term { (self.negate().intersection(&other.negate())).negate() } /// Indicate if this term is a subset of another term. /// Just like for sets, we say that t1 is a subset of t2 /// if and only if t1 ∩ t2 = t1. - pub(crate) fn subset_of(&self, other: &Term) -> bool { + pub(crate) fn subset_of(&self, other: &Term) -> bool { self == &self.intersection(other) } } @@ -120,7 +120,7 @@ pub(crate) enum Relation { } /// Relation between terms. -impl<'a, V: 'a + Version> Term { +impl<'a, I: Interval, V: 'a + Version> Term { /// Check if a set of terms satisfies this term. /// /// We say that a set of terms S "satisfies" a term t @@ -129,7 +129,7 @@ impl<'a, V: 'a + Version> Term { /// It turns out that this can also be expressed with set operations: /// S satisfies t if and only if ⋂ S ⊆ t #[cfg(test)] - fn satisfied_by(&self, terms_intersection: &Term) -> bool { + fn satisfied_by(&self, terms_intersection: &Term) -> bool { terms_intersection.subset_of(self) } @@ -142,13 +142,13 @@ impl<'a, V: 'a + Version> Term { /// S contradicts t if and only if ⋂ S is disjoint with t /// S contradicts t if and only if (⋂ S) ⋂ t = ∅ #[cfg(test)] - fn contradicted_by(&self, terms_intersection: &Term) -> bool { + fn contradicted_by(&self, terms_intersection: &Term) -> bool { terms_intersection.intersection(self) == Self::empty() } /// Check if a set of terms satisfies or contradicts a given term. /// Otherwise the relation is inconclusive. - pub(crate) fn relation_with(&self, other_terms_intersection: &Term) -> Relation { + pub(crate) fn relation_with(&self, other_terms_intersection: &Term) -> Relation { let full_intersection = self.intersection(other_terms_intersection); if &full_intersection == other_terms_intersection { Relation::Satisfied @@ -160,15 +160,15 @@ impl<'a, V: 'a + Version> Term { } } -impl AsRef> for Term { - fn as_ref(&self) -> &Term { - &self +impl, V: Version> AsRef> for Term { + fn as_ref(&self) -> &Term { + self } } // REPORT ###################################################################### -impl fmt::Display for Term { +impl, V: Version + fmt::Display> fmt::Display for Term { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { Self::Positive(range) => write!(f, "{}", range), @@ -182,13 +182,13 @@ impl fmt::Display for Term { #[cfg(test)] pub mod tests { use super::*; - use crate::version::NumberVersion; + use crate::version_trait::{NumberInterval, NumberVersion}; use proptest::prelude::*; - pub fn strategy() -> impl Strategy> { + pub fn strategy() -> impl Strategy> { prop_oneof![ - crate::range::tests::strategy().prop_map(Term::Positive), - crate::range::tests::strategy().prop_map(Term::Negative), + crate::range_trait::tests::strategy().prop_map(Term::Positive), + crate::range_trait::tests::strategy().prop_map(Term::Negative), ] } diff --git a/src/version_trait.rs b/src/version_trait.rs new file mode 100644 index 00000000..a33a398c --- /dev/null +++ b/src/version_trait.rs @@ -0,0 +1,390 @@ +// SPDX-License-Identifier: MPL-2.0 + +//! Traits and implementations to create and compare versions. + +use std::fmt::{self, Debug, Display}; +use std::ops::{Bound, RangeBounds}; +use std::str::FromStr; +use thiserror::Error; + +/// New trait for versions. +pub trait Version: Clone + Ord + Debug + Display { + /// Returns the minimum version. + fn minimum() -> Bound; + /// Returns the maximum version. + fn maximum() -> Bound; +} + +/// An interval is a bounded domain containing all values +/// between its starting and ending bounds. +pub trait Interval: RangeBounds + Debug + Clone + Eq + PartialEq { + /// Create an interval from its starting and ending bounds. + /// It's the caller responsability to order them correctly. + fn new(start_bound: Bound, end_bound: Bound) -> Self; +} + +// SemanticVersion ############################################################# + +/// Type for semantic versions: major.minor.patch. +#[derive(Debug, Copy, Clone, Ord, PartialOrd, Eq, PartialEq, Hash)] +pub struct SemanticVersion { + major: u32, + minor: u32, + patch: u32, +} + +#[cfg(feature = "serde")] +impl serde::Serialize for SemanticVersion { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + serializer.serialize_str(&format!("{}", self)) + } +} + +#[cfg(feature = "serde")] +impl<'de> serde::Deserialize<'de> for SemanticVersion { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + let s = String::deserialize(deserializer)?; + FromStr::from_str(&s).map_err(serde::de::Error::custom) + } +} + +// Constructors +impl SemanticVersion { + /// Create a version with "major", "minor" and "patch" values. + /// `version = major.minor.patch` + pub fn new(major: u32, minor: u32, patch: u32) -> Self { + Self { + major, + minor, + patch, + } + } + + /// Version 0.0.0. + pub fn zero() -> Self { + Self::new(0, 0, 0) + } + + /// Version 1.0.0. + pub fn one() -> Self { + Self::new(1, 0, 0) + } + + /// Version 2.0.0. + pub fn two() -> Self { + Self::new(2, 0, 0) + } +} + +// Convert a tuple (major, minor, patch) into a version. +impl From<(u32, u32, u32)> for SemanticVersion { + fn from(tuple: (u32, u32, u32)) -> Self { + let (major, minor, patch) = tuple; + Self::new(major, minor, patch) + } +} + +// Convert a version into a tuple (major, minor, patch). +impl From for (u32, u32, u32) { + fn from(v: SemanticVersion) -> Self { + (v.major, v.minor, v.patch) + } +} + +// Bump versions. +impl SemanticVersion { + /// Bump the patch number of a version. + pub fn bump_patch(self) -> Self { + Self::new(self.major, self.minor, self.patch + 1) + } + + /// Bump the minor number of a version. + pub fn bump_minor(self) -> Self { + Self::new(self.major, self.minor + 1, 0) + } + + /// Bump the major number of a version. + pub fn bump_major(self) -> Self { + Self::new(self.major + 1, 0, 0) + } +} + +/// Error creating [SemanticVersion] from [String]. +#[derive(Error, Debug, PartialEq)] +pub enum VersionParseError { + /// [SemanticVersion] must contain major, minor, patch versions. + #[error("version {full_version} must contain 3 numbers separated by dot")] + NotThreeParts { + /// [SemanticVersion] that was being parsed. + full_version: String, + }, + /// Wrapper around [ParseIntError](core::num::ParseIntError). + #[error("cannot parse '{version_part}' in '{full_version}' as u32: {parse_error}")] + ParseIntError { + /// [SemanticVersion] that was being parsed. + full_version: String, + /// A version part where parsing failed. + version_part: String, + /// A specific error resulted from parsing a part of the version as [u32]. + parse_error: String, + }, +} + +impl FromStr for SemanticVersion { + type Err = VersionParseError; + + fn from_str(s: &str) -> Result { + let parse_u32 = |part: &str| { + part.parse::().map_err(|e| Self::Err::ParseIntError { + full_version: s.to_string(), + version_part: part.to_string(), + parse_error: e.to_string(), + }) + }; + + let mut parts = s.split('.'); + match (parts.next(), parts.next(), parts.next(), parts.next()) { + (Some(major), Some(minor), Some(patch), None) => { + let major = parse_u32(major)?; + let minor = parse_u32(minor)?; + let patch = parse_u32(patch)?; + Ok(Self { + major, + minor, + patch, + }) + } + _ => Err(Self::Err::NotThreeParts { + full_version: s.to_string(), + }), + } + } +} + +#[test] +fn from_str_for_semantic_version() { + let parse = |str: &str| str.parse::(); + assert!(parse( + &SemanticVersion { + major: 0, + minor: 1, + patch: 0 + } + .to_string() + ) + .is_ok()); + assert!(parse("1.2.3").is_ok()); + assert_eq!( + parse("1.abc.3"), + Err(VersionParseError::ParseIntError { + full_version: "1.abc.3".to_owned(), + version_part: "abc".to_owned(), + parse_error: "invalid digit found in string".to_owned(), + }) + ); + assert_eq!( + parse("1.2.-3"), + Err(VersionParseError::ParseIntError { + full_version: "1.2.-3".to_owned(), + version_part: "-3".to_owned(), + parse_error: "invalid digit found in string".to_owned(), + }) + ); + assert_eq!( + parse("1.2.9876543210"), + Err(VersionParseError::ParseIntError { + full_version: "1.2.9876543210".to_owned(), + version_part: "9876543210".to_owned(), + parse_error: "number too large to fit in target type".to_owned(), + }) + ); + assert_eq!( + parse("1.2"), + Err(VersionParseError::NotThreeParts { + full_version: "1.2".to_owned(), + }) + ); + assert_eq!( + parse("1.2.3."), + Err(VersionParseError::NotThreeParts { + full_version: "1.2.3.".to_owned(), + }) + ); +} + +impl Display for SemanticVersion { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}.{}.{}", self.major, self.minor, self.patch) + } +} + +// Implement Version for SemanticVersion. +impl Version for SemanticVersion { + fn minimum() -> Bound { + Bound::Included(Self::zero()) + } + fn maximum() -> Bound { + Bound::Unbounded + } +} + +// SemanticInterval ############################################################ + +/// Interval for semantic versions. +#[derive(Debug, Clone, Eq, PartialEq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub struct SemanticInterval { + start: Bound, + end: Bound, +} + +impl RangeBounds for SemanticInterval { + fn start_bound(&self) -> Bound<&SemanticVersion> { + ref_bound(&self.start) + } + fn end_bound(&self) -> Bound<&SemanticVersion> { + ref_bound(&self.end) + } +} + +impl Interval for SemanticInterval { + fn new(start_bound: Bound, end_bound: Bound) -> Self { + let start = match &start_bound { + Bound::Unbounded => Bound::Included(SemanticVersion::zero()), + _ => start_bound, + }; + Self { + start, + end: end_bound, + } + } +} + +// Ease the creation of SemanticInterval from classic ranges: (2, 0, 0)..(2, 5, 0) +impl> From for SemanticInterval { + fn from(range: RB) -> Self { + Self::new( + map_bound(SemanticVersion::from, owned_bound(range.start_bound())), + map_bound(SemanticVersion::from, owned_bound(range.end_bound())), + ) + } +} + +// NumberVersion ############################################################### + +/// Simplest versions possible, just a positive number. +#[derive(Debug, Copy, Clone, Ord, PartialOrd, Eq, PartialEq, Hash)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize,))] +#[cfg_attr(feature = "serde", serde(transparent))] +pub struct NumberVersion(pub u32); + +// Convert an usize into a version. +impl From for NumberVersion { + fn from(v: u32) -> Self { + Self(v) + } +} + +// Convert a version into an usize. +impl From for u32 { + fn from(version: NumberVersion) -> Self { + version.0 + } +} + +impl Display for NumberVersion { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.0) + } +} + +impl Version for NumberVersion { + fn minimum() -> Bound { + Bound::Included(NumberVersion(0)) + } + fn maximum() -> Bound { + Bound::Unbounded + } +} + +// NumberInterval ############################################################## + +/// Interval for number versions. +#[derive(Debug, Clone, Eq, PartialEq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub struct NumberInterval { + start: Bound, + end: Bound, +} + +// Ease the creation of NumberInterval from classic ranges: 0..10 +impl> From for NumberInterval { + fn from(range: RB) -> Self { + Self::new( + map_bound(NumberVersion::from, owned_bound(range.start_bound())), + map_bound(NumberVersion::from, owned_bound(range.end_bound())), + ) + } +} + +impl RangeBounds for NumberInterval { + fn start_bound(&self) -> Bound<&NumberVersion> { + ref_bound(&self.start) + } + fn end_bound(&self) -> Bound<&NumberVersion> { + ref_bound(&self.end) + } +} + +impl Interval for NumberInterval { + fn new(start_bound: Bound, end_bound: Bound) -> Self { + Self { + start: start_bound, + end: end_bound, + } + } +} + +// Helper ################################################################## + +/// Map function inside a Bound. +pub fn map_bound T>(f: F, bound: Bound) -> Bound { + match bound { + Bound::Unbounded => Bound::Unbounded, + Bound::Excluded(b) => Bound::Included(f(b)), + Bound::Included(b) => Bound::Excluded(f(b)), + } +} + +/// Flip a bound by exchanging Excluded and Included. +pub fn flip_bound(bound: Bound) -> Bound { + match bound { + Bound::Unbounded => Bound::Unbounded, + Bound::Excluded(b) => Bound::Included(b), + Bound::Included(b) => Bound::Excluded(b), + } +} + +/// Will be obsolete when .as_ref() hits stable. +pub fn ref_bound(bound: &Bound) -> Bound<&V> { + match *bound { + Bound::Included(ref x) => Bound::Included(x), + Bound::Excluded(ref x) => Bound::Excluded(x), + Bound::Unbounded => Bound::Unbounded, + } +} + +/// Will be obsolete when .cloned() hits stable. +pub fn owned_bound(bound: Bound<&V>) -> Bound { + match bound { + Bound::Unbounded => Bound::Unbounded, + Bound::Included(x) => Bound::Included(x.clone()), + Bound::Excluded(x) => Bound::Excluded(x.clone()), + } +} diff --git a/tests/examples.rs b/tests/examples.rs index 29901caf..43bea58c 100644 --- a/tests/examples.rs +++ b/tests/examples.rs @@ -1,14 +1,27 @@ // SPDX-License-Identifier: MPL-2.0 -use pubgrub::range::Range; +use pubgrub::range_trait::Range; use pubgrub::solver::{resolve, OfflineDependencyProvider}; use pubgrub::type_aliases::Map; -use pubgrub::version::{NumberVersion, SemanticVersion}; +use pubgrub::version_trait::{NumberInterval, NumberVersion, SemanticInterval, SemanticVersion}; + +use log::LevelFilter; +use std::io::Write; + +fn init_log() { + let _ = env_logger::builder() + .filter_level(LevelFilter::Trace) + .format(|buf, record| writeln!(buf, "{}", record.args())) + .is_test(true) + .try_init(); +} #[test] /// https://github.com/dart-lang/pub/blob/master/doc/solver.md#no-conflicts fn no_conflict() { - let mut dependency_provider = OfflineDependencyProvider::<&str, SemanticVersion>::new(); + init_log(); + let mut dependency_provider = + OfflineDependencyProvider::<&str, SemanticInterval, SemanticVersion>::new(); #[rustfmt::skip] dependency_provider.add_dependencies( "root", (1, 0, 0), @@ -38,7 +51,9 @@ fn no_conflict() { #[test] /// https://github.com/dart-lang/pub/blob/master/doc/solver.md#avoiding-conflict-during-decision-making fn avoiding_conflict_during_decision_making() { - let mut dependency_provider = OfflineDependencyProvider::<&str, SemanticVersion>::new(); + init_log(); + let mut dependency_provider = + OfflineDependencyProvider::<&str, SemanticInterval, SemanticVersion>::new(); #[rustfmt::skip] dependency_provider.add_dependencies( "root", (1, 0, 0), @@ -73,7 +88,9 @@ fn avoiding_conflict_during_decision_making() { #[test] /// https://github.com/dart-lang/pub/blob/master/doc/solver.md#performing-conflict-resolution fn conflict_resolution() { - let mut dependency_provider = OfflineDependencyProvider::<&str, SemanticVersion>::new(); + init_log(); + let mut dependency_provider = + OfflineDependencyProvider::<&str, SemanticInterval, SemanticVersion>::new(); #[rustfmt::skip] dependency_provider.add_dependencies( "root", (1, 0, 0), @@ -106,7 +123,9 @@ fn conflict_resolution() { #[test] /// https://github.com/dart-lang/pub/blob/master/doc/solver.md#conflict-resolution-with-a-partial-satisfier fn conflict_with_partial_satisfier() { - let mut dependency_provider = OfflineDependencyProvider::<&str, SemanticVersion>::new(); + init_log(); + let mut dependency_provider = + OfflineDependencyProvider::<&str, SemanticInterval, SemanticVersion>::new(); #[rustfmt::skip] // root 1.0.0 depends on foo ^1.0.0 and target ^2.0.0 dependency_provider.add_dependencies( @@ -171,12 +190,14 @@ fn conflict_with_partial_satisfier() { /// /// Solution: a0, b0, c0, d0 fn double_choices() { - let mut dependency_provider = OfflineDependencyProvider::<&str, NumberVersion>::new(); - dependency_provider.add_dependencies("a", 0, vec![("b", Range::any()), ("c", Range::any())]); - dependency_provider.add_dependencies("b", 0, vec![("d", Range::exact(0))]); - dependency_provider.add_dependencies("b", 1, vec![("d", Range::exact(1))]); + init_log(); + let mut dependency_provider = + OfflineDependencyProvider::<&str, NumberInterval, NumberVersion>::new(); + dependency_provider.add_dependencies("a", 0, vec![("b", Range::full()), ("c", Range::full())]); + dependency_provider.add_dependencies("b", 0, vec![("d", Range::singleton(0))]); + dependency_provider.add_dependencies("b", 1, vec![("d", Range::singleton(1))]); dependency_provider.add_dependencies("c", 0, vec![]); - dependency_provider.add_dependencies("c", 1, vec![("d", Range::exact(2))]); + dependency_provider.add_dependencies("c", 1, vec![("d", Range::singleton(2))]); dependency_provider.add_dependencies("d", 0, vec![]); // Solution. diff --git a/tests/proptest.rs b/tests/proptest.rs index d2da3730..92dd7582 100644 --- a/tests/proptest.rs +++ b/tests/proptest.rs @@ -4,13 +4,13 @@ use std::{collections::BTreeSet as Set, error::Error}; use pubgrub::error::PubGrubError; use pubgrub::package::Package; -use pubgrub::range::Range; +use pubgrub::range_trait::Range; use pubgrub::report::{DefaultStringReporter, Reporter}; use pubgrub::solver::{ choose_package_with_fewest_versions, resolve, Dependencies, DependencyProvider, OfflineDependencyProvider, }; -use pubgrub::version::{NumberVersion, Version}; +use pubgrub::version_trait::{Interval, NumberInterval, NumberVersion, Version}; use proptest::collection::{btree_map, vec}; use proptest::prelude::*; @@ -24,10 +24,14 @@ mod sat_dependency_provider; /// The same as [OfflineDependencyProvider] but takes versions from the opposite end: /// if [OfflineDependencyProvider] returns versions from newest to oldest, this returns them from oldest to newest. #[derive(Clone)] -struct OldestVersionsDependencyProvider(OfflineDependencyProvider); +struct OldestVersionsDependencyProvider, V: Version>( + OfflineDependencyProvider, +); -impl DependencyProvider for OldestVersionsDependencyProvider { - fn choose_package_version, U: std::borrow::Borrow>>( +impl, V: Version> DependencyProvider + for OldestVersionsDependencyProvider +{ + fn choose_package_version, U: std::borrow::Borrow>>( &self, potential_packages: impl Iterator, ) -> Result<(T, Option), Box> { @@ -37,7 +41,7 @@ impl DependencyProvider for OldestVersionsDependen )) } - fn get_dependencies(&self, p: &P, v: &V) -> Result, Box> { + fn get_dependencies(&self, p: &P, v: &V) -> Result, Box> { self.0.get_dependencies(p, v) } } @@ -62,17 +66,17 @@ impl TimeoutDependencyProvider { } } -impl> DependencyProvider - for TimeoutDependencyProvider +impl, V: Version, DP: DependencyProvider> + DependencyProvider for TimeoutDependencyProvider { - fn choose_package_version, U: std::borrow::Borrow>>( + 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, v: &V) -> Result, Box> { + fn get_dependencies(&self, p: &P, v: &V) -> Result, Box> { self.dp.get_dependencies(p, v) } @@ -88,8 +92,9 @@ impl> DependencyProvider::new(); - dependency_provider.add_dependencies(0, 0, vec![(666, Range::any())]); + let mut dependency_provider = + OfflineDependencyProvider::<_, NumberInterval, NumberVersion>::new(); + dependency_provider.add_dependencies(0, 0, vec![(666, Range::full())]); // Run the algorithm. let _ = resolve( @@ -118,7 +123,7 @@ pub fn registry_strategy( bad_name: N, ) -> impl Strategy< Value = ( - OfflineDependencyProvider, + OfflineDependencyProvider, Vec<(N, NumberVersion)>, ), > { @@ -168,7 +173,7 @@ pub fn registry_strategy( move |(crate_vers_by_name, raw_dependencies, reverse_alphabetical, complicated_len)| { let mut list_of_pkgid: Vec<( (N, NumberVersion), - Option)>>, + Option)>>, )> = crate_vers_by_name .iter() .flat_map(|(name, vers)| { @@ -196,13 +201,13 @@ pub fn registry_strategy( deps.push(( dep_name, if c == 0 && d == s_last_index { - Range::any() + Range::full() } else if c == 0 { Range::strictly_lower_than(s[d].0 + 1) } else if d == s_last_index { Range::higher_than(s[c].0) } else if c == d { - Range::exact(s[c].0) + Range::singleton(s[c].0) } else { Range::between(s[c].0, s[d].0 + 1) }, @@ -210,7 +215,8 @@ pub fn registry_strategy( } } - let mut dependency_provider = OfflineDependencyProvider::::new(); + let mut dependency_provider = + OfflineDependencyProvider::::new(); let complicated_len = std::cmp::min(complicated_len, list_of_pkgid.len()); let complicated: Vec<_> = if reverse_alphabetical { @@ -226,7 +232,7 @@ pub fn registry_strategy( dependency_provider.add_dependencies( name, ver, - deps.unwrap_or_else(|| vec![(bad_name.clone(), Range::any())]), + deps.unwrap_or_else(|| vec![(bad_name.clone(), Range::full())]), ); } @@ -373,7 +379,7 @@ proptest! { .versions(package) .unwrap().collect(); let version = version_idx.get(&versions); - let dependencies: Vec<(u16, Range)> = match dependency_provider + let dependencies: Vec<(u16, Range)> = match dependency_provider .get_dependencies(package, version) .unwrap() { @@ -432,7 +438,7 @@ proptest! { Ok(used) => { // If resolution was successful, then unpublishing a version of a crate // that was not selected should not change that. - let mut smaller_dependency_provider = OfflineDependencyProvider::<_, NumberVersion>::new(); + let mut smaller_dependency_provider = OfflineDependencyProvider::<_,NumberInterval, NumberVersion>::new(); for &(n, v) in &all_versions { if used.get(&n) == Some(&v) // it was used || to_remove.get(&(n, v)).is_none() // or it is not one to be removed @@ -455,7 +461,7 @@ proptest! { Err(_) => { // If resolution was unsuccessful, then it should stay unsuccessful // even if any version of a crate is unpublished. - let mut smaller_dependency_provider = OfflineDependencyProvider::<_, NumberVersion>::new(); + let mut smaller_dependency_provider = OfflineDependencyProvider::<_, NumberInterval, NumberVersion>::new(); for &(n, v) in &all_versions { if to_remove.get(&(n, v)).is_none() // it is not one to be removed { diff --git a/tests/sat_dependency_provider.rs b/tests/sat_dependency_provider.rs index fa964ca8..6e2da979 100644 --- a/tests/sat_dependency_provider.rs +++ b/tests/sat_dependency_provider.rs @@ -1,9 +1,11 @@ // SPDX-License-Identifier: MPL-2.0 +use std::marker::PhantomData; + use pubgrub::package::Package; use pubgrub::solver::{Dependencies, DependencyProvider, OfflineDependencyProvider}; use pubgrub::type_aliases::{Map, SelectedDependencies}; -use pubgrub::version::Version; +use pubgrub::version_trait::{Interval, Version}; use varisat::ExtendFormula; const fn num_bits() -> usize { @@ -46,13 +48,14 @@ fn sat_at_most_one(solver: &mut impl varisat::ExtendFormula, vars: &[varisat::Va /// /// The SAT library does not optimize for the newer version, /// so the selected packages may not match the real resolver. -pub struct SatResolve { +pub struct SatResolve, V: Version> { solver: varisat::Solver<'static>, all_versions_by_p: Map>, + phantom: PhantomData, } -impl SatResolve { - pub fn new(dp: &OfflineDependencyProvider) -> Self { +impl, V: Version> SatResolve { + pub fn new(dp: &OfflineDependencyProvider) -> Self { let mut cnf = varisat::CnfFormula::new(); let mut all_versions = vec![]; @@ -107,6 +110,7 @@ impl SatResolve { Self { solver, all_versions_by_p, + phantom: PhantomData, } } diff --git a/tests/tests.rs b/tests/tests.rs index 7aeed03b..d7d498d9 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -1,20 +1,21 @@ // SPDX-License-Identifier: MPL-2.0 use pubgrub::error::PubGrubError; -use pubgrub::range::Range; +use pubgrub::range_trait::Range; use pubgrub::solver::{resolve, OfflineDependencyProvider}; -use pubgrub::version::NumberVersion; +use pubgrub::version_trait::{NumberInterval, NumberVersion}; #[test] fn same_result_on_repeated_runs() { - let mut dependency_provider = OfflineDependencyProvider::<_, NumberVersion>::new(); + let mut dependency_provider = + OfflineDependencyProvider::<_, NumberInterval, NumberVersion>::new(); dependency_provider.add_dependencies("c", 0, vec![]); dependency_provider.add_dependencies("c", 2, vec![]); dependency_provider.add_dependencies("b", 0, vec![]); dependency_provider.add_dependencies("b", 1, vec![("c", Range::between(0, 1))]); - dependency_provider.add_dependencies("a", 0, vec![("b", Range::any()), ("c", Range::any())]); + dependency_provider.add_dependencies("a", 0, vec![("b", Range::full()), ("c", Range::full())]); let name = "a"; let ver = NumberVersion(0); @@ -29,14 +30,15 @@ fn same_result_on_repeated_runs() { #[test] fn should_always_find_a_satisfier() { - let mut dependency_provider = OfflineDependencyProvider::<_, NumberVersion>::new(); - dependency_provider.add_dependencies("a", 0, vec![("b", Range::none())]); + let mut dependency_provider = + OfflineDependencyProvider::<_, NumberInterval, NumberVersion>::new(); + dependency_provider.add_dependencies("a", 0, vec![("b", Range::empty())]); assert!(matches!( resolve(&dependency_provider, "a", 0), Err(PubGrubError::DependencyOnTheEmptySet { .. }) )); - dependency_provider.add_dependencies("c", 0, vec![("a", Range::any())]); + dependency_provider.add_dependencies("c", 0, vec![("a", Range::full())]); assert!(matches!( resolve(&dependency_provider, "c", 0), Err(PubGrubError::DependencyOnTheEmptySet { .. }) @@ -45,8 +47,9 @@ fn should_always_find_a_satisfier() { #[test] fn cannot_depend_on_self() { - let mut dependency_provider = OfflineDependencyProvider::<_, NumberVersion>::new(); - dependency_provider.add_dependencies("a", 0, vec![("a", Range::any())]); + let mut dependency_provider = + OfflineDependencyProvider::<_, NumberInterval, NumberVersion>::new(); + dependency_provider.add_dependencies("a", 0, vec![("a", Range::full())]); assert!(matches!( resolve(&dependency_provider, "a", 0), Err(PubGrubError::SelfDependency { .. })