diff --git a/benches/backtracking.rs b/benches/backtracking.rs index c7aedaf5..8d84ce73 100644 --- a/benches/backtracking.rs +++ b/benches/backtracking.rs @@ -26,7 +26,7 @@ fn backtracking_singletons(c: &mut Criterion, package_count: u32, version_count: c.bench_function("backtracking_singletons", |b| { b.iter(|| { - let _ = pubgrub::resolve(&mut dependency_provider, 0u32, 0u32); + let _ = dependency_provider.resolve(0u32, 0u32); }) }); } @@ -59,7 +59,7 @@ fn backtracking_disjoint_versions(c: &mut Criterion, package_count: u32, version c.bench_function("backtracking_disjoint_versions", |b| { b.iter(|| { - let _ = pubgrub::resolve(&mut dependency_provider, 0u32, 0u32); + let _ = dependency_provider.resolve(0u32, 0u32); }) }); } @@ -83,7 +83,7 @@ fn backtracking_ranges(c: &mut Criterion, package_count: u32, version_count: u32 c.bench_function("backtracking_ranges", |b| { b.iter(|| { - let _ = pubgrub::resolve(&mut dependency_provider, 0u32, 0u32); + let _ = dependency_provider.resolve(0u32, 0u32); }) }); } diff --git a/benches/large_case.rs b/benches/large_case.rs index 2e59ce3b..2d47d321 100644 --- a/benches/large_case.rs +++ b/benches/large_case.rs @@ -7,20 +7,19 @@ use std::time::Duration; use criterion::*; use serde::de::Deserialize; -use pubgrub::{resolve, Map, OfflineDependencyProvider, Range, SemanticVersion, VersionSet}; +use pubgrub::{Map, OfflineDependencyProvider, Range, VersionRanges}; fn bench< 'a, P: Debug + Display + Clone + Eq + Hash + Deserialize<'a>, - VS: VersionSet + Deserialize<'a>, + R: VersionRanges + Deserialize<'a>, >( b: &mut Bencher, case: &'a str, ) where - ::V: Deserialize<'a>, + R::V: Deserialize<'a>, { - let mut dependency_provider: OfflineDependencyProvider = - ron::de::from_str(case).unwrap(); + let mut dependency_provider: OfflineDependencyProvider = ron::de::from_str(case).unwrap(); let dependencies = dependency_provider .packages() @@ -34,8 +33,8 @@ fn bench< b.iter(|| { for (p, versions) in &dependencies { - for n in versions { - let _ = resolve(&mut dependency_provider, p.clone(), n.clone()); + for v in versions { + let _ = dependency_provider.resolve(p.clone(), v.clone()); } } }); @@ -49,14 +48,10 @@ fn bench_nested(c: &mut Criterion) { let case = case.unwrap().path(); let name = case.file_name().unwrap().to_string_lossy(); let data = std::fs::read_to_string(&case).unwrap(); - if name.ends_with("u16_NumberVersion.ron") || name.ends_with("u16_u32.ron") { + if name.ends_with("u16_NumberVersion.ron") { group.bench_function(name, |b| { bench::>(b, &data); }); - } else if name.ends_with("str_SemanticVersion.ron") { - group.bench_function(name, |b| { - bench::<&str, Range>(b, &data); - }); } } diff --git a/benches/sudoku.rs b/benches/sudoku.rs index 9f2e175c..2ebe9e5a 100644 --- a/benches/sudoku.rs +++ b/benches/sudoku.rs @@ -4,8 +4,7 @@ // SPDX-License-Identifier: MPL-2.0 use pubgrub::{ - resolve, DefaultStringReporter, OfflineDependencyProvider, PubGrubError, Range, Reporter, - SelectedDependencies, + DefaultStringReporter, Map, OfflineDependencyProvider, PubGrubError, Range, Reporter, }; use std::fmt; use std::sync::Arc; @@ -119,19 +118,18 @@ fn encode_constraints( } } -fn solve(board: Vec<(SudokuPackage, Ranges>)>) -> SelectedDependencies { +fn solve(board: Vec<(SudokuPackage, Ranges>)>) -> Map> { let mut dependency_provider = DP::new(); encode_constraints(&mut dependency_provider); dependency_provider.add_dependencies(SudokuPackage::Root, Arc::new(1usize), board); - match resolve( - &mut dependency_provider, - SudokuPackage::Root, - Arc::new(1usize), - ) { + match dependency_provider.resolve(SudokuPackage::Root, Arc::new(1usize)) { Ok(sol) => sol, Err(PubGrubError::NoSolution(mut error)) => { error.derivation_tree.collapse_no_versions(); - eprintln!("{}", DefaultStringReporter::report(&error)); + eprintln!( + "{}", + DefaultStringReporter::report(&error, &dependency_provider) + ); std::process::exit(1); } Err(err) => panic!("{:?}", err), diff --git a/examples/branching_error_reporting.rs b/examples/branching_error_reporting.rs index 811b613e..fda327fe 100644 --- a/examples/branching_error_reporting.rs +++ b/examples/branching_error_reporting.rs @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MPL-2.0 use pubgrub::{ - resolve, DefaultStringReporter, OfflineDependencyProvider, PubGrubError, Ranges, Reporter, + DefaultStringReporter, OfflineDependencyProvider, PubGrubError, Ranges, Reporter, SemanticVersion, }; @@ -10,15 +10,16 @@ type SemVS = Ranges; // https://github.com/dart-lang/pub/blob/master/doc/solver.md#branching-error-reporting fn main() { let mut dependency_provider = OfflineDependencyProvider::<&str, SemVS>::new(); + #[rustfmt::skip] // root 1.0.0 depends on foo ^1.0.0 - dependency_provider.add_dependencies( + dependency_provider.add_dependencies( "root", (1, 0, 0), [("foo", Ranges::from_range_bounds((1, 0, 0)..(2, 0, 0)))], ); #[rustfmt::skip] // foo 1.0.0 depends on a ^1.0.0 and b ^1.0.0 - dependency_provider.add_dependencies( + dependency_provider.add_dependencies( "foo", (1, 0, 0), [ ("a", Ranges::from_range_bounds((1, 0, 0)..(2, 0, 0))), @@ -27,7 +28,7 @@ fn main() { ); #[rustfmt::skip] // foo 1.1.0 depends on x ^1.0.0 and y ^1.0.0 - dependency_provider.add_dependencies( + dependency_provider.add_dependencies( "foo", (1, 1, 0), [ ("x", Ranges::from_range_bounds((1, 0, 0)..(2, 0, 0))), @@ -36,7 +37,7 @@ fn main() { ); #[rustfmt::skip] // a 1.0.0 depends on b ^2.0.0 - dependency_provider.add_dependencies( + dependency_provider.add_dependencies( "a", (1, 0, 0), [("b", Ranges::from_range_bounds((2, 0, 0)..(3, 0, 0)))], ); @@ -45,7 +46,7 @@ fn main() { dependency_provider.add_dependencies("b", (2, 0, 0), []); #[rustfmt::skip] // x 1.0.0 depends on y ^2.0.0. - dependency_provider.add_dependencies( + dependency_provider.add_dependencies( "x", (1, 0, 0), [("y", Ranges::from_range_bounds((2, 0, 0)..(3, 0, 0)))], ); @@ -54,11 +55,14 @@ fn main() { dependency_provider.add_dependencies("y", (2, 0, 0), []); // Run the algorithm. - match resolve(&mut dependency_provider, "root", (1, 0, 0)) { + match dependency_provider.resolve("root", (1, 0, 0)) { Ok(sol) => println!("{:?}", sol), Err(PubGrubError::NoSolution(mut error)) => { error.derivation_tree.collapse_no_versions(); - eprintln!("{}", DefaultStringReporter::report(&error)); + eprintln!( + "{}", + DefaultStringReporter::report(&error, &dependency_provider) + ); std::process::exit(1); } Err(err) => panic!("{:?}", err), diff --git a/examples/caching_dependency_provider.rs b/examples/caching_dependency_provider.rs index 555b78aa..033f2701 100644 --- a/examples/caching_dependency_provider.rs +++ b/examples/caching_dependency_provider.rs @@ -1,65 +1,106 @@ // SPDX-License-Identifier: MPL-2.0 use std::cell::RefCell; -use std::collections::BTreeMap; use std::fmt::{Debug, Display}; use std::hash::Hash; use pubgrub::{ resolve, Dependencies, DependencyConstraints, DependencyProvider, Map, - OfflineDependencyProvider, PackageArena, PackageId, Ranges, + OfflineDependencyProvider, PackageArena, PackageId, PackageVersionWrapper, PubGrubError, + Ranges, SelectedDependencies, VersionIndex, VersionRanges, VersionSet, }; type NumVS = Ranges; -type CachedDeps = RefCell>>>; + +trait RemoteProvider: DependencyProvider

> { + type Pkg: Debug + Display + Clone + Eq + Hash; + type R: VersionRanges; + + fn resolve_parameters( + &self, + p: Self::Pkg, + v: impl Into<::V>, + ) -> Option<(PackageVersionWrapper, VersionIndex)>; +} + +impl RemoteProvider + for OfflineDependencyProvider +{ + type Pkg = P; + type R = R; + + fn resolve_parameters( + &self, + p: P, + v: impl Into<::V>, + ) -> Option<(PackageVersionWrapper

, VersionIndex)> { + self.resolve_parameters(p, v) + } +} // 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, R: VersionRanges> where DP::P: Debug + Display + Clone + Eq + Hash, { remote_dependencies: DP, - cached_dependencies: CachedDeps, + cached_dependencies: RefCell>>, } -impl CachingDependencyProvider +impl, R: VersionRanges> CachingDependencyProvider where DP::P: Debug + Display + Clone + Eq + Hash, { - pub fn new(remote_dependencies_provider: DP) -> Self { + fn new(remote_dependencies_provider: DP) -> Self { CachingDependencyProvider { remote_dependencies: remote_dependencies_provider, cached_dependencies: Default::default(), } } + + fn resolve( + &mut self, + p: ::Pkg, + v: impl Into, + ) -> Result, PubGrubError> { + let Some((p, v)) = self.remote_dependencies.resolve_parameters(p, v) else { + return Err(PubGrubError::NoRoot); + }; + resolve(self, p, v) + } } -impl> DependencyProvider for CachingDependencyProvider +impl, R: VersionRanges> DependencyProvider + for CachingDependencyProvider where DP::P: Debug + Display + Clone + Eq + Hash, + R::V: Clone, { // Cache dependencies if they were already queried fn get_dependencies( &mut self, package_id: PackageId, - version: &DP::V, + version_index: VersionIndex, package_store: &mut PackageArena, - ) -> Result, DP::Err> { + ) -> Result, DP::Err> { let mut cache = self.cached_dependencies.borrow_mut(); - if let Some(deps) = cache.get(&package_id).and_then(|vmap| vmap.get(version)) { + if let Some(deps) = cache + .get(&package_id) + .and_then(|vmap| vmap.get(&version_index)) + { return Ok(Dependencies::Available(deps.clone())); } match self .remote_dependencies - .get_dependencies(package_id, version, package_store) + .get_dependencies(package_id, version_index, package_store) { Ok(Dependencies::Available(deps)) => { cache .entry(package_id) .or_default() - .insert(version.clone(), deps.clone()); + .insert(version_index, deps.clone()); Ok(Dependencies::Available(deps)) } @@ -71,11 +112,11 @@ where fn choose_version( &mut self, package_id: PackageId, - ranges: &DP::VS, + set: VersionSet, package_store: &PackageArena, - ) -> Result, DP::Err> { + ) -> Result, DP::Err> { self.remote_dependencies - .choose_version(package_id, ranges, package_store) + .choose_version(package_id, set, package_store) } type Priority = DP::Priority; @@ -83,19 +124,35 @@ where fn prioritize( &mut self, package_id: PackageId, - ranges: &DP::VS, + set: VersionSet, package_store: &PackageArena, ) -> Self::Priority { self.remote_dependencies - .prioritize(package_id, ranges, package_store) + .prioritize(package_id, set, package_store) } type Err = DP::Err; type P = DP::P; - type V = DP::V; - type VS = DP::VS; type M = DP::M; + + fn package_version_display<'a>( + &'a self, + package: &'a Self::P, + version_index: VersionIndex, + ) -> impl Display + 'a { + self.remote_dependencies + .package_version_display(package, version_index) + } + + fn package_version_set_display<'a>( + &'a self, + package: &'a Self::P, + version_set: VersionSet, + ) -> impl Display + 'a { + self.remote_dependencies + .package_version_set_display(package, version_set) + } } fn main() { @@ -108,6 +165,6 @@ fn main() { let mut caching_dependencies_provider = CachingDependencyProvider::new(remote_dependencies_provider); - let solution = resolve(&mut caching_dependencies_provider, "root", 1u32); + let solution = caching_dependencies_provider.resolve("root", 1u32); println!("Solution: {:?}", solution); } diff --git a/examples/doc_interface.rs b/examples/doc_interface.rs index bcff3c16..b8c8e88c 100644 --- a/examples/doc_interface.rs +++ b/examples/doc_interface.rs @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MPL-2.0 -use pubgrub::{resolve, OfflineDependencyProvider, Ranges}; +use pubgrub::{OfflineDependencyProvider, Ranges}; type NumVS = Ranges; @@ -19,6 +19,6 @@ fn main() { dependency_provider.add_dependencies("icons", 1u32, []); // Run the algorithm. - let solution = resolve(&mut dependency_provider, "root", 1u32); - println!("Solution: {:?}", solution); + let solution = dependency_provider.resolve("root", 1u32).unwrap(); + println!("Solution: {solution:?}"); } diff --git a/examples/doc_interface_error.rs b/examples/doc_interface_error.rs index 794b3224..b5ee85f7 100644 --- a/examples/doc_interface_error.rs +++ b/examples/doc_interface_error.rs @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MPL-2.0 use pubgrub::{ - resolve, DefaultStringReporter, OfflineDependencyProvider, PubGrubError, Ranges, Reporter, + DefaultStringReporter, OfflineDependencyProvider, PubGrubError, Ranges, Reporter, SemanticVersion, }; @@ -71,11 +71,11 @@ fn main() { dependency_provider.add_dependencies("intl", (5, 0, 0), []); // Run the algorithm. - match resolve(&mut dependency_provider, "root", (1, 0, 0)) { - Ok(sol) => println!("{:?}", sol), + match dependency_provider.resolve("root", (1, 0, 0)) { + Ok(sol) => println!("Solution: {sol:?}"), Err(PubGrubError::NoSolution(mut error)) => { error.derivation_tree.collapse_no_versions(); - eprintln!("{}", DefaultStringReporter::report(&error)); + eprintln!("{}", DefaultStringReporter::report(&error, &dependency_provider)); } Err(err) => panic!("{:?}", err), }; diff --git a/examples/doc_interface_semantic.rs b/examples/doc_interface_semantic.rs index e579d7bd..74c6e4de 100644 --- a/examples/doc_interface_semantic.rs +++ b/examples/doc_interface_semantic.rs @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MPL-2.0 use pubgrub::{ - resolve, DefaultStringReporter, OfflineDependencyProvider, PubGrubError, Ranges, Reporter, + DefaultStringReporter, OfflineDependencyProvider, PubGrubError, Ranges, Reporter, SemanticVersion, }; @@ -62,11 +62,11 @@ fn main() { dependency_provider.add_dependencies("icons", (2, 0, 0), []); // Run the algorithm. - match resolve(&mut dependency_provider, "root", (1, 0, 0)) { - Ok(sol) => println!("{:?}", sol), + match dependency_provider.resolve("root", (1, 0, 0)) { + Ok(sol) => println!("Solution: {sol:?}"), Err(PubGrubError::NoSolution(mut error)) => { error.derivation_tree.collapse_no_versions(); - eprintln!("{}", DefaultStringReporter::report(&error)); + eprintln!("{}", DefaultStringReporter::report(&error, &dependency_provider)); } Err(err) => panic!("{:?}", err), }; diff --git a/examples/linear_error_reporting.rs b/examples/linear_error_reporting.rs index 005929e3..0144ef8b 100644 --- a/examples/linear_error_reporting.rs +++ b/examples/linear_error_reporting.rs @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MPL-2.0 use pubgrub::{ - resolve, DefaultStringReporter, OfflineDependencyProvider, PubGrubError, Ranges, Reporter, + DefaultStringReporter, OfflineDependencyProvider, PubGrubError, Ranges, Reporter, SemanticVersion, }; @@ -36,11 +36,14 @@ fn main() { dependency_provider.add_dependencies("baz", (3, 0, 0), []); // Run the algorithm. - match resolve(&mut dependency_provider, "root", (1, 0, 0)) { - Ok(sol) => println!("{:?}", sol), + match dependency_provider.resolve("root", (1, 0, 0)) { + Ok(sol) => println!("Solution: {sol:?}"), Err(PubGrubError::NoSolution(mut error)) => { error.derivation_tree.collapse_no_versions(); - eprintln!("{}", DefaultStringReporter::report(&error)); + eprintln!( + "{}", + DefaultStringReporter::report(&error, &dependency_provider) + ); std::process::exit(1); } Err(err) => panic!("{:?}", err), diff --git a/examples/many_versions.rs b/examples/many_versions.rs new file mode 100644 index 00000000..170ced3e --- /dev/null +++ b/examples/many_versions.rs @@ -0,0 +1,161 @@ +// SPDX-License-Identifier: MPL-2.0 + +use std::cmp::Reverse; +use std::convert::Infallible; +use std::fmt::Display; +use std::num::NonZeroU64; + +use pubgrub::{ + resolve, DefaultStringReporter, Dependencies, DependencyProvider, Map, PackageArena, PackageId, + PackageVersionWrapper, PubGrubError, Reporter, VersionIndex, VersionSet, +}; + +struct Provider { + version_counts: Map<&'static str, u64>, +} + +impl DependencyProvider for Provider { + type P = PackageVersionWrapper<&'static str>; + type M = &'static str; + type Priority = Reverse; + type Err = Infallible; + + fn prioritize( + &mut self, + _: PackageId, + set: VersionSet, + _: &PackageArena, + ) -> Self::Priority { + Reverse(set.count() as u64) + } + + fn choose_version( + &mut self, + _: PackageId, + set: VersionSet, + _: &PackageArena, + ) -> Result, Self::Err> { + Ok(set.last()) + } + + fn get_dependencies( + &mut self, + package: PackageId, + version_index: VersionIndex, + package_store: &mut PackageArena, + ) -> Result, Self::Err> { + let mut dep_map = Map::default(); + + let add_dep = |store: &mut PackageArena, dep_map: &mut Map<_, _>, (d, vs)| { + dep_map.insert(store.insert(d), vs); + }; + + let add_range_dep = + |store: &mut PackageArena, dep_map: &mut Map<_, _>, (dn, r, vc)| { + add_dep(store, dep_map, PackageVersionWrapper::new_dep(dn, r, vc)); + }; + + let add_singleton_dep = + |store: &mut PackageArena, dep_map: &mut Map<_, _>, (dn, v, vc)| { + let (d, vs) = PackageVersionWrapper::new_singleton_dep(dn, v, vc); + add_dep(store, dep_map, (d, vs)); + }; + + let wrapper = package_store.pkg(package).unwrap(); + let inner = wrapper.inner(version_index).map(|(&p, v)| (p, v)); + + if let Some((d, vs)) = wrapper.dependency(version_index) { + add_dep(package_store, &mut dep_map, (d, vs)); + } + + if let Some((name, true_version_index)) = inner { + match (name, true_version_index) { + ("root", 0) => { + { + let dn = "dep1"; + match self.version_counts.get(dn) { + Some(&vc) => { + add_singleton_dep(package_store, &mut dep_map, (dn, 1, vc)) + } + None => return Ok(Dependencies::Unavailable("unavailable")), + } + } + { + let dn = "dep2"; + match self.version_counts.get(dn) { + Some(&vc) => { + add_range_dep(package_store, &mut dep_map, (dn, 0..64, vc)) + } + None => return Ok(Dependencies::Unavailable("unavailable")), + } + } + } + ("dep1", 1) => { + let dn = "many"; + match self.version_counts.get(dn) { + Some(&vc) => add_range_dep(package_store, &mut dep_map, (dn, 0..10000, vc)), + None => return Ok(Dependencies::Unavailable("unavailable")), + } + } + ("dep2", 1) | ("many", _) => (), + _ => return Ok(Dependencies::Unavailable("unavailable")), + } + }; + + Ok(Dependencies::Available(dep_map)) + } + + fn package_version_display<'a>( + &'a self, + package: &'a Self::P, + version_index: VersionIndex, + ) -> impl Display + 'a { + match package.inner(version_index) { + Some((&pkg, true_version_index)) => format!("{pkg} @ {true_version_index}"), + None => format!("{package} @ {}", version_index.get()), + } + } + + fn package_version_set_display<'a>( + &'a self, + package: &'a Self::P, + version_set: VersionSet, + ) -> impl Display + 'a { + let version_indices = version_set + .iter() + .map(|version_index| match package.inner(version_index) { + Some((_, version_index)) => version_index, + None => version_index.get() as u64, + }) + .collect::>(); + + match package.inner_pkg() { + Some(p) => format!("{p} @ {version_indices:?}"), + _ => format!("{package} @ {version_indices:?}"), + } + } +} + +fn main() { + let (root_pkg, root_version_index) = + PackageVersionWrapper::new_pkg("root", 0, NonZeroU64::new(1).unwrap()); + + let mut provider = Provider { + version_counts: Map::from_iter([("root", 1), ("dep1", 63), ("dep2", 64), ("many", 10000)]), + }; + + match resolve(&mut provider, root_pkg, root_version_index) { + Ok(sol) => { + for (p, &v) in &sol { + let pv = provider.package_version_display(p, v); + println!("{pv}"); + } + } + Err(PubGrubError::NoSolution(mut error)) => { + error.derivation_tree.collapse_no_versions(); + eprintln!("{}", DefaultStringReporter::report(&error, &provider)); + std::process::exit(1); + } + Err(err) => panic!("{:?}", err), + } +} diff --git a/examples/unsat_root_message_no_version.rs b/examples/unsat_root_message_no_version.rs index 8e6c34a4..50a19107 100644 --- a/examples/unsat_root_message_no_version.rs +++ b/examples/unsat_root_message_no_version.rs @@ -4,9 +4,9 @@ use std::fmt::{self, Debug, Display}; use std::hash::Hash; use pubgrub::{ - resolve, DefaultStringReporter, Derived, External, Map, OfflineDependencyProvider, - PackageArena, PackageId, PubGrubError, Ranges, ReportFormatter, Reporter, SemanticVersion, - Term, + DefaultStringReporter, DependencyProvider, Derived, External, Map, OfflineDependencyProvider, + PackageArena, PackageId, PackageVersionWrapper, PubGrubError, Ranges, ReportFormatter, + Reporter, SemanticVersion, Term, VersionSet, }; #[derive(Clone, Debug, PartialEq, Eq, Hash)] @@ -24,8 +24,8 @@ impl Display for CustomPackage { } } +type Store = PackageArena>; type Dp = OfflineDependencyProvider>; -type Store = PackageArena; #[derive(Debug, Default)] struct CustomReportFormatter; @@ -33,37 +33,39 @@ struct CustomReportFormatter; impl ReportFormatter for CustomReportFormatter { type Output = String; - fn format_terms( - &self, - terms: &Map>>, - package_store: &Store, - ) -> String { + fn format_terms(&self, terms: &Map, package_store: &Store, dp: &Dp) -> String { let terms_vec: Vec<_> = terms .iter() - .map(|(&pid, v)| (pid, package_store.pkg(pid).unwrap(), v)) + .map(|(&pid, &v)| (pid, package_store.pkg(pid).unwrap(), v)) .collect(); match terms_vec.as_slice() { [] => "version solving failed".into(), - [(_, package @ CustomPackage::Root, Term::Positive(_))] => { - format!("{package} is forbidden") - } - [(_, package @ CustomPackage::Root, Term::Negative(_))] => { - format!("{package} is mandatory") - } - [(_, package @ CustomPackage::Package(_), Term::Positive(ranges))] => { - format!("{package} {ranges} is forbidden") - } - [(_, package @ CustomPackage::Package(_), Term::Negative(ranges))] => { - format!("{package} {ranges} is mandatory") - } - &[(pid1, _, Term::Positive(r1)), (pid2, _, Term::Negative(r2))] => { - External::FromDependencyOf(pid1, r1.clone(), pid2, r2.clone()) - .display::(package_store) + &[(pid, package, t)] => match package.inner_pkg() { + Some(&CustomPackage::Root) if t.is_positive() => format!("{package} is forbidden"), + Some(&CustomPackage::Root) => format!("{package} is mandatory"), + _ if t.is_positive() => format!( + "{} is forbidden", + dp.package_version_set_display( + package_store.pkg(pid).unwrap(), + t.version_set() + ), + ), + _ => format!( + "{} is mandatory", + dp.package_version_set_display( + package_store.pkg(pid).unwrap(), + t.version_set() + ), + ), + }, + &[(pid1, _, t1), (pid2, _, t2)] if t1.is_positive() && t2.is_negative() => { + External::FromDependencyOf(pid1, t1.version_set(), pid2, t2.version_set()) + .display(package_store, dp) .to_string() } - &[(pid1, _, Term::Negative(r1)), (pid2, _, Term::Positive(r2))] => { - External::FromDependencyOf(pid2, r2.clone(), pid1, r1.clone()) - .display::(package_store) + &[(pid1, _, t1), (pid2, _, t2)] if t1.is_negative() && t2.is_positive() => { + External::FromDependencyOf(pid2, t2.version_set(), pid1, t1.version_set()) + .display(package_store, dp) .to_string() } slice => { @@ -75,49 +77,72 @@ impl ReportFormatter for CustomReportFormatter { fn format_external( &self, - external: &External, &'static str>, + external: &External<&'static str>, package_store: &Store, + dp: &Dp, ) -> String { - match external { - External::NotRoot(package_id, version) => { - let package = package_store.pkg(*package_id).unwrap(); - format!("we are solving dependencies of {package} {version}") + match *external { + External::NotRoot(package_id, version_index) => { + let pkg = package_store.pkg(package_id).unwrap(); + format!( + "we are solving dependencies of {}", + dp.package_version_display(pkg, version_index), + ) } External::NoVersions(package_id, set) => { - let package = package_store.pkg(*package_id).unwrap(); - if set == &Ranges::full() { - format!("there is no available version for {package}") + let pkg = package_store.pkg(package_id).unwrap(); + if set == VersionSet::full() { + format!("there is no available version for {pkg}") } else { - format!("there is no version of {package} in {set}") + format!( + "there is no version of {}", + dp.package_version_set_display(pkg, set), + ) } } - External::Custom(package_id, set, reason) => { - let package = package_store.pkg(*package_id).unwrap(); - if set == &Ranges::full() { - format!("dependencies of {package} are unavailable because {reason}") + External::Custom(package_id, set, ref reason) => { + let pkg = package_store.pkg(package_id).unwrap(); + if set == VersionSet::full() { + format!("dependencies of {pkg} are unavailable because {reason}") } else { - format!("dependencies of {package} at version {set} are unavailable because {reason}") + format!( + "dependencies of {} are unavailable because {reason}", + dp.package_version_set_display(pkg, set), + ) } } External::FromDependencyOf(package_id, package_set, dep_id, dep_set) => { - let package = package_store.pkg(*package_id).unwrap(); - let dependency = package_store.pkg(*dep_id).unwrap(); - if package_set == &Ranges::full() && dep_set == &Ranges::full() { - format!("{package} depends on {dependency}") - } else if package_set == &Ranges::full() { - format!("{package} depends on {dependency} {dep_set}") - } else if dep_set == &Ranges::full() { - if matches!(package, CustomPackage::Root) { + let pkg = package_store.pkg(package_id).unwrap(); + let dep_pkg = package_store.pkg(dep_id).unwrap(); + if package_set == VersionSet::full() && dep_set == VersionSet::full() { + format!("{pkg} depends on {dep_pkg}") + } else if package_set == VersionSet::full() { + format!( + "{pkg} depends on {}", + dp.package_version_set_display(dep_pkg, dep_set), + ) + } else if dep_set == VersionSet::full() { + if pkg.inner_pkg() == Some(&CustomPackage::Root) { // Exclude the dummy version for root packages - format!("{package} depends on {dependency}") + format!("{pkg} depends on {dep_pkg}") } else { - format!("{package} {package_set} depends on {dependency}") + format!( + "{} depends on {dep_pkg}", + dp.package_version_set_display(pkg, package_set), + ) } - } else if matches!(package, CustomPackage::Root) { + } else if pkg.inner_pkg() == Some(&CustomPackage::Root) { // Exclude the dummy version for root packages - format!("{package} depends on {dependency} {dep_set}") + format!( + "{pkg} depends on {}", + dp.package_version_set_display(dep_pkg, dep_set), + ) } else { - format!("{package} {package_set} depends on {dependency} {dep_set}") + format!( + "{} depends on {}", + dp.package_version_set_display(pkg, package_set), + dp.package_version_set_display(dep_pkg, dep_set), + ) } } } @@ -126,17 +151,18 @@ impl ReportFormatter for CustomReportFormatter { /// Simplest case, we just combine two external incompatibilities. fn explain_both_external( &self, - external1: &External, &'static str>, - external2: &External, &'static str>, - current_terms: &Map>>, + external1: &External<&'static str>, + external2: &External<&'static str>, + current_terms: &Map, package_store: &Store, + dp: &Dp, ) -> String { // TODO: order should be chosen to make it more logical. format!( "Because {} and {}, {}.", - self.format_external(external1, package_store), - self.format_external(external2, package_store), - self.format_terms(current_terms, package_store) + self.format_external(external1, package_store, dp), + self.format_external(external2, package_store, dp), + self.format_terms(current_terms, package_store, dp) ) } @@ -144,20 +170,21 @@ impl ReportFormatter for CustomReportFormatter { fn explain_both_ref( &self, ref_id1: usize, - derived1: &Derived, &'static str>, + derived1: &Derived<&'static str>, ref_id2: usize, - derived2: &Derived, &'static str>, - current_terms: &Map>>, + derived2: &Derived<&'static str>, + current_terms: &Map, package_store: &Store, + dp: &Dp, ) -> String { // TODO: order should be chosen to make it more logical. format!( "Because {} ({}) and {} ({}), {}.", - self.format_terms(&derived1.terms, package_store), + self.format_terms(&derived1.terms, package_store, dp), ref_id1, - self.format_terms(&derived2.terms, package_store), + self.format_terms(&derived2.terms, package_store, dp), ref_id2, - self.format_terms(current_terms, package_store) + self.format_terms(current_terms, package_store, dp) ) } @@ -167,32 +194,34 @@ impl ReportFormatter for CustomReportFormatter { fn explain_ref_and_external( &self, ref_id: usize, - derived: &Derived, &'static str>, - external: &External, &'static str>, - current_terms: &Map>>, + derived: &Derived<&'static str>, + external: &External<&'static str>, + current_terms: &Map, package_store: &Store, + dp: &Dp, ) -> String { // TODO: order should be chosen to make it more logical. format!( "Because {} ({}) and {}, {}.", - self.format_terms(&derived.terms, package_store), + self.format_terms(&derived.terms, package_store, dp), ref_id, - self.format_external(external, package_store), - self.format_terms(current_terms, package_store) + self.format_external(external, package_store, dp), + self.format_terms(current_terms, package_store, dp) ) } /// Add an external cause to the chain of explanations. fn and_explain_external( &self, - external: &External, &'static str>, - current_terms: &Map>>, + external: &External<&'static str>, + current_terms: &Map, package_store: &Store, + dp: &Dp, ) -> String { format!( "And because {}, {}.", - self.format_external(external, package_store), - self.format_terms(current_terms, package_store) + self.format_external(external, package_store, dp), + self.format_terms(current_terms, package_store, dp) ) } @@ -200,31 +229,33 @@ impl ReportFormatter for CustomReportFormatter { fn and_explain_ref( &self, ref_id: usize, - derived: &Derived, &'static str>, - current_terms: &Map>>, + derived: &Derived<&'static str>, + current_terms: &Map, package_store: &Store, + dp: &Dp, ) -> String { format!( "And because {} ({}), {}.", - self.format_terms(&derived.terms, package_store), + self.format_terms(&derived.terms, package_store, dp), ref_id, - self.format_terms(current_terms, package_store) + self.format_terms(current_terms, package_store, dp) ) } /// Add an already explained incompat to the chain of explanations. fn and_explain_prior_and_external( &self, - prior_external: &External, &'static str>, - external: &External, &'static str>, - current_terms: &Map>>, + prior_external: &External<&'static str>, + external: &External<&'static str>, + current_terms: &Map, package_store: &Store, + dp: &Dp, ) -> String { format!( "And because {} and {}, {}.", - self.format_external(prior_external, package_store), - self.format_external(external, package_store), - self.format_terms(current_terms, package_store) + self.format_external(prior_external, package_store, dp), + self.format_external(external, package_store, dp), + self.format_terms(current_terms, package_store, dp) ) } } @@ -232,6 +263,7 @@ impl ReportFormatter for CustomReportFormatter { fn main() { let mut dependency_provider = OfflineDependencyProvider::>::new(); + // Define the root package with a dependency on a package we do not provide dependency_provider.add_dependencies( CustomPackage::Root, @@ -243,21 +275,28 @@ fn main() { ); // Run the algorithm - match resolve(&mut dependency_provider, CustomPackage::Root, (0, 0, 0)) { + match dependency_provider.resolve(CustomPackage::Root, (0, 0, 0)) { Ok(sol) => println!("{:?}", sol), Err(PubGrubError::NoSolution(error)) => { eprintln!("No solution.\n"); eprintln!("### Default report:"); eprintln!("```"); - eprintln!("{}", DefaultStringReporter::report(&error)); + eprintln!( + "{}", + DefaultStringReporter::report(&error, &dependency_provider) + ); eprintln!("```\n"); eprintln!("### Report with custom formatter:"); eprintln!("```"); eprintln!( "{}", - DefaultStringReporter::report_with_formatter(&error, &CustomReportFormatter) + DefaultStringReporter::report_with_formatter( + &error, + &CustomReportFormatter, + &dependency_provider + ) ); eprintln!("```"); std::process::exit(1); diff --git a/src/error.rs b/src/error.rs index e0f21033..b6b88612 100644 --- a/src/error.rs +++ b/src/error.rs @@ -12,7 +12,7 @@ pub struct NoSolutionError { /// Package store pub package_store: PackageArena, /// Derivate tree - pub derivation_tree: DerivationTree, + pub derivation_tree: DerivationTree, } /// Errors that may occur while solving dependencies. @@ -28,12 +28,10 @@ pub enum PubGrubError { /// Error arising when the implementer of [DependencyProvider] returned an error in the method /// [get_dependencies](DependencyProvider::get_dependencies). - #[error("Retrieving dependencies of {package} {version} failed")] + #[error("Retrieving dependencies of {package_version} failed")] ErrorRetrievingDependencies { - /// Package whose dependencies we want. - package: DP::P, - /// Version of the package for which we want the dependencies. - version: DP::V, + /// Represenatation of package and version whose dependencies we want. + package_version: String, /// Error raised by the implementer of /// [DependencyProvider]. source: DP::Err, @@ -73,13 +71,11 @@ where .field("derivation_tree", &err.derivation_tree) .finish(), Self::ErrorRetrievingDependencies { - package, - version, + package_version, source, } => f .debug_struct("ErrorRetrievingDependencies") - .field("package", package) - .field("version", version) + .field("package_version", package_version) .field("source", source) .finish(), Self::ErrorChoosingPackageVersion(arg0) => f diff --git a/src/helpers.rs b/src/helpers.rs new file mode 100644 index 00000000..6133921a --- /dev/null +++ b/src/helpers.rs @@ -0,0 +1,350 @@ +use std::fmt::{self, Display}; +use std::iter::repeat_n; +use std::num::NonZeroU64; +use std::rc::Rc; + +use crate::{VersionIndex, VersionSet}; + +/// Package allowing more than 63 versions +#[derive(Debug, Clone, Eq, PartialEq, Hash)] +pub struct Pkg

{ + pkg: P, + quotient: u64, + count: u64, +} + +impl

Pkg

{ + pub fn pkg(&self) -> &P { + &self.pkg + } +} + +impl Display for Pkg

{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{} (q={})", self.pkg, self.quotient) + } +} + +/// Virtual package ensuring package unicity +#[derive(Debug, Clone, Eq, PartialEq, Hash)] +pub struct VirtualPkg

{ + pkg: P, + quotient: u64, + count: u64, +} + +impl

VirtualPkg

{ + pub fn pkg(&self) -> &P { + &self.pkg + } +} + +impl Display for VirtualPkg

{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "VirtualPkg({}, q={}, c={})", + self.pkg, self.quotient, self.count + ) + } +} + +/// Virtual package dependency allowing more than 63 versions +#[derive(Debug, Clone, Eq, PartialEq, Hash)] +pub struct VirtualDep

