Upgrade pubgrub-rs version (#220)

Upgrades our PubGrub to 8951e37fe923a7edd5a78ed5f49f165b0fdc48de.
This commit is contained in:
Charlie Marsh 2023-10-29 13:25:55 -07:00 committed by GitHub
parent 1e4259a608
commit 4209e77c95
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 61 additions and 128 deletions

View file

@ -195,27 +195,14 @@ impl<'a, Context: BuildContext + Sync> Resolver<'a, Context> {
)); ));
continue; continue;
} }
Dependencies::Known(constraints) => { Dependencies::Known(constraints) if constraints.contains_key(package) => {
if constraints.contains_key(package) { return Err(PubGrubError::SelfDependency {
return Err(PubGrubError::SelfDependency { package: package.clone(),
package: package.clone(), version: version.clone(),
version: version.clone(),
}
.into());
} }
if let Some((dependent, _)) = constraints .into());
.iter()
.find(|(_, r)| r == &&Range::<PubGrubVersion>::empty())
{
return Err(PubGrubError::DependencyOnTheEmptySet {
package: package.clone(),
version: version.clone(),
dependent: dependent.clone(),
}
.into());
}
constraints
} }
Dependencies::Known(constraints) => constraints,
}; };
// Add that package and version if the dependencies are not problematic. // Add that package and version if the dependencies are not problematic.
@ -225,17 +212,6 @@ impl<'a, Context: BuildContext + Sync> Resolver<'a, Context> {
&dependencies, &dependencies,
); );
if state.incompatibility_store[dep_incompats.clone()]
.iter()
.any(|incompat| state.is_terminal(incompat))
{
// For a dependency incompatibility to be terminal,
// it can only mean that root depend on not root?
return Err(PubGrubError::Failure(
"Root package depends on itself at a different version?".into(),
)
.into());
}
state.partial_solution.add_version( state.partial_solution.add_version(
package.clone(), package.clone(),
version, version,

View file

@ -30,21 +30,6 @@ pub enum PubGrubError<P: Package, VS: VersionSet> {
source: Box<dyn std::error::Error + Send + Sync>, source: Box<dyn std::error::Error + Send + Sync>,
}, },
/// Error arising when the implementer of
/// [DependencyProvider](crate::solver::DependencyProvider)
/// returned a dependency on an empty range.
/// This technically means that the package can not be selected,
/// but is clearly some kind of mistake.
#[error("Package {dependent} required by {package} {version} depends on the empty set")]
DependencyOnTheEmptySet {
/// Package whose dependencies we want.
package: P,
/// Version of the package for which we want the dependencies.
version: VS::V,
/// The dependent package that requires us to pick from the empty set.
dependent: P,
},
/// Error arising when the implementer of /// Error arising when the implementer of
/// [DependencyProvider](crate::solver::DependencyProvider) /// [DependencyProvider](crate::solver::DependencyProvider)
/// returned a dependency on the requested package. /// returned a dependency on the requested package.

View file

@ -90,11 +90,6 @@ impl<P: Package, VS: VersionSet> State<P, VS> {
new_incompats_id_range new_incompats_id_range
} }
/// Check if an incompatibility is terminal.
pub fn is_terminal(&self, incompatibility: &Incompatibility<P, VS>) -> bool {
incompatibility.is_terminal(&self.root_package, &self.root_version)
}
/// Unit propagation is the core mechanism of the solving algorithm. /// Unit propagation is the core mechanism of the solving algorithm.
/// CF <https://github.com/dart-lang/pub/blob/master/doc/solver.md#unit-propagation> /// CF <https://github.com/dart-lang/pub/blob/master/doc/solver.md#unit-propagation>
pub fn unit_propagation(&mut self, package: P) -> Result<(), PubGrubError<P, VS>> { pub fn unit_propagation(&mut self, package: P) -> Result<(), PubGrubError<P, VS>> {
@ -240,7 +235,10 @@ impl<P: Package, VS: VersionSet> State<P, VS> {
/// It may not be trivial since those incompatibilities /// It may not be trivial since those incompatibilities
/// may already have derived others. /// may already have derived others.
fn merge_incompatibility(&mut self, id: IncompId<P, VS>) { fn merge_incompatibility(&mut self, id: IncompId<P, VS>) {
for (pkg, _term) in self.incompatibility_store[id].iter() { for (pkg, term) in self.incompatibility_store[id].iter() {
if cfg!(debug_assertions) {
assert_ne!(term, &crate::term::Term::any());
}
self.incompatibilities self.incompatibilities
.entry(pkg.clone()) .entry(pkg.clone())
.or_default() .or_default()

View file

@ -109,10 +109,14 @@ impl<P: Package, VS: VersionSet> Incompatibility<P, VS> {
let set1 = VS::singleton(version); let set1 = VS::singleton(version);
let (p2, set2) = dep; let (p2, set2) = dep;
Self { Self {
package_terms: SmallMap::Two([ package_terms: if set2 == &VS::empty() {
(package.clone(), Term::Positive(set1.clone())), SmallMap::One([(package.clone(), Term::Positive(set1.clone()))])
(p2.clone(), Term::Negative(set2.clone())), } else {
]), SmallMap::Two([
(package.clone(), Term::Positive(set1.clone())),
(p2.clone(), Term::Negative(set2.clone())),
])
},
kind: Kind::FromDependencyOf(package, set1, p2.clone(), set2.clone()), kind: Kind::FromDependencyOf(package, set1, p2.clone(), set2.clone()),
} }
} }

View file

@ -141,7 +141,13 @@ impl<P: Package, VS: VersionSet> PartialSolution<P, VS> {
AssignmentsIntersection::Decision(_) => panic!("Already existing decision"), AssignmentsIntersection::Decision(_) => panic!("Already existing decision"),
// Cannot be called if the versions is not contained in the terms intersection. // Cannot be called if the versions is not contained in the terms intersection.
AssignmentsIntersection::Derivations(term) => { AssignmentsIntersection::Derivations(term) => {
debug_assert!(term.contains(&version)) debug_assert!(
term.contains(&version),
"{}: {} was expected to be contained in {}",
package,
version,
term,
)
} }
}, },
} }

View file

@ -1,7 +1,5 @@
// SPDX-License-Identifier: MPL-2.0 // SPDX-License-Identifier: MPL-2.0
#![allow(clippy::all, unreachable_pub)]
//! PubGrub version solving algorithm. //! PubGrub version solving algorithm.
//! //!
//! Version solving consists in efficiently finding a set of packages and versions //! Version solving consists in efficiently finding a set of packages and versions
@ -215,6 +213,8 @@
//! with a cache, you may want to know that some versions //! with a cache, you may want to know that some versions
//! do not exist in your cache. //! do not exist in your cache.
#![allow(clippy::all, unreachable_pub)]
pub mod error; pub mod error;
pub mod package; pub mod package;
pub mod range; pub mod range;

View file

@ -178,36 +178,13 @@ pub fn resolve<P: Package, VS: VersionSet>(
version: v.clone(), version: v.clone(),
}); });
} }
Dependencies::Known(x) => { Dependencies::Known(x) => x,
if let Some((dependent, _)) = x.iter().find(|(_, r)| r == &&VS::empty()) {
return Err(PubGrubError::DependencyOnTheEmptySet {
package: p.clone(),
version: v.clone(),
dependent: dependent.clone(),
});
}
x
}
}; };
// Add that package and version if the dependencies are not problematic. // Add that package and version if the dependencies are not problematic.
let dep_incompats = let dep_incompats =
state.add_incompatibility_from_dependencies(p.clone(), v.clone(), &known_dependencies); state.add_incompatibility_from_dependencies(p.clone(), v.clone(), &known_dependencies);
// TODO: I don't think this check can actually happen.
// We might want to put it under #[cfg(debug_assertions)].
let incompatible_self_dependency = state.incompatibility_store[dep_incompats.clone()]
.iter()
.any(|incompat| state.is_terminal(incompat));
if incompatible_self_dependency {
// For a dependency incompatibility to be terminal,
// it can only mean that root depend on not root?
return Err(PubGrubError::Failure(
"Root package depends on itself at a different version?".into(),
));
}
state.partial_solution.add_version( state.partial_solution.add_version(
p.clone(), p.clone(),
v, v,

View file

@ -13,7 +13,7 @@ use pubgrub::solver::{
use pubgrub::version::{NumberVersion, SemanticVersion}; use pubgrub::version::{NumberVersion, SemanticVersion};
use pubgrub::version_set::VersionSet; use pubgrub::version_set::VersionSet;
use proptest::collection::{btree_map, vec}; use proptest::collection::{btree_map, btree_set, vec};
use proptest::prelude::*; use proptest::prelude::*;
use proptest::sample::Index; use proptest::sample::Index;
use proptest::string::string_regex; use proptest::string::string_regex;
@ -131,20 +131,15 @@ fn string_names() -> impl Strategy<Value = String> {
/// This strategy has a high probability of having valid dependencies /// This strategy has a high probability of having valid dependencies
pub fn registry_strategy<N: Package + Ord>( pub fn registry_strategy<N: Package + Ord>(
name: impl Strategy<Value = N>, name: impl Strategy<Value = N>,
bad_name: N,
) -> impl Strategy<Value = (OfflineDependencyProvider<N, NumVS>, Vec<(N, NumberVersion)>)> { ) -> impl Strategy<Value = (OfflineDependencyProvider<N, NumVS>, Vec<(N, NumberVersion)>)> {
let max_crates = 40; let max_crates = 40;
let max_versions = 15; let max_versions = 15;
let shrinkage = 40; let shrinkage = 40;
let complicated_len = 10usize; let complicated_len = 10usize;
// If this is false than the crate will depend on the nonexistent "bad"
// instead of the complex set we generated for it.
let allow_deps = prop::bool::weighted(0.99);
let a_version = ..(max_versions as u32); let a_version = ..(max_versions as u32);
let list_of_versions = btree_map(a_version, allow_deps, 1..=max_versions) let list_of_versions = btree_set(a_version, 1..=max_versions)
.prop_map(move |ver| ver.into_iter().collect::<Vec<_>>()); .prop_map(move |ver| ver.into_iter().collect::<Vec<_>>());
let list_of_crates_with_versions = btree_map(name, list_of_versions, 1..=max_crates); let list_of_crates_with_versions = btree_map(name, list_of_versions, 1..=max_crates);
@ -177,16 +172,12 @@ pub fn registry_strategy<N: Package + Ord>(
) )
.prop_map( .prop_map(
move |(crate_vers_by_name, raw_dependencies, reverse_alphabetical, complicated_len)| { move |(crate_vers_by_name, raw_dependencies, reverse_alphabetical, complicated_len)| {
let mut list_of_pkgid: Vec<((N, NumberVersion), Option<Vec<(N, NumVS)>>)> = let mut list_of_pkgid: Vec<((N, NumberVersion), Vec<(N, NumVS)>)> =
crate_vers_by_name crate_vers_by_name
.iter() .iter()
.flat_map(|(name, vers)| { .flat_map(|(name, vers)| {
vers.iter().map(move |x| { vers.iter()
( .map(move |x| ((name.clone(), NumberVersion::from(x)), vec![]))
(name.clone(), NumberVersion::from(x.0)),
if x.1 { Some(vec![]) } else { None },
)
})
}) })
.collect(); .collect();
let len_all_pkgid = list_of_pkgid.len(); let len_all_pkgid = list_of_pkgid.len();
@ -199,24 +190,24 @@ pub fn registry_strategy<N: Package + Ord>(
} }
let s = &crate_vers_by_name[&dep_name]; let s = &crate_vers_by_name[&dep_name];
let s_last_index = s.len() - 1; let s_last_index = s.len() - 1;
let (c, d) = order_index(c, d, s.len()); let (c, d) = order_index(c, d, s.len() + 1);
if let (_, Some(deps)) = &mut list_of_pkgid[b] { list_of_pkgid[b].1.push((
deps.push(( dep_name,
dep_name, if c > s_last_index {
if c == 0 && d == s_last_index { Range::empty()
Range::full() } else if c == 0 && d >= s_last_index {
} else if c == 0 { Range::full()
Range::strictly_lower_than(s[d].0 + 1) } else if c == 0 {
} else if d == s_last_index { Range::strictly_lower_than(s[d] + 1)
Range::higher_than(s[c].0) } else if d >= s_last_index {
} else if c == d { Range::higher_than(s[c])
Range::singleton(s[c].0) } else if c == d {
} else { Range::singleton(s[c])
Range::between(s[c].0, s[d].0 + 1) } else {
}, Range::between(s[c], s[d] + 1)
)) },
} ));
} }
let mut dependency_provider = OfflineDependencyProvider::<N, NumVS>::new(); let mut dependency_provider = OfflineDependencyProvider::<N, NumVS>::new();
@ -232,11 +223,7 @@ pub fn registry_strategy<N: Package + Ord>(
.collect(); .collect();
for ((name, ver), deps) in list_of_pkgid { for ((name, ver), deps) in list_of_pkgid {
dependency_provider.add_dependencies( dependency_provider.add_dependencies(name, ver, deps);
name,
ver,
deps.unwrap_or_else(|| vec![(bad_name.clone(), Range::full())]),
);
} }
(dependency_provider, complicated) (dependency_provider, complicated)
@ -252,7 +239,7 @@ fn meta_test_deep_trees_from_strategy() {
let mut dis = [0; 21]; let mut dis = [0; 21];
let strategy = registry_strategy(0u16..665, 666); let strategy = registry_strategy(0u16..665);
let mut test_runner = TestRunner::deterministic(); let mut test_runner = TestRunner::deterministic();
for _ in 0..128 { for _ in 0..128 {
let (dependency_provider, cases) = strategy let (dependency_provider, cases) = strategy
@ -299,7 +286,7 @@ proptest! {
#[test] #[test]
/// This test is mostly for profiling. /// This test is mostly for profiling.
fn prop_passes_string( fn prop_passes_string(
(dependency_provider, cases) in registry_strategy(string_names(), "bad".to_owned()) (dependency_provider, cases) in registry_strategy(string_names())
) { ) {
for (name, ver) in cases { for (name, ver) in cases {
let _ = resolve(&TimeoutDependencyProvider::new(dependency_provider.clone(), 50_000), name, ver); let _ = resolve(&TimeoutDependencyProvider::new(dependency_provider.clone(), 50_000), name, ver);
@ -309,7 +296,7 @@ proptest! {
#[test] #[test]
/// This test is mostly for profiling. /// This test is mostly for profiling.
fn prop_passes_int( fn prop_passes_int(
(dependency_provider, cases) in registry_strategy(0u16..665, 666) (dependency_provider, cases) in registry_strategy(0u16..665)
) { ) {
for (name, ver) in cases { for (name, ver) in cases {
let _ = resolve(&TimeoutDependencyProvider::new(dependency_provider.clone(), 50_000), name, ver); let _ = resolve(&TimeoutDependencyProvider::new(dependency_provider.clone(), 50_000), name, ver);
@ -318,7 +305,7 @@ proptest! {
#[test] #[test]
fn prop_sat_errors_the_same( fn prop_sat_errors_the_same(
(dependency_provider, cases) in registry_strategy(0u16..665, 666) (dependency_provider, cases) in registry_strategy(0u16..665)
) { ) {
let mut sat = SatResolve::new(&dependency_provider); let mut sat = SatResolve::new(&dependency_provider);
for (name, ver) in cases { for (name, ver) in cases {
@ -333,7 +320,7 @@ proptest! {
#[test] #[test]
/// This tests whether the algorithm is still deterministic. /// This tests whether the algorithm is still deterministic.
fn prop_same_on_repeated_runs( fn prop_same_on_repeated_runs(
(dependency_provider, cases) in registry_strategy(0u16..665, 666) (dependency_provider, cases) in registry_strategy(0u16..665)
) { ) {
for (name, ver) in cases { for (name, ver) in cases {
let one = resolve(&TimeoutDependencyProvider::new(dependency_provider.clone(), 50_000), name, ver); let one = resolve(&TimeoutDependencyProvider::new(dependency_provider.clone(), 50_000), name, ver);
@ -355,7 +342,7 @@ proptest! {
/// [ReverseDependencyProvider] changes what order the candidates /// [ReverseDependencyProvider] changes what order the candidates
/// are tried but not the existence of a solution. /// are tried but not the existence of a solution.
fn prop_reversed_version_errors_the_same( fn prop_reversed_version_errors_the_same(
(dependency_provider, cases) in registry_strategy(0u16..665, 666) (dependency_provider, cases) in registry_strategy(0u16..665)
) { ) {
let reverse_provider = OldestVersionsDependencyProvider(dependency_provider.clone()); let reverse_provider = OldestVersionsDependencyProvider(dependency_provider.clone());
for (name, ver) in cases { for (name, ver) in cases {
@ -371,7 +358,7 @@ proptest! {
#[test] #[test]
fn prop_removing_a_dep_cant_break( fn prop_removing_a_dep_cant_break(
(dependency_provider, cases) in registry_strategy(0u16..665, 666), (dependency_provider, cases) in registry_strategy(0u16..665),
indexes_to_remove in prop::collection::vec((any::<prop::sample::Index>(), any::<prop::sample::Index>(), any::<prop::sample::Index>()), 1..10) 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 packages: Vec<_> = dependency_provider.packages().collect();
@ -423,7 +410,7 @@ proptest! {
#[test] #[test]
fn prop_limited_independence_of_irrelevant_alternatives( fn prop_limited_independence_of_irrelevant_alternatives(
(dependency_provider, cases) in registry_strategy(0u16..665, 666), (dependency_provider, cases) in registry_strategy(0u16..665),
indexes_to_remove in prop::collection::vec(any::<prop::sample::Index>(), 1..10) indexes_to_remove in prop::collection::vec(any::<prop::sample::Index>(), 1..10)
) { ) {
let all_versions: Vec<(u16, NumberVersion)> = dependency_provider let all_versions: Vec<(u16, NumberVersion)> = dependency_provider

View file

@ -35,13 +35,13 @@ fn should_always_find_a_satisfier() {
dependency_provider.add_dependencies("a", 0, [("b", Range::empty())]); dependency_provider.add_dependencies("a", 0, [("b", Range::empty())]);
assert!(matches!( assert!(matches!(
resolve(&dependency_provider, "a", 0), resolve(&dependency_provider, "a", 0),
Err(PubGrubError::DependencyOnTheEmptySet { .. }) Err(PubGrubError::NoSolution { .. })
)); ));
dependency_provider.add_dependencies("c", 0, [("a", Range::full())]); dependency_provider.add_dependencies("c", 0, [("a", Range::full())]);
assert!(matches!( assert!(matches!(
resolve(&dependency_provider, "c", 0), resolve(&dependency_provider, "c", 0),
Err(PubGrubError::DependencyOnTheEmptySet { .. }) Err(PubGrubError::NoSolution { .. })
)); ));
} }