Switch pubgrub to experimental iter commit 02a19f7

vendor/pubgrub/Cargo.toml

@@ -8,7 +8,7 @@ authors = [
     "Alex Tokarev <aleksator@gmail.com>",
     "Jacob Finkelman <Eh2406@wayne.edu>",
 ]
-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
 

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<P: Package, VS: VersionSet, Priority: Ord + Clone> {
     root_package: P,
     root_version: VS::V,
 
-    pub incompatibilities: Map<P, Vec<IncompId<P, VS>>>,
+    incompatibilities: Map<P, Vec<IncompId<P, VS>>>,
 
     /// 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<P: Package, VS: VersionSet, Priority: Ord + Clone> State<P, VS, Priority> {
         &mut self,
         package: P,
         version: VS::V,
-        deps: &DependencyConstraints<P, VS>,
+        deps: impl IntoIterator<Item = (P, VS)>,
     ) -> std::ops::Range<IncompId<P, VS>> {
         // 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.

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<P: Package, VS: VersionSet> {
-    pub package_terms: SmallMap<P, Term<VS>>,
+    package_terms: SmallMap<P, Term<VS>>,
-    pub kind: Kind<P, VS>,
+    kind: Kind<P, VS>,
 }
 
 /// Type alias of unique identifiers for incompatibilities.
 pub type IncompId<P, VS> = Id<Incompatibility<P, VS>>;
 
 #[derive(Debug, Clone)]
-pub enum Kind<P: Package, VS: VersionSet> {
+enum Kind<P: Package, VS: VersionSet> {
     /// 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<P: Package, VS: VersionSet> Incompatibility<P, VS> {
     }
 
     /// 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<P: Package, VS: VersionSet> Incompatibility<P, VS> {
                     (p2.clone(), Term::Negative(set2.clone())),
                 ])
             },
-            kind: Kind::FromDependencyOf(package, set1, p2.clone(), set2.clone()),
+            kind: Kind::FromDependencyOf(package, set1, p2, set2),
         }
     }
 

vendor/pubgrub/src/internal/partial_solution.rs

@@ -165,7 +165,13 @@ impl<P: Package, VS: VersionSet, Priority: Ord + Clone> PartialSolution<P, VS, P
                     AssignmentsIntersection::Decision(_) => 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<P: Package, VS: VersionSet, Priority: Ord + Clone> PartialSolution<P, VS, P
         }
     }
 