{ + pkg: P, + version_indices: Rc<[VersionSet]>, + offset: u64, + quotient: u64, +} + +impl

VirtualDep

{ + pub fn pkg(&self) -> &P { + &self.pkg + } +} + +impl Display for VirtualDep

{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let vc = self + .version_indices + .iter() + .map(|vs| vs.count()) + .sum::(); + + write!( + f, + "VirtualDep({}, vc={vc}, o={}, q={})", + self.pkg, self.offset, self.quotient + ) + } +} + +/// Package wrapper used to allow more than 63 versions per package. +#[derive(Debug, Clone, Eq, PartialEq, Hash)] +pub enum PackageVersionWrapper { + /// Package allowing more than 63 versions + Pkg(Pkg

), + /// Virtual package ensuring package unicity + VirtualPkg(VirtualPkg

), + /// Virtual package dependency allowing more than 63 versions + VirtualDep(VirtualDep

), +} + +impl Display for PackageVersionWrapper

{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Pkg(p) => p.fmt(f), + Self::VirtualPkg(vp) => vp.fmt(f), + Self::VirtualDep(vd) => vd.fmt(f), + } + } +} + +impl PackageVersionWrapper

{ + /// Create a new package. + pub fn new_pkg( + pkg: P, + true_version_index: u64, + version_count: NonZeroU64, + ) -> (Self, VersionIndex) { + ( + Self::Pkg(Pkg { + pkg, + quotient: true_version_index / VersionIndex::MAX, + count: (version_count.get() - 1) / VersionIndex::MAX, + }), + VersionIndex::new((true_version_index % VersionIndex::MAX) as u8).unwrap(), + ) + } + + /// Create a new package dependency with no versions. + pub fn new_empty_dep(pkg: P) -> (Self, VersionSet) { + ( + Self::Pkg(Pkg { + pkg, + quotient: 0, + count: 0, + }), + VersionSet::empty(), + ) + } + + /// Create a new package dependency at the specified version. + pub fn new_singleton_dep( + pkg: P, + true_version_index: u64, + version_count: u64, + ) -> (Self, VersionSet) { + match NonZeroU64::new(version_count) { + Some(version_count) => { + assert!(true_version_index < version_count.get()); + let (this, v) = Self::new_pkg(pkg, true_version_index, version_count); + (this, VersionSet::singleton(v)) + } + None => Self::new_empty_dep(pkg), + } + } + + /// Create a new package dependency at the specified versions. + pub fn new_dep( + pkg: P, + true_version_indices: impl IntoIterator, + version_count: u64, + ) -> (Self, VersionSet) { + let Some(nz_version_count) = NonZeroU64::new(version_count) else { + return Self::new_empty_dep(pkg); + }; + if version_count <= VersionIndex::MAX { + let mut set = VersionSet::empty(); + for true_version_index in true_version_indices { + assert!(true_version_index < version_count); + let v = VersionIndex::new(true_version_index as u8).unwrap(); + set = set.union(VersionSet::singleton(v)); + } + return ( + Self::Pkg(Pkg { + pkg, + quotient: 0, + count: (version_count - 1) / VersionIndex::MAX, + }), + set, + ); + } + + let mut true_version_indices = true_version_indices.into_iter(); + + let Some(first) = true_version_indices.next() else { + return Self::new_empty_dep(pkg); + }; + assert!(first < version_count); + + let Some(second) = true_version_indices.next() else { + let (d, vs) = Self::new_pkg(pkg, first, nz_version_count); + return (d, VersionSet::singleton(vs)); + }; + assert!(second < version_count); + + let mut version_indices = Rc::from_iter(repeat_n( + VersionSet::empty(), + (1 + (version_count - 1) / VersionIndex::MAX) as usize, + )); + let versions_slice = Rc::make_mut(&mut version_indices); + + for true_version_index in [first, second].into_iter().chain(true_version_indices) { + assert!(true_version_index < version_count); + let index = (true_version_index / VersionIndex::MAX) as usize; + let v = VersionIndex::new((true_version_index % VersionIndex::MAX) as u8).unwrap(); + let set = versions_slice.get_mut(index).unwrap(); + *set = set.union(VersionSet::singleton(v)); + } + + let offset = 0; + let quotient = VersionIndex::MAX.pow(version_count.ilog(VersionIndex::MAX) - 1); + let version_set = Self::dep_version_set(&version_indices, offset, quotient); + + let this = Self::VirtualDep(VirtualDep { + pkg, + version_indices, + offset, + quotient, + }); + + (this, version_set) + } + + /// Clone and replace the package of this wrapper. + pub fn replace_pkg(&self, new_pkg: T) -> PackageVersionWrapper { + match *self { + Self::Pkg(Pkg { + pkg: _, + quotient, + count, + }) => PackageVersionWrapper::Pkg(Pkg { + pkg: new_pkg, + quotient, + count, + }), + Self::VirtualPkg(VirtualPkg { + pkg: _, + quotient, + count, + }) => PackageVersionWrapper::VirtualPkg(VirtualPkg { + pkg: new_pkg, + quotient, + count, + }), + Self::VirtualDep(VirtualDep { + pkg: _, + ref version_indices, + offset, + quotient, + }) => PackageVersionWrapper::VirtualDep(VirtualDep { + pkg: new_pkg, + version_indices: version_indices.clone(), + offset, + quotient, + }), + } + } + + /// Get the inner package if existing. + pub fn inner_pkg(&self) -> Option<&P> { + match self { + Self::Pkg(Pkg { pkg, .. }) => Some(pkg), + _ => None, + } + } + + /// Get the inner package if existing. + pub fn inner(&self, version_index: VersionIndex) -> Option<(&P, u64)> { + match self { + Self::Pkg(Pkg { pkg, quotient, .. }) => Some(( + pkg, + quotient * VersionIndex::MAX + version_index.get() as u64, + )), + _ => None, + } + } + + /// Get the inner package if existing. + pub fn into_inner(self, version_index: VersionIndex) -> Option<(P, u64)> { + match self { + Self::Pkg(Pkg { pkg, quotient, .. }) => Some(( + pkg, + quotient * VersionIndex::MAX + version_index.get() as u64, + )), + _ => None, + } + } + + /// Get the wrapper virtual dependency if existing. + pub fn dependency(&self, version_index: VersionIndex) -> Option<(Self, VersionSet)> { + match *self { + Self::Pkg(Pkg { + ref pkg, + quotient, + count, + }) + | Self::VirtualPkg(VirtualPkg { + ref pkg, + quotient, + count, + }) => { + if count == 0 { + None + } else { + Some(( + Self::VirtualPkg(VirtualPkg { + pkg: pkg.clone(), + quotient: quotient / VersionIndex::MAX, + count: count / VersionIndex::MAX, + }), + VersionSet::singleton( + VersionIndex::new((quotient % VersionIndex::MAX) as u8).unwrap(), + ), + )) + } + } + Self::VirtualDep(VirtualDep { + ref pkg, + ref version_indices, + offset, + quotient, + }) => { + let offset = offset + version_index.get() as u64 * quotient; + if quotient == 1 { + return Some(( + Self::Pkg(Pkg { + pkg: pkg.clone(), + quotient: offset, + count: (version_indices.len() - 1) as u64, + }), + version_indices[offset as usize], + )); + } + let quotient = quotient / VersionIndex::MAX; + let version_set = Self::dep_version_set(version_indices, offset, quotient); + + let this = Self::VirtualDep(VirtualDep { + pkg: pkg.clone(), + version_indices: version_indices.clone(), + offset, + quotient, + }); + + Some((this, version_set)) + } + } + } + + fn dep_version_set(sets: &[VersionSet], offset: u64, quotient: u64) -> VersionSet { + sets[offset as usize..] + .chunks(quotient as usize) + .take(VersionIndex::MAX as usize) + .enumerate() + .filter(|&(_, sets)| sets.iter().any(|&vs| vs != VersionSet::empty())) + .map(|(i, _)| VersionSet::singleton(VersionIndex::new(i as u8).unwrap())) + .fold(VersionSet::empty(), |acc, vs| acc.union(vs)) + } +} diff --git a/src/internal/core.rs b/src/internal/core.rs index b370ed8b..a650a234 100644 --- a/src/internal/core.rs +++ b/src/internal/core.rs @@ -10,14 +10,15 @@ use crate::{ Arena, DecisionLevel, IncompDpId, Incompatibility, PartialSolution, Relation, SatisfierSearch, SmallVec, }, - DependencyProvider, DerivationTree, Map, PackageArena, PackageId, Set, VersionSet, + DependencyProvider, DerivationTree, Map, PackageArena, PackageId, Set, Term, VersionIndex, + VersionSet, }; /// Current state of the PubGrub algorithm. #[derive(Clone)] pub(crate) struct State { root_package_id: PackageId, - root_version: DP::V, + root_version_index: VersionIndex, #[allow(clippy::type_complexity)] incompatibilities: Map>>, @@ -37,7 +38,7 @@ pub(crate) struct State { pub(crate) partial_solution: PartialSolution, /// The store is the reference storage for all incompatibilities. - pub(crate) incompatibility_store: Arena>, + pub(crate) 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 @@ -47,17 +48,17 @@ pub(crate) struct State { impl State { /// Initialization of PubGrub state. - pub(crate) fn init(root_package_id: PackageId, root_version: DP::V) -> Self { + pub(crate) fn init(root_package_id: PackageId, root_version_index: VersionIndex) -> Self { let mut incompatibility_store = Arena::new(); let not_root_id = incompatibility_store.alloc(Incompatibility::not_root( root_package_id, - root_version.clone(), + root_version_index, )); let mut incompatibilities = Map::default(); incompatibilities.insert(root_package_id, vec![not_root_id]); Self { root_package_id, - root_version, + root_version_index, incompatibilities, contradicted_incompatibilities: Map::default(), partial_solution: PartialSolution::empty(), @@ -68,7 +69,7 @@ impl State { } /// Add an incompatibility to the state. - pub(crate) fn add_incompatibility(&mut self, incompat: Incompatibility) { + pub(crate) fn add_incompatibility(&mut self, incompat: Incompatibility) { let id = self.incompatibility_store.alloc(incompat); self.merge_incompatibility(id); } @@ -77,19 +78,15 @@ impl State { pub(crate) fn add_incompatibility_from_dependencies( &mut self, package_id: PackageId, - version: DP::V, - deps: impl IntoIterator, + version_index: VersionIndex, + deps: impl IntoIterator, ) -> std::ops::Range> { // Create incompatibilities and allocate them in the store. - let new_incompats_id_range = - self.incompatibility_store - .alloc_iter(deps.into_iter().map(|dep| { - Incompatibility::from_dependency( - package_id, - ::singleton(version.clone()), - dep, - ) - })); + let vs = VersionSet::singleton(version_index); + let new_incompats_id_range = self.incompatibility_store.alloc_iter( + deps.into_iter() + .map(|dep| Incompatibility::from_dependency(package_id, vs, dep)), + ); // Merge the newly created incompatibilities with the older ones. for id in IncompDpId::::range_to_iter(new_incompats_id_range.clone()) { self.merge_incompatibility(id); @@ -103,7 +100,8 @@ impl State { &mut self, package_id: PackageId, package_store: &PackageArena, - ) -> Result<(), DerivationTree> { + dependency_provider: &DP, + ) -> Result<(), DerivationTree> { self.unit_propagation_buffer.clear(); self.unit_propagation_buffer.push(package_id); while let Some(current_package) = self.unit_propagation_buffer.pop() { @@ -125,7 +123,7 @@ impl State { Relation::Satisfied => { log::info!( "Start conflict resolution because incompat satisfied:\n {}", - current_incompat.display::(package_store) + current_incompat.display(package_store, dependency_provider) ); conflict_id = Some(incompat_id); break; @@ -157,7 +155,7 @@ impl State { } if let Some(incompat_id) = conflict_id { let (package_almost, root_cause) = self - .conflict_resolution(incompat_id, package_store) + .conflict_resolution(incompat_id, package_store, dependency_provider) .map_err(|terminal_incompat_id| { self.build_derivation_tree(terminal_incompat_id) })?; @@ -186,12 +184,13 @@ impl State { &mut self, incompatibility: IncompDpId, package_store: &PackageArena, + dependency_provider: &DP, ) -> Result<(PackageId, IncompDpId), IncompDpId> { let mut current_incompat_id = incompatibility; let mut current_incompat_changed = false; loop { if self.incompatibility_store[current_incompat_id] - .is_terminal(self.root_package_id, &self.root_version) + .is_terminal(self.root_package_id, self.root_version_index) { return Err(current_incompat_id); } else { @@ -219,7 +218,10 @@ impl State { package_id, &self.incompatibility_store, ); - log::info!("prior cause: {}", prior_cause.display::(package_store)); + log::info!( + "prior cause: {}", + prior_cause.display(package_store, dependency_provider) + ); current_incompat_id = self.incompatibility_store.alloc(prior_cause); current_incompat_changed = true; } @@ -283,7 +285,7 @@ impl State { } for (package_id, term) in self.incompatibility_store[id].iter() { if cfg!(debug_assertions) { - assert_ne!(term, &crate::term::Term::any()); + assert_ne!(term, Term::any()); } self.incompatibilities .entry(package_id) @@ -294,7 +296,7 @@ impl State { // Error reporting ######################################################### - fn build_derivation_tree(&self, incompat: IncompDpId) -> DerivationTree { + fn build_derivation_tree(&self, incompat: IncompDpId) -> DerivationTree { let mut all_ids: Set> = Set::default(); let mut shared_ids = Set::default(); let mut stack = vec![incompat]; diff --git a/src/internal/incompatibility.rs b/src/internal/incompatibility.rs index 0bdc5fe5..b3d625ca 100644 --- a/src/internal/incompatibility.rs +++ b/src/internal/incompatibility.rs @@ -9,7 +9,7 @@ use std::sync::Arc; use crate::{ internal::{Arena, Id, SmallMap}, term, DefaultStringReportFormatter, DependencyProvider, DerivationTree, Derived, External, Map, - PackageArena, PackageId, ReportFormatter, Set, Term, VersionSet, + PackageArena, PackageId, ReportFormatter, Set, Term, VersionIndex, VersionSet, }; /// An incompatibility is a set of terms for different packages @@ -28,29 +28,28 @@ use crate::{ /// during conflict resolution. More about all this in /// [PubGrub documentation](https://github.com/dart-lang/pub/blob/master/doc/solver.md#incompatibility). #[derive(Debug, Clone)] -pub(crate) struct Incompatibility { - package_terms: SmallMap>, - kind: Kind, +pub(crate) struct Incompatibility { + package_terms: SmallMap, + kind: Kind, } /// Type alias of unique identifiers for incompatibilities. -pub(crate) type IncompId = Id>; +pub(crate) type IncompId = Id>; -pub(crate) type IncompDpId = - IncompId<::VS, ::M>; +pub(crate) type IncompDpId = IncompId<::M>; #[derive(Debug, Clone)] -enum Kind { +enum Kind { /// Initial incompatibility aiming at picking the root package for the first decision. /// /// This incompatibility drives the resolution, it requires that we pick the (virtual) root /// packages. - NotRoot(PackageId, VS::V), + NotRoot(PackageId, VersionIndex), /// There are no versions in the given range for this package. /// /// This incompatibility is used when we tried all versions in a range and no version /// worked, so we have to backtrack - NoVersions(PackageId, VS), + NoVersions(PackageId, VersionSet), /// Incompatibility coming from the dependencies of a given package. /// /// If a@1 depends on b>=1,<2, we create an incompatibility with terms `{a 1, b <1,>=2}` with @@ -58,17 +57,17 @@ enum Kind { /// /// We can merge multiple dependents with the same version. For example, if a@1 depends on b and /// a@2 depends on b, we can say instead a@1||2 depends on b. - FromDependencyOf(PackageId, VS, PackageId, VS), + FromDependencyOf(PackageId, VersionSet, PackageId, VersionSet), /// Derived from two causes. Stores cause ids. /// /// For example, if a -> b and b -> c, we can derive a -> c. - DerivedFrom(IncompId, IncompId), + DerivedFrom(IncompId, IncompId), /// The package is unavailable for reasons outside pubgrub. /// /// Examples: /// * The version would require building the package, but builds are disabled. /// * The package is not available in the cache, but internet access has been disabled. - Custom(PackageId, VS, M), + Custom(PackageId, VersionSet, M), } /// A Relation describes how a set of terms can be compared to an incompatibility. @@ -88,23 +87,24 @@ pub(crate) enum Relation { Inconclusive, } -impl Incompatibility { +impl Incompatibility { /// Create the initial "not Root" incompatibility. - pub(crate) fn not_root(package_id: PackageId, version: VS::V) -> Self { + pub(crate) fn not_root(package_id: PackageId, version_index: VersionIndex) -> Self { Self { package_terms: SmallMap::One([( package_id, - Term::Negative(VS::singleton(version.clone())), + Term::negative(VersionSet::singleton(version_index)), )]), - kind: Kind::NotRoot(package_id, version), + kind: Kind::NotRoot(package_id, version_index), } } /// Create an incompatibility to remember that a given set does not contain any version. - pub(crate) fn no_versions(package_id: PackageId, term: Term) -> Self { - let set = match &term { - Term::Positive(r) => r.clone(), - Term::Negative(_) => panic!("No version should have a positive term"), + pub(crate) fn no_versions(package_id: PackageId, term: Term) -> Self { + let set = if term.is_positive() { + term.version_set() + } else { + panic!("No version should have a positive term") }; Self { package_terms: SmallMap::One([(package_id, term)]), @@ -114,10 +114,11 @@ impl Incompatibility { /// Create an incompatibility for a reason outside pubgrub. #[allow(dead_code)] // Used by uv - pub(crate) fn custom_term(package_id: PackageId, term: Term, metadata: M) -> Self { - let set = match &term { - Term::Positive(r) => r.clone(), - Term::Negative(_) => panic!("No version should have a positive term"), + pub(crate) fn custom_term(package_id: PackageId, term: Term, metadata: M) -> Self { + let set = if term.is_positive() { + term.version_set() + } else { + panic!("No version should have a positive term") }; Self { package_terms: SmallMap::One([(package_id, term)]), @@ -126,9 +127,13 @@ impl Incompatibility { } /// Create an incompatibility for a reason outside pubgrub. - pub(crate) fn custom_version(package_id: PackageId, version: VS::V, metadata: M) -> Self { - let set = VS::singleton(version); - let term = Term::Positive(set.clone()); + pub(crate) fn custom_version( + package_id: PackageId, + version_index: VersionIndex, + metadata: M, + ) -> Self { + let set = VersionSet::singleton(version_index); + let term = Term::positive(set); Self { package_terms: SmallMap::One([(package_id, term)]), kind: Kind::Custom(package_id, set, metadata), @@ -138,20 +143,20 @@ impl Incompatibility { /// Build an incompatibility from a given dependency. pub(crate) fn from_dependency( package_id: PackageId, - versions: VS, - dep: (PackageId, VS), + vs: VersionSet, + dep: (PackageId, VersionSet), ) -> Self { let (pid2, set2) = dep; Self { - package_terms: if set2 == VS::empty() { - SmallMap::One([(package_id, Term::Positive(versions.clone()))]) + package_terms: if set2 == VersionSet::empty() { + SmallMap::One([(package_id, Term::positive(vs))]) } else { SmallMap::Two([ - (package_id, Term::Positive(versions.clone())), - (pid2, Term::Negative(set2.clone())), + (package_id, Term::positive(vs)), + (pid2, Term::negative(set2)), ]) }, - kind: Kind::FromDependencyOf(package_id, versions, pid2, set2), + kind: Kind::FromDependencyOf(package_id, vs, pid2, set2), } } @@ -187,17 +192,17 @@ impl Incompatibility { if dep_term != other.get(pid2) { return None; } - return Some(Self::from_dependency( + Some(Self::from_dependency( pid1, self.get(pid1) .unwrap() .unwrap_positive() - .union(other.get(pid1).unwrap().unwrap_positive()), // It is safe to `simplify` here + .union(other.get(pid1).unwrap().unwrap_positive()), ( pid2, - dep_term.map_or(VS::empty(), |v| v.unwrap_negative().clone()), + dep_term.map_or(VersionSet::empty(), |v| v.unwrap_negative()), ), - )); + )) } /// Prior cause of two incompatibilities using the rule of resolution. @@ -218,9 +223,9 @@ impl Incompatibility { satisfier_cause_terms .iter() .filter(|(&pid, _)| pid != package_id), - |t1, t2| Some(t1.intersection(t2)), + |&t1, &t2| Some(t1.intersection(t2)), ); - let term = t1.union(satisfier_cause_terms.get(&package_id).unwrap()); + let term = t1.union(*satisfier_cause_terms.get(&package_id).unwrap()); if term != Term::any() { package_terms.insert(package_id, term); } @@ -232,29 +237,31 @@ impl Incompatibility { /// Check if an incompatibility should mark the end of the algorithm /// because it satisfies the root package. - pub(crate) fn is_terminal(&self, root_package_id: PackageId, root_version: &VS::V) -> bool { + pub(crate) fn is_terminal( + &self, + root_package_id: PackageId, + root_version_index: VersionIndex, + ) -> bool { if self.package_terms.len() == 0 { true } else if self.package_terms.len() > 1 { false } else { let (&package_id, term) = self.package_terms.iter().next().unwrap(); - (package_id == root_package_id) && term.contains(root_version) + (package_id == root_package_id) && term.contains(root_version_index) } } /// Get the term related to a given package (if it exists). - pub(crate) fn get(&self, package_id: PackageId) -> Option<&Term> { - self.package_terms.get(&package_id) + pub(crate) fn get(&self, package_id: PackageId) -> Option { + self.package_terms.get(&package_id).copied() } /// Iterate over packages. - pub(crate) fn iter(&self) -> impl Iterator)> { - self.package_terms.iter().map(|(&k, v)| (k, v)) + pub(crate) fn iter(&self) -> impl Iterator + use<'_, M> { + self.package_terms.iter().map(|(&k, &v)| (k, v)) } - // Reporting ############################################################### - /// Retrieve parent causes if of type DerivedFrom. pub(crate) fn causes(&self) -> Option<(Id, Id)> { match self.kind { @@ -268,8 +275,8 @@ impl Incompatibility { self_id: Id, shared_ids: &Set>, store: &Arena, - precomputed: &Map, Arc>>, - ) -> DerivationTree { + precomputed: &Map, Arc>>, + ) -> DerivationTree { match store[self_id].kind.clone() { Kind::DerivedFrom(id1, id2) => { let derived = Derived { @@ -286,34 +293,30 @@ impl Incompatibility { }; DerivationTree::Derived(derived) } - Kind::NotRoot(package_id, version) => { - DerivationTree::External(External::NotRoot(package_id, version)) + Kind::NotRoot(package_id, version_index) => { + DerivationTree::External(External::NotRoot(package_id, version_index)) } Kind::NoVersions(package_id, set) => { - DerivationTree::External(External::NoVersions(package_id, set.clone())) + DerivationTree::External(External::NoVersions(package_id, set)) } Kind::FromDependencyOf(package_id, set, dep_package_id, dep_set) => { DerivationTree::External(External::FromDependencyOf( package_id, - set.clone(), + set, dep_package_id, - dep_set.clone(), + dep_set, )) } - Kind::Custom(package_id, set, metadata) => DerivationTree::External(External::Custom( - package_id, - set.clone(), - metadata.clone(), - )), + Kind::Custom(package_id, set, metadata) => { + DerivationTree::External(External::Custom(package_id, set, metadata.clone())) + } } } -} -impl<'a, VS: VersionSet + 'a, M: Eq + Clone + Debug + Display + 'a> Incompatibility { /// CF definition of Relation enum. - pub(crate) fn relation(&self, terms: impl Fn(PackageId) -> Option<&'a Term>) -> Relation { + pub(crate) fn relation(&self, terms: impl Fn(PackageId) -> Option) -> Relation { let mut relation = Relation::Satisfied; - for (&package_id, incompat_term) in self.package_terms.iter() { + for (&package_id, &incompat_term) in self.package_terms.iter() { match terms(package_id).map(|term| incompat_term.relation_with(term)) { Some(term::Relation::Satisfied) => {} Some(term::Relation::Contradicted) => { @@ -335,26 +338,33 @@ impl<'a, VS: VersionSet + 'a, M: Eq + Clone + Debug + Display + 'a> Incompatibil } relation } -} -impl Incompatibility { - pub(crate) fn display<'a, DP: DependencyProvider>( + pub(crate) fn display<'a, DP: DependencyProvider>( &'a self, package_store: &'a PackageArena, - ) -> IncompatibilityDisplay<'a, DP> { + dependency_provider: &'a DP, + ) -> IncompatibilityDisplay<'a, DP, M> { IncompatibilityDisplay { incompatibility: self, package_store, + dependency_provider, } } } -pub(crate) struct IncompatibilityDisplay<'a, DP: DependencyProvider> { - incompatibility: &'a Incompatibility, +pub(crate) struct IncompatibilityDisplay< + 'a, + DP: DependencyProvider, + M: Eq + Clone + Debug + Display, +> { + incompatibility: &'a Incompatibility, package_store: &'a PackageArena, + dependency_provider: &'a DP, } -impl<'a, DP: DependencyProvider> Display for IncompatibilityDisplay<'a, DP> { +impl<'a, DP: DependencyProvider, M: Eq + Clone + Debug + Display> Display + for IncompatibilityDisplay<'a, DP, M> +{ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!( f, @@ -363,6 +373,7 @@ impl<'a, DP: DependencyProvider> Display for IncompatibilityDisplay<'a, DP> { &DefaultStringReportFormatter, &self.incompatibility.package_terms.as_map(), self.package_store, + self.dependency_provider ) ) } @@ -376,7 +387,6 @@ pub(crate) mod tests { use super::*; use crate::term::tests::strategy as term_strat; - use crate::Ranges; proptest! { /// For any three different packages p1, p2 and p3, @@ -390,13 +400,13 @@ pub(crate) mod tests { fn rule_of_resolution(t1 in term_strat(), t2 in term_strat(), t3 in term_strat()) { let mut store = Arena::new(); let i1 = store.alloc(Incompatibility { - package_terms: SmallMap::Two([(PackageId(1), t1.clone()), (PackageId(2), t2.negate())]), - kind: Kind::< _, String>::FromDependencyOf(PackageId(1), Ranges::full(), PackageId(2), Ranges::full()) + package_terms: SmallMap::Two([(PackageId(1), t1), (PackageId(2), t2.negate())]), + kind: Kind::::FromDependencyOf(PackageId(1), VersionSet::full(), PackageId(2), VersionSet::full()) }); let i2 = store.alloc(Incompatibility { - package_terms: SmallMap::Two([(PackageId(2), t2), (PackageId(3), t3.clone())]), - kind: Kind::< _, String>::FromDependencyOf(PackageId(2), Ranges::full(), PackageId(3), Ranges::full()) + package_terms: SmallMap::Two([(PackageId(2), t2), (PackageId(3), t3)]), + kind: Kind::::FromDependencyOf(PackageId(2), VersionSet::full(), PackageId(3), VersionSet::full()) }); let mut i3 = Map::default(); diff --git a/src/internal/partial_solution.rs b/src/internal/partial_solution.rs index 6eb74e54..8921c769 100644 --- a/src/internal/partial_solution.rs +++ b/src/internal/partial_solution.rs @@ -2,7 +2,7 @@ //! A Memory acts like a structured partial solution where terms are regrouped by package in a [Map]. -use std::fmt::{Debug, Display}; +use std::fmt::{self, Debug, Display}; use std::hash::BuildHasherDefault; use priority_queue::PriorityQueue; @@ -12,7 +12,7 @@ use crate::Map; use crate::{ internal::{Arena, IncompDpId, IncompId, Incompatibility, Relation, SmallMap, SmallVec}, DependencyProvider, FxIndexMap, PackageArena, PackageId, SelectedDependencies, Term, - VersionSet, + VersionIndex, VersionSet, }; #[derive(Debug, Copy, Clone, Ord, PartialOrd, Eq, PartialEq)] @@ -41,7 +41,7 @@ pub(crate) struct PartialSolution { /// the last time `prioritize` has been called. The inverse is not necessarily true, some packages in the range /// did not have a change. Within this range there is no sorting. #[allow(clippy::type_complexity)] - package_assignments: FxIndexMap>, + package_assignments: FxIndexMap>, /// `prioritized_potential_packages` is primarily a HashMap from a package with no desition and a positive assignment /// to its `Priority`. But, it also maintains a max heap of packages by `Priority` order. prioritized_potential_packages: @@ -68,7 +68,7 @@ pub(crate) struct PartialSolutionDisplay<'a, DP: DependencyProvider> { } impl<'a, DP: DependencyProvider> Display for PartialSolutionDisplay<'a, DP> { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let partial_solution = self.partial_solution; let mut assignments: Vec<_> = partial_solution .package_assignments @@ -93,15 +93,15 @@ impl<'a, DP: DependencyProvider> Display for PartialSolutionDisplay<'a, DP> { /// that have already been made for a given package, /// as well as the intersection of terms by all of these. #[derive(Clone, Debug)] -struct PackageAssignments { +struct PackageAssignments { smallest_decision_level: DecisionLevel, highest_decision_level: DecisionLevel, - dated_derivations: SmallVec>, - assignments_intersection: AssignmentsIntersection, + dated_derivations: SmallVec>, + assignments_intersection: AssignmentsIntersection, } -impl Display for PackageAssignments { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +impl Display for PackageAssignments { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let derivations: Vec<_> = self .dated_derivations .iter() @@ -119,30 +119,30 @@ impl Display for PackageAssignm } #[derive(Clone, Debug)] -struct DatedDerivation { +struct DatedDerivation { global_index: u32, decision_level: DecisionLevel, - cause: IncompId, - accumulated_intersection: Term, + cause: IncompId, + accumulated_intersection: Term, } -impl Display for DatedDerivation { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +impl Display for DatedDerivation { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{:?}, cause: {:?}", self.decision_level, self.cause) } } #[derive(Clone, Debug)] -enum AssignmentsIntersection { - Decision((u32, VS::V, Term)), - Derivations(Term), +enum AssignmentsIntersection { + Decision((u32, VersionIndex, Term)), + Derivations(Term), } -impl Display for AssignmentsIntersection { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +impl Display for AssignmentsIntersection { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - Self::Decision((lvl, version, _)) => { - write!(f, "Decision: level {}, v = {}", lvl, version) + Self::Decision((lvl, version_index, _)) => { + write!(f, "Decision: level {}, v = {}", lvl, version_index.get()) } Self::Derivations(term) => write!(f, "Derivations term: {}", term), } @@ -150,16 +150,16 @@ impl Display for AssignmentsIntersection { } #[derive(Clone, Debug)] -pub(crate) enum SatisfierSearch { +pub(crate) enum SatisfierSearch { DifferentDecisionLevels { previous_satisfier_level: DecisionLevel, }, SameDecisionLevels { - satisfier_cause: IncompId, + satisfier_cause: IncompId, }, } -type SatisfiedMap = SmallMap>, u32, DecisionLevel)>; +type SatisfiedMap = SmallMap>, u32, DecisionLevel)>; impl PartialSolution { /// Initialize an empty PartialSolution. @@ -175,12 +175,7 @@ impl PartialSolution { } /// Add a decision. - pub(crate) fn add_decision( - &mut self, - package_id: PackageId, - version: DP::V, - package_store: &PackageArena, - ) { + pub(crate) fn add_decision(&mut self, package_id: PackageId, version_index: VersionIndex) { // Check that add_decision is never used in the wrong context. if cfg!(debug_assertions) { match self.package_assignments.get_mut(&package_id) { @@ -191,10 +186,9 @@ impl PartialSolution { // Cannot be called if the versions is not contained in the terms' intersection. AssignmentsIntersection::Derivations(term) => { debug_assert!( - term.contains(&version), - "{}: {} was expected to be contained in {}", - package_store.pkg(package_id).unwrap(), - version, + term.contains(version_index), + "{} was expected to be contained in {}", + version_index.get(), term, ) } @@ -214,8 +208,8 @@ impl PartialSolution { pa.highest_decision_level = self.current_decision_level; pa.assignments_intersection = AssignmentsIntersection::Decision(( self.next_global_index, - version.clone(), - Term::exact(version), + version_index, + Term::exact(version_index), )); // Maintain that the beginning of the `package_assignments` Have all decisions in sorted order. if new_idx != old_idx { @@ -229,7 +223,7 @@ impl PartialSolution { &mut self, package_id: PackageId, cause: IncompDpId, - store: &Arena>, + store: &Arena>, ) { use indexmap::map::Entry; let mut dated_derivation = DatedDerivation { @@ -251,8 +245,8 @@ impl PartialSolution { panic!("add_derivation should not be called after a decision") } AssignmentsIntersection::Derivations(t) => { - *t = t.intersection(&dated_derivation.accumulated_intersection); - dated_derivation.accumulated_intersection = t.clone(); + *t = t.intersection(dated_derivation.accumulated_intersection); + dated_derivation.accumulated_intersection = *t; if t.is_positive() { // we can use `swap_indices` to make `changed_this_decision_level` only go down by 1 // but the copying is slower then the larger search @@ -264,7 +258,7 @@ impl PartialSolution { pa.dated_derivations.push(dated_derivation); } Entry::Vacant(v) => { - let term = dated_derivation.accumulated_intersection.clone(); + let term = dated_derivation.accumulated_intersection; if term.is_positive() { self.changed_this_decision_level = std::cmp::min(self.changed_this_decision_level, pa_last_index); @@ -281,7 +275,7 @@ impl PartialSolution { pub(crate) fn pick_highest_priority_pkg( &mut self, - mut prioritizer: impl FnMut(PackageId, &DP::VS) -> DP::Priority, + mut prioritizer: impl FnMut(PackageId, VersionSet) -> DP::Priority, ) -> Option { let check_all = self.changed_this_decision_level == self.current_decision_level.0.saturating_sub(1) as usize; @@ -318,8 +312,8 @@ impl PartialSolution { .package_assignments .iter() .take(self.current_decision_level.0 as usize) - .map(|(&pid, pa)| match &pa.assignments_intersection { - AssignmentsIntersection::Decision((_, v, _)) => (pid, v), + .map(|(&pid, pa)| match pa.assignments_intersection { + AssignmentsIntersection::Decision((_, version_index, _)) => (pid, version_index), AssignmentsIntersection::Derivations(_) => { panic!("Derivations in the Decision part") } @@ -329,7 +323,7 @@ impl PartialSolution { package_store .into_iter() .enumerate() - .filter_map(|(i, p)| used.get(&PackageId(i as u32)).map(|&v| (p, v.clone()))) + .filter_map(|(i, p)| used.get(&PackageId(i as u32)).map(|&v| (p, v))) .collect() } @@ -365,7 +359,7 @@ impl PartialSolution { // Reset the assignments intersection. pa.assignments_intersection = - AssignmentsIntersection::Derivations(last.accumulated_intersection.clone()); + AssignmentsIntersection::Derivations(last.accumulated_intersection); true } }); @@ -383,28 +377,29 @@ impl PartialSolution { pub(crate) fn add_version( &mut self, package_id: PackageId, - version: DP::V, - new_incompatibilities: std::ops::Range>, - store: &Arena>, + version_index: VersionIndex, + new_incompatibilities: std::ops::Range>, + store: &Arena>, package_store: &PackageArena, + dependency_provider: &DP, ) { if !self.has_ever_backtracked { // Nothing has yet gone wrong during this resolution. This call is unlikely to be the first problem. // So let's live with a little bit of risk and add the decision without checking the dependencies. // The worst that can happen is we will have to do a full backtrack which only removes this one decision. log::info!( - "add_decision: {} @ {} without checking dependencies", - package_store.pkg(package_id).unwrap(), - version, + "add_decision: {} without checking dependencies", + dependency_provider + .package_version_display(package_store.pkg(package_id).unwrap(), version_index) ); - self.add_decision(package_id, version, package_store); + self.add_decision(package_id, version_index); } else { // Check if any of the new dependencies preclude deciding on this crate version. - let exact = Term::exact(version.clone()); - let not_satisfied = |incompat: &Incompatibility| { + let exact = Term::exact(version_index); + let not_satisfied = |incompat: &Incompatibility| { incompat.relation(|pid| { if pid == package_id { - Some(&exact) + Some(exact) } else { self.term_intersection_for_package(pid) } @@ -415,31 +410,32 @@ impl PartialSolution { // would create a conflict (be satisfied). if store[new_incompatibilities].iter().all(not_satisfied) { log::info!( - "add_decision: {} @ {}", - package_store.pkg(package_id).unwrap(), - version + "add_decision: {}", + dependency_provider.package_version_display( + package_store.pkg(package_id).unwrap(), + version_index + ) ); - self.add_decision(package_id, version, package_store); + self.add_decision(package_id, version_index); } else { log::info!( - "not adding {} @ {} because of its dependencies", - package_store.pkg(package_id).unwrap(), - version + "not adding {} because of its dependencies", + dependency_provider.package_version_display( + package_store.pkg(package_id).unwrap(), + version_index + ) ); } } } /// Check if the terms in the partial solution satisfy the incompatibility. - pub(crate) fn relation(&self, incompat: &Incompatibility) -> Relation { + pub(crate) fn relation(&self, incompat: &Incompatibility) -> Relation { incompat.relation(|package_id| self.term_intersection_for_package(package_id)) } /// Retrieve intersection of terms related to package. - pub(crate) fn term_intersection_for_package( - &self, - package_id: PackageId, - ) -> Option<&Term> { + pub(crate) fn term_intersection_for_package(&self, package_id: PackageId) -> Option { self.package_assignments .get(&package_id) .map(|pa| pa.assignments_intersection.term()) @@ -449,10 +445,10 @@ impl PartialSolution { #[allow(clippy::type_complexity)] pub(crate) fn satisfier_search( &self, - incompat: &Incompatibility, - store: &Arena>, + incompat: &Incompatibility, + store: &Arena>, package_store: &PackageArena, - ) -> (PackageId, SatisfierSearch) { + ) -> (PackageId, SatisfierSearch) { let satisfied_map = Self::find_satisfier(incompat, &self.package_assignments, package_store); let (&satisfier_package, &(satisfier_cause, _, satisfier_decision_level)) = satisfied_map @@ -490,16 +486,16 @@ impl PartialSolution { /// to return a coherent previous_satisfier_level. #[allow(clippy::type_complexity)] fn find_satisfier( - incompat: &Incompatibility, - package_assignments: &FxIndexMap>, + incompat: &Incompatibility, + package_assignments: &FxIndexMap>, package_store: &PackageArena, - ) -> SatisfiedMap { + ) -> SatisfiedMap { let mut satisfied = SmallMap::Empty; for (package_id, incompat_term) in incompat.iter() { let pa = package_assignments.get(&package_id).expect("Must exist"); satisfied.insert( package_id, - pa.satisfier::(package_id, &incompat_term.negate(), package_store), + pa.satisfier::(package_id, incompat_term.negate(), package_store), ); } satisfied @@ -510,11 +506,11 @@ impl PartialSolution { /// and including that assignment plus satisfier. #[allow(clippy::type_complexity)] fn find_previous_satisfier( - incompat: &Incompatibility, + incompat: &Incompatibility, satisfier_package_id: PackageId, - mut satisfied_map: SatisfiedMap, - package_assignments: &FxIndexMap>, - store: &Arena>, + mut satisfied_map: SatisfiedMap, + package_assignments: &FxIndexMap>, + store: &Arena>, package_store: &PackageArena, ) -> DecisionLevel { // First, let's retrieve the previous derivations and the initial accum_term. @@ -524,9 +520,9 @@ impl PartialSolution { let accum_term = if let &Some(cause) = satisfier_cause { store[cause].get(satisfier_package_id).unwrap().negate() } else { - match &satisfier_pa.assignments_intersection { + match satisfier_pa.assignments_intersection { AssignmentsIntersection::Derivations(_) => panic!("must be a decision"), - AssignmentsIntersection::Decision((_, _, term)) => term.clone(), + AssignmentsIntersection::Decision((_, _, term)) => term, } }; @@ -538,7 +534,7 @@ impl PartialSolution { satisfier_package_id, satisfier_pa.satisfier::( satisfier_package_id, - &accum_term.intersection(&incompat_term.negate()), + accum_term.intersection(incompat_term.negate()), package_store, ), ); @@ -556,13 +552,13 @@ impl PartialSolution { } } -impl PackageAssignments { +impl PackageAssignments { fn satisfier( &self, package_id: PackageId, - start_term: &Term, + start_term: Term, package_store: &PackageArena, - ) -> (Option>, u32, DecisionLevel) { + ) -> (Option>, u32, DecisionLevel) { let empty = Term::empty(); // Indicate if we found a satisfier in the list of derivations, otherwise it will be the decision. let idx = self @@ -593,10 +589,10 @@ impl PackageAssignments } } -impl AssignmentsIntersection { +impl AssignmentsIntersection { /// Returns the term intersection of all assignments (decision included). - fn term(&self) -> &Term { - match self { + fn term(&self) -> Term { + match *self { Self::Decision((_, _, term)) => term, Self::Derivations(term) => term, } @@ -606,7 +602,7 @@ impl AssignmentsIntersection { /// selected version (no "decision") /// and if it contains at least one positive derivation term /// in the partial solution. - fn potential_package_filter(&self, package_id: PackageId) -> Option<(PackageId, &VS)> { + fn potential_package_filter(&self, package_id: PackageId) -> Option<(PackageId, VersionSet)> { match self { Self::Decision(_) => None, Self::Derivations(term_intersection) => { diff --git a/src/lib.rs b/src/lib.rs index ea62c283..9d84a536 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -46,7 +46,7 @@ //! dependency_provider.add_dependencies("icons", 1u32, []); //! //! // Run the algorithm. -//! let solution = resolve(&mut dependency_provider, &"root", 1u32).unwrap(); +//! let solution = dependency_provider.resolve(&"root", 1u32).unwrap(); //! ``` //! //! # DependencyProvider trait @@ -61,22 +61,21 @@ //! and [SemanticVersion] for versions. //! This may be done quite easily by implementing the following functions. //! ``` -//! # use pubgrub::{DependencyProvider, Dependencies, SemanticVersion, Ranges, DependencyConstraints, Map, PackageId, PackageArena}; +//! # use pubgrub::{DependencyProvider, Dependencies, Ranges, DependencyConstraints, Map, PackageId, PackageArena, VersionIndex, VersionSet}; //! # use std::error::Error; +//! # use std::fmt::Display; //! # use std::borrow::Borrow; //! # use std::convert::Infallible; //! # //! # struct MyDependencyProvider; //! # -//! type SemVS = Ranges; -//! //! impl DependencyProvider for MyDependencyProvider { //! fn choose_version( //! &mut self, //! package_id: PackageId, -//! range: &SemVS, +//! set: VersionSet, //! package_store: &PackageArena, -//! ) -> Result, Infallible> { +//! ) -> Result, Infallible> { //! unimplemented!() //! } //! @@ -84,7 +83,7 @@ //! fn prioritize( //! &mut self, //! package_id: PackageId, -//! range: &SemVS, +//! set: VersionSet, //! package_store: &PackageArena, //! ) -> Self::Priority { //! unimplemented!() @@ -93,17 +92,25 @@ //! fn get_dependencies( //! &mut self, //! package_id: PackageId, -//! version: &SemanticVersion, +//! version_index: VersionIndex, //! package_store: &mut PackageArena, -//! ) -> Result, Infallible> { +//! ) -> Result, Infallible> { //! Ok(Dependencies::Available(DependencyConstraints::default())) //! } //! //! type Err = Infallible; //! type P = String; -//! type V = SemanticVersion; -//! type VS = SemVS; //! type M = String; +//! +//! fn package_version_display<'a>(&'a self, package: &'a Self::P, version_index: VersionIndex) -> impl Display + 'a { +//! let s: String = unimplemented!(); +//! s +//! } +//! +//! fn package_version_set_display<'a>(&'a self, package: &'a Self::P, version_set: VersionSet) -> impl Display + 'a { +//! let s: String = unimplemented!(); +//! s +//! } //! } //! ``` //! @@ -157,22 +164,23 @@ //! //! This crate defines the following [Reporter] trait: //! ``` -//! # use pubgrub::{DerivationTree, DependencyProvider, NoSolutionError, ReportFormatter, VersionSet}; +//! # use pubgrub::{DerivationTree, DependencyProvider, NoSolutionError, ReportFormatter}; //! # use std::fmt::{Debug, Display}; //! # -//! pub trait Reporter { +//! pub trait Reporter { //! /// Output type of the report. //! type Output; //! //! /// Generate a report from the error //! /// describing the resolution failure using the default formatter. -//! fn report(error: &NoSolutionError) -> Self::Output; +//! fn report(error: &NoSolutionError, dependency_provider: &DP) -> Self::Output; //! //! /// Generate a report from the error //! /// describing the resolution failure using a custom formatter. //! fn report_with_formatter( //! error: &NoSolutionError, //! formatter: &impl ReportFormatter, +//! dependency_provider: &DP, //! ) -> Self::Output; //! } //! ``` @@ -191,11 +199,11 @@ //! # let root_package = "root"; //! # let root_version = 1u32; //! # -//! match resolve(&mut dependency_provider, &root_package, root_version) { +//! match dependency_provider.resolve(&root_package, root_version) { //! Ok(solution) => println!("{:?}", solution), //! Err(PubGrubError::NoSolution(mut error)) => { //! error.derivation_tree.collapse_no_versions(); -//! eprintln!("{}", DefaultStringReporter::report(&error)); +//! eprintln!("{}", DefaultStringReporter::report(&error, &dependency_provider)); //! } //! Err(err) => panic!("{:?}", err), //! }; @@ -222,31 +230,33 @@ #![warn(missing_docs)] mod error; +mod helpers; mod package; mod provider; mod report; +mod semantic; mod solver; mod term; mod type_aliases; mod version; -mod version_set; pub use error::{NoSolutionError, PubGrubError}; +pub use helpers::PackageVersionWrapper; pub use package::{PackageArena, PackageId}; -pub use provider::OfflineDependencyProvider; +pub use provider::{OfflineDependencyProvider, VersionRanges}; pub use report::{ DefaultStringReportFormatter, DefaultStringReporter, DerivationTree, Derived, External, ReportFormatter, Reporter, }; +pub use semantic::{SemanticVersion, VersionParseError}; pub use solver::{resolve, Dependencies, DependencyProvider}; pub use term::Term; pub use type_aliases::{ DependencyConstraints, FxIndexMap, FxIndexSet, Map, SelectedDependencies, Set, }; -pub use version::{SemanticVersion, VersionParseError}; +pub use version::{VersionIndex, VersionSet}; pub use version_ranges::Ranges; #[deprecated(note = "Use `Ranges` instead")] pub use version_ranges::Ranges as Range; -pub use version_set::VersionSet; mod internal; diff --git a/src/provider.rs b/src/provider.rs index 5857c878..4dfeb1a3 100644 --- a/src/provider.rs +++ b/src/provider.rs @@ -1,29 +1,122 @@ +use std::borrow::Borrow; use std::cmp::Reverse; -use std::collections::BTreeMap; use std::convert::Infallible; use std::fmt::{Debug, Display}; use std::hash::Hash; +use std::ops::Bound; -use crate::{Dependencies, DependencyProvider, Map, PackageArena, PackageId, VersionSet}; +use crate::{ + resolve, Dependencies, DependencyProvider, FxIndexSet, Map, PackageArena, PackageId, + PackageVersionWrapper, PubGrubError, Ranges, VersionIndex, VersionSet, +}; + +/// Version range. +pub trait VersionRanges: Debug + Display { + /// Associated version type. + type V: Debug + Display + Clone + Ord; + + /// Returns true if this version range contains the specified value. + fn contains(&self, version: &Self::V) -> bool; + + /// Returns true if this version range contains the specified values. + fn contains_many<'s, I, BV>(&'s self, versions: I) -> impl Iterator + 's + where + I: IntoIterator + 's, + BV: Borrow + 's; + + /// Returns the bounding range of this version range. + #[allow(clippy::type_complexity)] + fn bounding_range(&self) -> Option<(Bound<&Self::V>, Bound<&Self::V>)>; + + /// Returns a version range for the provided ordered versions. + fn from_ordered_versions(versions: impl IntoIterator + Clone) -> Self; +} + +impl VersionRanges for Ranges { + type V = V; + + fn contains(&self, version: &Self::V) -> bool { + self.contains(version) + } + + fn contains_many<'s, I, BV>(&'s self, versions: I) -> impl Iterator + 's + where + I: IntoIterator + 's, + BV: Borrow + 's, + { + self.contains_many(versions.into_iter()) + } + + fn bounding_range(&self) -> Option<(Bound<&Self::V>, Bound<&Self::V>)> { + self.bounding_range() + } + + fn from_ordered_versions(versions: impl IntoIterator + Clone) -> Self { + let all_iter = versions.clone().into_iter(); + let versions = versions.into_iter(); + let mut range = Ranges::empty(); + for (v, ok) in versions { + if ok { + range = range.union(&Ranges::singleton(v)); + } + } + range.simplify(all_iter.map(|(v, _)| v)) + } +} /// A basic implementation of [DependencyProvider]. #[derive(Debug, Clone, Default)] -#[cfg_attr( - feature = "serde", - derive(serde::Serialize, serde::Deserialize), - serde( - transparent, - bound( - serialize = "P: serde::Serialize, VS::V: serde::Serialize, VS: serde::Serialize", - deserialize = "P: serde::Deserialize<'de>, VS::V: serde::Deserialize<'de>, VS: serde::Deserialize<'de>", - ) - ) -)] -pub struct OfflineDependencyProvider { - dependencies: Map>>, +pub struct OfflineDependencyProvider { + #[allow(clippy::type_complexity)] + dependencies: Map)>>, +} + +#[cfg(feature = "serde")] +impl serde::Serialize + for OfflineDependencyProvider +where + P: serde::Serialize, + R::V: serde::Serialize, + R: serde::Serialize, +{ + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + use std::collections::BTreeMap; + + self.dependencies + .iter() + .map(|(p, versions)| (p, versions.iter().map(|(v, dmap)| (v, dmap)).collect())) + .collect::>>>() + .serialize(serializer) + } +} + +#[cfg(feature = "serde")] +impl<'de, P: Debug + Display + Clone + Eq + Hash, R: VersionRanges> serde::Deserialize<'de> + for OfflineDependencyProvider +where + P: serde::Deserialize<'de>, + R::V: serde::Deserialize<'de>, + R: serde::Deserialize<'de>, +{ + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + use std::collections::BTreeMap; + + Ok(Self { + dependencies: >>>::deserialize(deserializer)? + .into_iter() + .map(|(p, versions)| (p, versions.into_iter().collect())) + .collect(), + }) + } } -impl OfflineDependencyProvider { +impl OfflineDependencyProvider { /// Creates an empty OfflineDependencyProvider with no dependencies. pub fn new() -> Self { Self { @@ -41,18 +134,19 @@ impl OfflineDependencyPr /// 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, + version: impl Into, dependencies: I, ) { - *self - .dependencies - .entry(package) - .or_default() - .entry(version.into()) - .or_default() = dependencies.into_iter().collect(); + let version = version.into(); + let pkg_deps = self.dependencies.entry(package).or_default(); + + match pkg_deps.binary_search_by(|(v, _)| v.cmp(&version)) { + Ok(idx) => pkg_deps[idx].1 = dependencies.into_iter().collect(), + Err(idx) => pkg_deps.insert(idx, (version, dependencies.into_iter().collect())), + } } /// Lists packages that have been saved. @@ -62,13 +156,72 @@ impl OfflineDependencyPr /// Lists versions of saved packages in sorted order. /// Returns [None] if no information is available regarding that package. - pub fn versions(&self, p: &P) -> Option + Clone> { - Some(self.dependencies.get(p)?.keys()) + pub fn versions(&self, p: &P) -> Option + Clone> { + Some(self.dependencies.get(p)?.iter().map(|(v, _)| v)) } /// Lists dependencies of a given package and version. - pub fn dependencies(&self, p: &P, v: &VS::V) -> Option<&Map> { - self.dependencies.get(p)?.get(v) + pub fn dependencies(&self, p: &P, v: &R::V) -> Option<&Map> { + let pkg_deps = self.dependencies.get(p)?; + pkg_deps + .binary_search_by(|(version, _)| version.cmp(v)) + .map(|idx| &pkg_deps[idx].1) + .ok() + } + + /// Returns the resolve parameters from a root package and version. + pub fn resolve_parameters( + &self, + p: P, + v: impl Into, + ) -> Option<(PackageVersionWrapper

, VersionIndex)> { + let versions = self.dependencies.get(&p)?; + + let v = v.into(); + let true_version_index = self + .dependencies + .get(&p)? + .iter() + .enumerate() + .find(|&(_, (pv, _))| pv == &v) + .map(|(i, _)| i as u64)?; + + let (root_pkg, root_version_index) = PackageVersionWrapper::new_pkg( + p, + true_version_index, + (versions.len() as u64).try_into().unwrap(), + ); + + Some((root_pkg, root_version_index)) + } + + /// Finds a set of packages satisfying dependency bounds for a given package + version pair. + pub fn resolve( + &mut self, + p: P, + v: impl Into, + ) -> Result, PubGrubError> { + let (root_pkg, root_version_index) = self + .resolve_parameters(p, v) + .ok_or_else(|| PubGrubError::NoRoot)?; + + let res = resolve(self, root_pkg, root_version_index)?; + + Ok(res + .into_iter() + .filter_map(|(wrapper, version_index)| { + let (pkg, true_version_index) = wrapper.into_inner(version_index)?; + + let version = self + .dependencies + .get(&pkg) + .into_iter() + .flat_map(|versions| versions.iter().map(|(v, _)| v)) + .nth(true_version_index as usize)?; + + Some((pkg, version.clone())) + }) + .collect()) } } @@ -77,12 +230,10 @@ impl OfflineDependencyPr /// Currently packages are picked with the fewest versions contained in the constraints first. /// But, that may change in new versions if better heuristics are found. /// Versions are picked with the newest versions first. -impl DependencyProvider - for OfflineDependencyProvider +impl DependencyProvider + for OfflineDependencyProvider { - type P = P; - type V = VS::V; - type VS = VS; + type P = PackageVersionWrapper

