From 5aa15dd2bfdf02e6f411b0812b7f56341c75dd03 Mon Sep 17 00:00:00 2001 From: Zanie Date: Wed, 8 Nov 2023 17:05:22 -0600 Subject: [PATCH] Switch pubgrub to experimental iter commit 02a19f7 --- vendor/pubgrub/Cargo.toml | 6 +- vendor/pubgrub/src/internal/core.rs | 8 +- .../pubgrub/src/internal/incompatibility.rs | 12 +- .../pubgrub/src/internal/partial_solution.rs | 26 +- vendor/pubgrub/src/internal/small_vec.rs | 6 +- vendor/pubgrub/src/lib.rs | 9 +- vendor/pubgrub/src/range.rs | 21 +- vendor/pubgrub/src/solver.rs | 35 +-- vendor/pubgrub/src/term.rs | 2 +- vendor/pubgrub/src/type_aliases.rs | 7 - vendor/pubgrub/tests/proptest.rs | 239 ++++++++++++------ .../pubgrub/tests/sat_dependency_provider.rs | 47 ++-- 12 files changed, 247 insertions(+), 171 deletions(-) diff --git a/vendor/pubgrub/Cargo.toml b/vendor/pubgrub/Cargo.toml index 0f02caa10..a4c94e696 100644 --- a/vendor/pubgrub/Cargo.toml +++ b/vendor/pubgrub/Cargo.toml @@ -8,7 +8,7 @@ authors = [ "Alex Tokarev ", "Jacob Finkelman ", ] -edition = "2018" +edition = "2021" description = "PubGrub version solving algorithm" readme = "README.md" repository = "https://github.com/pubgrub-rs/pubgrub" @@ -20,10 +20,10 @@ include = ["Cargo.toml", "LICENSE", "README.md", "src/**", "tests/**", "examples # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] -thiserror = "1.0" -rustc-hash = "1.1.0" indexmap = "2.0.2" priority-queue = "1.1.1" +thiserror = "1.0" +rustc-hash = "1.1.0" serde = { version = "1.0", features = ["derive"], optional = true } log = "0.4.14" # for debug logs in tests diff --git a/vendor/pubgrub/src/internal/core.rs b/vendor/pubgrub/src/internal/core.rs index a4a23b341..ca1dec1c3 100644 --- a/vendor/pubgrub/src/internal/core.rs +++ b/vendor/pubgrub/src/internal/core.rs @@ -15,7 +15,7 @@ use crate::internal::partial_solution::{DecisionLevel, PartialSolution}; use crate::internal::small_vec::SmallVec; use crate::package::Package; use crate::report::DerivationTree; -use crate::type_aliases::{DependencyConstraints, Map}; +use crate::type_aliases::Map; use crate::version_set::VersionSet; /// Current state of the PubGrub algorithm. @@ -24,7 +24,7 @@ pub struct State { root_package: P, root_version: VS::V, - pub incompatibilities: Map>>, + incompatibilities: Map>>, /// Store the ids of incompatibilities that are already contradicted /// and will stay that way until the next conflict and backtrack is operated. @@ -75,12 +75,12 @@ impl State { &mut self, package: P, version: VS::V, - deps: &DependencyConstraints, + 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.iter().map(|dep| { + .alloc_iter(deps.into_iter().map(|dep| { Incompatibility::from_dependency(package.clone(), version.clone(), dep) })); // Merge the newly created incompatibilities with the older ones. diff --git a/vendor/pubgrub/src/internal/incompatibility.rs b/vendor/pubgrub/src/internal/incompatibility.rs index 592aaf398..fd33d6af0 100644 --- a/vendor/pubgrub/src/internal/incompatibility.rs +++ b/vendor/pubgrub/src/internal/incompatibility.rs @@ -30,15 +30,15 @@ use crate::version_set::VersionSet; /// [PubGrub documentation](https://github.com/dart-lang/pub/blob/master/doc/solver.md#incompatibility). #[derive(Debug, Clone)] pub struct Incompatibility { - pub package_terms: SmallMap>, - pub kind: Kind, + package_terms: SmallMap>, + kind: Kind, } /// Type alias of unique identifiers for incompatibilities. pub type IncompId = Id>; #[derive(Debug, Clone)] -pub enum Kind { +enum Kind { /// Initial incompatibility aiming at picking the root package for the first decision. NotRoot(P, VS::V), /// There are no versions in the given range for this package. @@ -105,11 +105,11 @@ impl Incompatibility { } /// Build an incompatibility from a given dependency. - pub fn from_dependency(package: P, version: VS::V, dep: (&P, &VS)) -> Self { + pub fn from_dependency(package: P, version: VS::V, dep: (P, VS)) -> Self { let set1 = VS::singleton(version); let (p2, set2) = dep; Self { - package_terms: if set2 == &VS::empty() { + package_terms: if set2 == VS::empty() { SmallMap::One([(package.clone(), Term::Positive(set1.clone()))]) } else { SmallMap::Two([ @@ -117,7 +117,7 @@ impl Incompatibility { (p2.clone(), Term::Negative(set2.clone())), ]) }, - kind: Kind::FromDependencyOf(package, set1, p2.clone(), set2.clone()), + kind: Kind::FromDependencyOf(package, set1, p2, set2), } } diff --git a/vendor/pubgrub/src/internal/partial_solution.rs b/vendor/pubgrub/src/internal/partial_solution.rs index 47f3bee2e..057dea134 100644 --- a/vendor/pubgrub/src/internal/partial_solution.rs +++ b/vendor/pubgrub/src/internal/partial_solution.rs @@ -165,7 +165,13 @@ impl PartialSolution panic!("Already existing decision"), // Cannot be called if the versions is not contained in the terms intersection. AssignmentsIntersection::Derivations(term) => { - debug_assert!(term.contains(&version)) + debug_assert!( + term.contains(&version), + "{}: {} was expected to be contained in {}", + package, + version, + term, + ) } }, } @@ -246,24 +252,6 @@ impl PartialSolution impl Iterator { - let check_all = self.changed_this_decision_level - == self.current_decision_level.0.saturating_sub(1) as usize; - let current_decision_level = self.current_decision_level; - self.package_assignments - .get_range(self.changed_this_decision_level..) - .unwrap() - .iter() - .filter(move |(_, pa)| { - // We only actually need to update the package if its Been changed - // since the last time we called prioritize. - // Which means it's highest decision level is the current decision level, - // or if we backtracked in the mean time. - check_all || pa.highest_decision_level == current_decision_level - }) - .filter_map(|(p, pa)| pa.assignments_intersection.potential_package_filter(p)) - } - pub fn pick_highest_priority_pkg( &mut self, prioritizer: impl Fn(&P, &VS) -> Priority, diff --git a/vendor/pubgrub/src/internal/small_vec.rs b/vendor/pubgrub/src/internal/small_vec.rs index 4c2a97df3..fa720435d 100644 --- a/vendor/pubgrub/src/internal/small_vec.rs +++ b/vendor/pubgrub/src/internal/small_vec.rs @@ -166,9 +166,9 @@ impl IntoIterator for SmallVec { fn into_iter(self) -> Self::IntoIter { match self { SmallVec::Empty => SmallVecIntoIter::Empty, - SmallVec::One(a) => SmallVecIntoIter::One(IntoIterator::into_iter(a)), - SmallVec::Two(a) => SmallVecIntoIter::Two(IntoIterator::into_iter(a)), - SmallVec::Flexible(v) => SmallVecIntoIter::Flexible(IntoIterator::into_iter(v)), + SmallVec::One(a) => SmallVecIntoIter::One(a.into_iter()), + SmallVec::Two(a) => SmallVecIntoIter::Two(a.into_iter()), + SmallVec::Flexible(v) => SmallVecIntoIter::Flexible(v.into_iter()), } } } diff --git a/vendor/pubgrub/src/lib.rs b/vendor/pubgrub/src/lib.rs index 0150c52ab..f41058bd7 100644 --- a/vendor/pubgrub/src/lib.rs +++ b/vendor/pubgrub/src/lib.rs @@ -102,8 +102,10 @@ //! &self, //! package: &String, //! version: &SemanticVersion, -//! ) -> Result, Box> { -//! unimplemented!() +//! ) -> Result + Clone>, Box> +//! { +//! unimplemented!(); +//! Ok(Dependencies::Known([])) //! } //! } //! ``` @@ -217,7 +219,8 @@ //! with a cache, you may want to know that some versions //! do not exist in your cache. -#![allow(clippy::all, unreachable_pub)] +#![allow(clippy::rc_buffer)] +#![warn(missing_docs)] pub mod error; pub mod package; diff --git a/vendor/pubgrub/src/range.rs b/vendor/pubgrub/src/range.rs index 47e76e44f..be7ff250f 100644 --- a/vendor/pubgrub/src/range.rs +++ b/vendor/pubgrub/src/range.rs @@ -195,7 +195,7 @@ impl Range { .segments .last() .expect("if there is a first element, there must be a last element"); - (bound_as_ref(start), bound_as_ref(&end.1)) + (start.as_ref(), end.1.as_ref()) }) } @@ -264,15 +264,6 @@ impl Range { } } -/// Implementation of [`Bound::as_ref`] which is currently marked as unstable. -fn bound_as_ref(bound: &Bound) -> Bound<&V> { - match bound { - Included(v) => Included(v), - Excluded(v) => Excluded(v), - Unbounded => Unbounded, - } -} - fn valid_segment(start: &Bound, end: &Bound) -> bool { match (start, end) { (Included(s), Included(e)) => s <= e, @@ -307,7 +298,7 @@ impl Range { (Included(i), Excluded(e)) | (Excluded(e), Included(i)) if i <= e => Excluded(e), (Included(i), Excluded(e)) | (Excluded(e), Included(i)) if e < i => Included(i), - (s, Unbounded) | (Unbounded, s) => bound_as_ref(s), + (s, Unbounded) | (Unbounded, s) => s.as_ref(), _ => unreachable!(), } .cloned(); @@ -317,7 +308,7 @@ impl Range { (Included(i), Excluded(e)) | (Excluded(e), Included(i)) if i >= e => Excluded(e), (Included(i), Excluded(e)) | (Excluded(e), Included(i)) if e > i => Included(i), - (s, Unbounded) | (Unbounded, s) => bound_as_ref(s), + (s, Unbounded) | (Unbounded, s) => s.as_ref(), _ => unreachable!(), } .cloned(); @@ -373,7 +364,7 @@ impl Display for Range { } else { for (idx, segment) in self.segments.iter().enumerate() { if idx > 0 { - write!(f, ", ")?; + write!(f, " | ")?; } match segment { (Unbounded, Unbounded) => write!(f, "*")?, @@ -382,9 +373,9 @@ impl Display for Range { (Included(v), Unbounded) => write!(f, ">={v}")?, (Included(v), Included(b)) => { if v == b { - write!(f, "=={v}")? + write!(f, "{v}")? } else { - write!(f, ">={v},<={b}")? + write!(f, ">={v}, <={b}")? } } (Included(v), Excluded(b)) => write!(f, ">={v}, <{b}")?, diff --git a/vendor/pubgrub/src/solver.rs b/vendor/pubgrub/src/solver.rs index 8e0d06b14..956e46be7 100644 --- a/vendor/pubgrub/src/solver.rs +++ b/vendor/pubgrub/src/solver.rs @@ -73,10 +73,10 @@ use std::collections::{BTreeMap, BTreeSet as Set}; use std::error::Error; use crate::error::PubGrubError; -pub use crate::internal::core::State; -pub use crate::internal::incompatibility::{Incompatibility, Kind}; +use crate::internal::core::State; +use crate::internal::incompatibility::Incompatibility; use crate::package::Package; -use crate::type_aliases::{DependencyConstraints, Map, SelectedDependencies}; +use crate::type_aliases::{Map, SelectedDependencies}; use crate::version_set::VersionSet; use log::{debug, info}; @@ -162,7 +162,7 @@ pub fn resolve( )); continue; } - Dependencies::Known(x) if x.contains_key(p) => { + Dependencies::Known(x) if x.clone().into_iter().any(|(d, _)| &d == p) => { return Err(PubGrubError::SelfDependency { package: p.clone(), version: v.clone(), @@ -175,12 +175,12 @@ pub fn resolve( let dep_incompats = state.add_incompatibility_from_dependencies( p.clone(), v.clone(), - &known_dependencies, + known_dependencies, ); state.partial_solution.add_version( p.clone(), - v, + v.clone(), dep_incompats, &state.incompatibility_store, ); @@ -196,11 +196,11 @@ pub fn resolve( /// An enum used by [DependencyProvider] that holds information about package dependencies. /// For each [Package] there is a set of versions allowed as a dependency. #[derive(Clone)] -pub enum Dependencies { +pub enum Dependencies { /// Package dependencies are unavailable. Unknown, /// Container for all available package versions. - Known(DependencyConstraints), + Known(T), } /// Trait that allows the algorithm to retrieve available packages and their dependencies. @@ -255,7 +255,7 @@ pub trait DependencyProvider { &self, package: &P, version: &VS::V, - ) -> Result, Box>; + ) -> Result + Clone>, Box>; /// This is called fairly regularly during the resolution, /// if it returns an Err then resolution will be terminated. @@ -279,7 +279,7 @@ pub trait DependencyProvider { )] #[cfg_attr(feature = "serde", serde(transparent))] pub struct OfflineDependencyProvider { - dependencies: Map>>, + dependencies: Map>>, } impl OfflineDependencyProvider { @@ -329,8 +329,8 @@ impl OfflineDependencyProvider { /// Lists dependencies of a given package and version. /// Returns [None] if no information is available regarding that package and version pair. - fn dependencies(&self, package: &P, version: &VS::V) -> Option> { - self.dependencies.get(package)?.get(version).cloned() + fn dependencies(&self, package: &P, version: &VS::V) -> Option<&Map> { + self.dependencies.get(package)?.get(version) } } @@ -364,11 +364,16 @@ impl DependencyProvider for OfflineDependency fn get_dependencies( &self, package: &P, - version: &VS::V, - ) -> Result, Box> { + version: &::V, + ) -> Result + Clone>, Box> + { Ok(match self.dependencies(package, version) { None => Dependencies::Unknown, - Some(dependencies) => Dependencies::Known(dependencies), + Some(dependencies) => Dependencies::Known( + dependencies + .into_iter() + .map(|(p, vs)| (p.clone(), vs.clone())), + ), }) } } diff --git a/vendor/pubgrub/src/term.rs b/vendor/pubgrub/src/term.rs index 895afdfe0..cf7aa6f7b 100644 --- a/vendor/pubgrub/src/term.rs +++ b/vendor/pubgrub/src/term.rs @@ -64,7 +64,7 @@ impl Term { /// Unwrap the set contained in a positive term. /// Will panic if used on a negative set. - pub fn unwrap_positive(&self) -> &VS { + pub(crate) fn unwrap_positive(&self) -> &VS { match self { Self::Positive(set) => set, _ => panic!("Negative term cannot unwrap positive set"), diff --git a/vendor/pubgrub/src/type_aliases.rs b/vendor/pubgrub/src/type_aliases.rs index 11cc37c75..c0912659f 100644 --- a/vendor/pubgrub/src/type_aliases.rs +++ b/vendor/pubgrub/src/type_aliases.rs @@ -8,10 +8,3 @@ pub type Map = rustc_hash::FxHashMap; /// Concrete dependencies picked by the library during [resolve](crate::solver::resolve) /// from [DependencyConstraints]. pub type SelectedDependencies = Map; - -/// Holds information about all possible versions a given package can accept. -/// There is a difference in semantics between an empty map -/// inside [DependencyConstraints] and [Dependencies::Unknown](crate::solver::Dependencies::Unknown): -/// the former means the package has no dependency and it is a known fact, -/// while the latter means they could not be fetched by the [DependencyProvider](crate::solver::DependencyProvider). -pub type DependencyConstraints = Map; diff --git a/vendor/pubgrub/tests/proptest.rs b/vendor/pubgrub/tests/proptest.rs index 145c5c577..d5ac3f816 100644 --- a/vendor/pubgrub/tests/proptest.rs +++ b/vendor/pubgrub/tests/proptest.rs @@ -5,8 +5,9 @@ use std::{collections::BTreeSet as Set, error::Error}; use pubgrub::error::PubGrubError; use pubgrub::package::Package; use pubgrub::range::Range; -use pubgrub::report::{DefaultStringReporter, Reporter}; +use pubgrub::report::{DefaultStringReporter, DerivationTree, External, Reporter}; use pubgrub::solver::{resolve, Dependencies, DependencyProvider, OfflineDependencyProvider}; +use pubgrub::type_aliases::SelectedDependencies; use pubgrub::version::{NumberVersion, SemanticVersion}; use pubgrub::version_set::VersionSet; @@ -33,7 +34,8 @@ impl DependencyProvider &self, p: &P, v: &VS::V, - ) -> Result, Box> { + ) -> Result + Clone>, Box> + { self.0.get_dependencies(p, v) } @@ -85,7 +87,8 @@ impl> DependencyProvid &self, p: &P, v: &VS::V, - ) -> Result, Box> { + ) -> Result + Clone>, Box> + { self.dp.get_dependencies(p, v) } @@ -112,6 +115,18 @@ impl> DependencyProvid } } +fn timeout_resolve>( + dependency_provider: DP, + name: P, + version: impl Into, +) -> Result, PubGrubError> { + resolve( + &TimeoutDependencyProvider::new(dependency_provider, 50_000), + name, + version, + ) +} + type NumVS = Range; type SemVS = Range; @@ -283,6 +298,110 @@ fn meta_test_deep_trees_from_strategy() { ); } +/// Removes versions from the dependency provider where the retain function returns false. +/// Solutions are constructed as a set of versions. +/// If there are fewer versions available, there are fewer valid solutions available. +/// If there was no solution to a resolution in the original dependency provider, +/// 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, + mut retain: impl FnMut(&N, &VS::V) -> bool, +) -> OfflineDependencyProvider { + let mut smaller_dependency_provider = OfflineDependencyProvider::new(); + + for n in dependency_provider.packages() { + for v in dependency_provider.versions(n).unwrap() { + if !retain(n, v) { + continue; + } + let deps = match dependency_provider.get_dependencies(&n, &v).unwrap() { + Dependencies::Unknown => panic!(), + Dependencies::Known(deps) => deps, + }; + smaller_dependency_provider.add_dependencies(n.clone(), v.clone(), deps) + } + } + smaller_dependency_provider +} + +/// Removes dependencies from the dependency provider where the retain function returns false. +/// Solutions are constraned by having to fulfill all the dependencies. +/// If there are fewer dependencies required, there are more valid solutions. +/// If there was a solution to a resolution in the original dependency provider, +/// 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, + mut retain: impl FnMut(&N, &VS::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() { + let deps = match dependency_provider.get_dependencies(&n, &v).unwrap() { + Dependencies::Unknown => panic!(), + Dependencies::Known(deps) => deps, + }; + smaller_dependency_provider.add_dependencies( + n.clone(), + v.clone(), + deps.into_iter().filter_map(|(dep, range)| { + if !retain(n, v, &dep) { + None + } else { + Some((dep.clone(), range.clone())) + } + }), + ); + } + } + smaller_dependency_provider +} + +fn errors_the_same_with_only_report_dependencies( + dependency_provider: OfflineDependencyProvider, + name: N, + ver: NumberVersion, +) { + let Err(PubGrubError::NoSolution(tree)) = + timeout_resolve(dependency_provider.clone(), name.clone(), ver) + else { + return; + }; + + fn recursive( + to_retain: &mut Vec<(N, VS, N)>, + tree: &DerivationTree, + ) { + match tree { + DerivationTree::External(External::FromDependencyOf(n1, vs1, n2, _)) => { + to_retain.push((n1.clone(), vs1.clone(), n2.clone())); + } + DerivationTree::Derived(d) => { + recursive(to_retain, &*d.cause1); + recursive(to_retain, &*d.cause2); + } + _ => {} + } + } + + let mut to_retain = Vec::new(); + recursive(&mut to_retain, &tree); + + let removed_provider = retain_dependencies(&dependency_provider, |p, v, d| { + to_retain + .iter() + .any(|(n1, vs1, n2)| n1 == p && vs1.contains(v) && n2 == d) + }); + + assert!( + timeout_resolve(removed_provider.clone(), name, ver).is_err(), + "The full index errored filtering to only dependencies in the derivation tree succeeded" + ); +} + proptest! { #![proptest_config(ProptestConfig { max_shrink_iters: @@ -303,7 +422,7 @@ proptest! { (dependency_provider, cases) in registry_strategy(string_names()) ) { for (name, ver) in cases { - let _ = resolve(&TimeoutDependencyProvider::new(dependency_provider.clone(), 50_000), name, ver); + _ = timeout_resolve(dependency_provider.clone(), name, ver); } } @@ -313,7 +432,7 @@ proptest! { (dependency_provider, cases) in registry_strategy(0u16..665) ) { for (name, ver) in cases { - let _ = resolve(&TimeoutDependencyProvider::new(dependency_provider.clone(), 50_000), name, ver); + _ = timeout_resolve(dependency_provider.clone(), name, ver); } } @@ -323,11 +442,17 @@ proptest! { ) { let mut sat = SatResolve::new(&dependency_provider); for (name, ver) in cases { - if let Ok(s) = resolve(&TimeoutDependencyProvider::new(dependency_provider.clone(), 50_000), name, ver) { - prop_assert!(sat.sat_is_valid_solution(&s)); - } else { - prop_assert!(!sat.sat_resolve(&name, &ver)); - } + let res = timeout_resolve(dependency_provider.clone(), name, ver); + sat.check_resolve(&res, &name, &ver); + } + } + + #[test] + fn prop_errors_the_same_with_only_report_dependencies( + (dependency_provider, cases) in registry_strategy(0u16..665) + ) { + for (name, ver) in cases { + errors_the_same_with_only_report_dependencies(dependency_provider.clone(), name, ver); } } @@ -337,9 +462,9 @@ proptest! { (dependency_provider, cases) in registry_strategy(0u16..665) ) { for (name, ver) in cases { - let one = resolve(&TimeoutDependencyProvider::new(dependency_provider.clone(), 50_000), name, ver); + let one = timeout_resolve(dependency_provider.clone(), name, ver); for _ in 0..3 { - match (&one, &resolve(&TimeoutDependencyProvider::new(dependency_provider.clone(), 50_000), name, ver)) { + match (&one, &timeout_resolve(dependency_provider.clone(), name, ver)) { (Ok(l), Ok(r)) => assert_eq!(l, r), (Err(PubGrubError::NoSolution(derivation_l)), Err(PubGrubError::NoSolution(derivation_r))) => { prop_assert_eq!( @@ -360,8 +485,8 @@ proptest! { ) { let reverse_provider = OldestVersionsDependencyProvider(dependency_provider.clone()); for (name, ver) in cases { - let l = resolve(&TimeoutDependencyProvider::new(dependency_provider.clone(), 50_000), name, ver); - let r = resolve(&TimeoutDependencyProvider::new(reverse_provider.clone(), 50_000), name, ver); + let l = timeout_resolve(dependency_provider.clone(), name, ver); + let r = timeout_resolve(reverse_provider.clone(), name, ver); match (&l, &r) { (Ok(_), Ok(_)) => (), (Err(_), Err(_)) => (), @@ -376,7 +501,7 @@ proptest! { indexes_to_remove in prop::collection::vec((any::(), any::(), any::()), 1..10) ) { let packages: Vec<_> = dependency_provider.packages().collect(); - let mut removed_provider = dependency_provider.clone(); + let mut to_remove = Set::new(); for (package_idx, version_idx, dep_idx) in indexes_to_remove { let package = package_idx.get(&packages); let versions: Vec<_> = dependency_provider @@ -391,29 +516,17 @@ proptest! { Dependencies::Known(d) => d.into_iter().collect(), }; if !dependencies.is_empty() { - let dependency = dep_idx.get(&dependencies).0; - removed_provider.add_dependencies( - **package, - **version, - dependencies.into_iter().filter(|x| x.0 != dependency), - ) + to_remove.insert((package, **version, dep_idx.get(&dependencies).0)); } } + let removed_provider = retain_dependencies( + &dependency_provider, + |p, v, d| {!to_remove.contains(&(&p, *v, *d))} + ); for (name, ver) in cases { - if resolve( - &TimeoutDependencyProvider::new(dependency_provider.clone(), 50_000), - name, - ver, - ) - .is_ok() - { + if timeout_resolve(dependency_provider.clone(), name, ver).is_ok() { prop_assert!( - resolve( - &TimeoutDependencyProvider::new(removed_provider.clone(), 50_000), - name, - ver - ) - .is_ok(), + timeout_resolve(removed_provider.clone(), name, ver).is_ok(), "full index worked for `{} = \"={}\"` but removing some deps broke it!", name, ver, @@ -438,24 +551,16 @@ proptest! { .collect(); let to_remove: Set<(_, _)> = indexes_to_remove.iter().map(|x| x.get(&all_versions)).cloned().collect(); for (name, ver) in cases { - match resolve(&TimeoutDependencyProvider::new(dependency_provider.clone(), 50_000), name, ver) { + match timeout_resolve(dependency_provider.clone(), name, ver) { Ok(used) => { // If resolution was successful, then unpublishing a version of a crate // that was not selected should not change that. - let mut smaller_dependency_provider = OfflineDependencyProvider::<_, NumVS>::new(); - for &(n, v) in &all_versions { - if used.get(&n) == Some(&v) // it was used - || to_remove.get(&(n, v)).is_none() // or it is not one to be removed - { - let deps = match dependency_provider.get_dependencies(&n, &v).unwrap() { - Dependencies::Unknown => panic!(), - Dependencies::Known(deps) => deps, - }; - smaller_dependency_provider.add_dependencies(n, v, deps) - } - } + let smaller_dependency_provider = retain_versions(&dependency_provider, |n, v| { + used.get(&n) == Some(&v) // it was used + || to_remove.get(&(*n, *v)).is_none() // or it is not one to be removed + }); prop_assert!( - resolve(&TimeoutDependencyProvider::new(smaller_dependency_provider.clone(), 50_000), name, ver).is_ok(), + timeout_resolve(smaller_dependency_provider.clone(), name, ver).is_ok(), "unpublishing {:?} stopped `{} = \"={}\"` from working", to_remove, name, @@ -465,19 +570,11 @@ proptest! { Err(_) => { // If resolution was unsuccessful, then it should stay unsuccessful // even if any version of a crate is unpublished. - let mut smaller_dependency_provider = OfflineDependencyProvider::<_, NumVS>::new(); - for &(n, v) in &all_versions { - if to_remove.get(&(n, v)).is_none() // it is not one to be removed - { - let deps = match dependency_provider.get_dependencies(&n, &v).unwrap() { - Dependencies::Unknown => panic!(), - Dependencies::Known(deps) => deps, - }; - smaller_dependency_provider.add_dependencies(n, v, deps) - } - } + let smaller_dependency_provider = retain_versions(&dependency_provider, |n, v| { + to_remove.get(&(*n, *v)).is_some() // it is one to be removed + }); prop_assert!( - resolve(&TimeoutDependencyProvider::new(smaller_dependency_provider.clone(), 50_000), name, ver).is_err(), + timeout_resolve(smaller_dependency_provider.clone(), name, ver).is_err(), "full index did not work for `{} = \"={}\"` but unpublishing {:?} fixed it!", name, ver, @@ -495,19 +592,17 @@ fn large_case() { for case in std::fs::read_dir("test-examples").unwrap() { let case = case.unwrap().path(); let name = case.file_name().unwrap().to_string_lossy(); - eprintln!("{}", name); + 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") { let dependency_provider: OfflineDependencyProvider = ron::de::from_str(&data).unwrap(); let mut sat = SatResolve::new(&dependency_provider); for p in dependency_provider.packages() { - for n in dependency_provider.versions(p).unwrap() { - if let Ok(s) = resolve(&dependency_provider, p.clone(), n.clone()) { - assert!(sat.sat_is_valid_solution(&s)); - } else { - assert!(!sat.sat_resolve(p, &n)); - } + for v in dependency_provider.versions(p).unwrap() { + let res = resolve(&dependency_provider, p.clone(), v); + sat.check_resolve(&res, p, v); } } } else if name.ends_with("str_SemanticVersion.ron") { @@ -515,14 +610,12 @@ fn large_case() { ron::de::from_str(&data).unwrap(); let mut sat = SatResolve::new(&dependency_provider); for p in dependency_provider.packages() { - for n in dependency_provider.versions(p).unwrap() { - if let Ok(s) = resolve(&dependency_provider, p, n.clone()) { - assert!(sat.sat_is_valid_solution(&s)); - } else { - assert!(!sat.sat_resolve(p, &n)); - } + for v in dependency_provider.versions(p).unwrap() { + let res = resolve(&dependency_provider, p.clone(), v); + sat.check_resolve(&res, p, v); } } } + eprintln!(" in {}s", start_time.elapsed().as_secs()) } } diff --git a/vendor/pubgrub/tests/sat_dependency_provider.rs b/vendor/pubgrub/tests/sat_dependency_provider.rs index 97ecab3e8..fe03d9cbf 100644 --- a/vendor/pubgrub/tests/sat_dependency_provider.rs +++ b/vendor/pubgrub/tests/sat_dependency_provider.rs @@ -1,23 +1,12 @@ // SPDX-License-Identifier: MPL-2.0 +use pubgrub::error::PubGrubError; use pubgrub::package::Package; use pubgrub::solver::{Dependencies, DependencyProvider, OfflineDependencyProvider}; use pubgrub::type_aliases::{Map, SelectedDependencies}; use pubgrub::version_set::VersionSet; use varisat::ExtendFormula; -const fn num_bits() -> usize { - std::mem::size_of::() * 8 -} - -fn log_bits(x: usize) -> usize { - if x == 0 { - return 0; - } - assert!(x > 0); - (num_bits::() as u32 - x.leading_zeros()) as usize -} - fn sat_at_most_one(solver: &mut impl varisat::ExtendFormula, vars: &[varisat::Var]) { if vars.len() <= 1 { return; @@ -32,7 +21,8 @@ fn sat_at_most_one(solver: &mut impl varisat::ExtendFormula, vars: &[varisat::Va } // use the "Binary Encoding" from // https://www.it.uu.se/research/group/astra/ModRef10/papers/Alan%20M.%20Frisch%20and%20Paul%20A.%20Giannoros.%20SAT%20Encodings%20of%20the%20At-Most-k%20Constraint%20-%20ModRef%202010.pdf - let bits: Vec = solver.new_var_iter(log_bits(vars.len())).collect(); + let len_bits = vars.len().ilog2() as usize + 1; + let bits: Vec = solver.new_var_iter(len_bits).collect(); for (i, p) in vars.iter().enumerate() { for (j, &bit) in bits.iter().enumerate() { solver.add_clause(&[p.negative(), bit.lit(((1 << j) & i) > 0)]); @@ -79,10 +69,10 @@ impl SatResolve { Dependencies::Unknown => panic!(), Dependencies::Known(d) => d, }; - for (p1, range) in &deps { + for (p1, range) in deps { let empty_vec = vec![]; let mut matches: Vec = all_versions_by_p - .get(p1) + .get(&p1) .unwrap_or(&empty_vec) .iter() .filter(|(v1, _)| range.contains(v1)) @@ -110,7 +100,7 @@ impl SatResolve { } } - pub fn sat_resolve(&mut self, name: &P, ver: &VS::V) -> bool { + pub fn resolve(&mut self, name: &P, ver: &VS::V) -> bool { if let Some(vers) = self.all_versions_by_p.get(name) { if let Some((_, var)) = vers.iter().find(|(v, _)| v == ver) { self.solver.assume(&[var.positive()]); @@ -126,16 +116,13 @@ impl SatResolve { } } - pub fn sat_is_valid_solution(&mut self, pids: &SelectedDependencies) -> bool { + pub fn is_valid_solution(&mut self, pids: &SelectedDependencies) -> bool { let mut assumption = vec![]; for (p, vs) in &self.all_versions_by_p { + let pid_for_p = pids.get(p); for (v, var) in vs { - assumption.push(if pids.get(p) == Some(v) { - var.positive() - } else { - var.negative() - }) + assumption.push(var.lit(pid_for_p == Some(v))) } } @@ -145,4 +132,20 @@ impl SatResolve { .solve() .expect("docs say it can't error in default config") } + + pub fn check_resolve( + &mut self, + res: &Result, PubGrubError>, + p: &P, + v: &VS::V, + ) { + match res { + Ok(s) => { + assert!(self.is_valid_solution(s)); + } + Err(_) => { + assert!(!self.resolve(p, v)); + } + } + } }