-    pub fn prioritized_packages(&self) -> impl Iterator<Item = (&P, &VS)> {
-        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,

vendor/pubgrub/src/internal/small_vec.rs

@@ -166,9 +166,9 @@ impl<T> IntoIterator for SmallVec<T> {
     fn into_iter(self) -> Self::IntoIter {
         match self {
             SmallVec::Empty => SmallVecIntoIter::Empty,
-            SmallVec::One(a) => SmallVecIntoIter::One(IntoIterator::into_iter(a)),
+            SmallVec::One(a) => SmallVecIntoIter::One(a.into_iter()),
-            SmallVec::Two(a) => SmallVecIntoIter::Two(IntoIterator::into_iter(a)),
+            SmallVec::Two(a) => SmallVecIntoIter::Two(a.into_iter()),
-            SmallVec::Flexible(v) => SmallVecIntoIter::Flexible(IntoIterator::into_iter(v)),
+            SmallVec::Flexible(v) => SmallVecIntoIter::Flexible(v.into_iter()),
         }
     }
 }

vendor/pubgrub/src/lib.rs

@@ -102,8 +102,10 @@
 //!         &self,
 //!         package: &String,
 //!         version: &SemanticVersion,
-//!     ) -> Result<Dependencies<String, SemVS>, Box<dyn Error + Send + Sync>> {
+//!     ) -> Result<Dependencies<impl IntoIterator<Item = (String, SemVS)> + Clone>, Box<dyn Error + Send + Sync>>
-//!         unimplemented!()
+//!     {
+//!         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;

vendor/pubgrub/src/range.rs

@@ -195,7 +195,7 @@ impl<V: Ord> Range<V> {
                 .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<V: Ord> Range<V> {
     }
 }
 
-/// Implementation of [`Bound::as_ref`] which is currently marked as unstable.
-fn bound_as_ref<V>(bound: &Bound<V>) -> Bound<&V> {
-    match bound {
-        Included(v) => Included(v),
-        Excluded(v) => Excluded(v),
-        Unbounded => Unbounded,
-    }
-}
-
 fn valid_segment<T: PartialOrd>(start: &Bound<T>, end: &Bound<T>) -> bool {
     match (start, end) {
         (Included(s), Included(e)) => s <= e,
@@ -307,7 +298,7 @@ impl<V: Ord + Clone> Range<V> {
 
                 (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<V: Ord + Clone> Range<V> {
 
                 (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<V: Display + Eq> Display for Range<V> {
         } else {
             for (idx, segment) in self.segments.iter().enumerate() {
                 if idx > 0 {
-                    write!(f, ", ")?;
+                    write!(f, " | ")?;
                 }
                 match segment {
                     (Unbounded, Unbounded) => write!(f, "*")?,
@@ -382,7 +373,7 @@ impl<V: Display + Eq> Display for Range<V> {
                     (Included(v), Unbounded) => write!(f, ">={v}")?,
                     (Included(v), Included(b)) => {
                         if v == b {
-                            write!(f, "=={v}")?
+                            write!(f, "{v}")?
                         } else {
                             write!(f, ">={v}, <={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;
+use crate::internal::core::State;
-pub use crate::internal::incompatibility::{Incompatibility, Kind};
+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<P: Package, VS: VersionSet>(
                     ));
                     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<P: Package, VS: VersionSet>(
             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<P: Package, VS: VersionSet>(
 /// 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<P: Package, VS: VersionSet> {
+pub enum Dependencies<T> {
     /// Package dependencies are unavailable.
     Unknown,
     /// Container for all available package versions.
-    Known(DependencyConstraints<P, VS>),
+    Known(T),
 }
 
 /// Trait that allows the algorithm to retrieve available packages and their dependencies.
@@ -255,7 +255,7 @@ pub trait DependencyProvider<P: Package, VS: VersionSet> {
         &self,
         package: &P,
         version: &VS::V,
-    ) -> Result<Dependencies<P, VS>, Box<dyn Error + Send + Sync>>;
+    ) -> Result<Dependencies<impl IntoIterator<Item = (P, VS)> + Clone>, Box<dyn Error + Send + Sync>>;
 
     /// 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<P: Package, VS: VersionSet> {
 )]
 #[cfg_attr(feature = "serde", serde(transparent))]
 pub struct OfflineDependencyProvider<P: Package, VS: VersionSet> {
-    dependencies: Map<P, BTreeMap<VS::V, DependencyConstraints<P, VS>>>,
+    dependencies: Map<P, BTreeMap<VS::V, Map<P, VS>>>,
 }
 
 impl<P: Package, VS: VersionSet> OfflineDependencyProvider<P, VS> {
@@ -329,8 +329,8 @@ impl<P: Package, VS: VersionSet> OfflineDependencyProvider<P, VS> {
 
     /// 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<DependencyConstraints<P, VS>> {
+    fn dependencies(&self, package: &P, version: &VS::V) -> Option<&Map<P, VS>> {
-        self.dependencies.get(package)?.get(version).cloned()
+        self.dependencies.get(package)?.get(version)
     }
 }
 
@@ -364,11 +364,16 @@ impl<P: Package, VS: VersionSet> DependencyProvider<P, VS> for OfflineDependency
     fn get_dependencies(
         &self,
         package: &P,
-        version: &VS::V,
+        version: &<VS as VersionSet>::V,
-    ) -> Result<Dependencies<P, VS>, Box<dyn Error + Send + Sync>> {
+    ) -> Result<Dependencies<impl IntoIterator<Item = (P, VS)> + Clone>, Box<dyn Error + Send + Sync>>
+    {
         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())),
+            ),
         })
     }
 }

vendor/pubgrub/src/term.rs

@@ -64,7 +64,7 @@ impl<VS: VersionSet> Term<VS> {
 
     /// 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"),

vendor/pubgrub/src/type_aliases.rs

@@ -8,10 +8,3 @@ pub type Map<K, V> = rustc_hash::FxHashMap<K, V>;
 /// Concrete dependencies picked by the library during [resolve](crate::solver::resolve)
 /// from [DependencyConstraints].
 pub type SelectedDependencies<P, V> = Map<P, V>;
-
-/// Holds information about all possible versions a given package can accept.
-/// There is a difference in semantics between an empty map
-/// inside [DependencyConstraints] and [Dependencies::Unknown](crate::solver::Dependencies::Unknown):
-/// the former means the package has no dependency and it is a known fact,
-/// while the latter means they could not be fetched by the [DependencyProvider](crate::solver::DependencyProvider).
-pub type DependencyConstraints<P, VS> = Map<P, VS>;

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<P: Package, VS: VersionSet> DependencyProvider<P, VS>
         &self,
         p: &P,
         v: &VS::V,
-    ) -> Result<Dependencies<P, VS>, Box<dyn Error + Send + Sync>> {
+    ) -> Result<Dependencies<impl IntoIterator<Item = (P, VS)> + Clone>, Box<dyn Error + Send + Sync>>
+    {
         self.0.get_dependencies(p, v)
     }
 
@@ -85,7 +87,8 @@ impl<P: Package, VS: VersionSet, DP: DependencyProvider<P, VS>> DependencyProvid
         &self,
         p: &P,
         v: &VS::V,
-    ) -> Result<Dependencies<P, VS>, Box<dyn Error + Send + Sync>> {
+    ) -> Result<Dependencies<impl IntoIterator<Item = (P, VS)> + Clone>, Box<dyn Error + Send + Sync>>
+    {
         self.dp.get_dependencies(p, v)
     }
 
@@ -112,6 +115,18 @@ impl<P: Package, VS: VersionSet, DP: DependencyProvider<P, VS>> DependencyProvid
     }
 }
 
+fn timeout_resolve<P: Package, VS: VersionSet, DP: DependencyProvider<P, VS>>(
+    dependency_provider: DP,
+    name: P,
+    version: impl Into<VS::V>,
+) -> Result<SelectedDependencies<P, VS::V>, PubGrubError<P, VS>> {
+    resolve(
+        &TimeoutDependencyProvider::new(dependency_provider, 50_000),
+        name,
+        version,
+    )
+}
+
 type NumVS = Range<NumberVersion>;
 type SemVS = Range<SemanticVersion>;
 
@@ -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<N: Package + Ord, VS: VersionSet>(
+    dependency_provider: &OfflineDependencyProvider<N, VS>,
+    mut retain: impl FnMut(&N, &VS::V) -> bool,
+) -> OfflineDependencyProvider<N, VS> {
+    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<N: Package + Ord, VS: VersionSet>(
+    dependency_provider: &OfflineDependencyProvider<N, VS>,
+    mut retain: impl FnMut(&N, &VS::V, &N) -> bool,
+) -> OfflineDependencyProvider<N, VS> {
+    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<N: Package + Ord>(
+    dependency_provider: OfflineDependencyProvider<N, NumVS>,
+    name: N,
+    ver: NumberVersion,
+) {
+    let Err(PubGrubError::NoSolution(tree)) =
+        timeout_resolve(dependency_provider.clone(), name.clone(), ver)
+    else {
+        return;
+    };
+
+    fn recursive<N: Package + Ord, VS: VersionSet>(
+        to_retain: &mut Vec<(N, VS, N)>,
+        tree: &DerivationTree<N, VS>,
+    ) {
+        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,12 +442,18 @@ 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) {
+            let res = timeout_resolve(dependency_provider.clone(), name, ver);
-                prop_assert!(sat.sat_is_valid_solution(&s));
+            sat.check_resolve(&res, &name, &ver);
-            } else {
-                prop_assert!(!sat.sat_resolve(&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);
+        }
     }
 
     #[test]
@@ -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 l = timeout_resolve(dependency_provider.clone(), name, ver);
-            let r = resolve(&TimeoutDependencyProvider::new(reverse_provider.clone(), 50_000), 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::<prop::sample::Index>(), any::<prop::sample::Index>(), any::<prop::sample::Index>()), 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;
+                to_remove.insert((package, **version, dep_idx.get(&dependencies).0));
-                removed_provider.add_dependencies(
-                    **package,
-                    **version,
-                    dependencies.into_iter().filter(|x| x.0 != dependency),
-                )
             }
         }
+        let removed_provider = retain_dependencies(
+            &dependency_provider,
+            |p, v, d| {!to_remove.contains(&(&p, *v, *d))}
+        );
         for (name, ver) in cases {
-            if resolve(
+            if timeout_resolve(dependency_provider.clone(), name, ver).is_ok() {
-                &TimeoutDependencyProvider::new(dependency_provider.clone(), 50_000),
-                name,
-                ver,
-            )
-            .is_ok()
-            {
                 prop_assert!(
-                    resolve(
+                    timeout_resolve(removed_provider.clone(), name, ver).is_ok(),
-                        &TimeoutDependencyProvider::new(removed_provider.clone(), 50_000),
-                        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();
+                    let smaller_dependency_provider = retain_versions(&dependency_provider, |n, v| {
-                    for &(n, v) in &all_versions {
+                            used.get(&n) == Some(&v) // it was used
-                        if used.get(&n) == Some(&v) // it was used
+                            || to_remove.get(&(*n, *v)).is_none() // or it is not one to be removed
-                           || to_remove.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)
-                        }
-                    }
                     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();
+                    let smaller_dependency_provider = retain_versions(&dependency_provider, |n, v| {
-                    for &(n, v) in &all_versions {
+                        to_remove.get(&(*n, *v)).is_some() // it is one to be removed
-                        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)
-                        }
-                    }
                     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<u16, NumVS> =
                 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() {
+                for v in dependency_provider.versions(p).unwrap() {
-                    if let Ok(s) = resolve(&dependency_provider, p.clone(), n.clone()) {
+                    let res = resolve(&dependency_provider, p.clone(), v);
-                        assert!(sat.sat_is_valid_solution(&s));
+                    sat.check_resolve(&res, p, v);
-                    } else {
-                        assert!(!sat.sat_resolve(p, &n));
-                    }
                 }
             }
         } 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() {
+                for v in dependency_provider.versions(p).unwrap() {
-                    if let Ok(s) = resolve(&dependency_provider, p, n.clone()) {
+                    let res = resolve(&dependency_provider, p.clone(), v);
-                        assert!(sat.sat_is_valid_solution(&s));
+                    sat.check_resolve(&res, p, v);
-                    } else {
-                        assert!(!sat.sat_resolve(p, &n));
-                    }
                 }
             }
         }
+        eprintln!(" in {}s", start_time.elapsed().as_secs())
     }
 }

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<T>() -> usize {
-    std::mem::size_of::<T>() * 8
-}
-
-fn log_bits(x: usize) -> usize {
-    if x == 0 {
-        return 0;
-    }
-    assert!(x > 0);
-    (num_bits::<usize>() 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<varisat::Var> = solver.new_var_iter(log_bits(vars.len())).collect();
+    let len_bits = vars.len().ilog2() as usize + 1;
+    let bits: Vec<varisat::Var> = 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<P: Package, VS: VersionSet> SatResolve<P, VS> {
                 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<varisat::Lit> = all_versions_by_p
-                    .get(p1)
+                    .get(&p1)
                     .unwrap_or(&empty_vec)
                     .iter()
                     .filter(|(v1, _)| range.contains(v1))
@@ -110,7 +100,7 @@ impl<P: Package, VS: VersionSet> SatResolve<P, VS> {
         }
     }
 
-    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<P: Package, VS: VersionSet> SatResolve<P, VS> {
         }
     }
 
-    pub fn sat_is_valid_solution(&mut self, pids: &SelectedDependencies<P, VS::V>) -> bool {
+    pub fn is_valid_solution(&mut self, pids: &SelectedDependencies<P, VS::V>) -> 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) {
+                assumption.push(var.lit(pid_for_p == Some(v)))
-                    var.positive()
-                } else {
-                    var.negative()
-                })
             }
         }
 
@@ -145,4 +132,20 @@ impl<P: Package, VS: VersionSet> SatResolve<P, VS> {
             .solve()
             .expect("docs say it can't error in default config")
     }
+
+    pub fn check_resolve(
+        &mut self,
+        res: &Result<SelectedDependencies<P, VS::V>, PubGrubError<P, VS>>,
+        p: &P,
+        v: &VS::V,
+    ) {
+        match res {
+            Ok(s) => {
+                assert!(self.is_valid_solution(s));
+            }
+            Err(_) => {
+                assert!(!self.resolve(p, v));
+            }
+        }
+    }
 }