; type M = &'static str; type Err = Infallible; @@ -90,14 +241,11 @@ impl DependencyProvider #[inline] fn choose_version( &mut self, - package_id: PackageId, - range: &VS, - package_store: &PackageArena, - ) -> Result, Infallible> { - Ok(package_store - .pkg(package_id) - .and_then(|p| self.dependencies.get(p)) - .and_then(|versions| versions.keys().rev().find(|v| range.contains(v)).cloned())) + _: PackageId, + set: VersionSet, + _: &PackageArena, + ) -> Result, Infallible> { + Ok(set.last()) } type Priority = Reverse; @@ -106,39 +254,149 @@ impl DependencyProvider fn prioritize( &mut self, package_id: PackageId, - range: &VS, - package_store: &PackageArena, + set: VersionSet, + _: &PackageArena, ) -> Self::Priority { - let count = package_store - .pkg(package_id) - .and_then(|p| self.dependencies.get(p)) - .map(|versions| versions.keys().filter(|v| range.contains(v)).count()) - .unwrap_or(0); - - Reverse(((count as u64) << 32) + package_id.0 as u64) + Reverse(((set.count() as u64) << 32) + package_id.0 as u64) } #[inline] fn get_dependencies( &mut self, package_id: PackageId, - version: &VS::V, + version_index: VersionIndex, package_store: &mut PackageArena, - ) -> Result, Infallible> { + ) -> Result, Infallible> { + let mut dep_map = Map::default(); + + let wrapper = package_store.pkg(package_id).unwrap(); + let inner = wrapper.inner(version_index).map(|(p, v)| (p.clone(), v)); + + if let Some((d, vs)) = wrapper.dependency(version_index) { + dep_map.insert(package_store.insert(d), vs); + } + + let Some((pkg, true_version_index)) = inner else { + return Ok(Dependencies::Available(dep_map)); + }; + let msg = "dependencies could not be determined"; - let Some(deps) = self - .dependencies - .get(package_store.pkg(package_id).unwrap()) - .and_then(|d| d.get(version)) + let Some(pkg_deps) = self.dependencies.get(&pkg) else { + return Ok(Dependencies::Unavailable(msg)); + }; + let Some(deps) = pkg_deps + .iter() + .map(|(_, dmap)| dmap) + .nth(true_version_index as usize) else { return Ok(Dependencies::Unavailable(msg)); }; - Ok(Dependencies::Available( - deps.iter() - .map(|(dep, r)| (package_store.insert(dep.clone()), r.clone())) - .collect(), - )) + for (dep, r) in deps { + let empty_vec = vec![]; + let versions = self.dependencies.get(dep).unwrap_or(&empty_vec); + let version_count = versions.len() as u64; + let dep = dep.clone(); + + let (d, vs) = match r.bounding_range() { + None => PackageVersionWrapper::new_empty_dep(dep), + Some((Bound::Unbounded, Bound::Unbounded)) => { + PackageVersionWrapper::new_dep(dep, 0..version_count, version_count) + } + Some((Bound::Included(start), Bound::Included(end))) if start == end => { + if let Ok(idx) = versions.binary_search_by(|(v, _)| v.cmp(start)) { + PackageVersionWrapper::new_singleton_dep(dep, idx as u64, version_count) + } else { + PackageVersionWrapper::new_empty_dep(dep) + } + } + Some((start, end)) => { + let start_idx = match start { + Bound::Unbounded => 0, + Bound::Included(start) | Bound::Excluded(start) => { + versions.partition_point(|(v, _)| v < start) + } + }; + let end_idx = match end { + Bound::Unbounded => version_count as usize, + Bound::Included(end) | Bound::Excluded(end) => { + versions.partition_point(|(v, _)| v <= end) + } + }; + + let true_version_indices = r + .contains_many(versions[start_idx..end_idx].iter().map(|(v, _)| v)) + .enumerate() + .filter(|&(_, ok)| ok) + .map(|(i, _)| (start_idx + i) as u64); + + PackageVersionWrapper::new_dep(dep, true_version_indices, version_count) + } + }; + + dep_map.insert(package_store.insert(d), vs); + } + + Ok(Dependencies::Available(dep_map)) + } + + fn package_version_display<'a>( + &'a self, + package: &'a Self::P, + version_index: VersionIndex, + ) -> impl Display + 'a { + match package.inner(version_index) { + None => format!("{package} @ {}", version_index.get()), + Some((pkg, true_version_index)) => { + if let Some(version) = self + .dependencies + .get(pkg) + .into_iter() + .flat_map(|versions| versions.iter().map(|(v, _)| v)) + .nth(true_version_index as usize) + { + format!("{pkg} @ {version}") + } else { + format!("{pkg} @ ") + } + } + } + } + + fn package_version_set_display<'a>( + &'a self, + package: &'a Self::P, + version_set: VersionSet, + ) -> impl Display + 'a { + match package.inner_pkg() { + Some(p) => { + let true_version_indices = version_set + .iter() + .map(|v| package.inner(v).unwrap().1 as usize) + .collect::>(); + + let versions = self + .dependencies + .get(p) + .map(|versions| { + R::from_ordered_versions(versions.iter().enumerate().map(|(i, (v, _))| { + let ok = true_version_indices.contains(&i); + (v.clone(), ok) + })) + }) + .unwrap_or_else(|| R::from_ordered_versions([])); + + format!("{p} @ {versions}") + } + _ => { + let version_indices = version_set + .iter() + .map(|version_index| version_index.get()) + .collect::>(); + + format!("{package} @ {version_indices:?}") + } + } } } diff --git a/src/report.rs b/src/report.rs index 53bc985e..421a78b6 100644 --- a/src/report.rs +++ b/src/report.rs @@ -8,7 +8,8 @@ use std::ops::Deref; use std::sync::Arc; use crate::{ - DependencyProvider, Map, NoSolutionError, PackageArena, PackageId, Set, Term, VersionSet, + DependencyProvider, Map, NoSolutionError, PackageArena, PackageId, Set, Term, VersionIndex, + VersionSet, }; /// Reporter trait. @@ -17,44 +18,45 @@ pub trait Reporter { type Output; /// Generate a report from the error describing the resolution failure using the default formatter. - fn report(error: &NoSolutionError) -> Self::Output; + fn report(error: &NoSolutionError, dependency_provider: &DP) -> Self::Output; /// Generate a report from the error describing the resolution failure using a custom formatter. fn report_with_formatter( error: &NoSolutionError, formatter: &impl ReportFormatter, + dependency_provider: &DP, ) -> Self::Output; } /// Derivation tree resulting in the impossibility /// to solve the dependencies of our root package. #[derive(Debug, Clone)] -pub enum DerivationTree { +pub enum DerivationTree { /// External incompatibility. - External(External), + External(External), /// Incompatibility derived from two others. - Derived(Derived), + Derived(Derived), } /// Incompatibilities that are not derived from others, /// they have their own reason. #[derive(Debug, Clone)] -pub enum External { +pub enum External { /// Initial incompatibility aiming at picking the root package for the first decision. - NotRoot(PackageId, VS::V), + NotRoot(PackageId, VersionIndex), /// There are no versions in the given set for this package. - NoVersions(PackageId, VS), + NoVersions(PackageId, VersionSet), /// Incompatibility coming from the dependencies of a given package. - FromDependencyOf(PackageId, VS, PackageId, VS), + FromDependencyOf(PackageId, VersionSet, PackageId, VersionSet), /// The package is unusable for reasons outside pubgrub. - Custom(PackageId, VS, M), + Custom(PackageId, VersionSet, M), } /// Incompatibility derived from two others. #[derive(Debug, Clone)] -pub struct Derived { +pub struct Derived { /// 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. @@ -62,12 +64,12 @@ pub struct Derived { /// and refer to the explanation for the other times. pub shared_id: Option, /// First cause. - pub cause1: Arc>, + pub cause1: Arc>, /// Second cause. - pub cause2: Arc>, + pub cause2: Arc>, } -impl DerivationTree { +impl DerivationTree { /// Get all packages referred to in the derivation tree. pub fn packages(&self) -> Set { let mut packages = Set::default(); @@ -132,7 +134,7 @@ impl DerivationTree { } } - fn merge_no_versions(self, package_id: PackageId, set: VS) -> Option { + fn merge_no_versions(self, package_id: PackageId, set: VersionSet) -> Option { match self { // TODO: take care of the Derived case. // Once done, we can remove the Option. @@ -147,7 +149,7 @@ impl DerivationTree { if pid1 == package_id { Some(DerivationTree::External(External::FromDependencyOf( pid1, - r1.union(&set), + r1.union(set), pid2, r2, ))) @@ -156,7 +158,7 @@ impl DerivationTree { pid1, r1, pid2, - r2.union(&set), + r2.union(set), ))) } } @@ -166,61 +168,91 @@ impl DerivationTree { } } -impl External { +impl External { /// Returns an object implementing `Display` for this `External`. - pub fn display<'a, DP: DependencyProvider>( + pub fn display<'a, DP: DependencyProvider>( &'a self, package_store: &'a PackageArena, + dependency_provider: &'a DP, ) -> ExternalDisplay<'a, DP> { ExternalDisplay { external: self, package_store, + dependency_provider, } } } pub struct ExternalDisplay<'a, DP: DependencyProvider> { - external: &'a External, + external: &'a External, package_store: &'a PackageArena, + dependency_provider: &'a DP, } impl<'a, DP: DependencyProvider> Display for ExternalDisplay<'a, DP> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match *self.external { - External::NotRoot(package_id, ref version) => { + External::NotRoot(package_id, version_index) => { let p = self.package_store.pkg(package_id).unwrap(); - write!(f, "we are solving dependencies of {p} {version}") + write!( + f, + "we are solving dependencies of {}", + self.dependency_provider + .package_version_display(p, version_index), + ) } - External::NoVersions(package_id, ref set) => { + External::NoVersions(package_id, set) => { let p = self.package_store.pkg(package_id).unwrap(); - if set == &DP::VS::full() { + if set == VersionSet::full() { write!(f, "there is no available version for {p}") } else { - write!(f, "there is no version of {p} in {set}") + write!( + f, + "there is no version of {}", + self.dependency_provider.package_version_set_display(p, set), + ) } } - External::Custom(package_id, ref set, ref metadata) => { + External::Custom(package_id, set, ref metadata) => { let p = self.package_store.pkg(package_id).unwrap(); - if set == &DP::VS::full() { + if set == VersionSet::full() { write!(f, "dependencies of {p} are unavailable ({metadata})") } else { write!( f, - "dependencies of {p} at version {set} are unavailable ({metadata})", + "dependencies of {} are unavailable ({metadata})", + self.dependency_provider.package_version_set_display(p, set), ) } } - External::FromDependencyOf(pid, ref set_p, did, ref set_dep) => { + External::FromDependencyOf(pid, set_p, did, set_dep) => { let p = self.package_store.pkg(pid).unwrap(); let d = self.package_store.pkg(did).unwrap(); - if set_p == &DP::VS::full() && set_dep == &DP::VS::full() { + if set_p == VersionSet::full() && set_dep == VersionSet::full() { write!(f, "{p} depends on {d}") - } else if set_p == &DP::VS::full() { - write!(f, "{p} depends on {d} {set_dep}") - } else if set_dep == &DP::VS::full() { - write!(f, "{p} {set_p} depends on {d}") + } else if set_p == VersionSet::full() { + write!( + f, + "{p} depends on {}", + self.dependency_provider + .package_version_set_display(d, set_dep), + ) + } else if set_dep == VersionSet::full() { + write!( + f, + "{} depends on {d}", + self.dependency_provider + .package_version_set_display(p, set_p), + ) } else { - write!(f, "{p} {set_p} depends on {d} {set_dep}") + write!( + f, + "{} depends on {}", + self.dependency_provider + .package_version_set_display(p, set_p), + self.dependency_provider + .package_version_set_display(d, set_dep), + ) } } } @@ -235,24 +267,27 @@ pub trait ReportFormatter { /// Format an [External] incompatibility. fn format_external( &self, - external: &External, + external: &External, package_store: &PackageArena, + dependency_provider: &DP, ) -> Self::Output; /// Format terms of an incompatibility. fn format_terms( &self, - terms: &Map>, + terms: &Map, package_store: &PackageArena, + dependency_provider: &DP, ) -> Self::Output; /// Simplest case, we just combine two external incompatibilities. fn explain_both_external( &self, - external1: &External, - external2: &External, - current_terms: &Map>, + external1: &External, + external2: &External, + current_terms: &Map, package_store: &PackageArena, + dependency_provider: &DP, ) -> Self::Output; /// Both causes have already been explained so we use their refs. @@ -260,11 +295,12 @@ pub trait ReportFormatter { fn explain_both_ref( &self, ref_id1: usize, - derived1: &Derived, + derived1: &Derived, ref_id2: usize, - derived2: &Derived, - current_terms: &Map>, + derived2: &Derived, + current_terms: &Map, package_store: &PackageArena, + dependency_provider: &DP, ) -> Self::Output; /// One cause is derived (already explained so one-line), @@ -273,36 +309,40 @@ pub trait ReportFormatter { fn explain_ref_and_external( &self, ref_id: usize, - derived: &Derived, - external: &External, - current_terms: &Map>, + derived: &Derived, + external: &External, + current_terms: &Map, package_store: &PackageArena, + dependency_provider: &DP, ) -> Self::Output; /// Add an external cause to the chain of explanations. fn and_explain_external( &self, - external: &External, - current_terms: &Map>, + external: &External, + current_terms: &Map, package_store: &PackageArena, + dependency_provider: &DP, ) -> Self::Output; /// Add an already explained incompat to the chain of explanations. fn and_explain_ref( &self, ref_id: usize, - derived: &Derived, - current_terms: &Map>, + derived: &Derived, + current_terms: &Map, package_store: &PackageArena, + dependency_provider: &DP, ) -> Self::Output; /// Add an already explained incompat to the chain of explanations. fn and_explain_prior_and_external( &self, - prior_external: &External, - external: &External, - current_terms: &Map>, + prior_external: &External, + external: &External, + current_terms: &Map, package_store: &PackageArena, + dependency_provider: &DP, ) -> Self::Output; } @@ -315,49 +355,70 @@ impl ReportFormatter for DefaultStringReportFormatte fn format_external( &self, - external: &External, + external: &External, package_store: &PackageArena, + dependency_provider: &DP, ) -> String { - external.display::(package_store).to_string() + external + .display(package_store, dependency_provider) + .to_string() } fn format_terms( &self, - terms: &Map>, + terms: &Map, package_store: &PackageArena, + dependency_provider: &DP, ) -> Self::Output { - let terms_vec: Vec<_> = terms.iter().map(|(&pid, t)| (pid, t)).collect(); + let terms_vec: Vec<_> = terms.iter().map(|(&pid, &t)| (pid, t)).collect(); match *terms_vec.as_slice() { [] => "version solving failed".into(), // TODO: special case when that unique package is root. - [(pid, Term::Positive(range))] => { - let p = package_store.pkg(pid).unwrap(); - format!("{p} {range} is forbidden") - } - [(pid, Term::Negative(range))] => { - let p = package_store.pkg(pid).unwrap(); - format!("{p} {range} is mandatory") + [(pid, term)] => { + let pkg = package_store.pkg(pid).unwrap(); + if term.is_positive() { + format!( + "{} is forbidden", + dependency_provider.package_version_set_display(pkg, term.version_set()), + ) + } else { + format!( + "{} is mandatory", + dependency_provider.package_version_set_display(pkg, term.version_set()), + ) + } } - - [(pid1, Term::Positive(r1)), (pid2, Term::Negative(r2))] => { - ReportFormatter::::format_external( - self, - &External::FromDependencyOf(pid1, r1.clone(), pid2, r2.clone()), + [(pid1, t1), (pid2, t2)] if t1.is_positive() && t2.is_negative() => { + let r1 = t1.version_set(); + let r2 = t2.version_set(); + self.format_external( + &External::FromDependencyOf(pid1, r1, pid2, r2), package_store, + dependency_provider, ) } - [(pid1, Term::Negative(r1)), (pid2, Term::Positive(r2))] => { - ReportFormatter::::format_external( - self, - &External::FromDependencyOf(pid2, r2.clone(), pid1, r1.clone()), + [(pid1, t1), (pid2, t2)] if t1.is_negative() && t2.is_positive() => { + let r1 = t1.version_set(); + let r2 = t2.version_set(); + self.format_external( + &External::FromDependencyOf(pid2, r2, pid1, r1), package_store, + dependency_provider, ) } ref slice => { let str_terms: Vec<_> = slice .iter() - .map(|&(package_id, t)| { - format!("{} {}", package_store.pkg(package_id).unwrap(), t) + .map(|&(pid, t)| { + let pvs = dependency_provider.package_version_set_display( + package_store.pkg(pid).unwrap(), + t.version_set(), + ); + if t.is_positive() { + format!("{pvs}") + } else { + format!("Not ( {pvs} )") + } }) .collect(); str_terms.join(", ") + " are incompatible" @@ -368,17 +429,23 @@ impl ReportFormatter for DefaultStringReportFormatte /// Simplest case, we just combine two external incompatibilities. fn explain_both_external( &self, - external1: &External, - external2: &External, - current_terms: &Map>, + external1: &External, + external2: &External, + current_terms: &Map, package_store: &PackageArena, + dependency_provider: &DP, ) -> String { // TODO: order should be chosen to make it more logical. format!( "Because {} and {}, {}.", - ReportFormatter::::format_external(self, external1, package_store), - ReportFormatter::::format_external(self, external2, package_store), - ReportFormatter::::format_terms(self, current_terms, package_store) + self.format_external(external1, package_store, dependency_provider), + self.format_external(external2, package_store, dependency_provider), + ReportFormatter::::format_terms( + self, + current_terms, + package_store, + dependency_provider + ) ) } @@ -386,20 +453,36 @@ impl ReportFormatter for DefaultStringReportFormatte fn explain_both_ref( &self, ref_id1: usize, - derived1: &Derived, + derived1: &Derived, ref_id2: usize, - derived2: &Derived, - current_terms: &Map>, + derived2: &Derived, + current_terms: &Map, package_store: &PackageArena, + dependency_provider: &DP, ) -> String { // TODO: order should be chosen to make it more logical. format!( "Because {} ({}) and {} ({}), {}.", - ReportFormatter::::format_terms(self, &derived1.terms, package_store), + ReportFormatter::::format_terms( + self, + &derived1.terms, + package_store, + dependency_provider + ), ref_id1, - ReportFormatter::::format_terms(self, &derived2.terms, package_store), + ReportFormatter::::format_terms( + self, + &derived2.terms, + package_store, + dependency_provider + ), ref_id2, - ReportFormatter::::format_terms(self, current_terms, package_store) + ReportFormatter::::format_terms( + self, + current_terms, + package_store, + dependency_provider + ) ) } @@ -409,32 +492,49 @@ impl ReportFormatter for DefaultStringReportFormatte fn explain_ref_and_external( &self, ref_id: usize, - derived: &Derived, - external: &External, - current_terms: &Map>, + derived: &Derived, + external: &External, + current_terms: &Map, package_store: &PackageArena, + dependency_provider: &DP, ) -> String { // TODO: order should be chosen to make it more logical. format!( "Because {} ({}) and {}, {}.", - ReportFormatter::::format_terms(self, &derived.terms, package_store), + ReportFormatter::::format_terms( + self, + &derived.terms, + package_store, + dependency_provider + ), ref_id, - ReportFormatter::::format_external(self, external, package_store), - ReportFormatter::::format_terms(self, current_terms, package_store) + self.format_external(external, package_store, dependency_provider), + ReportFormatter::::format_terms( + self, + current_terms, + package_store, + dependency_provider + ) ) } /// Add an external cause to the chain of explanations. fn and_explain_external( &self, - external: &External, - current_terms: &Map>, + external: &External, + current_terms: &Map, package_store: &PackageArena, + dependency_provider: &DP, ) -> String { format!( "And because {}, {}.", - ReportFormatter::::format_external(self, external, package_store), - ReportFormatter::::format_terms(self, current_terms, package_store) + self.format_external(external, package_store, dependency_provider), + ReportFormatter::::format_terms( + self, + current_terms, + package_store, + dependency_provider + ) ) } @@ -442,31 +542,48 @@ impl ReportFormatter for DefaultStringReportFormatte fn and_explain_ref( &self, ref_id: usize, - derived: &Derived, - current_terms: &Map>, + derived: &Derived, + current_terms: &Map, package_store: &PackageArena, + dependency_provider: &DP, ) -> String { format!( "And because {} ({}), {}.", - ReportFormatter::::format_terms(self, &derived.terms, package_store), + ReportFormatter::::format_terms( + self, + &derived.terms, + package_store, + dependency_provider + ), ref_id, - ReportFormatter::::format_terms(self, current_terms, package_store) + ReportFormatter::::format_terms( + self, + current_terms, + package_store, + dependency_provider + ) ) } /// Add an already explained incompat to the chain of explanations. fn and_explain_prior_and_external( &self, - prior_external: &External, - external: &External, - current_terms: &Map>, + prior_external: &External, + external: &External, + current_terms: &Map, package_store: &PackageArena, + dependency_provider: &DP, ) -> String { format!( "And because {} and {}, {}.", - ReportFormatter::::format_external(self, prior_external, package_store), - ReportFormatter::::format_external(self, external, package_store), - ReportFormatter::::format_terms(self, current_terms, package_store) + self.format_external(prior_external, package_store, dependency_provider), + self.format_external(external, package_store, dependency_provider), + ReportFormatter::::format_terms( + self, + current_terms, + package_store, + dependency_provider + ) ) } } @@ -494,11 +611,12 @@ impl DefaultStringReporter { fn build_recursive>( &mut self, - derived: &Derived, + derived: &Derived, formatter: &F, package_store: &PackageArena, + dependency_provider: &DP, ) { - self.build_recursive_helper(derived, formatter, package_store); + self.build_recursive_helper(derived, formatter, package_store, dependency_provider); if let Some(id) = derived.shared_id { #[allow(clippy::map_entry)] // `add_line_ref` not compatible with proposed fix. if !self.shared_with_ref.contains_key(&id) { @@ -510,9 +628,10 @@ impl DefaultStringReporter { fn build_recursive_helper>( &mut self, - current: &Derived, + current: &Derived, formatter: &F, package_store: &PackageArena, + dependency_provider: &DP, ) { match (current.cause1.deref(), current.cause2.deref()) { (DerivationTree::External(external1), DerivationTree::External(external2)) => { @@ -522,16 +641,31 @@ impl DefaultStringReporter { external2, ¤t.terms, package_store, + dependency_provider, )); } (DerivationTree::Derived(derived), DerivationTree::External(external)) => { // One cause is derived, so we explain this first // then we add the one-line external part // and finally conclude with the current incompatibility. - self.report_one_each(derived, external, ¤t.terms, formatter, package_store); + self.report_one_each( + derived, + external, + ¤t.terms, + formatter, + package_store, + dependency_provider, + ); } (DerivationTree::External(external), DerivationTree::Derived(derived)) => { - self.report_one_each(derived, external, ¤t.terms, formatter, package_store); + self.report_one_each( + derived, + external, + ¤t.terms, + formatter, + package_store, + dependency_provider, + ); } (DerivationTree::Derived(derived1), DerivationTree::Derived(derived2)) => { // This is the most complex case since both causes are also derived. @@ -548,26 +682,39 @@ impl DefaultStringReporter { derived2, ¤t.terms, package_store, + dependency_provider, )), // Otherwise, if one only has a line number reference, // we recursively call the one without reference and then // add the one with reference to conclude. (Some(ref1), None) => { - self.build_recursive(derived2, formatter, package_store); + self.build_recursive( + derived2, + formatter, + package_store, + dependency_provider, + ); self.lines.push(formatter.and_explain_ref( ref1, derived1, ¤t.terms, package_store, + dependency_provider, )); } (None, Some(ref2)) => { - self.build_recursive(derived1, formatter, package_store); + self.build_recursive( + derived1, + formatter, + package_store, + dependency_provider, + ); self.lines.push(formatter.and_explain_ref( ref2, derived2, ¤t.terms, package_store, + dependency_provider, )); } // Finally, if no line reference exists yet, @@ -578,20 +725,36 @@ impl DefaultStringReporter { // recursively call on the second node, // and finally conclude. (None, None) => { - self.build_recursive(derived1, formatter, package_store); + self.build_recursive( + derived1, + formatter, + package_store, + dependency_provider, + ); if derived1.shared_id.is_some() { self.lines.push("".into()); - self.build_recursive(current, formatter, package_store); + self.build_recursive( + current, + formatter, + package_store, + dependency_provider, + ); } else { self.add_line_ref(); let ref1 = self.ref_count; self.lines.push("".into()); - self.build_recursive(derived2, formatter, package_store); + self.build_recursive( + derived2, + formatter, + package_store, + dependency_provider, + ); self.lines.push(formatter.and_explain_ref( ref1, derived1, ¤t.terms, package_store, + dependency_provider, )); } } @@ -606,11 +769,12 @@ impl DefaultStringReporter { /// has already been explained or not. fn report_one_each>( &mut self, - derived: &Derived, - external: &External, - current_terms: &Map>, + derived: &Derived, + external: &External, + current_terms: &Map, formatter: &F, package_store: &PackageArena, + dependency_provider: &DP, ) { match self.line_ref_of(derived.shared_id) { Some(ref_id) => self.lines.push(formatter.explain_ref_and_external( @@ -619,6 +783,7 @@ impl DefaultStringReporter { external, current_terms, package_store, + dependency_provider, )), None => self.report_recurse_one_each( derived, @@ -626,6 +791,7 @@ impl DefaultStringReporter { current_terms, formatter, package_store, + dependency_provider, ), } } @@ -633,41 +799,45 @@ impl DefaultStringReporter { /// Report one derived (without a line ref yet) and one external. fn report_recurse_one_each>( &mut self, - derived: &Derived, - external: &External, - current_terms: &Map>, + derived: &Derived, + external: &External, + current_terms: &Map, formatter: &F, package_store: &PackageArena, + dependency_provider: &DP, ) { match (derived.cause1.deref(), derived.cause2.deref()) { // If the derived cause has itself one external prior cause, // we can chain the external explanations. (DerivationTree::Derived(prior_derived), DerivationTree::External(prior_external)) => { - self.build_recursive(prior_derived, formatter, package_store); + self.build_recursive(prior_derived, formatter, package_store, dependency_provider); self.lines.push(formatter.and_explain_prior_and_external( prior_external, external, current_terms, package_store, + dependency_provider, )); } // If the derived cause has itself one external prior cause, // we can chain the external explanations. (DerivationTree::External(prior_external), DerivationTree::Derived(prior_derived)) => { - self.build_recursive(prior_derived, formatter, package_store); + self.build_recursive(prior_derived, formatter, package_store, dependency_provider); self.lines.push(formatter.and_explain_prior_and_external( prior_external, external, current_terms, package_store, + dependency_provider, )); } _ => { - self.build_recursive(derived, formatter, package_store); + self.build_recursive(derived, formatter, package_store, dependency_provider); self.lines.push(formatter.and_explain_external( external, current_terms, package_store, + dependency_provider, )); } } @@ -691,15 +861,20 @@ impl DefaultStringReporter { impl Reporter for DefaultStringReporter { type Output = String; - fn report(error: &NoSolutionError) -> Self::Output { + fn report(error: &NoSolutionError, dependency_provider: &DP) -> Self::Output { let formatter = DefaultStringReportFormatter; match &error.derivation_tree { DerivationTree::External(external) => { - ReportFormatter::::format_external(&formatter, external, &error.package_store) + formatter.format_external(external, &error.package_store, dependency_provider) } DerivationTree::Derived(derived) => { let mut reporter = Self::new(); - reporter.build_recursive::(derived, &formatter, &error.package_store); + reporter.build_recursive( + derived, + &formatter, + &error.package_store, + dependency_provider, + ); reporter.lines.join("\n") } } @@ -708,14 +883,20 @@ impl Reporter for DefaultStringReporter { fn report_with_formatter( error: &NoSolutionError, formatter: &impl ReportFormatter, + dependency_provider: &DP, ) -> Self::Output { match &error.derivation_tree { DerivationTree::External(external) => { - ReportFormatter::::format_external(formatter, external, &error.package_store) + formatter.format_external(external, &error.package_store, dependency_provider) } DerivationTree::Derived(derived) => { let mut reporter = Self::new(); - reporter.build_recursive(derived, formatter, &error.package_store); + reporter.build_recursive( + derived, + formatter, + &error.package_store, + dependency_provider, + ); reporter.lines.join("\n") } } diff --git a/src/semantic.rs b/src/semantic.rs new file mode 100644 index 00000000..4c952fb2 --- /dev/null +++ b/src/semantic.rs @@ -0,0 +1,222 @@ +// SPDX-License-Identifier: MPL-2.0 + +//! Semantic version implementation. + +use std::fmt::{self, Debug, Display}; +use std::str::FromStr; + +use thiserror::Error; + +/// 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 &(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 an &version into a version. +impl From<&SemanticVersion> for SemanticVersion { + fn from(v: &SemanticVersion) -> Self { + *v + } +} + +// 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, Eq)] +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) + } +} diff --git a/src/solver.rs b/src/solver.rs index 90347b36..bfd61bd1 100644 --- a/src/solver.rs +++ b/src/solver.rs @@ -37,7 +37,7 @@ //! # let mut dependency_provider = OfflineDependencyProvider::<&str, NumVS>::new(); //! # let package = "root"; //! # let version = 1u32; -//! let solution = resolve(&mut dependency_provider, &package, version)?; +//! let solution = dependency_provider.resolve(&package, version)?; //! # Ok(()) //! # } //! # fn main() { @@ -63,7 +63,7 @@ use log::{debug, info}; use crate::{ internal::{Incompatibility, State}, DependencyConstraints, DerivationTree, External, Map, NoSolutionError, PackageArena, PackageId, - PubGrubError, SelectedDependencies, VersionSet, + PubGrubError, SelectedDependencies, VersionIndex, VersionSet, }; /// Main function of the library. @@ -71,12 +71,12 @@ use crate::{ pub fn resolve( dependency_provider: &mut DP, package: DP::P, - version: impl Into, + version_index: VersionIndex, ) -> Result, PubGrubError> { let mut package_store = PackageArena::new(); let package_id = package_store.insert(package); - let mut state: State = State::init(package_id, version.into()); - let mut added_dependencies: Map> = Map::default(); + let mut state: State = State::init(package_id, version_index); + let mut added_dependencies: Map> = Map::default(); let mut next = package_id; loop { dependency_provider @@ -84,7 +84,7 @@ pub fn resolve( .map_err(PubGrubError::ErrorInShouldCancel)?; info!("unit_propagation: {}", package_store.pkg(next).unwrap()); - match state.unit_propagation(next, &package_store) { + match state.unit_propagation(next, &package_store, dependency_provider) { Ok(t) => t, Err(DerivationTree::External(External::NoVersions(PackageId(0), _))) => { return Err(PubGrubError::NoRoot) @@ -123,107 +123,92 @@ pub fn resolve( .choose_version(next, term_intersection.unwrap_positive(), &package_store) .map_err(PubGrubError::ErrorChoosingPackageVersion)?; - info!( - "DP chose: {} @ {decision:?}", - package_store.pkg(next).unwrap() - ); - // Pick the next compatible version. let v = match decision { None => { - let inc = Incompatibility::no_versions(next, term_intersection.clone()); + info!( + "DP chose: {} (no versions)", + package_store.pkg(next).unwrap() + ); + let inc = Incompatibility::no_versions(next, term_intersection); state.add_incompatibility(inc); continue; } - Some(x) => x, + Some(v) => { + info!( + "DP chose: {}", + dependency_provider + .package_version_display(package_store.pkg(next).unwrap(), v) + ); + v + } }; - if !term_intersection.contains(&v) { + if !term_intersection.contains(v) { return Err(PubGrubError::Failure( "choose_package_version picked an incompatible version".into(), )); } - let is_new_dependency = added_dependencies - .entry(next) - .or_default() - .insert(v.clone()); + let is_new_dependency = added_dependencies.entry(next).or_default().insert(v); if is_new_dependency { // Retrieve that package dependencies. let pid = next; - let dependencies: Dependencies< - ::VS, - ::M, - > = dependency_provider - .get_dependencies(pid, &v, &mut package_store) + let dependencies = dependency_provider + .get_dependencies(pid, v, &mut package_store) .map_err(|err| PubGrubError::ErrorRetrievingDependencies { - package: package_store.pkg(pid).unwrap().clone(), - version: v.clone(), + package_version: dependency_provider + .package_version_display(package_store.pkg(pid).unwrap(), v) + .to_string(), source: err, })?; let dependencies = match dependencies { Dependencies::Unavailable(reason) => { - state.add_incompatibility(Incompatibility::custom_version( - pid, - v.clone(), - reason, - )); + state.add_incompatibility(Incompatibility::custom_version(pid, v, reason)); continue; } Dependencies::Available(x) => x, }; // Add that package and version if the dependencies are not problematic. - let dep_incompats = - state.add_incompatibility_from_dependencies(pid, v.clone(), dependencies); + let dep_incompats = state.add_incompatibility_from_dependencies(pid, v, dependencies); state.partial_solution.add_version( pid, - v.clone(), + v, dep_incompats, &state.incompatibility_store, &package_store, + dependency_provider, ); } else { // `dep_incompats` are already in `incompatibilities` so we know there are not satisfied // terms and can add the decision directly. info!( - "add_decision (not first time): {} @ {}", - package_store.pkg(next).unwrap(), - v + "add_decision (not first time): {}", + dependency_provider.package_version_display(package_store.pkg(next).unwrap(), v) ); - state.partial_solution.add_decision(next, v, &package_store); + state.partial_solution.add_decision(next, v); } } } /// An enum used by [DependencyProvider] that holds information about package dependencies. #[derive(Clone)] -pub enum Dependencies { +pub enum Dependencies { /// Package dependencies are unavailable with the reason why they are missing. Unavailable(M), /// Container for all available package versions. - Available(DependencyConstraints), + Available(DependencyConstraints), } /// 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 { /// How this provider stores the name of the packages. - type P: Debug + Display + Clone + Eq + Hash; - - /// How this provider stores the versions of the packages. - /// - /// A common choice is [`SemanticVersion`][crate::version::SemanticVersion]. - type V: Debug + Display + Clone + Ord; - - /// How this provider stores the version requirements for the packages. - /// The requirements must be able to process the same kind of version as this dependency provider. - /// - /// A common choice is [`Ranges`][version_ranges::Ranges]. - type VS: VersionSet; + type P: Debug + Display + Eq + Hash; /// Type for custom incompatibilities. /// @@ -266,7 +251,7 @@ pub trait DependencyProvider { fn prioritize( &mut self, package_id: PackageId, - range: &Self::VS, + set: VersionSet, package_store: &PackageArena, ) -> Self::Priority; /// The type returned from `prioritize`. The resolver does not care what type this is @@ -287,9 +272,9 @@ pub trait DependencyProvider { fn choose_version( &mut self, package_id: PackageId, - range: &Self::VS, + set: VersionSet, package_store: &PackageArena, - ) -> Result, Self::Err>; + ) -> Result, Self::Err>; /// Retrieves the package dependencies. /// Return [Dependencies::Unavailable] if its dependencies are unavailable. @@ -297,9 +282,9 @@ pub trait DependencyProvider { fn get_dependencies( &mut self, package_id: PackageId, - version: &Self::V, + version_index: VersionIndex, package_store: &mut PackageArena, - ) -> Result, Self::Err>; + ) -> Result, Self::Err>; /// This is called fairly regularly during the resolution, /// if it returns an Err then resolution will be terminated. @@ -309,4 +294,18 @@ pub trait DependencyProvider { fn should_cancel(&mut self) -> Result<(), Self::Err> { Ok(()) } + + /// Get a representation of a package version. + fn package_version_display<'a>( + &'a self, + package: &'a Self::P, + version_index: VersionIndex, + ) -> impl Display + 'a; + + /// Get a representation of a package version set. + fn package_version_set_display<'a>( + &'a self, + package: &'a Self::P, + version_set: VersionSet, + ) -> impl Display + 'a; } diff --git a/src/term.rs b/src/term.rs index 38faf087..8f2766e3 100644 --- a/src/term.rs +++ b/src/term.rs @@ -5,147 +5,157 @@ use std::fmt::{self, Display}; -use crate::VersionSet; +use crate::{VersionIndex, VersionSet}; /// A positive or negative expression regarding a set of versions. /// -/// `Positive(r)` and `Negative(r.complement())` are not equivalent: -/// * the term `Positive(r)` is satisfied if the package is selected AND the selected version is in `r`. -/// * the term `Negative(r.complement())` is satisfied if the package is not selected OR the selected version is in `r`. +/// `Term::positive(vs)` and `Term::negative(vs.complement())` are not equivalent: +/// * `Term::positive(vs)` is satisfied if the package is selected AND the selected version is in `vs`. +/// * `Term::negative(vs.complement())` is satisfied if the package is not selected OR the selected version is in `vs`. /// -/// A `Positive` term in the partial solution requires a version to be selected, but a `Negative` term +/// A positive term in the partial solution requires a version to be selected, but a negative term /// allows for a solution that does not have that package selected. -/// Specifically, `Positive(VS::empty())` means that there was a conflict (we need to select a version for the package -/// but can't pick any), while `Negative(VS::full())` would mean it is fine as long as we don't select the package. -#[derive(Debug, Clone, Eq, PartialEq)] -pub enum Term { +/// Specifically, `Term::positive(VersionSet::empty())` means that there was a conflict +/// (we need to select a version for the package but can't pick any), +/// while `Term::negative(VersionSet::full())` would mean it is fine as long as we don't select the package. +#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)] +#[repr(transparent)] +pub struct Term(u64); + +impl Term { + /// Contruct a positive `Term`. /// 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(VS), - /// The term `not (v < 3.0.0)` is a negative expression + #[inline] + pub(crate) fn positive(vs: VersionSet) -> Self { + Self(vs.0) + } + + /// Contruct a negative `Term`. + /// For example, `not (v < 3.0.0)` is a negative expression /// that is evaluated true if a version >= 3.0.0 is selected /// or if no version is selected at all. - Negative(VS), -} + #[inline] + pub(crate) fn negative(vs: VersionSet) -> Self { + Self(!vs.0) + } -/// Base methods. -impl Term { /// A term that is always true. + #[inline] pub(crate) fn any() -> Self { - Self::Negative(VS::empty()) + Self(!0) } /// A term that is never true. + #[inline] pub(crate) fn empty() -> Self { - Self::Positive(VS::empty()) + Self(0) } /// A positive term containing exactly that version. - pub(crate) fn exact(version: VS::V) -> Self { - Self::Positive(VS::singleton(version)) + #[inline] + pub(crate) fn exact(version_index: VersionIndex) -> Self { + Self::positive(VersionSet::singleton(version_index)) } /// Simply check if a term is positive. - pub(crate) fn is_positive(&self) -> bool { - match self { - Self::Positive(_) => true, - Self::Negative(_) => false, - } + #[inline] + pub fn is_positive(self) -> bool { + self.0 & 1 == 0 + } + + /// Simply check if a term is negative. + #[inline] + pub fn is_negative(self) -> bool { + self.0 & 1 != 0 } /// Negate a term. /// Evaluation of a negated term always returns /// the opposite of the evaluation of the original one. - pub(crate) fn negate(&self) -> Self { - match self { - Self::Positive(set) => Self::Negative(set.clone()), - Self::Negative(set) => Self::Positive(set.clone()), + #[inline] + pub(crate) fn negate(self) -> Self { + Self(!self.0) + } + + /// Get the inner version set. + #[inline] + pub fn version_set(self) -> VersionSet { + if self.is_positive() { + VersionSet(self.0) + } else { + VersionSet(!self.0) } } /// Evaluate a term regarding a given choice of version. - pub(crate) fn contains(&self, v: &VS::V) -> bool { - match self { - Self::Positive(set) => set.contains(v), - Self::Negative(set) => !set.contains(v), - } + #[inline] + pub(crate) fn contains(self, v: VersionIndex) -> bool { + self.0 & VersionSet::singleton(v).0 != 0 } /// Unwrap the set contained in a positive term. /// Will panic if used on a negative set. - pub(crate) fn unwrap_positive(&self) -> &VS { - match self { - Self::Positive(set) => set, - _ => panic!("Negative term cannot unwrap positive set"), + #[inline] + pub(crate) fn unwrap_positive(self) -> VersionSet { + if self.is_positive() { + VersionSet(self.0) + } else { + panic!("Negative term cannot unwrap positive set") } } /// Unwrap the set contained in a negative term. /// Will panic if used on a positive set. - pub(crate) fn unwrap_negative(&self) -> &VS { - match self { - Self::Negative(set) => set, - _ => panic!("Positive term cannot unwrap negative set"), + #[inline] + pub(crate) fn unwrap_negative(self) -> VersionSet { + if self.is_negative() { + VersionSet(!self.0) + } else { + panic!("Positive term cannot unwrap negative set") } } -} -/// Set operations with terms. -impl Term { /// Compute the intersection of two terms. - /// /// The intersection is negative (unselected package is allowed) /// if all terms are negative. - pub(crate) fn intersection(&self, other: &Self) -> Self { - match (self, other) { - (Self::Positive(r1), Self::Positive(r2)) => Self::Positive(r1.intersection(r2)), - (Self::Positive(p), Self::Negative(n)) | (Self::Negative(n), Self::Positive(p)) => { - Self::Positive(n.complement().intersection(p)) - } - (Self::Negative(r1), Self::Negative(r2)) => Self::Negative(r1.union(r2)), - } - } - - /// Check whether two terms are mutually exclusive. - /// - /// An optimization for the native implementation of checking whether the intersection of two sets is empty. - pub(crate) fn is_disjoint(&self, other: &Self) -> bool { - match (self, other) { - (Self::Positive(r1), Self::Positive(r2)) => r1.is_disjoint(r2), - // Unselected package is allowed in both terms, so they are never disjoint. - (Self::Negative(_), Self::Negative(_)) => false, - // If the positive term is a subset of the negative term, it lies fully in the region that the negative - // term excludes. - (Self::Positive(p), Self::Negative(n)) | (Self::Negative(n), Self::Positive(p)) => { - p.subset_of(n) - } - } + #[inline] + pub(crate) fn intersection(self, other: Self) -> Self { + Self(self.0 & other.0) } /// Compute the union of two terms. /// If at least one term is negative, the union is also negative (unselected package is allowed). - pub(crate) fn union(&self, other: &Self) -> Self { - match (self, other) { - (Self::Positive(r1), Self::Positive(r2)) => Self::Positive(r1.union(r2)), - (Self::Positive(p), Self::Negative(n)) | (Self::Negative(n), Self::Positive(p)) => { - Self::Negative(p.complement().intersection(n)) - } - (Self::Negative(r1), Self::Negative(r2)) => Self::Negative(r1.intersection(r2)), - } + #[inline] + pub(crate) fn union(self, other: Self) -> Self { + Self(self.0 | other.0) + } + + /// Check whether two terms are mutually exclusive. + #[inline] + pub(crate) fn is_disjoint(self, other: Self) -> bool { + self.0 & other.0 == 0 } /// 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: &Self) -> bool { - match (self, other) { - (Self::Positive(r1), Self::Positive(r2)) => r1.subset_of(r2), - (Self::Positive(r1), Self::Negative(r2)) => r1.is_disjoint(r2), - // Only a negative term allows the unselected package, - // so it can never be a subset of a positive term. - (Self::Negative(_), Self::Positive(_)) => false, - (Self::Negative(r1), Self::Negative(r2)) => r2.subset_of(r1), + #[inline] + pub(crate) fn subset_of(self, other: Self) -> bool { + self.0 & other.0 == self.0 + } + + /// Check if a set of terms satisfies or contradicts a given term. + /// Otherwise the relation is inconclusive. + #[inline] + pub(crate) fn relation_with(self, other_terms_intersection: Self) -> Relation { + if other_terms_intersection.subset_of(self) { + Relation::Satisfied + } else if other_terms_intersection.is_disjoint(self) { + Relation::Contradicted + } else { + Relation::Inconclusive } } } @@ -165,89 +175,71 @@ pub(crate) enum Relation { Inconclusive, } -/// Relation between terms. -impl Term { - /// Check if a set of terms satisfies this term. - /// - /// We say that a set of terms S "satisfies" a term t - /// if t must be true whenever every term in S is true. - /// - /// 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: &Self) -> bool { - terms_intersection.subset_of(self) - } - - /// Check if a set of terms contradicts this term. - /// - /// We say that a set of terms S "contradicts" a term t - /// if t must be false whenever every term in S is true. - /// - /// It turns out that this can also be expressed with set operations: - /// 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: &Self) -> 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: &Self) -> Relation { - if other_terms_intersection.subset_of(self) { - Relation::Satisfied - } else if self.is_disjoint(other_terms_intersection) { - Relation::Contradicted - } else { - Relation::Inconclusive +impl Display for Term { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if self.is_negative() { + write!(f, "Not ( ")?; } - } -} -impl AsRef for Term { - fn as_ref(&self) -> &Self { - self - } -} - -// REPORT ###################################################################### + let mut list = f.debug_list(); + for v in self.version_set().iter() { + list.entry(&v.get()); + } + list.finish()?; -impl Display for Term { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::Positive(set) => write!(f, "{}", set), - Self::Negative(set) => write!(f, "Not ( {} )", set), + if self.is_negative() { + write!(f, " )")?; } + + Ok(()) } } -// TESTS ####################################################################### - #[cfg(test)] pub mod tests { - use super::*; use proptest::prelude::*; - use version_ranges::Ranges; - pub fn strategy() -> impl Strategy>> { - prop_oneof![ - version_ranges::proptest_strategy().prop_map(Term::Negative), - version_ranges::proptest_strategy().prop_map(Term::Positive), - ] + use super::*; + + impl Term { + /// Check if a set of terms satisfies this term. + /// + /// We say that a set of terms S "satisfies" a term t + /// if t must be true whenever every term in S is true. + /// + /// It turns out that this can also be expressed with set operations: + /// S satisfies t if and only if ⋂ S ⊆ t + fn satisfied_by(self, terms_intersection: Self) -> bool { + terms_intersection.subset_of(self) + } + + /// Check if a set of terms contradicts this term. + /// + /// We say that a set of terms S "contradicts" a term t + /// if t must be false whenever every term in S is true. + /// + /// It turns out that this can also be expressed with set operations: + /// S contradicts t if and only if ⋂ S is disjoint with t + /// S contradicts t if and only if (⋂ S) ⋂ t = ∅ + fn contradicted_by(self, terms_intersection: Self) -> bool { + terms_intersection.intersection(self) == Self::empty() + } } - proptest! { - // Testing relation -------------------------------- + pub fn strategy() -> impl Strategy { + any::().prop_map(Term) + } + proptest! { + /// Testing relation #[test] fn relation_with(term1 in strategy(), term2 in strategy()) { - match term1.relation_with(&term2) { - Relation::Satisfied => assert!(term1.satisfied_by(&term2)), - Relation::Contradicted => assert!(term1.contradicted_by(&term2)), + match term1.relation_with(term2) { + Relation::Satisfied => assert!(term1.satisfied_by(term2)), + Relation::Contradicted => assert!(term1.contradicted_by(term2)), Relation::Inconclusive => { - assert!(!term1.satisfied_by(&term2)); - assert!(!term1.contradicted_by(&term2)); + assert!(!term1.satisfied_by(term2)); + assert!(!term1.contradicted_by(term2)); } } } @@ -256,30 +248,30 @@ pub mod tests { #[test] fn positive_negative(term1 in strategy(), term2 in strategy()) { let intersection_positive = term1.is_positive() || term2.is_positive(); - let union_positive = term1.is_positive() && term2.is_positive(); - assert_eq!(term1.intersection(&term2).is_positive(), intersection_positive); - assert_eq!(term1.union(&term2).is_positive(), union_positive); + let union_positive = term1.is_positive() & term2.is_positive(); + assert_eq!(term1.intersection(term2).is_positive(), intersection_positive); + assert_eq!(term1.union(term2).is_positive(), union_positive); } #[test] fn is_disjoint_through_intersection(r1 in strategy(), r2 in strategy()) { - let disjoint_def = r1.intersection(&r2) == Term::empty(); - assert_eq!(r1.is_disjoint(&r2), disjoint_def); + let disjoint_def = r1.intersection(r2) == Term::empty(); + assert_eq!(r1.is_disjoint(r2), disjoint_def); } #[test] fn subset_of_through_intersection(r1 in strategy(), r2 in strategy()) { - let disjoint_def = r1.intersection(&r2) == r1; - assert_eq!(r1.subset_of(&r2), disjoint_def); + let disjoint_def = r1.intersection(r2) == r1; + assert_eq!(r1.subset_of(r2), disjoint_def); } #[test] fn union_through_intersection(r1 in strategy(), r2 in strategy()) { let union_def = r1 .negate() - .intersection(&r2.negate()) + .intersection(r2.negate()) .negate(); - assert_eq!(r1.union(&r2), union_def); + assert_eq!(r1.union(r2), union_def); } } } diff --git a/src/type_aliases.rs b/src/type_aliases.rs index 2994c60d..8fab6667 100644 --- a/src/type_aliases.rs +++ b/src/type_aliases.rs @@ -2,7 +2,7 @@ //! Publicly exported type aliases. -use crate::{DependencyProvider, PackageId}; +use crate::{DependencyProvider, PackageId, VersionIndex, VersionSet}; /// Map implementation used by the library. pub type Map = rustc_hash::FxHashMap; @@ -18,12 +18,11 @@ pub type FxIndexSet = indexmap::IndexSet; /// Concrete dependencies picked by the library during [resolve](crate::solver::resolve) /// from [DependencyConstraints]. -pub type SelectedDependencies = - Map<::P, ::V>; +pub type SelectedDependencies = Map<::P, VersionIndex>; /// Holds information about all possible versions a given package can accept. /// There is a difference in semantics between an empty map /// inside [DependencyConstraints] and [Dependencies::Unavailable](crate::solver::Dependencies::Unavailable): /// the former means the package has no dependency and it is a known fact, /// while the latter means they could not be fetched by the [DependencyProvider]. -pub type DependencyConstraints = Map; +pub type DependencyConstraints = Map; diff --git a/src/version.rs b/src/version.rs index 445f5d7e..cd63c352 100644 --- a/src/version.rs +++ b/src/version.rs @@ -1,222 +1,124 @@ // SPDX-License-Identifier: MPL-2.0 -//! Traits and implementations to create and compare versions. - -use std::fmt::{self, Debug, Display}; -use std::str::FromStr; - -use thiserror::Error; - -/// 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)) +/// Type for identifying a version index. +#[derive(Debug, Default, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)] +#[repr(transparent)] +pub struct VersionIndex(u8); + +impl VersionIndex { + /// Maximum possible version index. + pub const MAX: u64 = (u64::BITS - 1) as u64; + + /// Constructor for a version index. + #[inline] + pub fn new(v: u8) -> Option { + if v < Self::MAX as u8 { + Some(Self(v)) + } else { + None + } } -} -#[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) + /// Get the inner version index. + #[inline] + pub fn get(self) -> u8 { + self.0 } } -// 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, - } - } +/// Type for identifying a set of version indices. +#[derive(Debug, Default, Copy, Clone, Eq, PartialEq, Hash)] +#[repr(transparent)] +pub struct VersionSet(pub(crate) u64); - /// Version 0.0.0. - pub fn zero() -> Self { - Self::new(0, 0, 0) +impl VersionSet { + /// Constructor for an empty set containing no version index. + #[inline] + pub fn empty() -> Self { + Self(0) } - /// Version 1.0.0. - pub fn one() -> Self { - Self::new(1, 0, 0) + /// Constructor for the set containing all version indices. + #[inline] + pub fn full() -> Self { + Self(u64::MAX & (!1)) } - /// Version 2.0.0. - pub fn two() -> Self { - Self::new(2, 0, 0) + /// Constructor for a set containing exactly one version index. + #[inline] + pub fn singleton(v: VersionIndex) -> Self { + Self(2 << v.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) + /// Compute the complement of this set. + #[inline] + pub fn complement(self) -> Self { + Self((!self.0) & (!1)) } -} -// Convert a &(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) + /// Compute the intersection with another set. + #[inline] + pub fn intersection(self, other: Self) -> Self { + Self(self.0 & other.0) } -} -// Convert an &version into a version. -impl From<&SemanticVersion> for SemanticVersion { - fn from(v: &SemanticVersion) -> Self { - *v + /// Compute the union with another set. + #[inline] + pub fn union(self, other: Self) -> Self { + Self(self.0 | other.0) } -} -// 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) + /// Evaluate membership of a version index in this set. + #[inline] + pub fn contains(self, v: VersionIndex) -> bool { + self.intersection(Self::singleton(v)) != Self::empty() } -} -// 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) + /// Whether the set has no overlapping version indices. + #[inline] + pub fn is_disjoint(self, other: Self) -> bool { + self.intersection(other) == Self::empty() } - /// Bump the minor number of a version. - pub fn bump_minor(self) -> Self { - Self::new(self.major, self.minor + 1, 0) + /// Whether all version indices of `self` are contained in `other`. + #[inline] + pub fn subset_of(self, other: Self) -> bool { + self == self.intersection(other) } - /// Bump the major number of a version. - pub fn bump_major(self) -> Self { - Self::new(self.major + 1, 0, 0) + /// Get an iterator over the version indices contained in the set. + #[inline] + pub fn iter(self) -> impl Iterator { + (0..VersionIndex::MAX) + .filter(move |v| self.0 & (2 << v) != 0) + .map(|v| VersionIndex(v as u8)) } -} -/// Error creating [SemanticVersion] from [String]. -#[derive(Error, Debug, PartialEq, Eq)] -pub enum VersionParseError { - /// [SemanticVersion] must contain major, minor, patch versions. - #[error("version {full_version} must contain 3 numbers separated by dot")] - 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(), - }), + /// Get the first version index of the set. + #[inline] + pub fn first(self) -> Option { + if self != Self::empty() { + Some(VersionIndex((self.0 >> 1).trailing_zeros() as u8)) + } else { + None } } -} -#[test] -fn from_str_for_semantic_version() { - let parse = |str: &str| str.parse::(); - assert!(parse( - &SemanticVersion { - major: 0, - minor: 1, - patch: 0 + /// Get the last version index of the set. + #[inline] + pub fn last(self) -> Option { + if self != Self::empty() { + Some(VersionIndex( + (VersionIndex::MAX - (self.0 >> 1).leading_zeros() as u64) as u8, + )) + } else { + None } - .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) + /// Count the number of version indices contained in the set. + #[inline] + pub fn count(self) -> usize { + self.0.count_ones() as usize } } diff --git a/src/version_set.rs b/src/version_set.rs deleted file mode 100644 index 1c4ac04a..00000000 --- a/src/version_set.rs +++ /dev/null @@ -1,112 +0,0 @@ -// SPDX-License-Identifier: MPL-2.0 - -//! As its name suggests, the [VersionSet] trait describes sets of versions. -//! -//! One needs to define -//! - the associate type for versions, -//! - two constructors for the empty set and a singleton set, -//! - the complement and intersection set operations, -//! - and a function to evaluate membership of versions. -//! -//! Two functions are automatically derived, thanks to the mathematical properties of sets. -//! You can overwrite those implementations, but we highly recommend that you don't, -//! except if you are confident in a correct implementation that brings much performance gains. -//! -//! It is also extremely important that the `Eq` trait is correctly implemented. -//! In particular, you can only use `#[derive(Eq, PartialEq)]` if `Eq` is strictly equivalent to the -//! structural equality, i.e. if version sets have canonical representations. -//! Such problems may arise if your implementations of `complement()` and `intersection()` do not -//! return canonical representations so be careful there. - -use std::fmt::{Debug, Display}; - -use crate::Ranges; - -/// Trait describing sets of versions. -pub trait VersionSet: Debug + Display + Clone + Eq { - /// Version type associated with the sets manipulated. - type V: Debug + Display + Clone + Ord; - - // Constructors - /// Constructor for an empty set containing no version. - fn empty() -> Self; - /// Constructor for a set containing exactly one version. - fn singleton(v: Self::V) -> Self; - - // Operations - /// Compute the complement of this set. - fn complement(&self) -> Self; - /// Compute the intersection with another set. - fn intersection(&self, other: &Self) -> Self; - - // Membership - /// Evaluate membership of a version in this set. - fn contains(&self, v: &Self::V) -> bool; - - // Automatically implemented functions ########################### - - /// Constructor for the set containing all versions. - /// Automatically implemented as `Self::empty().complement()`. - fn full() -> Self { - Self::empty().complement() - } - - /// Compute the union with another set. - /// Thanks to set properties, this is automatically implemented as: - /// `self.complement().intersection(&other.complement()).complement()` - fn union(&self, other: &Self) -> Self { - self.complement() - .intersection(&other.complement()) - .complement() - } - - /// Whether the range have no overlapping segments. - fn is_disjoint(&self, other: &Self) -> bool { - self.intersection(other) == Self::empty() - } - - /// Whether all range of `self` are contained in `other`. - fn subset_of(&self, other: &Self) -> bool { - self == &self.intersection(other) - } -} - -impl VersionSet for Ranges { - type V = T; - - fn empty() -> Self { - Ranges::empty() - } - - fn singleton(v: Self::V) -> Self { - Ranges::singleton(v) - } - - fn complement(&self) -> Self { - Ranges::complement(self) - } - - fn intersection(&self, other: &Self) -> Self { - Ranges::intersection(self, other) - } - - fn contains(&self, v: &Self::V) -> bool { - Ranges::contains(self, v) - } - - fn full() -> Self { - Ranges::full() - } - - fn union(&self, other: &Self) -> Self { - Ranges::union(self, other) - } - - fn is_disjoint(&self, other: &Self) -> bool { - Ranges::is_disjoint(self, other) - } - - fn subset_of(&self, other: &Self) -> bool { - Ranges::subset_of(self, other) - } -} diff --git a/tests/examples.rs b/tests/examples.rs index 8e121a71..0b490a37 100644 --- a/tests/examples.rs +++ b/tests/examples.rs @@ -1,8 +1,8 @@ // SPDX-License-Identifier: MPL-2.0 use pubgrub::{ - resolve, DefaultStringReporter, Map, OfflineDependencyProvider, PubGrubError, Ranges, - Reporter as _, SemanticVersion, Set, + DefaultStringReporter, Map, OfflineDependencyProvider, PubGrubError, Ranges, Reporter as _, + SemanticVersion, Set, }; type NumVS = Ranges; @@ -39,7 +39,7 @@ fn no_conflict() { dependency_provider.add_dependencies("bar", (2, 0, 0), []); // Run the algorithm. - let computed_solution = resolve(&mut dependency_provider, "root", (1, 0, 0)).unwrap(); + let computed_solution = dependency_provider.resolve("root", (1, 0, 0)).unwrap(); // Solution. let mut expected_solution = Map::default(); @@ -75,7 +75,7 @@ fn avoiding_conflict_during_decision_making() { dependency_provider.add_dependencies("bar", (2, 0, 0), []); // Run the algorithm. - let computed_solution = resolve(&mut dependency_provider, "root", (1, 0, 0)).unwrap(); + let computed_solution = dependency_provider.resolve("root", (1, 0, 0)).unwrap(); // Solution. let mut expected_solution = Map::default(); @@ -110,7 +110,7 @@ fn conflict_resolution() { ); // Run the algorithm. - let computed_solution = resolve(&mut dependency_provider, "root", (1, 0, 0)).unwrap(); + let computed_solution = dependency_provider.resolve("root", (1, 0, 0)).unwrap(); // Solution. let mut expected_solution = Map::default(); @@ -168,7 +168,7 @@ fn conflict_with_partial_satisfier() { dependency_provider.add_dependencies("target", (1, 0, 0), []); // Run the algorithm. - let computed_solution = resolve(&mut dependency_provider, "root", (1, 0, 0)).unwrap(); + let computed_solution = dependency_provider.resolve("root", (1, 0, 0)).unwrap(); // Solution. let mut expected_solution = Map::default(); @@ -207,7 +207,7 @@ fn double_choices() { expected_solution.insert("d", 0u32); // Run the algorithm. - let computed_solution = resolve(&mut dependency_provider, "a", 0u32).unwrap(); + let computed_solution = dependency_provider.resolve("a", 0u32).unwrap(); assert_eq!(expected_solution, computed_solution); } @@ -230,27 +230,28 @@ fn confusing_with_lots_of_holes() { // This package is part of the dependency tree, but it's not part of the conflict dependency_provider.add_dependencies("baz", 1u32, vec![]); - let Err(PubGrubError::NoSolution(mut error)) = resolve(&mut dependency_provider, "root", 1u32) - else { + let Err(PubGrubError::NoSolution(mut error)) = dependency_provider.resolve("root", 1u32) else { unreachable!() }; assert_eq!( - &DefaultStringReporter::report(&error), - r#"Because there is no available version for bar and foo 1 | 2 | 3 | 4 | 5 depends on bar, foo 1 | 2 | 3 | 4 | 5 is forbidden. -And because there is no version of foo in <1 | >1, <2 | >2, <3 | >3, <4 | >4, <5 | >5 and root 1 depends on foo, root 1 is forbidden."# + &DefaultStringReporter::report(&error, &dependency_provider), + r#"Because foo @ * depends on bar @ ∅ and root @ 1 depends on foo @ *, root @ 1 is forbidden."# ); + error.derivation_tree.collapse_no_versions(); assert_eq!( - &DefaultStringReporter::report(&error), - "Because foo depends on bar and root 1 depends on foo, root 1 is forbidden." + &DefaultStringReporter::report(&error, &dependency_provider), + r#"Because foo @ * depends on bar @ ∅ and root @ 1 depends on foo @ *, root @ 1 is forbidden."# ); + assert_eq!( - error.derivation_tree.packages(), + error + .derivation_tree + .packages() + .into_iter() + .filter_map(|p| error.package_store.pkg(p).unwrap().inner_pkg().copied()) + .collect::>(), // baz isn't shown. - Set::from_iter([ - error.package_store.get(&"root").unwrap(), - error.package_store.get(&"foo").unwrap(), - error.package_store.get(&"bar").unwrap(), - ]) + Set::from_iter(["root", "foo", "bar"]), ); } diff --git a/tests/proptest.rs b/tests/proptest.rs index 8ccf92d7..65c8e0fb 100644 --- a/tests/proptest.rs +++ b/tests/proptest.rs @@ -15,8 +15,9 @@ use proptest::sample::Index; use pubgrub::{ resolve, DefaultStringReporter, Dependencies, DependencyProvider, DerivationTree, External, - NoSolutionError, OfflineDependencyProvider, PackageArena, PackageId, PubGrubError, Ranges, - Reporter, SelectedDependencies, Set, VersionSet, + Map, NoSolutionError, OfflineDependencyProvider, PackageArena, PackageId, + PackageVersionWrapper, PubGrubError, Ranges, Reporter, SelectedDependencies, Set, VersionIndex, + VersionRanges, VersionSet, }; use crate::sat_dependency_provider::SatResolve; @@ -24,54 +25,62 @@ use crate::sat_dependency_provider::SatResolve; /// 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( + OfflineDependencyProvider, ); -impl DependencyProvider - for OldestVersionsDependencyProvider +impl DependencyProvider + for OldestVersionsDependencyProvider { fn get_dependencies( &mut self, pid: PackageId, - v: &VS::V, + v: VersionIndex, package_store: &mut PackageArena, - ) -> Result, Infallible> { + ) -> Result, Infallible> { self.0.get_dependencies(pid, v, package_store) } fn choose_version( &mut self, - package_id: PackageId, - range: &VS, - package_store: &PackageArena, - ) -> Result, Infallible> { - Ok(self - .0 - .versions(package_store.pkg(package_id).unwrap()) - .into_iter() - .flatten() - .find(|&v| range.contains(v)) - .cloned()) + _: PackageId, + set: VersionSet, + _: &PackageArena, + ) -> Result, Infallible> { + Ok(set.first()) } - type Priority = as DependencyProvider>::Priority; + type Priority = as DependencyProvider>::Priority; fn prioritize( &mut self, package_id: PackageId, - range: &VS, + set: VersionSet, package_store: &PackageArena, ) -> Self::Priority { - self.0.prioritize(package_id, range, package_store) + self.0.prioritize(package_id, set, package_store) } type Err = Infallible; - type P = P; - type V = VS::V; - type VS = VS; + type P = PackageVersionWrapper

; type M = &'static str; + + fn package_version_display<'a>( + &'a self, + package: &'a Self::P, + version_index: VersionIndex, + ) -> impl Display + 'a { + self.0.package_version_display(package, version_index) + } + + fn package_version_set_display<'a>( + &'a self, + package: &'a Self::P, + version_set: VersionSet, + ) -> impl Display + 'a { + self.0.package_version_set_display(package, version_set) + } } /// The same as DP but it has a timeout. @@ -98,10 +107,10 @@ impl DependencyProvider for TimeoutDependencyProvider, - ) -> Result, DP::Err> { - self.dp.get_dependencies(pid, v, package_store) + ) -> Result, DP::Err> { + self.dp.get_dependencies(pid, version_index, package_store) } fn should_cancel(&mut self) -> Result<(), DP::Err> { @@ -115,10 +124,10 @@ impl DependencyProvider for TimeoutDependencyProvider, - ) -> Result, DP::Err> { - self.dp.choose_version(package_id, range, package_store) + ) -> Result, DP::Err> { + self.dp.choose_version(package_id, set, package_store) } type Priority = DP::Priority; @@ -126,33 +135,44 @@ impl DependencyProvider for TimeoutDependencyProvider, ) -> Self::Priority { - self.dp.prioritize(package_id, range, package_store) + self.dp.prioritize(package_id, set, package_store) } type Err = DP::Err; type P = DP::P; - type V = ::V; - type VS = DP::VS; type M = DP::M; + + fn package_version_display<'a>( + &'a self, + package: &'a Self::P, + version_index: VersionIndex, + ) -> impl Display + 'a { + self.dp.package_version_display(package, version_index) + } + + fn package_version_set_display<'a>( + &'a self, + package: &'a Self::P, + version_set: VersionSet, + ) -> impl Display + 'a { + self.dp.package_version_set_display(package, version_set) + } } fn timeout_resolve( dependency_provider: DP, - name: DP::P, - version: impl Into, + pkg: DP::P, + version_index: VersionIndex, ) -> Result< SelectedDependencies>, PubGrubError>, > { - resolve( - &mut TimeoutDependencyProvider::new(dependency_provider, 50_000), - name, - version, - ) + let mut dp = TimeoutDependencyProvider::new(dependency_provider, 50_000); + resolve(&mut dp, pkg, version_index) } type NumVS = Ranges; @@ -164,11 +184,9 @@ fn should_cancel_can_panic() { dependency_provider.add_dependencies(0, 0u32, [(666, Ranges::full())]); // Run the algorithm. - let _ = resolve( - &mut TimeoutDependencyProvider::new(dependency_provider, 1), - 0, - 0u32, - ); + let (p, v) = dependency_provider.resolve_parameters(0, 0u32).unwrap(); + let mut dp = TimeoutDependencyProvider::new(dependency_provider, 1); + let _ = resolve(&mut dp, p, v); } fn string_names() -> impl Strategy { @@ -303,7 +321,7 @@ fn meta_test_deep_trees_from_strategy() { .current(); for (name, ver) in cases { - let res = resolve(&mut dependency_provider, name, ver); + let res = dependency_provider.resolve(name, ver); dis[res .as_ref() .map(|x| std::cmp::min(x.len(), dis.len()) - 1) @@ -328,10 +346,10 @@ fn meta_test_deep_trees_from_strategy() { /// then there must still be no solution with some options removed. /// If there was a solution to a resolution in the original dependency provider, /// there may not be a solution after versions are removes iif removed versions were critical for all valid solutions. -fn retain_versions( - dependency_provider: &OfflineDependencyProvider, - retain: impl Fn(&N, &VS::V) -> bool, -) -> OfflineDependencyProvider { +fn retain_versions( + dependency_provider: &OfflineDependencyProvider, + retain: impl Fn(&N, &R::V) -> bool, +) -> OfflineDependencyProvider { let mut smaller_dependency_provider = OfflineDependencyProvider::new(); for n in dependency_provider.packages() { for v in dependency_provider.versions(n).unwrap() { @@ -342,7 +360,15 @@ fn retain_versions( smaller_dependency_provider.add_dependencies( n.clone(), v.clone(), - deps.iter().map(|(p, r)| (p.clone(), r.clone())), + deps.iter().map(|(p, r)| { + let r = R::from_ordered_versions( + dependency_provider + .versions(p) + .unwrap() + .map(|v| (v.clone(), r.contains(v))), + ); + (p.clone(), r) + }), ) } } @@ -356,10 +382,10 @@ fn retain_versions( /// then there must still be a solution after dependencies are removed. /// If there was no solution to a resolution in the original dependency provider, /// there may now be a solution after dependencies are removed. -fn retain_dependencies( - dependency_provider: &OfflineDependencyProvider, - retain: impl Fn(&N, &VS::V, &N) -> bool, -) -> OfflineDependencyProvider { +fn retain_dependencies( + dependency_provider: &OfflineDependencyProvider, + retain: impl Fn(&N, &R::V, &N) -> bool, +) -> OfflineDependencyProvider { let mut smaller_dependency_provider = OfflineDependencyProvider::new(); for n in dependency_provider.packages() { for v in dependency_provider.versions(n).unwrap() { @@ -367,11 +393,17 @@ fn retain_dependencies( smaller_dependency_provider.add_dependencies( n.clone(), v.clone(), - deps.iter().filter_map(|(dep, range)| { + deps.iter().filter_map(|(dep, r)| { if !retain(n, v, dep) { None } else { - Some((dep.clone(), range.clone())) + let r = R::from_ordered_versions( + dependency_provider + .versions(dep) + .unwrap() + .map(|v| (v.clone(), r.contains(v))), + ); + Some((dep.clone(), r)) } }), ); @@ -385,49 +417,73 @@ fn errors_the_same_with_only_report_dependencies( - to_retain: &mut Vec<(PackageId, VS, PackageId)>, - tree: &DerivationTree, + fn recursive( + to_retain: &mut Map>>, + tree: &DerivationTree, + package_store: &PackageArena>, + dependency_provider: &OfflineDependencyProvider, ) { match tree { - DerivationTree::External(External::FromDependencyOf(n1, vs1, n2, _)) => { - to_retain.push((*n1, vs1.clone(), *n2)); + &DerivationTree::External(External::FromDependencyOf(n1, vs1, n2, _)) => { + let pkg1 = package_store.pkg(n1).unwrap(); + let pkg2 = package_store.pkg(n2).unwrap(); + + if let Some(n1) = pkg1.inner_pkg() { + let n2 = match pkg2 { + PackageVersionWrapper::Pkg(p) => p.pkg(), + PackageVersionWrapper::VirtualPkg(vp) => vp.pkg(), + PackageVersionWrapper::VirtualDep(vd) => vd.pkg(), + }; + + for v in vs1.iter() { + let v1 = *dependency_provider + .versions(n1) + .unwrap() + .nth(pkg1.inner(v).unwrap().1 as usize) + .unwrap(); + + to_retain + .entry(n1.clone()) + .or_default() + .entry(v1) + .or_default() + .insert(n2.clone()); + } + } } DerivationTree::Derived(d) => { - recursive(to_retain, &*d.cause1); - recursive(to_retain, &*d.cause2); + recursive(to_retain, &*d.cause1, package_store, dependency_provider); + recursive(to_retain, &*d.cause2, package_store, dependency_provider); } _ => {} } } - let mut to_retain = Vec::new(); - recursive(&mut to_retain, &error.derivation_tree); - - let package_store = &error.package_store; - let to_retain = to_retain - .into_iter() - .map(|(n1, r, n2)| { - let n1 = package_store.pkg(n1).unwrap(); - let n2 = package_store.pkg(n2).unwrap(); - (n1, r, n2) - }) - .collect::>(); + let mut to_retain = Map::default(); + recursive( + &mut to_retain, + &error.derivation_tree, + &error.package_store, + &dependency_provider, + ); let removed_provider = retain_dependencies(&dependency_provider, |p, v, d| { - to_retain - .iter() - .any(|&(n1, ref vs1, n2)| n1 == p && vs1.contains(v) && n2 == d) + (|| to_retain.get(p)?.get(v)?.get(d))().is_some() }); + let (p, v) = removed_provider.resolve_parameters(name, ver).unwrap(); + assert!( - timeout_resolve(removed_provider.clone(), name, ver).is_err(), + timeout_resolve(removed_provider, p, v).is_err(), "The full index errored filtering to only dependencies in the derivation tree succeeded" ); } @@ -452,7 +508,8 @@ proptest! { (dependency_provider, cases) in registry_strategy(string_names()) ) { for (name, ver) in cases { - _ = timeout_resolve(dependency_provider.clone(), name, ver); + let (p, v) = dependency_provider.resolve_parameters(name, ver).unwrap(); + _ = timeout_resolve(dependency_provider.clone(), p, v); } } @@ -462,17 +519,19 @@ proptest! { (dependency_provider, cases) in registry_strategy(0u16..665) ) { for (name, ver) in cases { - _ = timeout_resolve(dependency_provider.clone(), name, ver); + let (p, v) = dependency_provider.resolve_parameters(name, ver).unwrap(); + _ = timeout_resolve(dependency_provider.clone(), p, v); } } #[test] fn prop_sat_errors_the_same( - (mut dependency_provider, cases) in registry_strategy(0u16..665) + (dependency_provider, cases) in registry_strategy(0u16..665) ) { - let mut sat = SatResolve::new(&mut dependency_provider); + let mut sat = SatResolve::new(&dependency_provider); for (name, ver) in cases { - let res = timeout_resolve(dependency_provider.clone(), name, ver); + let (p, v) = dependency_provider.resolve_parameters(name, ver).unwrap(); + let res = timeout_resolve(dependency_provider.clone(), p, v); sat.check_resolve(&res, &name, &ver); } } @@ -492,18 +551,18 @@ proptest! { (dependency_provider, cases) in registry_strategy(0u16..665) ) { for (name, ver) in cases { - let one = timeout_resolve(dependency_provider.clone(), name, ver); + let (p, v) = dependency_provider.resolve_parameters(name, ver).unwrap(); + let one = timeout_resolve(dependency_provider.clone(), p.clone(), v); for _ in 0..3 { - match (&one, &timeout_resolve(dependency_provider.clone(), name, ver)) { + match (&one, &timeout_resolve(dependency_provider.clone(), p.clone(), v)) { (Ok(l), Ok(r)) => assert_eq!(l, r), (Err(PubGrubError::NoSolution(error_l)), Err(PubGrubError::NoSolution(error_r))) => { - type Dp = OfflineDependencyProvider>; let (error_l, error_r) = (error_l.clone(), error_r.clone()); - let error_l = NoSolutionError:: { package_store: error_l.package_store, derivation_tree: error_l.derivation_tree }; - let error_r = NoSolutionError:: { package_store: error_r.package_store, derivation_tree: error_r.derivation_tree }; + let error_l = NoSolutionError { package_store: error_l.package_store, derivation_tree: error_l.derivation_tree }; + let error_r = NoSolutionError { package_store: error_r.package_store, derivation_tree: error_r.derivation_tree }; prop_assert_eq!( - DefaultStringReporter::report(&error_l), - DefaultStringReporter::report(&error_r) + DefaultStringReporter::report(&error_l, &dependency_provider), + DefaultStringReporter::report(&error_r, &dependency_provider) ); } _ => panic!("not the same result") @@ -520,8 +579,9 @@ proptest! { ) { let reverse_provider = OldestVersionsDependencyProvider(dependency_provider.clone()); for (name, ver) in cases { - let l = timeout_resolve(dependency_provider.clone(), name, ver); - let r = timeout_resolve(reverse_provider.clone(), name, ver); + let (p, v) = dependency_provider.resolve_parameters(name, ver).unwrap(); + let l = timeout_resolve(dependency_provider.clone(), p.clone(), v); + let r = timeout_resolve(reverse_provider.clone(), p, v); match (&l, &r) { (Ok(_), Ok(_)) => (), (Err(_), Err(_)) => (), @@ -539,7 +599,7 @@ proptest! { let mut to_remove = Set::default(); for (package_idx, version_idx, dep_idx) in indexes_to_remove { let pkg = *package_idx.get(&packages); - let versions: Vec<_> = dependency_provider + let versions: Vec<_> = dependency_provider .versions(&pkg) .unwrap() .copied() @@ -552,15 +612,14 @@ proptest! { } let removed_provider = retain_dependencies( &dependency_provider, - |&p, &v, &d| {!to_remove.contains(&(p, v, d))} + |&p, &v, &d| !to_remove.contains(&(p, v, d)) ); for (name, ver) in cases { - if timeout_resolve(dependency_provider.clone(), name, ver).is_ok() { + let (p, v) = dependency_provider.resolve_parameters(name, ver).unwrap(); + if timeout_resolve(dependency_provider.clone(), p.clone(), v).is_ok() { prop_assert!( - timeout_resolve(removed_provider.clone(), name, ver).is_ok(), - "full index worked for `{} = \"={}\"` but removing some deps broke it!", - name, - ver, + timeout_resolve(removed_provider.clone(), p, v).is_ok(), + "full index worked for `{name} = \"={ver}\"` but removing some deps broke it!" ) } } @@ -573,29 +632,28 @@ proptest! { ) { let all_versions: Vec<(u16, u32)> = dependency_provider .packages() - .flat_map(|&p| { - dependency_provider - .versions(&p) - .unwrap() - .map(move |&v| (p, v)) - }) + .flat_map(|&p| dependency_provider.versions(&p).unwrap().map(move |&v| (p, v))) .collect(); let to_remove: Set<(_, _)> = indexes_to_remove.iter().map(|x| x.get(&all_versions)).cloned().collect(); for (name, ver) in cases { - match timeout_resolve(dependency_provider.clone(), name, ver) { - Ok(used_packages) => { + let (p, v) = dependency_provider.resolve_parameters(name, ver).unwrap(); + match timeout_resolve(dependency_provider.clone(), p.clone(), v) { + Ok(used) => { + let used_packages = used + .iter() + .filter_map(|(wrapper, &version_index)| wrapper.inner(version_index)) + .map(|(p, v)| (p, *dependency_provider.versions(p).unwrap().nth(v as usize).unwrap())) + .collect::>(); // If resolution was successful, then unpublishing a version of a crate // that was not selected should not change that. let smaller_dependency_provider = retain_versions(&dependency_provider, |&n, &v| { used_packages.get(&n) == Some(&v) // it was used || !to_remove.contains(&(n, v)) // or it is not one to be removed }); + let (p, v) = smaller_dependency_provider.resolve_parameters(name, ver).unwrap(); prop_assert!( - timeout_resolve(smaller_dependency_provider.clone(), name, ver).is_ok(), - "unpublishing {:?} stopped `{} = \"={}\"` from working", - to_remove, - name, - ver + timeout_resolve(smaller_dependency_provider.clone(), p, v).is_ok(), + "unpublishing {to_remove:?} stopped `{name} = \"={ver}\"` from working" ) } Err(_) => { @@ -604,13 +662,10 @@ proptest! { let smaller_dependency_provider = retain_versions(&dependency_provider, |&n, &v| { to_remove.contains(&(n, v)) // it is one to be removed }); - if smaller_dependency_provider.versions(&name).is_some(){ + if let Some((p, v)) = smaller_dependency_provider.resolve_parameters(name, ver) { prop_assert!( - timeout_resolve(smaller_dependency_provider.clone(), name, ver).is_err(), - "full index did not work for `{} = \"={}\"` but unpublishing {:?} fixed it!", - name, - ver, - to_remove, + timeout_resolve(smaller_dependency_provider.clone(), p, v).is_err(), + "full index did not work for `{name} = \"={ver}\"` but unpublishing {to_remove:?} fixed it!" ) } } @@ -628,38 +683,21 @@ fn large_case() { eprint!("{} ", name); let data = std::fs::read_to_string(&case).unwrap(); let start_time = std::time::Instant::now(); - if name.ends_with("u16_NumberVersion.ron") || name.ends_with("u16_u32.ron") { + if name.ends_with("u16_NumberVersion.ron") { let mut dependency_provider: OfflineDependencyProvider = ron::de::from_str(&data).unwrap(); - let mut sat = SatResolve::new(&mut dependency_provider); - let packages = dependency_provider.packages().cloned().collect::>(); - for p in packages { - let versions = dependency_provider - .versions(&p) - .unwrap() - .copied() - .collect::>(); - for v in versions { - let res = resolve(&mut dependency_provider, p, v); - sat.check_resolve(&res, &p, &v); - } - } - } else if name.ends_with("str_SemanticVersion.ron") { - let mut dependency_provider: OfflineDependencyProvider< - &str, - Ranges, - > = ron::de::from_str(&data).unwrap(); - let mut sat = SatResolve::new(&mut dependency_provider); + let mut sat = SatResolve::new(&dependency_provider); let packages = dependency_provider.packages().cloned().collect::>(); - for p in packages { + for name in packages { let versions = dependency_provider - .versions(&p) + .versions(&name) .unwrap() .copied() .collect::>(); - for v in versions { + for ver in versions { + let (p, v) = dependency_provider.resolve_parameters(name, ver).unwrap(); let res = resolve(&mut dependency_provider, p, v); - sat.check_resolve(&res, &p, &v); + sat.check_resolve(&res, &name, &ver); } } } diff --git a/tests/sat_dependency_provider.rs b/tests/sat_dependency_provider.rs index 58bc88d6..ac137150 100644 --- a/tests/sat_dependency_provider.rs +++ b/tests/sat_dependency_provider.rs @@ -1,11 +1,12 @@ // SPDX-License-Identifier: MPL-2.0 +use std::collections::BTreeMap; use std::fmt::{Debug, Display}; use std::hash::Hash; use pubgrub::{ - DependencyProvider, Map, OfflineDependencyProvider, PubGrubError, SelectedDependencies, - VersionSet, + DependencyProvider, Map, OfflineDependencyProvider, PackageVersionWrapper, PubGrubError, + SelectedDependencies, VersionRanges, }; use varisat::ExtendFormula; @@ -38,17 +39,17 @@ fn sat_at_most_one(solver: &mut impl ExtendFormula, vars: &[varisat::Var]) { /// /// 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 { solver: varisat::Solver<'static>, - all_versions_by_p: Map>, + all_versions_by_p: Map>, } -impl SatResolve { - pub fn new(dp: &mut OfflineDependencyProvider) -> Self { +impl SatResolve { + pub fn new>(dp: &OfflineDependencyProvider) -> Self { let mut cnf = varisat::CnfFormula::new(); let mut all_versions = vec![]; - let mut all_versions_by_p: Map> = Map::default(); + let mut all_versions_by_p: Map> = Map::default(); for p in dp.packages() { let mut versions_for_p = vec![]; @@ -59,7 +60,7 @@ impl SatResolve { all_versions_by_p .entry(p.clone()) .or_default() - .push((v.clone(), new_var)); + .insert(v.clone(), new_var); } // no two versions of the same package sat_at_most_one(&mut cnf, &versions_for_p); @@ -69,12 +70,11 @@ impl SatResolve { for (p, v, var) in &all_versions { let deps = dp.dependencies(p, v).unwrap(); for (p1, range) in deps { - let empty_vec = vec![]; let mut matches: Vec = all_versions_by_p .get(p1) - .unwrap_or(&empty_vec) + .unwrap_or(&BTreeMap::new()) .iter() - .filter(|(v1, _)| range.contains(v1)) + .filter(|&(v1, _)| range.contains(v1)) .map(|(_, var1)| var1.positive()) .collect(); // ^ the `dep` is satisfied or @@ -99,32 +99,35 @@ impl SatResolve { } } - pub fn resolve(&mut self, name: &P, ver: &VS::V) -> bool { + pub fn resolve(&mut self, name: &P, ver: &V) -> bool { if let Some(vers) = self.all_versions_by_p.get(name) { - if let Some((_, var)) = vers.iter().find(|(v, _)| v == ver) { + if let Some((_, var)) = vers.iter().find(|&(v, _)| v == ver) { self.solver.assume(&[var.positive()]); - self.solver + return self + .solver .solve() - .expect("docs say it can't error in default config") - } else { - false + .expect("docs say it can't error in default config"); } - } else { - false } + false } - pub fn is_valid_solution>( + pub fn is_valid_solution>>( &mut self, pids: &SelectedDependencies, ) -> bool { + let pids = pids + .iter() + .filter_map(|(wrapper, &version_index)| wrapper.inner(version_index)) + .collect::>(); + let mut assumption = vec![]; for (p, vs) in &self.all_versions_by_p { - let pid_for_p = pids.get(p); - for (v, var) in vs { - assumption.push(var.lit(pid_for_p == Some(v))) + let pid_for_p = pids.get(p).map(|&v| v as usize); + for (i, var) in vs.values().enumerate() { + assumption.push(var.lit(pid_for_p == Some(i))) } } @@ -135,19 +138,15 @@ impl SatResolve { .expect("docs say it can't error in default config") } - pub fn check_resolve>( + pub fn check_resolve>>( &mut self, res: &Result, PubGrubError>, p: &P, - v: &VS::V, + v: &V, ) { match res { - Ok(s) => { - assert!(self.is_valid_solution::(s)); - } - Err(_) => { - assert!(!self.resolve(p, v)); - } + Ok(s) => assert!(self.is_valid_solution::(s)), + Err(_) => assert!(!self.resolve(p, v)), } } } diff --git a/tests/tests.rs b/tests/tests.rs index f16f4ae8..473fd58d 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MPL-2.0 -use pubgrub::{resolve, OfflineDependencyProvider, PubGrubError, Ranges}; +use pubgrub::{OfflineDependencyProvider, PubGrubError, Ranges}; type NumVS = Ranges; @@ -17,9 +17,9 @@ fn same_result_on_repeated_runs() { let name = "a"; let ver: u32 = 0; - let one = resolve(&mut dependency_provider, name, ver); + let one = dependency_provider.resolve(name, ver); for _ in 0..10 { - match (&one, &resolve(&mut dependency_provider, name, ver)) { + match (&one, &dependency_provider.resolve(name, ver)) { (Ok(l), Ok(r)) => assert_eq!(l, r), _ => panic!("not the same result"), } @@ -31,13 +31,13 @@ fn should_always_find_a_satisfier() { let mut dependency_provider = OfflineDependencyProvider::<_, NumVS>::new(); dependency_provider.add_dependencies("a", 0u32, [("b", Ranges::empty())]); assert!(matches!( - resolve(&mut dependency_provider, "a", 0u32), + dependency_provider.resolve("a", 0u32), Err(PubGrubError::NoSolution { .. }) )); dependency_provider.add_dependencies("c", 0u32, [("a", Ranges::full())]); assert!(matches!( - resolve(&mut dependency_provider, "c", 0u32), + dependency_provider.resolve("c", 0u32), Err(PubGrubError::NoSolution { .. }) )); } @@ -46,7 +46,7 @@ fn should_always_find_a_satisfier() { fn depend_on_self() { let mut dependency_provider = OfflineDependencyProvider::<_, NumVS>::new(); dependency_provider.add_dependencies("a", 0u32, [("a", Ranges::full())]); - assert!(resolve(&mut dependency_provider, "a", 0u32).is_ok()); + assert!(dependency_provider.resolve("a", 0u32).is_ok()); dependency_provider.add_dependencies("a", 66u32, [("a", Ranges::singleton(111u32))]); - assert!(resolve(&mut dependency_provider, "a", 66u32).is_err()); + assert!(dependency_provider.resolve("a", 66u32).is_err()); }