From cc4c3e784abaa0a0ae8c94aab114929f1772e94a Mon Sep 17 00:00:00 2001 From: x-hgg-x <39058530+x-hgg-x@users.noreply.github.com> Date: Sat, 28 Sep 2024 20:01:18 +0200 Subject: [PATCH 1/4] test: separate resolver tests into multiple files --- crates/resolver-tests/src/helpers.rs | 213 ++++++ crates/resolver-tests/src/lib.rs | 851 ++++------------------- crates/resolver-tests/src/sat.rs | 416 +++++++++++ crates/resolver-tests/tests/proptests.rs | 273 ++++++++ crates/resolver-tests/tests/resolve.rs | 482 +------------ crates/resolver-tests/tests/validated.rs | 215 ++++++ 6 files changed, 1244 insertions(+), 1206 deletions(-) create mode 100644 crates/resolver-tests/src/helpers.rs create mode 100644 crates/resolver-tests/src/sat.rs create mode 100644 crates/resolver-tests/tests/proptests.rs create mode 100644 crates/resolver-tests/tests/validated.rs diff --git a/crates/resolver-tests/src/helpers.rs b/crates/resolver-tests/src/helpers.rs new file mode 100644 index 00000000000..b253451f088 --- /dev/null +++ b/crates/resolver-tests/src/helpers.rs @@ -0,0 +1,213 @@ +use std::collections::BTreeMap; +use std::fmt::Debug; +use std::sync::OnceLock; + +use cargo::core::dependency::DepKind; +use cargo::core::{Dependency, GitReference, PackageId, SourceId, Summary}; +use cargo::util::IntoUrl; + +pub trait ToDep { + fn to_dep(self) -> Dependency; + fn to_opt_dep(self) -> Dependency; + fn to_dep_with(self, features: &[&'static str]) -> Dependency; +} + +impl ToDep for &'static str { + fn to_dep(self) -> Dependency { + Dependency::parse(self, Some("1.0.0"), registry_loc()).unwrap() + } + fn to_opt_dep(self) -> Dependency { + let mut dep = self.to_dep(); + dep.set_optional(true); + dep + } + fn to_dep_with(self, features: &[&'static str]) -> Dependency { + let mut dep = self.to_dep(); + dep.set_default_features(false); + dep.set_features(features.into_iter().copied()); + dep + } +} + +impl ToDep for Dependency { + fn to_dep(self) -> Dependency { + self + } + fn to_opt_dep(mut self) -> Dependency { + self.set_optional(true); + self + } + fn to_dep_with(mut self, features: &[&'static str]) -> Dependency { + self.set_default_features(false); + self.set_features(features.into_iter().copied()); + self + } +} + +pub trait ToPkgId { + fn to_pkgid(&self) -> PackageId; +} + +impl ToPkgId for PackageId { + fn to_pkgid(&self) -> PackageId { + *self + } +} + +impl<'a> ToPkgId for &'a str { + fn to_pkgid(&self) -> PackageId { + PackageId::try_new(*self, "1.0.0", registry_loc()).unwrap() + } +} + +impl, U: AsRef> ToPkgId for (T, U) { + fn to_pkgid(&self) -> PackageId { + let (name, vers) = self; + PackageId::try_new(name.as_ref(), vers.as_ref(), registry_loc()).unwrap() + } +} + +#[macro_export] +macro_rules! pkg { + ($pkgid:expr => [$($deps:expr),* $(,)? ]) => ({ + use $crate::helpers::ToDep; + let d: Vec = vec![$($deps.to_dep()),*]; + $crate::helpers::pkg_dep($pkgid, d) + }); + + ($pkgid:expr) => ({ + $crate::helpers::pkg($pkgid) + }) +} + +fn registry_loc() -> SourceId { + static EXAMPLE_DOT_COM: OnceLock = OnceLock::new(); + let example_dot = EXAMPLE_DOT_COM.get_or_init(|| { + SourceId::for_registry(&"https://example.com".into_url().unwrap()).unwrap() + }); + *example_dot +} + +pub fn pkg(name: T) -> Summary { + pkg_dep(name, Vec::new()) +} + +pub fn pkg_dep(name: T, dep: Vec) -> Summary { + let pkgid = name.to_pkgid(); + let link = if pkgid.name().ends_with("-sys") { + Some(pkgid.name()) + } else { + None + }; + Summary::new(name.to_pkgid(), dep, &BTreeMap::new(), link, None).unwrap() +} + +pub fn pkg_dep_with( + name: T, + dep: Vec, + features: &[(&'static str, &[&'static str])], +) -> Summary { + let pkgid = name.to_pkgid(); + let link = if pkgid.name().ends_with("-sys") { + Some(pkgid.name()) + } else { + None + }; + let features = features + .into_iter() + .map(|&(name, values)| (name.into(), values.into_iter().map(|&v| v.into()).collect())) + .collect(); + Summary::new(name.to_pkgid(), dep, &features, link, None).unwrap() +} + +pub fn pkg_id(name: &str) -> PackageId { + PackageId::try_new(name, "1.0.0", registry_loc()).unwrap() +} + +fn pkg_id_loc(name: &str, loc: &str) -> PackageId { + let remote = loc.into_url(); + let master = GitReference::Branch("master".to_string()); + let source_id = SourceId::for_git(&remote.unwrap(), master).unwrap(); + + PackageId::try_new(name, "1.0.0", source_id).unwrap() +} + +pub fn pkg_loc(name: &str, loc: &str) -> Summary { + let link = if name.ends_with("-sys") { + Some(name) + } else { + None + }; + Summary::new( + pkg_id_loc(name, loc), + Vec::new(), + &BTreeMap::new(), + link, + None, + ) + .unwrap() +} + +pub fn remove_dep(sum: &Summary, ind: usize) -> Summary { + let mut deps = sum.dependencies().to_vec(); + deps.remove(ind); + // note: more things will need to be copied over in the future, but it works for now. + Summary::new(sum.package_id(), deps, &BTreeMap::new(), sum.links(), None).unwrap() +} + +pub fn dep(name: &str) -> Dependency { + dep_req(name, "*") +} + +pub fn dep_req(name: &str, req: &str) -> Dependency { + Dependency::parse(name, Some(req), registry_loc()).unwrap() +} + +pub fn dep_req_kind(name: &str, req: &str, kind: DepKind) -> Dependency { + let mut dep = dep_req(name, req); + dep.set_kind(kind); + dep +} + +pub fn dep_loc(name: &str, location: &str) -> Dependency { + let url = location.into_url().unwrap(); + let master = GitReference::Branch("master".to_string()); + let source_id = SourceId::for_git(&url, master).unwrap(); + Dependency::parse(name, Some("1.0.0"), source_id).unwrap() +} + +pub fn dep_kind(name: &str, kind: DepKind) -> Dependency { + dep(name).set_kind(kind).clone() +} + +pub fn registry(pkgs: Vec) -> Vec { + pkgs +} + +pub fn names(names: &[P]) -> Vec { + names.iter().map(|name| name.to_pkgid()).collect() +} + +pub fn loc_names(names: &[(&'static str, &'static str)]) -> Vec { + names + .iter() + .map(|&(name, loc)| pkg_id_loc(name, loc)) + .collect() +} + +/// Assert `xs` contains `elems` +#[track_caller] +pub fn assert_contains(xs: &[A], elems: &[A]) { + for elem in elems { + assert!( + xs.contains(elem), + "missing element\nset: {xs:?}\nmissing: {elem:?}" + ); + } +} + +#[track_caller] +pub fn assert_same(a: &[A], b: &[A]) { + assert_eq!(a.len(), b.len(), "not equal\n{a:?}\n{b:?}"); + assert_contains(b, a); +} diff --git a/crates/resolver-tests/src/lib.rs b/crates/resolver-tests/src/lib.rs index a3d5dad7344..a6ab7aba7fb 100644 --- a/crates/resolver-tests/src/lib.rs +++ b/crates/resolver-tests/src/lib.rs @@ -4,31 +4,33 @@ #![allow(clippy::print_stderr)] +pub mod helpers; +pub mod sat; + use std::cmp::{max, min}; -use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; +use std::collections::{BTreeMap, HashSet}; use std::fmt; -use std::fmt::Write; -use std::sync::OnceLock; use std::task::Poll; use std::time::Instant; use cargo::core::dependency::DepKind; use cargo::core::resolver::{self, ResolveOpts, VersionOrdering, VersionPreferences}; -use cargo::core::FeatureMap; +use cargo::core::Resolve; use cargo::core::ResolveVersion; +use cargo::core::SourceId; use cargo::core::{Dependency, PackageId, Registry, Summary}; -use cargo::core::{FeatureValue, Resolve}; -use cargo::core::{GitReference, SourceId}; use cargo::sources::source::QueryKind; use cargo::sources::IndexSummary; -use cargo::util::interning::{InternedString, INTERNED_DEFAULT}; -use cargo::util::{CargoResult, GlobalContext, IntoUrl}; +use cargo::util::interning::InternedString; +use cargo::util::{CargoResult, GlobalContext}; + +use crate::helpers::{dep_req, dep_req_kind, pkg_dep, pkg_id, ToPkgId}; +use crate::sat::SatResolver; use proptest::collection::{btree_map, vec}; use proptest::prelude::*; use proptest::sample::Index; use proptest::string::string_regex; -use varisat::ExtendFormula; pub fn resolve(deps: Vec, registry: &[Summary]) -> CargoResult> { Ok( @@ -205,604 +207,6 @@ pub fn resolve_with_global_context_raw( resolve } -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 -} - -// At this point is possible to select every version of every package. -// So we need to mark certain versions as incompatible with each other. -// We could add a clause not A, not B for all A and B that are incompatible, -fn sat_at_most_one(solver: &mut varisat::Solver<'_>, vars: &[varisat::Var]) { - if vars.len() <= 1 { - return; - } else if vars.len() == 2 { - solver.add_clause(&[vars[0].negative(), vars[1].negative()]); - return; - } else if vars.len() == 3 { - solver.add_clause(&[vars[0].negative(), vars[1].negative()]); - solver.add_clause(&[vars[0].negative(), vars[2].negative()]); - solver.add_clause(&[vars[1].negative(), vars[2].negative()]); - return; - } - // There are more efficient ways to do it for large numbers of versions. - // - // 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(); - for (i, p) in vars.iter().enumerate() { - for b in 0..bits.len() { - solver.add_clause(&[p.negative(), bits[b].lit(((1 << b) & i) > 0)]); - } - } -} - -fn sat_at_most_one_by_key( - solver: &mut varisat::Solver<'_>, - data: impl Iterator, -) -> HashMap> { - // no two packages with the same keys set - let mut by_keys: HashMap> = HashMap::new(); - for (p, v) in data { - by_keys.entry(p).or_default().push(v) - } - for key in by_keys.values() { - sat_at_most_one(solver, key); - } - by_keys -} - -fn find_compatible_dep_summaries_by_name_in_toml( - pkg_dependencies: &[Dependency], - by_name: &HashMap>, -) -> HashMap> { - let empty_vec = vec![]; - - pkg_dependencies - .iter() - .map(|dep| { - let name_in_toml = dep.name_in_toml(); - - let compatible_summaries = by_name - .get(&dep.package_name()) - .unwrap_or(&empty_vec) - .iter() - .filter(|s| dep.matches_id(s.package_id())) - .filter(|s| dep.features().iter().all(|f| s.features().contains_key(f))) - .cloned() - .collect::>(); - - (name_in_toml, compatible_summaries) - }) - .collect() -} - -fn process_pkg_features( - solver: &mut varisat::Solver<'_>, - var_for_is_packages_used: &HashMap, - var_for_is_packages_features_used: &HashMap>, - pkg_feature_var_map: &HashMap, - pkg_features: &FeatureMap, - compatible_dep_summaries_by_name_in_toml: &HashMap>, -) { - // add clauses for package features - for (&feature_name, feature_values) in pkg_features { - for feature_value in feature_values { - let pkg_feature_var = pkg_feature_var_map[&feature_name]; - - match *feature_value { - FeatureValue::Feature(other_feature_name) => { - solver.add_clause(&[ - pkg_feature_var.negative(), - pkg_feature_var_map[&other_feature_name].positive(), - ]); - } - FeatureValue::Dep { dep_name } => { - let dep_clause = compatible_dep_summaries_by_name_in_toml[&dep_name] - .iter() - .map(|dep| var_for_is_packages_used[&dep.package_id()].positive()) - .chain([pkg_feature_var.negative()]) - .collect::>(); - - solver.add_clause(&dep_clause); - } - FeatureValue::DepFeature { - dep_name, - dep_feature: dep_feature_name, - weak, - } => { - for dep in &compatible_dep_summaries_by_name_in_toml[&dep_name] { - let dep_var = var_for_is_packages_used[&dep.package_id()]; - let dep_feature_var = - var_for_is_packages_features_used[&dep.package_id()][&dep_feature_name]; - - solver.add_clause(&[ - pkg_feature_var.negative(), - dep_var.negative(), - dep_feature_var.positive(), - ]); - } - - if !weak { - let dep_clause = compatible_dep_summaries_by_name_in_toml[&dep_name] - .iter() - .map(|dep| var_for_is_packages_used[&dep.package_id()].positive()) - .chain([pkg_feature_var.negative()]) - .collect::>(); - - solver.add_clause(&dep_clause); - } - } - } - } - } -} - -fn process_pkg_dependencies( - solver: &mut varisat::Solver<'_>, - var_for_is_packages_used: &HashMap, - var_for_is_packages_features_used: &HashMap>, - pkg_var: varisat::Var, - pkg_dependencies: &[Dependency], - compatible_dep_summaries_by_name_in_toml: &HashMap>, -) { - for dep in pkg_dependencies { - let compatible_dep_summaries = - &compatible_dep_summaries_by_name_in_toml[&dep.name_in_toml()]; - - // add clauses for package dependency features - for dep_summary in compatible_dep_summaries { - let dep_package_id = dep_summary.package_id(); - - let default_feature = if dep.uses_default_features() - && dep_summary.features().contains_key(&*INTERNED_DEFAULT) - { - Some(&INTERNED_DEFAULT) - } else { - None - }; - - for &feature_name in default_feature.into_iter().chain(dep.features()) { - solver.add_clause(&[ - pkg_var.negative(), - var_for_is_packages_used[&dep_package_id].negative(), - var_for_is_packages_features_used[&dep_package_id][&feature_name].positive(), - ]); - } - } - - // active packages need to activate each of their non-optional dependencies - if !dep.is_optional() { - let dep_clause = compatible_dep_summaries - .iter() - .map(|d| var_for_is_packages_used[&d.package_id()].positive()) - .chain([pkg_var.negative()]) - .collect::>(); - - solver.add_clause(&dep_clause); - } - } -} - -/// Resolution can be reduced to the SAT problem. So this is an alternative implementation -/// of the resolver that uses a SAT library for the hard work. This is intended to be easy to read, -/// as compared to the real resolver. -/// -/// For the subset of functionality that are currently made by `registry_strategy`, -/// this will find a valid resolution if one exists. -/// -/// The SAT library does not optimize for the newer version, -/// so the selected packages may not match the real resolver. -pub struct SatResolver { - solver: varisat::Solver<'static>, - old_root_vars: Vec, - var_for_is_packages_used: HashMap, - var_for_is_packages_features_used: HashMap>, - by_name: HashMap>, -} - -impl SatResolver { - pub fn new(registry: &[Summary]) -> Self { - let mut solver = varisat::Solver::new(); - - // That represents each package version which is set to "true" if the packages in the lock file and "false" if it is unused. - let var_for_is_packages_used = registry - .iter() - .map(|s| (s.package_id(), solver.new_var())) - .collect::>(); - - // That represents each feature of each package version, which is set to "true" if the package feature is used. - let var_for_is_packages_features_used = registry - .iter() - .map(|s| { - ( - s.package_id(), - (s.features().keys().map(|&f| (f, solver.new_var()))).collect(), - ) - }) - .collect::>>(); - - // if a package feature is used, then the package is used - for (package, pkg_feature_var_map) in &var_for_is_packages_features_used { - for (_, package_feature_var) in pkg_feature_var_map { - let package_var = var_for_is_packages_used[package]; - solver.add_clause(&[package_feature_var.negative(), package_var.positive()]); - } - } - - // no two packages with the same links set - sat_at_most_one_by_key( - &mut solver, - registry - .iter() - .map(|s| (s.links(), var_for_is_packages_used[&s.package_id()])) - .filter(|(l, _)| l.is_some()), - ); - - // no two semver compatible versions of the same package - sat_at_most_one_by_key( - &mut solver, - var_for_is_packages_used - .iter() - .map(|(p, &v)| (p.as_activations_key(), v)), - ); - - let mut by_name: HashMap> = HashMap::new(); - - for p in registry { - by_name.entry(p.name()).or_default().push(p.clone()) - } - - for pkg in registry { - let pkg_id = pkg.package_id(); - let pkg_dependencies = pkg.dependencies(); - let pkg_features = pkg.features(); - - let compatible_dep_summaries_by_name_in_toml = - find_compatible_dep_summaries_by_name_in_toml(pkg_dependencies, &by_name); - - process_pkg_features( - &mut solver, - &var_for_is_packages_used, - &var_for_is_packages_features_used, - &var_for_is_packages_features_used[&pkg_id], - pkg_features, - &compatible_dep_summaries_by_name_in_toml, - ); - - process_pkg_dependencies( - &mut solver, - &var_for_is_packages_used, - &var_for_is_packages_features_used, - var_for_is_packages_used[&pkg_id], - pkg_dependencies, - &compatible_dep_summaries_by_name_in_toml, - ); - } - - // We don't need to `solve` now. We know that "use nothing" will satisfy all the clauses so far. - // But things run faster if we let it spend some time figuring out how the constraints interact before we add assumptions. - solver - .solve() - .expect("docs say it can't error in default config"); - - SatResolver { - solver, - old_root_vars: Vec::new(), - var_for_is_packages_used, - var_for_is_packages_features_used, - by_name, - } - } - - pub fn sat_resolve(&mut self, root_dependencies: &[Dependency]) -> bool { - let SatResolver { - solver, - old_root_vars, - var_for_is_packages_used, - var_for_is_packages_features_used, - by_name, - } = self; - - let root_var = solver.new_var(); - - // root package is always used - // root vars from previous runs are deactivated - let assumption = old_root_vars - .iter() - .map(|v| v.negative()) - .chain([root_var.positive()]) - .collect::>(); - - old_root_vars.push(root_var); - - let compatible_dep_summaries_by_name_in_toml = - find_compatible_dep_summaries_by_name_in_toml(root_dependencies, &by_name); - - process_pkg_dependencies( - solver, - var_for_is_packages_used, - var_for_is_packages_features_used, - root_var, - root_dependencies, - &compatible_dep_summaries_by_name_in_toml, - ); - - solver.assume(&assumption); - - solver - .solve() - .expect("docs say it can't error in default config") - } - - pub fn sat_is_valid_solution(&mut self, pkgs: &[(PackageId, Vec)]) -> bool { - let contains_pkg = |pkg| pkgs.iter().any(|(p, _)| p == pkg); - let contains_pkg_feature = - |pkg, f| pkgs.iter().any(|(p, flist)| p == pkg && flist.contains(f)); - - for (p, _) in pkgs { - if p.name() != "root" && !self.var_for_is_packages_used.contains_key(p) { - return false; - } - } - - // root vars from previous runs are deactivated - let assumption = (self.old_root_vars.iter().map(|v| v.negative())) - .chain( - self.var_for_is_packages_used - .iter() - .map(|(p, v)| v.lit(contains_pkg(p))), - ) - .chain( - self.var_for_is_packages_features_used - .iter() - .flat_map(|(p, fmap)| { - fmap.iter() - .map(move |(f, v)| v.lit(contains_pkg_feature(p, f))) - }), - ) - .collect::>(); - - self.solver.assume(&assumption); - - self.solver - .solve() - .expect("docs say it can't error in default config") - } - - fn used_packages(&self) -> Option { - self.solver.model().map(|lits| { - let lits: HashSet<_> = lits - .iter() - .filter(|l| l.is_positive()) - .map(|l| l.var()) - .collect(); - - let mut used_packages = BTreeMap::>::new(); - for (&p, v) in self.var_for_is_packages_used.iter() { - if lits.contains(v) { - used_packages.entry(p).or_default(); - } - } - for (&p, map) in &self.var_for_is_packages_features_used { - for (&f, v) in map { - if lits.contains(v) { - used_packages - .get_mut(&p) - .expect("the feature is activated without the package being activated") - .insert(f); - } - } - } - - let mut out = String::from("used:\n"); - for (package, feature_names) in used_packages { - writeln!(&mut out, " {package}").unwrap(); - for feature_name in feature_names { - writeln!(&mut out, " + {feature_name}").unwrap(); - } - } - - out - }) - } -} - -pub trait ToDep { - fn to_dep(self) -> Dependency; - fn to_opt_dep(self) -> Dependency; - fn to_dep_with(self, features: &[&'static str]) -> Dependency; -} - -impl ToDep for &'static str { - fn to_dep(self) -> Dependency { - Dependency::parse(self, Some("1.0.0"), registry_loc()).unwrap() - } - fn to_opt_dep(self) -> Dependency { - let mut dep = self.to_dep(); - dep.set_optional(true); - dep - } - fn to_dep_with(self, features: &[&'static str]) -> Dependency { - let mut dep = self.to_dep(); - dep.set_default_features(false); - dep.set_features(features.into_iter().copied()); - dep - } -} - -impl ToDep for Dependency { - fn to_dep(self) -> Dependency { - self - } - fn to_opt_dep(mut self) -> Dependency { - self.set_optional(true); - self - } - fn to_dep_with(mut self, features: &[&'static str]) -> Dependency { - self.set_default_features(false); - self.set_features(features.into_iter().copied()); - self - } -} - -pub trait ToPkgId { - fn to_pkgid(&self) -> PackageId; -} - -impl ToPkgId for PackageId { - fn to_pkgid(&self) -> PackageId { - *self - } -} - -impl<'a> ToPkgId for &'a str { - fn to_pkgid(&self) -> PackageId { - PackageId::try_new(*self, "1.0.0", registry_loc()).unwrap() - } -} - -impl, U: AsRef> ToPkgId for (T, U) { - fn to_pkgid(&self) -> PackageId { - let (name, vers) = self; - PackageId::try_new(name.as_ref(), vers.as_ref(), registry_loc()).unwrap() - } -} - -#[macro_export] -macro_rules! pkg { - ($pkgid:expr => [$($deps:expr),* $(,)? ]) => ({ - let d: Vec = vec![$($deps.to_dep()),*]; - $crate::pkg_dep($pkgid, d) - }); - - ($pkgid:expr) => ({ - $crate::pkg($pkgid) - }) -} - -fn registry_loc() -> SourceId { - static EXAMPLE_DOT_COM: OnceLock = OnceLock::new(); - let example_dot = EXAMPLE_DOT_COM.get_or_init(|| { - SourceId::for_registry(&"https://example.com".into_url().unwrap()).unwrap() - }); - *example_dot -} - -pub fn pkg(name: T) -> Summary { - pkg_dep(name, Vec::new()) -} - -pub fn pkg_dep(name: T, dep: Vec) -> Summary { - let pkgid = name.to_pkgid(); - let link = if pkgid.name().ends_with("-sys") { - Some(pkgid.name()) - } else { - None - }; - Summary::new(name.to_pkgid(), dep, &BTreeMap::new(), link, None).unwrap() -} - -pub fn pkg_dep_with( - name: T, - dep: Vec, - features: &[(&'static str, &[&'static str])], -) -> Summary { - let pkgid = name.to_pkgid(); - let link = if pkgid.name().ends_with("-sys") { - Some(pkgid.name()) - } else { - None - }; - let features = features - .into_iter() - .map(|&(name, values)| (name.into(), values.into_iter().map(|&v| v.into()).collect())) - .collect(); - Summary::new(name.to_pkgid(), dep, &features, link, None).unwrap() -} - -pub fn pkg_id(name: &str) -> PackageId { - PackageId::try_new(name, "1.0.0", registry_loc()).unwrap() -} - -fn pkg_id_loc(name: &str, loc: &str) -> PackageId { - let remote = loc.into_url(); - let master = GitReference::Branch("master".to_string()); - let source_id = SourceId::for_git(&remote.unwrap(), master).unwrap(); - - PackageId::try_new(name, "1.0.0", source_id).unwrap() -} - -pub fn pkg_loc(name: &str, loc: &str) -> Summary { - let link = if name.ends_with("-sys") { - Some(name) - } else { - None - }; - Summary::new( - pkg_id_loc(name, loc), - Vec::new(), - &BTreeMap::new(), - link, - None, - ) - .unwrap() -} - -pub fn remove_dep(sum: &Summary, ind: usize) -> Summary { - let mut deps = sum.dependencies().to_vec(); - deps.remove(ind); - // note: more things will need to be copied over in the future, but it works for now. - Summary::new(sum.package_id(), deps, &BTreeMap::new(), sum.links(), None).unwrap() -} - -pub fn dep(name: &str) -> Dependency { - dep_req(name, "*") -} - -pub fn dep_req(name: &str, req: &str) -> Dependency { - Dependency::parse(name, Some(req), registry_loc()).unwrap() -} - -pub fn dep_req_kind(name: &str, req: &str, kind: DepKind) -> Dependency { - let mut dep = dep_req(name, req); - dep.set_kind(kind); - dep -} - -pub fn dep_loc(name: &str, location: &str) -> Dependency { - let url = location.into_url().unwrap(); - let master = GitReference::Branch("master".to_string()); - let source_id = SourceId::for_git(&url, master).unwrap(); - Dependency::parse(name, Some("1.0.0"), source_id).unwrap() -} - -pub fn dep_kind(name: &str, kind: DepKind) -> Dependency { - dep(name).set_kind(kind).clone() -} - -pub fn registry(pkgs: Vec) -> Vec { - pkgs -} - -pub fn names(names: &[P]) -> Vec { - names.iter().map(|name| name.to_pkgid()).collect() -} - -pub fn loc_names(names: &[(&'static str, &'static str)]) -> Vec { - names - .iter() - .map(|&(name, loc)| pkg_id_loc(name, loc)) - .collect() -} - /// By default `Summary` and `Dependency` have a very verbose `Debug` representation. /// This replaces with a representation that uses constructors from this file. /// @@ -853,40 +257,6 @@ impl fmt::Debug for PrettyPrintRegistry { } } -#[test] -fn meta_test_deep_pretty_print_registry() { - assert_eq!( - &format!( - "{:?}", - PrettyPrintRegistry(vec![ - pkg!(("foo", "1.0.1") => [dep_req("bar", "1")]), - pkg!(("foo", "1.0.0") => [dep_req("bar", "2")]), - pkg!(("foo", "2.0.0") => [dep_req("bar", "*")]), - pkg!(("bar", "1.0.0") => [dep_req("baz", "=1.0.2"), - dep_req("other", "1")]), - pkg!(("bar", "2.0.0") => [dep_req("baz", "=1.0.1")]), - pkg!(("baz", "1.0.2") => [dep_req("other", "2")]), - pkg!(("baz", "1.0.1")), - pkg!(("cat", "1.0.2") => [dep_req_kind("other", "2", DepKind::Build)]), - pkg!(("cat", "1.0.3") => [dep_req_kind("other", "2", DepKind::Development)]), - pkg!(("dep_req", "1.0.0")), - pkg!(("dep_req", "2.0.0")), - ]) - ), - "vec![pkg!((\"foo\", \"1.0.1\") => [dep_req(\"bar\", \"^1\"),]),\ - pkg!((\"foo\", \"1.0.0\") => [dep_req(\"bar\", \"^2\"),]),\ - pkg!((\"foo\", \"2.0.0\") => [dep(\"bar\"),]),\ - pkg!((\"bar\", \"1.0.0\") => [dep_req(\"baz\", \"=1.0.2\"),dep_req(\"other\", \"^1\"),]),\ - pkg!((\"bar\", \"2.0.0\") => [dep_req(\"baz\", \"=1.0.1\"),]),\ - pkg!((\"baz\", \"1.0.2\") => [dep_req(\"other\", \"^2\"),]),\ - pkg!((\"baz\", \"1.0.1\")),\ - pkg!((\"cat\", \"1.0.2\") => [dep_req_kind(\"other\", \"^2\", DepKind::Build, false),]),\ - pkg!((\"cat\", \"1.0.3\") => [dep_req_kind(\"other\", \"^2\", DepKind::Development, false),]),\ - pkg!((\"dep_req\", \"1.0.0\")),\ - pkg!((\"dep_req\", \"2.0.0\")),]" - ) -} - /// This generates a random registry index. /// Unlike `vec((Name, Ver, vec((Name, VerRq), ..), ..)`, /// this strategy has a high probability of having valid dependencies. @@ -1019,102 +389,125 @@ pub fn registry_strategy( ) } -/// This test is to test the generator to ensure -/// that it makes registries with large dependency trees -#[test] -fn meta_test_deep_trees_from_strategy() { - use proptest::strategy::ValueTree; - use proptest::test_runner::TestRunner; +#[cfg(test)] +mod tests { + use super::*; + use crate::helpers::registry; - let mut dis = [0; 21]; + #[test] + fn meta_test_deep_pretty_print_registry() { + assert_eq!( + &format!( + "{:?}", + PrettyPrintRegistry(vec![ + pkg!(("foo", "1.0.1") => [dep_req("bar", "1")]), + pkg!(("foo", "1.0.0") => [dep_req("bar", "2")]), + pkg!(("foo", "2.0.0") => [dep_req("bar", "*")]), + pkg!(("bar", "1.0.0") => [dep_req("baz", "=1.0.2"), + dep_req("other", "1")]), + pkg!(("bar", "2.0.0") => [dep_req("baz", "=1.0.1")]), + pkg!(("baz", "1.0.2") => [dep_req("other", "2")]), + pkg!(("baz", "1.0.1")), + pkg!(("cat", "1.0.2") => [dep_req_kind("other", "2", DepKind::Build)]), + pkg!(("cat", "1.0.3") => [dep_req_kind("other", "2", DepKind::Development)]), + pkg!(("dep_req", "1.0.0")), + pkg!(("dep_req", "2.0.0")), + ]) + ), + "vec![pkg!((\"foo\", \"1.0.1\") => [dep_req(\"bar\", \"^1\"),]),\ + pkg!((\"foo\", \"1.0.0\") => [dep_req(\"bar\", \"^2\"),]),\ + pkg!((\"foo\", \"2.0.0\") => [dep(\"bar\"),]),\ + pkg!((\"bar\", \"1.0.0\") => [dep_req(\"baz\", \"=1.0.2\"),dep_req(\"other\", \"^1\"),]),\ + pkg!((\"bar\", \"2.0.0\") => [dep_req(\"baz\", \"=1.0.1\"),]),\ + pkg!((\"baz\", \"1.0.2\") => [dep_req(\"other\", \"^2\"),]),\ + pkg!((\"baz\", \"1.0.1\")),\ + pkg!((\"cat\", \"1.0.2\") => [dep_req_kind(\"other\", \"^2\", DepKind::Build, false),]),\ + pkg!((\"cat\", \"1.0.3\") => [dep_req_kind(\"other\", \"^2\", DepKind::Development, false),]),\ + pkg!((\"dep_req\", \"1.0.0\")),\ + pkg!((\"dep_req\", \"2.0.0\")),]" + ) + } - let strategy = registry_strategy(50, 20, 60); - let mut test_runner = TestRunner::deterministic(); - for _ in 0..128 { - let PrettyPrintRegistry(input) = strategy - .new_tree(&mut TestRunner::new_with_rng( - Default::default(), - test_runner.new_rng(), - )) - .unwrap() - .current(); - let reg = registry(input.clone()); - for this in input.iter().rev().take(10) { - let res = resolve( - vec![dep_req(&this.name(), &format!("={}", this.version()))], - ®, - ); - dis[res - .as_ref() - .map(|x| min(x.len(), dis.len()) - 1) - .unwrap_or(0)] += 1; - if dis.iter().all(|&x| x > 0) { - return; + /// This test is to test the generator to ensure + /// that it makes registries with large dependency trees + #[test] + fn meta_test_deep_trees_from_strategy() { + use proptest::strategy::ValueTree; + use proptest::test_runner::TestRunner; + + let mut dis = [0; 21]; + + let strategy = registry_strategy(50, 20, 60); + let mut test_runner = TestRunner::deterministic(); + for _ in 0..128 { + let PrettyPrintRegistry(input) = strategy + .new_tree(&mut TestRunner::new_with_rng( + Default::default(), + test_runner.new_rng(), + )) + .unwrap() + .current(); + let reg = registry(input.clone()); + for this in input.iter().rev().take(10) { + let res = resolve( + vec![dep_req(&this.name(), &format!("={}", this.version()))], + ®, + ); + dis[res + .as_ref() + .map(|x| min(x.len(), dis.len()) - 1) + .unwrap_or(0)] += 1; + if dis.iter().all(|&x| x > 0) { + return; + } } } - } - panic!( - "In 1280 tries we did not see a wide enough distribution of dependency trees! dis: {:?}", - dis - ); -} - -/// This test is to test the generator to ensure -/// that it makes registries that include multiple versions of the same library -#[test] -fn meta_test_multiple_versions_strategy() { - use proptest::strategy::ValueTree; - use proptest::test_runner::TestRunner; - - let mut dis = [0; 10]; + panic!( + "In 1280 tries we did not see a wide enough distribution \ + of dependency trees! dis: {dis:?}" + ); + } - let strategy = registry_strategy(50, 20, 60); - let mut test_runner = TestRunner::deterministic(); - for _ in 0..128 { - let PrettyPrintRegistry(input) = strategy - .new_tree(&mut TestRunner::new_with_rng( - Default::default(), - test_runner.new_rng(), - )) - .unwrap() - .current(); - let reg = registry(input.clone()); - for this in input.iter().rev().take(10) { - let res = resolve( - vec![dep_req(&this.name(), &format!("={}", this.version()))], - ®, - ); - if let Ok(mut res) = res { - let res_len = res.len(); - res.sort_by_key(|s| s.name()); - res.dedup_by_key(|s| s.name()); - dis[min(res_len - res.len(), dis.len() - 1)] += 1; - } - if dis.iter().all(|&x| x > 0) { - return; + /// This test is to test the generator to ensure + /// that it makes registries that include multiple versions of the same library + #[test] + fn meta_test_multiple_versions_strategy() { + use proptest::strategy::ValueTree; + use proptest::test_runner::TestRunner; + + let mut dis = [0; 10]; + + let strategy = registry_strategy(50, 20, 60); + let mut test_runner = TestRunner::deterministic(); + for _ in 0..128 { + let PrettyPrintRegistry(input) = strategy + .new_tree(&mut TestRunner::new_with_rng( + Default::default(), + test_runner.new_rng(), + )) + .unwrap() + .current(); + let reg = registry(input.clone()); + for this in input.iter().rev().take(10) { + let res = resolve( + vec![dep_req(&this.name(), &format!("={}", this.version()))], + ®, + ); + if let Ok(mut res) = res { + let res_len = res.len(); + res.sort_by_key(|s| s.name()); + res.dedup_by_key(|s| s.name()); + dis[min(res_len - res.len(), dis.len() - 1)] += 1; + } + if dis.iter().all(|&x| x > 0) { + return; + } } } - } - panic!( - "In 1280 tries we did not see a wide enough distribution of multiple versions of the same library! dis: {:?}", - dis - ); -} - -/// Assert `xs` contains `elems` -#[track_caller] -pub fn assert_contains(xs: &[A], elems: &[A]) { - for elem in elems { - assert!( - xs.contains(elem), - "missing element\nset: {xs:?}\nmissing: {elem:?}" + panic!( + "In 1280 tries we did not see a wide enough distribution \ + of multiple versions of the same library! dis: {dis:?}" ); } } - -#[track_caller] -pub fn assert_same(a: &[A], b: &[A]) { - assert_eq!(a.len(), b.len(), "not equal\n{a:?}\n{b:?}"); - assert_contains(b, a); -} diff --git a/crates/resolver-tests/src/sat.rs b/crates/resolver-tests/src/sat.rs new file mode 100644 index 00000000000..c7f98bd91f5 --- /dev/null +++ b/crates/resolver-tests/src/sat.rs @@ -0,0 +1,416 @@ +use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; +use std::fmt::Write; + +use cargo::core::{Dependency, FeatureMap, FeatureValue, PackageId, Summary}; +use cargo::util::interning::{InternedString, INTERNED_DEFAULT}; +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 +} + +// At this point is possible to select every version of every package. +// So we need to mark certain versions as incompatible with each other. +// We could add a clause not A, not B for all A and B that are incompatible, +fn sat_at_most_one(solver: &mut varisat::Solver<'_>, vars: &[varisat::Var]) { + if vars.len() <= 1 { + return; + } else if vars.len() == 2 { + solver.add_clause(&[vars[0].negative(), vars[1].negative()]); + return; + } else if vars.len() == 3 { + solver.add_clause(&[vars[0].negative(), vars[1].negative()]); + solver.add_clause(&[vars[0].negative(), vars[2].negative()]); + solver.add_clause(&[vars[1].negative(), vars[2].negative()]); + return; + } + // There are more efficient ways to do it for large numbers of versions. + // + // 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(); + for (i, p) in vars.iter().enumerate() { + for b in 0..bits.len() { + solver.add_clause(&[p.negative(), bits[b].lit(((1 << b) & i) > 0)]); + } + } +} + +fn sat_at_most_one_by_key( + solver: &mut varisat::Solver<'_>, + data: impl Iterator, +) -> HashMap> { + // no two packages with the same keys set + let mut by_keys: HashMap> = HashMap::new(); + for (p, v) in data { + by_keys.entry(p).or_default().push(v) + } + for key in by_keys.values() { + sat_at_most_one(solver, key); + } + by_keys +} + +fn find_compatible_dep_summaries_by_name_in_toml( + pkg_dependencies: &[Dependency], + by_name: &HashMap>, +) -> HashMap> { + let empty_vec = vec![]; + + pkg_dependencies + .iter() + .map(|dep| { + let name_in_toml = dep.name_in_toml(); + + let compatible_summaries = by_name + .get(&dep.package_name()) + .unwrap_or(&empty_vec) + .iter() + .filter(|s| dep.matches_id(s.package_id())) + .filter(|s| dep.features().iter().all(|f| s.features().contains_key(f))) + .cloned() + .collect::>(); + + (name_in_toml, compatible_summaries) + }) + .collect() +} + +fn process_pkg_features( + solver: &mut varisat::Solver<'_>, + var_for_is_packages_used: &HashMap, + var_for_is_packages_features_used: &HashMap>, + pkg_feature_var_map: &HashMap, + pkg_features: &FeatureMap, + compatible_dep_summaries_by_name_in_toml: &HashMap>, +) { + // add clauses for package features + for (&feature_name, feature_values) in pkg_features { + for feature_value in feature_values { + let pkg_feature_var = pkg_feature_var_map[&feature_name]; + + match *feature_value { + FeatureValue::Feature(other_feature_name) => { + solver.add_clause(&[ + pkg_feature_var.negative(), + pkg_feature_var_map[&other_feature_name].positive(), + ]); + } + FeatureValue::Dep { dep_name } => { + let dep_clause = compatible_dep_summaries_by_name_in_toml[&dep_name] + .iter() + .map(|dep| var_for_is_packages_used[&dep.package_id()].positive()) + .chain([pkg_feature_var.negative()]) + .collect::>(); + + solver.add_clause(&dep_clause); + } + FeatureValue::DepFeature { + dep_name, + dep_feature: dep_feature_name, + weak, + } => { + for dep in &compatible_dep_summaries_by_name_in_toml[&dep_name] { + let dep_var = var_for_is_packages_used[&dep.package_id()]; + let dep_feature_var = + var_for_is_packages_features_used[&dep.package_id()][&dep_feature_name]; + + solver.add_clause(&[ + pkg_feature_var.negative(), + dep_var.negative(), + dep_feature_var.positive(), + ]); + } + + if !weak { + let dep_clause = compatible_dep_summaries_by_name_in_toml[&dep_name] + .iter() + .map(|dep| var_for_is_packages_used[&dep.package_id()].positive()) + .chain([pkg_feature_var.negative()]) + .collect::>(); + + solver.add_clause(&dep_clause); + } + } + } + } + } +} + +fn process_pkg_dependencies( + solver: &mut varisat::Solver<'_>, + var_for_is_packages_used: &HashMap, + var_for_is_packages_features_used: &HashMap>, + pkg_var: varisat::Var, + pkg_dependencies: &[Dependency], + compatible_dep_summaries_by_name_in_toml: &HashMap>, +) { + for dep in pkg_dependencies { + let compatible_dep_summaries = + &compatible_dep_summaries_by_name_in_toml[&dep.name_in_toml()]; + + // add clauses for package dependency features + for dep_summary in compatible_dep_summaries { + let dep_package_id = dep_summary.package_id(); + + let default_feature = if dep.uses_default_features() + && dep_summary.features().contains_key(&*INTERNED_DEFAULT) + { + Some(&INTERNED_DEFAULT) + } else { + None + }; + + for &feature_name in default_feature.into_iter().chain(dep.features()) { + solver.add_clause(&[ + pkg_var.negative(), + var_for_is_packages_used[&dep_package_id].negative(), + var_for_is_packages_features_used[&dep_package_id][&feature_name].positive(), + ]); + } + } + + // active packages need to activate each of their non-optional dependencies + if !dep.is_optional() { + let dep_clause = compatible_dep_summaries + .iter() + .map(|d| var_for_is_packages_used[&d.package_id()].positive()) + .chain([pkg_var.negative()]) + .collect::>(); + + solver.add_clause(&dep_clause); + } + } +} + +/// Resolution can be reduced to the SAT problem. So this is an alternative implementation +/// of the resolver that uses a SAT library for the hard work. This is intended to be easy to read, +/// as compared to the real resolver. +/// +/// For the subset of functionality that are currently made by `registry_strategy`, +/// this will find a valid resolution if one exists. +/// +/// The SAT library does not optimize for the newer version, +/// so the selected packages may not match the real resolver. +pub struct SatResolver { + solver: varisat::Solver<'static>, + old_root_vars: Vec, + var_for_is_packages_used: HashMap, + var_for_is_packages_features_used: HashMap>, + by_name: HashMap>, +} + +impl SatResolver { + pub fn new(registry: &[Summary]) -> Self { + let mut solver = varisat::Solver::new(); + + // That represents each package version which is set to "true" if the packages in the lock file and "false" if it is unused. + let var_for_is_packages_used = registry + .iter() + .map(|s| (s.package_id(), solver.new_var())) + .collect::>(); + + // That represents each feature of each package version, which is set to "true" if the package feature is used. + let var_for_is_packages_features_used = registry + .iter() + .map(|s| { + ( + s.package_id(), + (s.features().keys().map(|&f| (f, solver.new_var()))).collect(), + ) + }) + .collect::>>(); + + // if a package feature is used, then the package is used + for (package, pkg_feature_var_map) in &var_for_is_packages_features_used { + for (_, package_feature_var) in pkg_feature_var_map { + let package_var = var_for_is_packages_used[package]; + solver.add_clause(&[package_feature_var.negative(), package_var.positive()]); + } + } + + // no two packages with the same links set + sat_at_most_one_by_key( + &mut solver, + registry + .iter() + .map(|s| (s.links(), var_for_is_packages_used[&s.package_id()])) + .filter(|(l, _)| l.is_some()), + ); + + // no two semver compatible versions of the same package + sat_at_most_one_by_key( + &mut solver, + var_for_is_packages_used + .iter() + .map(|(p, &v)| (p.as_activations_key(), v)), + ); + + let mut by_name: HashMap> = HashMap::new(); + + for p in registry { + by_name.entry(p.name()).or_default().push(p.clone()) + } + + for pkg in registry { + let pkg_id = pkg.package_id(); + let pkg_dependencies = pkg.dependencies(); + let pkg_features = pkg.features(); + + let compatible_dep_summaries_by_name_in_toml = + find_compatible_dep_summaries_by_name_in_toml(pkg_dependencies, &by_name); + + process_pkg_features( + &mut solver, + &var_for_is_packages_used, + &var_for_is_packages_features_used, + &var_for_is_packages_features_used[&pkg_id], + pkg_features, + &compatible_dep_summaries_by_name_in_toml, + ); + + process_pkg_dependencies( + &mut solver, + &var_for_is_packages_used, + &var_for_is_packages_features_used, + var_for_is_packages_used[&pkg_id], + pkg_dependencies, + &compatible_dep_summaries_by_name_in_toml, + ); + } + + // We don't need to `solve` now. We know that "use nothing" will satisfy all the clauses so far. + // But things run faster if we let it spend some time figuring out how the constraints interact before we add assumptions. + solver + .solve() + .expect("docs say it can't error in default config"); + + SatResolver { + solver, + old_root_vars: Vec::new(), + var_for_is_packages_used, + var_for_is_packages_features_used, + by_name, + } + } + + pub fn sat_resolve(&mut self, root_dependencies: &[Dependency]) -> bool { + let SatResolver { + solver, + old_root_vars, + var_for_is_packages_used, + var_for_is_packages_features_used, + by_name, + } = self; + + let root_var = solver.new_var(); + + // root package is always used + // root vars from previous runs are deactivated + let assumption = old_root_vars + .iter() + .map(|v| v.negative()) + .chain([root_var.positive()]) + .collect::>(); + + old_root_vars.push(root_var); + + let compatible_dep_summaries_by_name_in_toml = + find_compatible_dep_summaries_by_name_in_toml(root_dependencies, &by_name); + + process_pkg_dependencies( + solver, + var_for_is_packages_used, + var_for_is_packages_features_used, + root_var, + root_dependencies, + &compatible_dep_summaries_by_name_in_toml, + ); + + solver.assume(&assumption); + + solver + .solve() + .expect("docs say it can't error in default config") + } + + pub fn sat_is_valid_solution(&mut self, pkgs: &[(PackageId, Vec)]) -> bool { + let contains_pkg = |pkg| pkgs.iter().any(|(p, _)| p == pkg); + let contains_pkg_feature = + |pkg, f| pkgs.iter().any(|(p, flist)| p == pkg && flist.contains(f)); + + for (p, _) in pkgs { + if p.name() != "root" && !self.var_for_is_packages_used.contains_key(p) { + return false; + } + } + + // root vars from previous runs are deactivated + let assumption = (self.old_root_vars.iter().map(|v| v.negative())) + .chain( + self.var_for_is_packages_used + .iter() + .map(|(p, v)| v.lit(contains_pkg(p))), + ) + .chain( + self.var_for_is_packages_features_used + .iter() + .flat_map(|(p, fmap)| { + fmap.iter() + .map(move |(f, v)| v.lit(contains_pkg_feature(p, f))) + }), + ) + .collect::>(); + + self.solver.assume(&assumption); + + self.solver + .solve() + .expect("docs say it can't error in default config") + } + + pub fn used_packages(&self) -> Option { + self.solver.model().map(|lits| { + let lits: HashSet<_> = lits + .iter() + .filter(|l| l.is_positive()) + .map(|l| l.var()) + .collect(); + + let mut used_packages = BTreeMap::>::new(); + for (&p, v) in self.var_for_is_packages_used.iter() { + if lits.contains(v) { + used_packages.entry(p).or_default(); + } + } + for (&p, map) in &self.var_for_is_packages_features_used { + for (&f, v) in map { + if lits.contains(v) { + used_packages + .get_mut(&p) + .expect("the feature is activated without the package being activated") + .insert(f); + } + } + } + + let mut out = String::from("used:\n"); + for (package, feature_names) in used_packages { + writeln!(&mut out, " {package}").unwrap(); + for feature_name in feature_names { + writeln!(&mut out, " + {feature_name}").unwrap(); + } + } + + out + }) + } +} diff --git a/crates/resolver-tests/tests/proptests.rs b/crates/resolver-tests/tests/proptests.rs new file mode 100644 index 00000000000..d1affe68900 --- /dev/null +++ b/crates/resolver-tests/tests/proptests.rs @@ -0,0 +1,273 @@ +use std::io::IsTerminal; + +use cargo::util::GlobalContext; +use cargo_util::is_ci; + +use resolver_tests::{ + helpers::{dep_req, registry, remove_dep}, + registry_strategy, resolve, resolve_and_validated, resolve_with_global_context, + sat::SatResolver, + PrettyPrintRegistry, +}; + +use proptest::prelude::*; + +// NOTE: proptest is a form of fuzz testing. It generates random input and makes sure that +// certain universal truths are upheld. Therefore, it can pass when there is a problem, +// but if it fails then there really is something wrong. When testing something as +// complicated as the resolver, the problems can be very subtle and hard to generate. +// We have had a history of these tests only failing on PRs long after a bug is introduced. +// If you have one of these test fail please report it on #6258, +// and if you did not change the resolver then feel free to retry without concern. +proptest! { + #![proptest_config(ProptestConfig { + max_shrink_iters: + if is_ci() || !std::io::stderr().is_terminal() { + // This attempts to make sure that CI will fail fast, + 0 + } else { + // but that local builds will give a small clear test case. + u32::MAX + }, + result_cache: prop::test_runner::basic_result_cache, + .. ProptestConfig::default() + })] + + /// NOTE: if you think this test has failed spuriously see the note at the top of this macro. + #[test] + fn prop_passes_validation( + PrettyPrintRegistry(input) in registry_strategy(50, 20, 60) + ) { + let reg = registry(input.clone()); + let mut sat_resolver = SatResolver::new(®); + + // There is only a small chance that a crate will be interesting. + // So we try some of the most complicated. + for this in input.iter().rev().take(20) { + let _ = resolve_and_validated( + vec![dep_req(&this.name(), &format!("={}", this.version()))], + ®, + &mut sat_resolver, + ); + } + } + + /// NOTE: if you think this test has failed spuriously see the note at the top of this macro. + #[test] + fn prop_minimum_version_errors_the_same( + PrettyPrintRegistry(input) in registry_strategy(50, 20, 60) + ) { + let mut gctx = GlobalContext::default().unwrap(); + gctx.nightly_features_allowed = true; + gctx + .configure( + 1, + false, + None, + false, + false, + false, + &None, + &["minimal-versions".to_string()], + &[], + ) + .unwrap(); + + let reg = registry(input.clone()); + + // There is only a small chance that a crate will be interesting. + // So we try some of the most complicated. + for this in input.iter().rev().take(10) { + let deps = vec![dep_req(&this.name(), &format!("={}", this.version()))]; + let res = resolve(deps.clone(), ®); + let mres = resolve_with_global_context(deps, ®, &gctx); + + // `minimal-versions` changes what order the candidates are tried but not the existence of a solution. + prop_assert_eq!( + res.is_ok(), + mres.is_ok(), + "minimal-versions and regular resolver disagree about whether `{} = \"={}\"` can resolve", + this.name(), + this.version() + ) + } + } + + /// NOTE: if you think this test has failed spuriously see the note at the top of this macro. + #[test] + fn prop_direct_minimum_version_error_implications( + PrettyPrintRegistry(input) in registry_strategy(50, 20, 60) + ) { + let mut gctx = GlobalContext::default().unwrap(); + gctx.nightly_features_allowed = true; + gctx + .configure( + 1, + false, + None, + false, + false, + false, + &None, + &["direct-minimal-versions".to_string()], + &[], + ) + .unwrap(); + + let reg = registry(input.clone()); + + // There is only a small chance that a crate will be interesting. + // So we try some of the most complicated. + for this in input.iter().rev().take(10) { + let deps = vec![dep_req(&this.name(), &format!("={}", this.version()))]; + let res = resolve(deps.clone(), ®); + let mres = resolve_with_global_context(deps, ®, &gctx); + + // `direct-minimal-versions` reduces the number of available solutions, + // so we verify that we do not come up with solutions not seen in `maximal-versions`. + if res.is_err() { + prop_assert!( + mres.is_err(), + "direct-minimal-versions should not have more solutions than the regular, maximal resolver but found one when resolving `{} = \"={}\"`", + this.name(), + this.version() + ) + } + if mres.is_ok() { + prop_assert!( + res.is_ok(), + "direct-minimal-versions should not have more solutions than the regular, maximal resolver but found one when resolving `{} = \"={}\"`", + this.name(), + this.version() + ) + } + } + } + + /// NOTE: if you think this test has failed spuriously see the note at the top of this macro. + #[test] + fn prop_removing_a_dep_cant_break( + PrettyPrintRegistry(input) in registry_strategy(50, 20, 60), + indexes_to_remove in prop::collection::vec((any::(), any::()), ..10) + ) { + let reg = registry(input.clone()); + let mut removed_input = input.clone(); + for (summary_idx, dep_idx) in indexes_to_remove { + if !removed_input.is_empty() { + let summary_idx = summary_idx.index(removed_input.len()); + let deps = removed_input[summary_idx].dependencies(); + if !deps.is_empty() { + let new = remove_dep(&removed_input[summary_idx], dep_idx.index(deps.len())); + removed_input[summary_idx] = new; + } + } + } + let removed_reg = registry(removed_input); + + // There is only a small chance that a crate will be interesting. + // So we try some of the most complicated. + for this in input.iter().rev().take(10) { + if resolve( + vec![dep_req(&this.name(), &format!("={}", this.version()))], + ®, + ).is_ok() { + prop_assert!( + resolve( + vec![dep_req(&this.name(), &format!("={}", this.version()))], + &removed_reg, + ).is_ok(), + "full index worked for `{} = \"={}\"` but removing some deps broke it!", + this.name(), + this.version(), + ) + } + } + } + + /// NOTE: if you think this test has failed spuriously see the note at the top of this macro. + #[test] + fn prop_limited_independence_of_irrelevant_alternatives( + PrettyPrintRegistry(input) in registry_strategy(50, 20, 60), + indexes_to_unpublish in prop::collection::vec(any::(), ..10) + ) { + let reg = registry(input.clone()); + + // There is only a small chance that a crate will be interesting. + // So we try some of the most complicated. + for this in input.iter().rev().take(10) { + let res = resolve( + vec![dep_req(&this.name(), &format!("={}", this.version()))], + ®, + ); + + match res { + Ok(r) => { + // If resolution was successful, then unpublishing a version of a crate + // that was not selected should not change that. + let not_selected: Vec<_> = input + .iter() + .cloned() + .filter(|x| !r.contains(&x.package_id())) + .collect(); + + if !not_selected.is_empty() { + let indexes_to_unpublish: Vec<_> = indexes_to_unpublish.iter().map(|x| x.get(¬_selected)).collect(); + + let new_reg = registry( + input + .iter() + .cloned() + .filter(|x| !indexes_to_unpublish.contains(&x)) + .collect(), + ); + + let res = resolve( + vec![dep_req(&this.name(), &format!("={}", this.version()))], + &new_reg, + ); + + // Note: that we can not assert that the two `res` are identical + // as the resolver does depend on irrelevant alternatives. + // It uses how constrained a dependency requirement is + // to determine what order to evaluate requirements. + + prop_assert!( + res.is_ok(), + "unpublishing {:?} stopped `{} = \"={}\"` from working", + indexes_to_unpublish.iter().map(|x| x.package_id()).collect::>(), + this.name(), + this.version() + ) + } + } + + Err(_) => { + // If resolution was unsuccessful, then it should stay unsuccessful + // even if any version of a crate is unpublished. + let indexes_to_unpublish: Vec<_> = indexes_to_unpublish.iter().map(|x| x.get(&input)).collect(); + + let new_reg = registry( + input + .iter() + .cloned() + .filter(|x| !indexes_to_unpublish.contains(&x)) + .collect(), + ); + + let res = resolve( + vec![dep_req(&this.name(), &format!("={}", this.version()))], + &new_reg, + ); + + prop_assert!( + res.is_err(), + "full index did not work for `{} = \"={}\"` but unpublishing {:?} fixed it!", + this.name(), + this.version(), + indexes_to_unpublish.iter().map(|x| x.package_id()).collect::>(), + ) + } + } + } + } +} diff --git a/crates/resolver-tests/tests/resolve.rs b/crates/resolver-tests/tests/resolve.rs index 5ac0a83dcbb..cc489266a8b 100644 --- a/crates/resolver-tests/tests/resolve.rs +++ b/crates/resolver-tests/tests/resolve.rs @@ -1,279 +1,15 @@ -use std::io::IsTerminal; - use cargo::core::dependency::DepKind; use cargo::core::Dependency; use cargo::util::GlobalContext; -use cargo_util::is_ci; use resolver_tests::{ - assert_contains, assert_same, dep, dep_kind, dep_loc, dep_req, loc_names, names, pkg, pkg_dep, - pkg_dep_with, pkg_id, pkg_loc, registry, registry_strategy, remove_dep, resolve, - resolve_and_validated, resolve_with_global_context, PrettyPrintRegistry, SatResolver, ToDep, - ToPkgId, + helpers::{ + assert_contains, assert_same, dep, dep_kind, dep_loc, dep_req, loc_names, names, pkg_id, + pkg_loc, registry, ToDep, ToPkgId, + }, + pkg, resolve, resolve_with_global_context, }; -use proptest::prelude::*; - -// NOTE: proptest is a form of fuzz testing. It generates random input and makes sure that -// certain universal truths are upheld. Therefore, it can pass when there is a problem, -// but if it fails then there really is something wrong. When testing something as -// complicated as the resolver, the problems can be very subtle and hard to generate. -// We have had a history of these tests only failing on PRs long after a bug is introduced. -// If you have one of these test fail please report it on #6258, -// and if you did not change the resolver then feel free to retry without concern. -proptest! { - #![proptest_config(ProptestConfig { - max_shrink_iters: - if is_ci() || !std::io::stderr().is_terminal() { - // This attempts to make sure that CI will fail fast, - 0 - } else { - // but that local builds will give a small clear test case. - u32::MAX - }, - result_cache: prop::test_runner::basic_result_cache, - .. ProptestConfig::default() - })] - - /// NOTE: if you think this test has failed spuriously see the note at the top of this macro. - #[test] - fn prop_passes_validation( - PrettyPrintRegistry(input) in registry_strategy(50, 20, 60) - ) { - let reg = registry(input.clone()); - let mut sat_resolver = SatResolver::new(®); - - // There is only a small chance that a crate will be interesting. - // So we try some of the most complicated. - for this in input.iter().rev().take(20) { - let _ = resolve_and_validated( - vec![dep_req(&this.name(), &format!("={}", this.version()))], - ®, - &mut sat_resolver, - ); - } - } - - /// NOTE: if you think this test has failed spuriously see the note at the top of this macro. - #[test] - fn prop_minimum_version_errors_the_same( - PrettyPrintRegistry(input) in registry_strategy(50, 20, 60) - ) { - let mut gctx = GlobalContext::default().unwrap(); - gctx.nightly_features_allowed = true; - gctx - .configure( - 1, - false, - None, - false, - false, - false, - &None, - &["minimal-versions".to_string()], - &[], - ) - .unwrap(); - - let reg = registry(input.clone()); - - // There is only a small chance that a crate will be interesting. - // So we try some of the most complicated. - for this in input.iter().rev().take(10) { - let deps = vec![dep_req(&this.name(), &format!("={}", this.version()))]; - let res = resolve(deps.clone(), ®); - let mres = resolve_with_global_context(deps, ®, &gctx); - - // `minimal-versions` changes what order the candidates are tried but not the existence of a solution. - prop_assert_eq!( - res.is_ok(), - mres.is_ok(), - "minimal-versions and regular resolver disagree about whether `{} = \"={}\"` can resolve", - this.name(), - this.version() - ) - } - } - - /// NOTE: if you think this test has failed spuriously see the note at the top of this macro. - #[test] - fn prop_direct_minimum_version_error_implications( - PrettyPrintRegistry(input) in registry_strategy(50, 20, 60) - ) { - let mut gctx = GlobalContext::default().unwrap(); - gctx.nightly_features_allowed = true; - gctx - .configure( - 1, - false, - None, - false, - false, - false, - &None, - &["direct-minimal-versions".to_string()], - &[], - ) - .unwrap(); - - let reg = registry(input.clone()); - - // There is only a small chance that a crate will be interesting. - // So we try some of the most complicated. - for this in input.iter().rev().take(10) { - let deps = vec![dep_req(&this.name(), &format!("={}", this.version()))]; - let res = resolve(deps.clone(), ®); - let mres = resolve_with_global_context(deps, ®, &gctx); - - // `direct-minimal-versions` reduces the number of available solutions, - // so we verify that we do not come up with solutions not seen in `maximal-versions`. - if res.is_err() { - prop_assert!( - mres.is_err(), - "direct-minimal-versions should not have more solutions than the regular, maximal resolver but found one when resolving `{} = \"={}\"`", - this.name(), - this.version() - ) - } - if mres.is_ok() { - prop_assert!( - res.is_ok(), - "direct-minimal-versions should not have more solutions than the regular, maximal resolver but found one when resolving `{} = \"={}\"`", - this.name(), - this.version() - ) - } - } - } - - /// NOTE: if you think this test has failed spuriously see the note at the top of this macro. - #[test] - fn prop_removing_a_dep_cant_break( - PrettyPrintRegistry(input) in registry_strategy(50, 20, 60), - indexes_to_remove in prop::collection::vec((any::(), any::()), ..10) - ) { - let reg = registry(input.clone()); - let mut removed_input = input.clone(); - for (summary_idx, dep_idx) in indexes_to_remove { - if !removed_input.is_empty() { - let summary_idx = summary_idx.index(removed_input.len()); - let deps = removed_input[summary_idx].dependencies(); - if !deps.is_empty() { - let new = remove_dep(&removed_input[summary_idx], dep_idx.index(deps.len())); - removed_input[summary_idx] = new; - } - } - } - let removed_reg = registry(removed_input); - - // There is only a small chance that a crate will be interesting. - // So we try some of the most complicated. - for this in input.iter().rev().take(10) { - if resolve( - vec![dep_req(&this.name(), &format!("={}", this.version()))], - ®, - ).is_ok() { - prop_assert!( - resolve( - vec![dep_req(&this.name(), &format!("={}", this.version()))], - &removed_reg, - ).is_ok(), - "full index worked for `{} = \"={}\"` but removing some deps broke it!", - this.name(), - this.version(), - ) - } - } - } - - /// NOTE: if you think this test has failed spuriously see the note at the top of this macro. - #[test] - fn prop_limited_independence_of_irrelevant_alternatives( - PrettyPrintRegistry(input) in registry_strategy(50, 20, 60), - indexes_to_unpublish in prop::collection::vec(any::(), ..10) - ) { - let reg = registry(input.clone()); - - // There is only a small chance that a crate will be interesting. - // So we try some of the most complicated. - for this in input.iter().rev().take(10) { - let res = resolve( - vec![dep_req(&this.name(), &format!("={}", this.version()))], - ®, - ); - - match res { - Ok(r) => { - // If resolution was successful, then unpublishing a version of a crate - // that was not selected should not change that. - let not_selected: Vec<_> = input - .iter() - .cloned() - .filter(|x| !r.contains(&x.package_id())) - .collect(); - - if !not_selected.is_empty() { - let indexes_to_unpublish: Vec<_> = indexes_to_unpublish.iter().map(|x| x.get(¬_selected)).collect(); - - let new_reg = registry( - input - .iter() - .cloned() - .filter(|x| !indexes_to_unpublish.contains(&x)) - .collect(), - ); - - let res = resolve( - vec![dep_req(&this.name(), &format!("={}", this.version()))], - &new_reg, - ); - - // Note: that we can not assert that the two `res` are identical - // as the resolver does depend on irrelevant alternatives. - // It uses how constrained a dependency requirement is - // to determine what order to evaluate requirements. - - prop_assert!( - res.is_ok(), - "unpublishing {:?} stopped `{} = \"={}\"` from working", - indexes_to_unpublish.iter().map(|x| x.package_id()).collect::>(), - this.name(), - this.version() - ) - } - } - - Err(_) => { - // If resolution was unsuccessful, then it should stay unsuccessful - // even if any version of a crate is unpublished. - let indexes_to_unpublish: Vec<_> = indexes_to_unpublish.iter().map(|x| x.get(&input)).collect(); - - let new_reg = registry( - input - .iter() - .cloned() - .filter(|x| !indexes_to_unpublish.contains(&x)) - .collect(), - ); - - let res = resolve( - vec![dep_req(&this.name(), &format!("={}", this.version()))], - &new_reg, - ); - - prop_assert!( - res.is_err(), - "full index did not work for `{} = \"={}\"` but unpublishing {:?} fixed it!", - this.name(), - this.version(), - indexes_to_unpublish.iter().map(|x| x.package_id()).collect::>(), - ) - } - } - } - } -} - #[test] #[should_panic(expected = "assertion failed: !name.is_empty()")] fn test_dependency_with_empty_name() { @@ -1201,214 +937,6 @@ fn large_conflict_cache() { let _ = resolve(root_deps, ®); } -#[test] -fn off_by_one_bug() { - let input = vec![ - pkg!(("A-sys", "0.0.1")), - pkg!(("A-sys", "0.0.4")), - pkg!(("A-sys", "0.0.6")), - pkg!(("A-sys", "0.0.7")), - pkg!(("NA", "0.0.0") => [dep_req("A-sys", "<= 0.0.5"),]), - pkg!(("NA", "0.0.1") => [dep_req("A-sys", ">= 0.0.6, <= 0.0.8"),]), - pkg!(("a", "0.0.1")), - pkg!(("a", "0.0.2")), - pkg!(("aa", "0.0.0") => [dep_req("A-sys", ">= 0.0.4, <= 0.0.6"),dep_req("NA", "<= 0.0.0"),]), - pkg!(("f", "0.0.3") => [dep("NA"),dep_req("a", "<= 0.0.2"),dep("aa"),]), - ]; - - let reg = registry(input); - let mut sat_resolver = SatResolver::new(®); - assert!(resolve_and_validated(vec![dep("f")], ®, &mut sat_resolver).is_ok()); -} - -#[test] -fn conflict_store_bug() { - let input = vec![ - pkg!(("A", "0.0.3")), - pkg!(("A", "0.0.5")), - pkg!(("A", "0.0.9") => [dep("bad"),]), - pkg!(("A", "0.0.10") => [dep("bad"),]), - pkg!(("L-sys", "0.0.1") => [dep("bad"),]), - pkg!(("L-sys", "0.0.5")), - pkg!(("R", "0.0.4") => [ - dep_req("L-sys", "= 0.0.5"), - ]), - pkg!(("R", "0.0.6")), - pkg!(("a-sys", "0.0.5")), - pkg!(("a-sys", "0.0.11")), - pkg!(("c", "0.0.12") => [ - dep_req("R", ">= 0.0.3, <= 0.0.4"), - ]), - pkg!(("c", "0.0.13") => [ - dep_req("a-sys", ">= 0.0.8, <= 0.0.11"), - ]), - pkg!(("c0", "0.0.6") => [ - dep_req("L-sys", "<= 0.0.2"), - ]), - pkg!(("c0", "0.0.10") => [ - dep_req("A", ">= 0.0.9, <= 0.0.10"), - dep_req("a-sys", "= 0.0.5"), - ]), - pkg!("j" => [ - dep_req("A", ">= 0.0.3, <= 0.0.5"), - dep_req("R", ">=0.0.4, <= 0.0.6"), - dep_req("c", ">= 0.0.9"), - dep_req("c0", ">= 0.0.6"), - ]), - ]; - - let reg = registry(input); - let mut sat_resolver = SatResolver::new(®); - assert!(resolve_and_validated(vec![dep("j")], ®, &mut sat_resolver).is_err()); -} - -#[test] -fn conflict_store_more_then_one_match() { - let input = vec![ - pkg!(("A", "0.0.0")), - pkg!(("A", "0.0.1")), - pkg!(("A-sys", "0.0.0")), - pkg!(("A-sys", "0.0.1")), - pkg!(("A-sys", "0.0.2")), - pkg!(("A-sys", "0.0.3")), - pkg!(("A-sys", "0.0.12")), - pkg!(("A-sys", "0.0.16")), - pkg!(("B-sys", "0.0.0")), - pkg!(("B-sys", "0.0.1")), - pkg!(("B-sys", "0.0.2") => [dep_req("A-sys", "= 0.0.12"),]), - pkg!(("BA-sys", "0.0.0") => [dep_req("A-sys","= 0.0.16"),]), - pkg!(("BA-sys", "0.0.1") => [dep("bad"),]), - pkg!(("BA-sys", "0.0.2") => [dep("bad"),]), - pkg!("nA" => [ - dep("A"), - dep_req("A-sys", "<= 0.0.3"), - dep("B-sys"), - dep("BA-sys"), - ]), - ]; - let reg = registry(input); - let mut sat_resolver = SatResolver::new(®); - assert!(resolve_and_validated(vec![dep("nA")], ®, &mut sat_resolver).is_err()); -} - -#[test] -fn bad_lockfile_from_8249() { - let input = vec![ - pkg!(("a-sys", "0.2.0")), - pkg!(("a-sys", "0.1.0")), - pkg!(("b", "0.1.0") => [ - dep_req("a-sys", "0.1"), // should be optional: true, but not needed for now - ]), - pkg!(("c", "1.0.0") => [ - dep_req("b", "=0.1.0"), - ]), - pkg!("foo" => [ - dep_req("a-sys", "=0.2.0"), - { - let mut b = dep_req("b", "=0.1.0"); - b.set_features(vec!["a-sys"]); - b - }, - dep_req("c", "=1.0.0"), - ]), - ]; - let reg = registry(input); - let mut sat_resolver = SatResolver::new(®); - assert!(resolve_and_validated(vec![dep("foo")], ®, &mut sat_resolver).is_err()); -} - -#[test] -fn registry_with_features() { - let reg = registry(vec![ - pkg!("a"), - pkg!("b"), - pkg_dep_with( - "image", - vec!["a".to_opt_dep(), "b".to_opt_dep(), "jpg".to_dep()], - &[("default", &["a"]), ("b", &["dep:b"])], - ), - pkg!("jpg"), - pkg!("log"), - pkg!("man"), - pkg_dep_with("rgb", vec!["man".to_opt_dep()], &[("man", &["dep:man"])]), - pkg_dep_with( - "dep", - vec![ - "image".to_dep_with(&["b"]), - "log".to_opt_dep(), - "rgb".to_opt_dep(), - ], - &[ - ("default", &["log", "image/default"]), - ("man", &["rgb?/man"]), - ], - ), - ]); - - for deps in [ - vec!["dep".to_dep_with(&["default", "man", "log", "rgb"])], - vec!["dep".to_dep_with(&["default"])], - vec!["dep".to_dep_with(&[])], - vec!["dep".to_dep_with(&["man"])], - vec!["dep".to_dep_with(&["man", "rgb"])], - ] { - let mut sat_resolver = SatResolver::new(®); - assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_ok()); - } -} - -#[test] -fn missing_feature() { - let reg = registry(vec![pkg!("a")]); - let mut sat_resolver = SatResolver::new(®); - assert!(resolve_and_validated(vec!["a".to_dep_with(&["f"])], ®, &mut sat_resolver).is_err()); -} - -#[test] -fn conflict_feature_and_sys() { - let reg = registry(vec![ - pkg(("a-sys", "1.0.0")), - pkg(("a-sys", "2.0.0")), - pkg_dep_with( - ("a", "1.0.0"), - vec![dep_req("a-sys", "1.0.0")], - &[("f", &[])], - ), - pkg_dep_with( - ("a", "2.0.0"), - vec![dep_req("a-sys", "2.0.0")], - &[("g", &[])], - ), - pkg_dep("dep", vec![dep_req("a", "2.0.0")]), - ]); - - let deps = vec![dep_req("a", "*").to_dep_with(&["f"]), dep("dep")]; - let mut sat_resolver = SatResolver::new(®); - assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_err()); -} - -#[test] -fn conflict_weak_features() { - let reg = registry(vec![ - pkg(("a-sys", "1.0.0")), - pkg(("a-sys", "2.0.0")), - pkg_dep("a1", vec![dep_req("a-sys", "1.0.0").to_opt_dep()]), - pkg_dep("a2", vec![dep_req("a-sys", "2.0.0").to_opt_dep()]), - pkg_dep_with( - "dep", - vec!["a1".to_opt_dep(), "a2".to_opt_dep()], - &[("a1", &["a1?/a-sys"]), ("a2", &["a2?/a-sys"])], - ), - ]); - - let deps = vec![dep("dep").to_dep_with(&["a1", "a2"])]; - - // The following asserts should be updated when support for weak dependencies - // is added to the dependency resolver. - assert!(resolve(deps.clone(), ®).is_err()); - assert!(SatResolver::new(®).sat_resolve(&deps)); -} - #[test] fn cyclic_good_error_message() { let input = vec![ diff --git a/crates/resolver-tests/tests/validated.rs b/crates/resolver-tests/tests/validated.rs new file mode 100644 index 00000000000..40902b61a32 --- /dev/null +++ b/crates/resolver-tests/tests/validated.rs @@ -0,0 +1,215 @@ +use cargo::core::Dependency; + +use resolver_tests::{ + helpers::{dep, dep_req, pkg, pkg_dep, pkg_dep_with, registry, ToDep}, + pkg, resolve, resolve_and_validated, + sat::SatResolver, +}; + +#[test] +fn off_by_one_bug() { + let input = vec![ + pkg!(("A-sys", "0.0.1")), + pkg!(("A-sys", "0.0.4")), + pkg!(("A-sys", "0.0.6")), + pkg!(("A-sys", "0.0.7")), + pkg!(("NA", "0.0.0") => [dep_req("A-sys", "<= 0.0.5"),]), + pkg!(("NA", "0.0.1") => [dep_req("A-sys", ">= 0.0.6, <= 0.0.8"),]), + pkg!(("a", "0.0.1")), + pkg!(("a", "0.0.2")), + pkg!(("aa", "0.0.0") => [dep_req("A-sys", ">= 0.0.4, <= 0.0.6"),dep_req("NA", "<= 0.0.0"),]), + pkg!(("f", "0.0.3") => [dep("NA"),dep_req("a", "<= 0.0.2"),dep("aa"),]), + ]; + + let reg = registry(input); + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(vec![dep("f")], ®, &mut sat_resolver).is_ok()); +} + +#[test] +fn conflict_store_bug() { + let input = vec![ + pkg!(("A", "0.0.3")), + pkg!(("A", "0.0.5")), + pkg!(("A", "0.0.9") => [dep("bad"),]), + pkg!(("A", "0.0.10") => [dep("bad"),]), + pkg!(("L-sys", "0.0.1") => [dep("bad"),]), + pkg!(("L-sys", "0.0.5")), + pkg!(("R", "0.0.4") => [ + dep_req("L-sys", "= 0.0.5"), + ]), + pkg!(("R", "0.0.6")), + pkg!(("a-sys", "0.0.5")), + pkg!(("a-sys", "0.0.11")), + pkg!(("c", "0.0.12") => [ + dep_req("R", ">= 0.0.3, <= 0.0.4"), + ]), + pkg!(("c", "0.0.13") => [ + dep_req("a-sys", ">= 0.0.8, <= 0.0.11"), + ]), + pkg!(("c0", "0.0.6") => [ + dep_req("L-sys", "<= 0.0.2"), + ]), + pkg!(("c0", "0.0.10") => [ + dep_req("A", ">= 0.0.9, <= 0.0.10"), + dep_req("a-sys", "= 0.0.5"), + ]), + pkg!("j" => [ + dep_req("A", ">= 0.0.3, <= 0.0.5"), + dep_req("R", ">=0.0.4, <= 0.0.6"), + dep_req("c", ">= 0.0.9"), + dep_req("c0", ">= 0.0.6"), + ]), + ]; + + let reg = registry(input); + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(vec![dep("j")], ®, &mut sat_resolver).is_err()); +} + +#[test] +fn conflict_store_more_then_one_match() { + let input = vec![ + pkg!(("A", "0.0.0")), + pkg!(("A", "0.0.1")), + pkg!(("A-sys", "0.0.0")), + pkg!(("A-sys", "0.0.1")), + pkg!(("A-sys", "0.0.2")), + pkg!(("A-sys", "0.0.3")), + pkg!(("A-sys", "0.0.12")), + pkg!(("A-sys", "0.0.16")), + pkg!(("B-sys", "0.0.0")), + pkg!(("B-sys", "0.0.1")), + pkg!(("B-sys", "0.0.2") => [dep_req("A-sys", "= 0.0.12"),]), + pkg!(("BA-sys", "0.0.0") => [dep_req("A-sys","= 0.0.16"),]), + pkg!(("BA-sys", "0.0.1") => [dep("bad"),]), + pkg!(("BA-sys", "0.0.2") => [dep("bad"),]), + pkg!("nA" => [ + dep("A"), + dep_req("A-sys", "<= 0.0.3"), + dep("B-sys"), + dep("BA-sys"), + ]), + ]; + let reg = registry(input); + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(vec![dep("nA")], ®, &mut sat_resolver).is_err()); +} + +#[test] +fn bad_lockfile_from_8249() { + let input = vec![ + pkg!(("a-sys", "0.2.0")), + pkg!(("a-sys", "0.1.0")), + pkg!(("b", "0.1.0") => [ + dep_req("a-sys", "0.1"), // should be optional: true, but not needed for now + ]), + pkg!(("c", "1.0.0") => [ + dep_req("b", "=0.1.0"), + ]), + pkg!("foo" => [ + dep_req("a-sys", "=0.2.0"), + { + let mut b = dep_req("b", "=0.1.0"); + b.set_features(vec!["a-sys"]); + b + }, + dep_req("c", "=1.0.0"), + ]), + ]; + let reg = registry(input); + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(vec![dep("foo")], ®, &mut sat_resolver).is_err()); +} + +#[test] +fn registry_with_features() { + let reg = registry(vec![ + pkg!("a"), + pkg!("b"), + pkg_dep_with( + "image", + vec!["a".to_opt_dep(), "b".to_opt_dep(), "jpg".to_dep()], + &[("default", &["a"]), ("b", &["dep:b"])], + ), + pkg!("jpg"), + pkg!("log"), + pkg!("man"), + pkg_dep_with("rgb", vec!["man".to_opt_dep()], &[("man", &["dep:man"])]), + pkg_dep_with( + "dep", + vec![ + "image".to_dep_with(&["b"]), + "log".to_opt_dep(), + "rgb".to_opt_dep(), + ], + &[ + ("default", &["log", "image/default"]), + ("man", &["rgb?/man"]), + ], + ), + ]); + + for deps in [ + vec!["dep".to_dep_with(&["default", "man", "log", "rgb"])], + vec!["dep".to_dep_with(&["default"])], + vec!["dep".to_dep_with(&[])], + vec!["dep".to_dep_with(&["man"])], + vec!["dep".to_dep_with(&["man", "rgb"])], + ] { + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_ok()); + } +} + +#[test] +fn missing_feature() { + let reg = registry(vec![pkg!("a")]); + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(vec!["a".to_dep_with(&["f"])], ®, &mut sat_resolver).is_err()); +} + +#[test] +fn conflict_feature_and_sys() { + let reg = registry(vec![ + pkg(("a-sys", "1.0.0")), + pkg(("a-sys", "2.0.0")), + pkg_dep_with( + ("a", "1.0.0"), + vec![dep_req("a-sys", "1.0.0")], + &[("f", &[])], + ), + pkg_dep_with( + ("a", "2.0.0"), + vec![dep_req("a-sys", "2.0.0")], + &[("g", &[])], + ), + pkg_dep("dep", vec![dep_req("a", "2.0.0")]), + ]); + + let deps = vec![dep_req("a", "*").to_dep_with(&["f"]), dep("dep")]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_err()); +} + +#[test] +fn conflict_weak_features() { + let reg = registry(vec![ + pkg(("a-sys", "1.0.0")), + pkg(("a-sys", "2.0.0")), + pkg_dep("a1", vec![dep_req("a-sys", "1.0.0").to_opt_dep()]), + pkg_dep("a2", vec![dep_req("a-sys", "2.0.0").to_opt_dep()]), + pkg_dep_with( + "dep", + vec!["a1".to_opt_dep(), "a2".to_opt_dep()], + &[("a1", &["a1?/a-sys"]), ("a2", &["a2?/a-sys"])], + ), + ]); + + let deps = vec![dep("dep").to_dep_with(&["a1", "a2"])]; + + // The following asserts should be updated when support for weak dependencies + // is added to the dependency resolver. + assert!(resolve(deps.clone(), ®).is_err()); + assert!(SatResolver::new(®).sat_resolve(&deps)); +} From 04e427075896be6f6d05afab863b3f5c93b40f86 Mon Sep 17 00:00:00 2001 From: x-hgg-x <39058530+x-hgg-x@users.noreply.github.com> Date: Sun, 29 Sep 2024 13:46:47 +0200 Subject: [PATCH 2/4] test: add more resolver helper methods --- crates/resolver-tests/src/helpers.rs | 61 +++++++++++++++++++++--- crates/resolver-tests/src/lib.rs | 34 ++++++++----- crates/resolver-tests/tests/resolve.rs | 4 +- crates/resolver-tests/tests/validated.rs | 32 ++++++------- 4 files changed, 91 insertions(+), 40 deletions(-) diff --git a/crates/resolver-tests/src/helpers.rs b/crates/resolver-tests/src/helpers.rs index b253451f088..2706c4d4732 100644 --- a/crates/resolver-tests/src/helpers.rs +++ b/crates/resolver-tests/src/helpers.rs @@ -8,40 +8,60 @@ use cargo::util::IntoUrl; pub trait ToDep { fn to_dep(self) -> Dependency; - fn to_opt_dep(self) -> Dependency; - fn to_dep_with(self, features: &[&'static str]) -> Dependency; + fn opt(self) -> Dependency; + fn with(self, features: &[&'static str]) -> Dependency; + fn with_default(self) -> Dependency; + fn rename(self, name: &str) -> Dependency; } impl ToDep for &'static str { fn to_dep(self) -> Dependency { Dependency::parse(self, Some("1.0.0"), registry_loc()).unwrap() } - fn to_opt_dep(self) -> Dependency { + fn opt(self) -> Dependency { let mut dep = self.to_dep(); dep.set_optional(true); dep } - fn to_dep_with(self, features: &[&'static str]) -> Dependency { + fn with(self, features: &[&'static str]) -> Dependency { let mut dep = self.to_dep(); dep.set_default_features(false); dep.set_features(features.into_iter().copied()); dep } + fn with_default(self) -> Dependency { + let mut dep = self.to_dep(); + dep.set_default_features(true); + dep + } + fn rename(self, name: &str) -> Dependency { + let mut dep = self.to_dep(); + dep.set_explicit_name_in_toml(name); + dep + } } impl ToDep for Dependency { fn to_dep(self) -> Dependency { self } - fn to_opt_dep(mut self) -> Dependency { + fn opt(mut self) -> Dependency { self.set_optional(true); self } - fn to_dep_with(mut self, features: &[&'static str]) -> Dependency { + fn with(mut self, features: &[&'static str]) -> Dependency { self.set_default_features(false); self.set_features(features.into_iter().copied()); self } + fn with_default(mut self) -> Dependency { + self.set_default_features(true); + self + } + fn rename(mut self, name: &str) -> Dependency { + self.set_explicit_name_in_toml(name); + self + } } pub trait ToPkgId { @@ -120,10 +140,23 @@ pub fn pkg_dep_with( Summary::new(name.to_pkgid(), dep, &features, link, None).unwrap() } +pub fn pkg_dep_link(name: T, link: &str, dep: Vec) -> Summary { + Summary::new(name.to_pkgid(), dep, &BTreeMap::new(), Some(link), None).unwrap() +} + pub fn pkg_id(name: &str) -> PackageId { PackageId::try_new(name, "1.0.0", registry_loc()).unwrap() } +pub fn pkg_id_source(name: &str, source: &str) -> PackageId { + PackageId::try_new( + name, + "1.0.0", + SourceId::for_registry(&source.into_url().unwrap()).unwrap(), + ) + .unwrap() +} + fn pkg_id_loc(name: &str, loc: &str) -> PackageId { let remote = loc.into_url(); let master = GitReference::Branch("master".to_string()); @@ -169,6 +202,12 @@ pub fn dep_req_kind(name: &str, req: &str, kind: DepKind) -> Dependency { dep } +pub fn dep_req_platform(name: &str, req: &str, platform: &str) -> Dependency { + let mut dep = dep_req(name, req); + dep.set_platform(Some(platform.parse().unwrap())); + dep +} + pub fn dep_loc(name: &str, location: &str) -> Dependency { let url = location.into_url().unwrap(); let master = GitReference::Branch("master".to_string()); @@ -177,7 +216,15 @@ pub fn dep_loc(name: &str, location: &str) -> Dependency { } pub fn dep_kind(name: &str, kind: DepKind) -> Dependency { - dep(name).set_kind(kind).clone() + let mut dep = dep(name); + dep.set_kind(kind); + dep +} + +pub fn dep_platform(name: &str, platform: &str) -> Dependency { + let mut dep = dep(name); + dep.set_platform(Some(platform.parse().unwrap())); + dep } pub fn registry(pkgs: Vec) -> Vec { diff --git a/crates/resolver-tests/src/lib.rs b/crates/resolver-tests/src/lib.rs index a6ab7aba7fb..0fda8cf62c9 100644 --- a/crates/resolver-tests/src/lib.rs +++ b/crates/resolver-tests/src/lib.rs @@ -41,14 +41,27 @@ pub fn resolve(deps: Vec, registry: &[Summary]) -> CargoResult, registry: &[Summary], sat_resolver: &mut SatResolver, ) -> CargoResult)>> { - let resolve = - resolve_with_global_context_raw(deps.clone(), registry, &GlobalContext::default().unwrap()); + resolve_and_validated_raw(deps, registry, pkg_id("root"), sat_resolver) +} + +// Verify that the resolution of cargo resolver can pass the verification of SAT +pub fn resolve_and_validated_raw( + deps: Vec, + registry: &[Summary], + root_pkg_id: PackageId, + sat_resolver: &mut SatResolver, +) -> CargoResult)>> { + let resolve = resolve_with_global_context_raw( + deps.clone(), + registry, + root_pkg_id, + &GlobalContext::default().unwrap(), + ); match resolve { Err(e) => { @@ -61,7 +74,7 @@ pub fn resolve_and_validated( Err(e) } Ok(resolve) => { - let mut stack = vec![pkg_id("root")]; + let mut stack = vec![root_pkg_id]; let mut used = HashSet::new(); let mut links = HashSet::new(); while let Some(p) = stack.pop() { @@ -106,13 +119,14 @@ pub fn resolve_with_global_context( registry: &[Summary], gctx: &GlobalContext, ) -> CargoResult)>> { - let resolve = resolve_with_global_context_raw(deps, registry, gctx)?; + let resolve = resolve_with_global_context_raw(deps, registry, pkg_id("root"), gctx)?; Ok(collect_features(&resolve)) } pub fn resolve_with_global_context_raw( deps: Vec, registry: &[Summary], + root_pkg_id: PackageId, gctx: &GlobalContext, ) -> CargoResult { struct MyRegistry<'a> { @@ -175,14 +189,8 @@ pub fn resolve_with_global_context_raw( used: HashSet::new(), }; - let root_summary = Summary::new( - pkg_id("root"), - deps, - &BTreeMap::new(), - None::<&String>, - None, - ) - .unwrap(); + let root_summary = + Summary::new(root_pkg_id, deps, &BTreeMap::new(), None::<&String>, None).unwrap(); let opts = ResolveOpts::everything(); diff --git a/crates/resolver-tests/tests/resolve.rs b/crates/resolver-tests/tests/resolve.rs index cc489266a8b..be358459a07 100644 --- a/crates/resolver-tests/tests/resolve.rs +++ b/crates/resolver-tests/tests/resolve.rs @@ -5,7 +5,7 @@ use cargo::util::GlobalContext; use resolver_tests::{ helpers::{ assert_contains, assert_same, dep, dep_kind, dep_loc, dep_req, loc_names, names, pkg_id, - pkg_loc, registry, ToDep, ToPkgId, + pkg_loc, registry, ToPkgId, }, pkg, resolve, resolve_with_global_context, }; @@ -14,7 +14,7 @@ use resolver_tests::{ #[should_panic(expected = "assertion failed: !name.is_empty()")] fn test_dependency_with_empty_name() { // Bug 5229, dependency-names must not be empty - "".to_dep(); + dep(""); } #[test] diff --git a/crates/resolver-tests/tests/validated.rs b/crates/resolver-tests/tests/validated.rs index 40902b61a32..68fc0bcd840 100644 --- a/crates/resolver-tests/tests/validated.rs +++ b/crates/resolver-tests/tests/validated.rs @@ -129,20 +129,16 @@ fn registry_with_features() { pkg!("b"), pkg_dep_with( "image", - vec!["a".to_opt_dep(), "b".to_opt_dep(), "jpg".to_dep()], + vec!["a".opt(), "b".opt(), "jpg".to_dep()], &[("default", &["a"]), ("b", &["dep:b"])], ), pkg!("jpg"), pkg!("log"), pkg!("man"), - pkg_dep_with("rgb", vec!["man".to_opt_dep()], &[("man", &["dep:man"])]), + pkg_dep_with("rgb", vec!["man".opt()], &[("man", &["dep:man"])]), pkg_dep_with( "dep", - vec![ - "image".to_dep_with(&["b"]), - "log".to_opt_dep(), - "rgb".to_opt_dep(), - ], + vec!["image".with(&["b"]), "log".opt(), "rgb".opt()], &[ ("default", &["log", "image/default"]), ("man", &["rgb?/man"]), @@ -151,11 +147,11 @@ fn registry_with_features() { ]); for deps in [ - vec!["dep".to_dep_with(&["default", "man", "log", "rgb"])], - vec!["dep".to_dep_with(&["default"])], - vec!["dep".to_dep_with(&[])], - vec!["dep".to_dep_with(&["man"])], - vec!["dep".to_dep_with(&["man", "rgb"])], + vec!["dep".with(&["default", "man", "log", "rgb"])], + vec!["dep".with(&["default"])], + vec!["dep".with(&[])], + vec!["dep".with(&["man"])], + vec!["dep".with(&["man", "rgb"])], ] { let mut sat_resolver = SatResolver::new(®); assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_ok()); @@ -166,7 +162,7 @@ fn registry_with_features() { fn missing_feature() { let reg = registry(vec![pkg!("a")]); let mut sat_resolver = SatResolver::new(®); - assert!(resolve_and_validated(vec!["a".to_dep_with(&["f"])], ®, &mut sat_resolver).is_err()); + assert!(resolve_and_validated(vec!["a".with(&["f"])], ®, &mut sat_resolver).is_err()); } #[test] @@ -187,7 +183,7 @@ fn conflict_feature_and_sys() { pkg_dep("dep", vec![dep_req("a", "2.0.0")]), ]); - let deps = vec![dep_req("a", "*").to_dep_with(&["f"]), dep("dep")]; + let deps = vec![dep_req("a", "*").with(&["f"]), dep("dep")]; let mut sat_resolver = SatResolver::new(®); assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_err()); } @@ -197,16 +193,16 @@ fn conflict_weak_features() { let reg = registry(vec![ pkg(("a-sys", "1.0.0")), pkg(("a-sys", "2.0.0")), - pkg_dep("a1", vec![dep_req("a-sys", "1.0.0").to_opt_dep()]), - pkg_dep("a2", vec![dep_req("a-sys", "2.0.0").to_opt_dep()]), + pkg_dep("a1", vec![dep_req("a-sys", "1.0.0").opt()]), + pkg_dep("a2", vec![dep_req("a-sys", "2.0.0").opt()]), pkg_dep_with( "dep", - vec!["a1".to_opt_dep(), "a2".to_opt_dep()], + vec!["a1".opt(), "a2".opt()], &[("a1", &["a1?/a-sys"]), ("a2", &["a2?/a-sys"])], ), ]); - let deps = vec![dep("dep").to_dep_with(&["a1", "a2"])]; + let deps = vec![dep("dep").with(&["a1", "a2"])]; // The following asserts should be updated when support for weak dependencies // is added to the dependency resolver. From 870f6d31d708b3315a0b1ad0ca8f3ecc17a3b617 Mon Sep 17 00:00:00 2001 From: x-hgg-x <39058530+x-hgg-x@users.noreply.github.com> Date: Sun, 29 Sep 2024 13:49:27 +0200 Subject: [PATCH 3/4] test: refactor sat resolver clauses computation Add new intermediate boolean variables for each dependency and each dependency feature declared in a package. This is used to simplify computation of the sat clauses. Add support for multiple dependencies with the same name, if their kind or target is different. A non-weak dependency feature for an optional dependency now activates a feature of the same name in the sat resolver. --- Cargo.lock | 1 + crates/resolver-tests/Cargo.toml | 1 + crates/resolver-tests/src/sat.rs | 422 ++++++++++++++++------- crates/resolver-tests/tests/validated.rs | 325 ++++++++++++++++- 4 files changed, 615 insertions(+), 134 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5f83785bb2c..2b1f206ed8c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2936,6 +2936,7 @@ name = "resolver-tests" version = "0.0.0" dependencies = [ "cargo", + "cargo-platform 0.1.8", "cargo-util", "cargo-util-schemas", "proptest", diff --git a/crates/resolver-tests/Cargo.toml b/crates/resolver-tests/Cargo.toml index 44f90690051..b63e3ffcee5 100644 --- a/crates/resolver-tests/Cargo.toml +++ b/crates/resolver-tests/Cargo.toml @@ -6,6 +6,7 @@ publish = false [dependencies] cargo.workspace = true +cargo-platform.workspace = true cargo-util-schemas.workspace = true cargo-util.workspace = true proptest.workspace = true diff --git a/crates/resolver-tests/src/sat.rs b/crates/resolver-tests/src/sat.rs index c7f98bd91f5..60546e85d1e 100644 --- a/crates/resolver-tests/src/sat.rs +++ b/crates/resolver-tests/src/sat.rs @@ -1,8 +1,10 @@ use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; use std::fmt::Write; +use cargo::core::dependency::DepKind; use cargo::core::{Dependency, FeatureMap, FeatureValue, PackageId, Summary}; use cargo::util::interning::{InternedString, INTERNED_DEFAULT}; +use cargo_platform::Platform; use varisat::ExtendFormula; const fn num_bits() -> usize { @@ -34,7 +36,7 @@ fn sat_at_most_one(solver: &mut varisat::Solver<'_>, vars: &[varisat::Var]) { } // There are more efficient ways to do it for large numbers of versions. // - // use the "Binary Encoding" from + // 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(); for (i, p) in vars.iter().enumerate() { @@ -48,7 +50,7 @@ fn sat_at_most_one_by_key( solver: &mut varisat::Solver<'_>, data: impl Iterator, ) -> HashMap> { - // no two packages with the same keys set + // No two packages with the same keys set let mut by_keys: HashMap> = HashMap::new(); for (p, v) in data { by_keys.entry(p).or_default().push(v) @@ -59,40 +61,130 @@ fn sat_at_most_one_by_key( by_keys } -fn find_compatible_dep_summaries_by_name_in_toml( +type DependencyVarMap<'a> = + HashMap), varisat::Var>>; + +type DependencyFeatureVarMap<'a> = HashMap< + InternedString, + HashMap<(DepKind, Option<&'a Platform>), HashMap>, +>; + +fn create_dependencies_vars<'a>( + solver: &mut varisat::Solver<'_>, + pkg_var: varisat::Var, + pkg_dependencies: &'a [Dependency], + pkg_features: &FeatureMap, +) -> (DependencyVarMap<'a>, DependencyFeatureVarMap<'a>) { + let mut var_for_is_dependencies_used = DependencyVarMap::new(); + let mut var_for_is_dependencies_features_used = DependencyFeatureVarMap::new(); + + for dep in pkg_dependencies { + let (name, kind, platform) = (dep.name_in_toml(), dep.kind(), dep.platform()); + + var_for_is_dependencies_used + .entry(name) + .or_default() + .insert((kind, platform), solver.new_var()); + + let dep_feature_var_map = dep + .features() + .iter() + .map(|&f| (f, solver.new_var())) + .collect(); + + var_for_is_dependencies_features_used + .entry(name) + .or_default() + .insert((kind, platform), dep_feature_var_map); + } + + for feature_values in pkg_features.values() { + for feature_value in feature_values { + let FeatureValue::DepFeature { + dep_name, + dep_feature, + weak: _, + } = *feature_value + else { + continue; + }; + + for dep_features_vars in var_for_is_dependencies_features_used + .get_mut(&dep_name) + .expect("feature dep name exists") + .values_mut() + { + dep_features_vars.insert(dep_feature, solver.new_var()); + } + } + } + + // If a package dependency is used, then the package is used + for dep_var_map in var_for_is_dependencies_used.values() { + for dep_var in dep_var_map.values() { + solver.add_clause(&[dep_var.negative(), pkg_var.positive()]); + } + } + + // If a dependency feature is used, then the dependency is used + for (&dep_name, map) in &mut var_for_is_dependencies_features_used { + for (&(dep_kind, dep_platform), dep_feature_var_map) in map { + for dep_feature_var in dep_feature_var_map.values() { + let dep_var_map = &var_for_is_dependencies_used[&dep_name]; + let dep_var = dep_var_map[&(dep_kind, dep_platform)]; + solver.add_clause(&[dep_feature_var.negative(), dep_var.positive()]); + } + } + } + + ( + var_for_is_dependencies_used, + var_for_is_dependencies_features_used, + ) +} + +fn process_pkg_dependencies( + solver: &mut varisat::Solver<'_>, + var_for_is_dependencies_used: &DependencyVarMap<'_>, + var_for_is_dependencies_features_used: &DependencyFeatureVarMap<'_>, + pkg_var: varisat::Var, pkg_dependencies: &[Dependency], - by_name: &HashMap>, -) -> HashMap> { - let empty_vec = vec![]; +) { + // Add clauses for package dependencies + for dep in pkg_dependencies { + let (name, kind, platform) = (dep.name_in_toml(), dep.kind(), dep.platform()); + let dep_var_map = &var_for_is_dependencies_used[&name]; + let dep_var = dep_var_map[&(kind, platform)]; - pkg_dependencies - .iter() - .map(|dep| { - let name_in_toml = dep.name_in_toml(); + if !dep.is_optional() { + solver.add_clause(&[pkg_var.negative(), dep_var.positive()]); + } - let compatible_summaries = by_name - .get(&dep.package_name()) - .unwrap_or(&empty_vec) - .iter() - .filter(|s| dep.matches_id(s.package_id())) - .filter(|s| dep.features().iter().all(|f| s.features().contains_key(f))) - .cloned() - .collect::>(); + for &feature_name in dep.features() { + let dep_feature_var = + &var_for_is_dependencies_features_used[&name][&(kind, platform)][&feature_name]; - (name_in_toml, compatible_summaries) - }) - .collect() + solver.add_clause(&[dep_var.negative(), dep_feature_var.positive()]); + } + } } fn process_pkg_features( solver: &mut varisat::Solver<'_>, - var_for_is_packages_used: &HashMap, - var_for_is_packages_features_used: &HashMap>, + var_for_is_dependencies_used: &DependencyVarMap<'_>, + var_for_is_dependencies_features_used: &DependencyFeatureVarMap<'_>, pkg_feature_var_map: &HashMap, + pkg_dependencies: &[Dependency], pkg_features: &FeatureMap, - compatible_dep_summaries_by_name_in_toml: &HashMap>, + check_dev_dependencies: bool, ) { - // add clauses for package features + let optional_dependencies = pkg_dependencies + .iter() + .filter(|dep| dep.is_optional()) + .map(|dep| (dep.kind(), dep.platform(), dep.name_in_toml())) + .collect::>(); + + // Add clauses for package features for (&feature_name, feature_values) in pkg_features { for feature_value in feature_values { let pkg_feature_var = pkg_feature_var_map[&feature_name]; @@ -105,39 +197,56 @@ fn process_pkg_features( ]); } FeatureValue::Dep { dep_name } => { - let dep_clause = compatible_dep_summaries_by_name_in_toml[&dep_name] - .iter() - .map(|dep| var_for_is_packages_used[&dep.package_id()].positive()) - .chain([pkg_feature_var.negative()]) - .collect::>(); - - solver.add_clause(&dep_clause); + // Add a clause for each dependency with the provided name (normal/build/dev with target) + for (&(dep_kind, _), &dep_var) in &var_for_is_dependencies_used[&dep_name] { + if dep_kind == DepKind::Development && !check_dev_dependencies { + continue; + } + solver.add_clause(&[pkg_feature_var.negative(), dep_var.positive()]); + } } FeatureValue::DepFeature { dep_name, dep_feature: dep_feature_name, weak, } => { - for dep in &compatible_dep_summaries_by_name_in_toml[&dep_name] { - let dep_var = var_for_is_packages_used[&dep.package_id()]; - let dep_feature_var = - var_for_is_packages_features_used[&dep.package_id()][&dep_feature_name]; + // Behavior of the feature: + // * if dependency `dep_name` is not optional, its feature `"dep_feature_name"` is activated. + // * if dependency `dep_name` is optional: + // - if this is a weak dependency feature: + // - feature `"dep_feature_name"` of dependency `dep_name` is activated if `dep_name` has been activated via another feature. + // - if this is not a weak dependency feature: + // - feature `dep_name` is activated if it exists. + // - dependency `dep_name` is activated. + // - feature `"dep_feature_name"` of dependency `dep_name` is activated. + + // Add clauses for each dependency with the provided name (normal/build/dev with target) + let dep_var_map = &var_for_is_dependencies_used[&dep_name]; + for (&(dep_kind, dep_platform), &dep_var) in dep_var_map { + if dep_kind == DepKind::Development && !check_dev_dependencies { + continue; + } + + let dep_feature_var = &var_for_is_dependencies_features_used[&dep_name] + [&(dep_kind, dep_platform)][&dep_feature_name]; solver.add_clause(&[ pkg_feature_var.negative(), dep_var.negative(), dep_feature_var.positive(), ]); - } - if !weak { - let dep_clause = compatible_dep_summaries_by_name_in_toml[&dep_name] - .iter() - .map(|dep| var_for_is_packages_used[&dep.package_id()].positive()) - .chain([pkg_feature_var.negative()]) - .collect::>(); - - solver.add_clause(&dep_clause); + let key = (dep_kind, dep_platform, dep_name); + if !weak && optional_dependencies.contains(&key) { + solver.add_clause(&[pkg_feature_var.negative(), dep_var.positive()]); + + if let Some(other_feature_var) = pkg_feature_var_map.get(&dep_name) { + solver.add_clause(&[ + pkg_feature_var.negative(), + other_feature_var.positive(), + ]); + } + } } } } @@ -145,48 +254,77 @@ fn process_pkg_features( } } -fn process_pkg_dependencies( +fn process_compatible_dep_summaries( solver: &mut varisat::Solver<'_>, + var_for_is_dependencies_used: &DependencyVarMap<'_>, + var_for_is_dependencies_features_used: &DependencyFeatureVarMap<'_>, var_for_is_packages_used: &HashMap, var_for_is_packages_features_used: &HashMap>, - pkg_var: varisat::Var, + by_name: &HashMap>, pkg_dependencies: &[Dependency], - compatible_dep_summaries_by_name_in_toml: &HashMap>, + check_dev_dependencies: bool, ) { for dep in pkg_dependencies { - let compatible_dep_summaries = - &compatible_dep_summaries_by_name_in_toml[&dep.name_in_toml()]; + if dep.kind() == DepKind::Development && !check_dev_dependencies { + continue; + } - // add clauses for package dependency features - for dep_summary in compatible_dep_summaries { - let dep_package_id = dep_summary.package_id(); + let (name, kind, platform) = (dep.name_in_toml(), dep.kind(), dep.platform()); + let dep_var_map = &var_for_is_dependencies_used[&name]; + let dep_var = dep_var_map[&(kind, platform)]; - let default_feature = if dep.uses_default_features() - && dep_summary.features().contains_key(&*INTERNED_DEFAULT) - { - Some(&INTERNED_DEFAULT) - } else { - None - }; + let dep_feature_var_map = &var_for_is_dependencies_features_used[&name][&(kind, platform)]; - for &feature_name in default_feature.into_iter().chain(dep.features()) { - solver.add_clause(&[ - pkg_var.negative(), - var_for_is_packages_used[&dep_package_id].negative(), - var_for_is_packages_features_used[&dep_package_id][&feature_name].positive(), - ]); - } - } + let compatible_summaries = by_name + .get(&dep.package_name()) + .into_iter() + .flatten() + .filter(|s| dep.matches(s)) + .filter(|s| dep.features().iter().all(|f| s.features().contains_key(f))) + .cloned() + .collect::>(); - // active packages need to activate each of their non-optional dependencies - if !dep.is_optional() { - let dep_clause = compatible_dep_summaries + // At least one compatible package should be activated + let dep_clause = compatible_summaries + .iter() + .map(|s| var_for_is_packages_used[&s.package_id()].positive()) + .chain([dep_var.negative()]) + .collect::>(); + + solver.add_clause(&dep_clause); + + for (&feature_name, &dep_feature_var) in dep_feature_var_map { + // At least one compatible package with the additional feature should be activated + let dep_feature_clause = compatible_summaries .iter() - .map(|d| var_for_is_packages_used[&d.package_id()].positive()) - .chain([pkg_var.negative()]) + .filter_map(|s| { + var_for_is_packages_features_used[&s.package_id()].get(&feature_name) + }) + .map(|var| var.positive()) + .chain([dep_feature_var.negative()]) .collect::>(); - solver.add_clause(&dep_clause); + solver.add_clause(&dep_feature_clause); + } + + if dep.uses_default_features() { + // For the selected package for this dependency, the `"default"` feature should be activated if it exists + let mut dep_default_clause = vec![dep_var.negative()]; + + for s in &compatible_summaries { + let s_pkg_id = s.package_id(); + let s_var = var_for_is_packages_used[&s_pkg_id]; + let s_feature_var_map = &var_for_is_packages_features_used[&s_pkg_id]; + + if let Some(s_default_feature_var) = s_feature_var_map.get(&INTERNED_DEFAULT) { + dep_default_clause + .extend_from_slice(&[s_var.negative(), s_default_feature_var.positive()]); + } else { + dep_default_clause.push(s_var.positive()); + } + } + + solver.add_clause(&dep_default_clause); } } } @@ -209,44 +347,50 @@ pub struct SatResolver { } impl SatResolver { - pub fn new(registry: &[Summary]) -> Self { + pub fn new<'a>(registry: impl IntoIterator) -> Self { + let check_dev_dependencies = false; + + let mut by_name: HashMap> = HashMap::new(); + for pkg in registry { + by_name.entry(pkg.name()).or_default().push(pkg.clone()) + } + let mut solver = varisat::Solver::new(); - // That represents each package version which is set to "true" if the packages in the lock file and "false" if it is unused. - let var_for_is_packages_used = registry - .iter() - .map(|s| (s.package_id(), solver.new_var())) - .collect::>(); + // Create boolean variables for packages and packages features + let mut var_for_is_packages_used = HashMap::new(); + let mut var_for_is_packages_features_used = HashMap::<_, HashMap<_, _>>::new(); - // That represents each feature of each package version, which is set to "true" if the package feature is used. - let var_for_is_packages_features_used = registry - .iter() - .map(|s| { - ( - s.package_id(), - (s.features().keys().map(|&f| (f, solver.new_var()))).collect(), - ) - }) - .collect::>>(); - - // if a package feature is used, then the package is used - for (package, pkg_feature_var_map) in &var_for_is_packages_features_used { - for (_, package_feature_var) in pkg_feature_var_map { - let package_var = var_for_is_packages_used[package]; - solver.add_clause(&[package_feature_var.negative(), package_var.positive()]); + for pkg in by_name.values().flatten() { + let pkg_id = pkg.package_id(); + + var_for_is_packages_used.insert(pkg_id, solver.new_var()); + + var_for_is_packages_features_used.insert( + pkg_id, + (pkg.features().keys().map(|&f| (f, solver.new_var()))).collect(), + ); + } + + // If a package feature is used, then the package is used + for (&pkg_id, pkg_feature_var_map) in &var_for_is_packages_features_used { + for pkg_feature_var in pkg_feature_var_map.values() { + let pkg_var = var_for_is_packages_used[&pkg_id]; + solver.add_clause(&[pkg_feature_var.negative(), pkg_var.positive()]); } } - // no two packages with the same links set + // No two packages with the same links set sat_at_most_one_by_key( &mut solver, - registry - .iter() + by_name + .values() + .flatten() .map(|s| (s.links(), var_for_is_packages_used[&s.package_id()])) .filter(|(l, _)| l.is_some()), ); - // no two semver compatible versions of the same package + // No two semver compatible versions of the same package sat_at_most_one_by_key( &mut solver, var_for_is_packages_used @@ -254,36 +398,43 @@ impl SatResolver { .map(|(p, &v)| (p.as_activations_key(), v)), ); - let mut by_name: HashMap> = HashMap::new(); - - for p in registry { - by_name.entry(p.name()).or_default().push(p.clone()) - } - - for pkg in registry { + for pkg in by_name.values().flatten() { let pkg_id = pkg.package_id(); let pkg_dependencies = pkg.dependencies(); let pkg_features = pkg.features(); + let pkg_var = var_for_is_packages_used[&pkg_id]; + + // Create boolean variables for dependencies and dependencies features + let (var_for_is_dependencies_used, var_for_is_dependencies_features_used) = + create_dependencies_vars(&mut solver, pkg_var, pkg_dependencies, pkg_features); - let compatible_dep_summaries_by_name_in_toml = - find_compatible_dep_summaries_by_name_in_toml(pkg_dependencies, &by_name); + process_pkg_dependencies( + &mut solver, + &var_for_is_dependencies_used, + &var_for_is_dependencies_features_used, + pkg_var, + pkg_dependencies, + ); process_pkg_features( &mut solver, - &var_for_is_packages_used, - &var_for_is_packages_features_used, + &var_for_is_dependencies_used, + &var_for_is_dependencies_features_used, &var_for_is_packages_features_used[&pkg_id], + pkg_dependencies, pkg_features, - &compatible_dep_summaries_by_name_in_toml, + check_dev_dependencies, ); - process_pkg_dependencies( + process_compatible_dep_summaries( &mut solver, + &var_for_is_dependencies_used, + &var_for_is_dependencies_features_used, &var_for_is_packages_used, &var_for_is_packages_features_used, - var_for_is_packages_used[&pkg_id], + &by_name, pkg_dependencies, - &compatible_dep_summaries_by_name_in_toml, + check_dev_dependencies, ); } @@ -313,28 +464,39 @@ impl SatResolver { let root_var = solver.new_var(); - // root package is always used - // root vars from previous runs are deactivated - let assumption = old_root_vars - .iter() - .map(|v| v.negative()) - .chain([root_var.positive()]) - .collect::>(); - - old_root_vars.push(root_var); - - let compatible_dep_summaries_by_name_in_toml = - find_compatible_dep_summaries_by_name_in_toml(root_dependencies, &by_name); + // Create boolean variables for dependencies and dependencies features + let (var_for_is_dependencies_used, var_for_is_dependencies_features_used) = + create_dependencies_vars(solver, root_var, root_dependencies, &FeatureMap::new()); process_pkg_dependencies( solver, + &var_for_is_dependencies_used, + &var_for_is_dependencies_features_used, + root_var, + root_dependencies, + ); + + process_compatible_dep_summaries( + solver, + &var_for_is_dependencies_used, + &var_for_is_dependencies_features_used, var_for_is_packages_used, var_for_is_packages_features_used, - root_var, + by_name, root_dependencies, - &compatible_dep_summaries_by_name_in_toml, + true, ); + // Root package is always used. + // Root vars from previous runs are deactivated. + let assumption = old_root_vars + .iter() + .map(|v| v.negative()) + .chain([root_var.positive()]) + .collect::>(); + + old_root_vars.push(root_var); + solver.assume(&assumption); solver @@ -353,7 +515,7 @@ impl SatResolver { } } - // root vars from previous runs are deactivated + // Root vars from previous runs are deactivated let assumption = (self.old_root_vars.iter().map(|v| v.negative())) .chain( self.var_for_is_packages_used diff --git a/crates/resolver-tests/tests/validated.rs b/crates/resolver-tests/tests/validated.rs index 68fc0bcd840..5eaccf29252 100644 --- a/crates/resolver-tests/tests/validated.rs +++ b/crates/resolver-tests/tests/validated.rs @@ -1,7 +1,10 @@ -use cargo::core::Dependency; +use cargo::core::{dependency::DepKind, Dependency}; use resolver_tests::{ - helpers::{dep, dep_req, pkg, pkg_dep, pkg_dep_with, registry, ToDep}, + helpers::{ + dep, dep_kind, dep_platform, dep_req, dep_req_kind, dep_req_platform, pkg, pkg_dep, + pkg_dep_with, registry, ToDep, + }, pkg, resolve, resolve_and_validated, sat::SatResolver, }; @@ -165,6 +168,59 @@ fn missing_feature() { assert!(resolve_and_validated(vec!["a".with(&["f"])], ®, &mut sat_resolver).is_err()); } +#[test] +fn missing_dep_feature() { + let reg = registry(vec![ + pkg("a"), + pkg_dep_with("dep", vec![dep("a")], &[("f", &["a/a"])]), + ]); + + let deps = vec![dep("dep").with(&["f"])]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_err()); +} + +#[test] +fn missing_weak_dep_feature() { + let reg = registry(vec![ + pkg("a"), + pkg_dep_with("dep1", vec![dep("a")], &[("f", &["a/a"])]), + pkg_dep_with("dep2", vec!["a".opt()], &[("f", &["a/a"])]), + pkg_dep_with("dep3", vec!["a".opt()], &[("f", &["a?/a"])]), + pkg_dep_with("dep4", vec!["x".opt()], &[("f", &["x?/a"])]), + ]); + + let deps = vec![dep("dep1").with(&["f"])]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_err()); + + let deps = vec![dep("dep2").with(&["f"])]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_err()); + + let deps = vec![dep("dep2").with(&["a", "f"])]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_err()); + + // Weak dependencies are not supported yet in the dependency resolver + let deps = vec![dep("dep3").with(&["f"])]; + assert!(resolve(deps.clone(), ®).is_err()); + assert!(SatResolver::new(®).sat_resolve(&deps)); + + let deps = vec![dep("dep3").with(&["a", "f"])]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_err()); + + // Weak dependencies are not supported yet in the dependency resolver + let deps = vec![dep("dep4").with(&["f"])]; + assert!(resolve(deps.clone(), ®).is_err()); + assert!(SatResolver::new(®).sat_resolve(&deps)); + + let deps = vec![dep("dep4").with(&["x", "f"])]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_err()); +} + #[test] fn conflict_feature_and_sys() { let reg = registry(vec![ @@ -204,8 +260,269 @@ fn conflict_weak_features() { let deps = vec![dep("dep").with(&["a1", "a2"])]; - // The following asserts should be updated when support for weak dependencies - // is added to the dependency resolver. + // Weak dependencies are not supported yet in the dependency resolver + assert!(resolve(deps.clone(), ®).is_err()); + assert!(SatResolver::new(®).sat_resolve(&deps)); +} + +#[test] +fn multiple_dep_kinds_and_targets() { + let reg = registry(vec![ + pkg(("a-sys", "1.0.0")), + pkg(("a-sys", "2.0.0")), + pkg_dep_with( + "dep1", + vec![ + dep_req_platform("a-sys", "1.0.0", "cfg(all())").opt(), + dep_req("a-sys", "1.0.0").opt(), + dep_req_kind("a-sys", "2.0.0", DepKind::Build).opt(), + ], + &[("default", &["dep:a-sys"])], + ), + pkg_dep_with( + "dep2", + vec![ + dep_req_platform("a-sys", "1.0.0", "cfg(all())").opt(), + dep_req("a-sys", "1.0.0").opt(), + dep_req_kind("a-sys", "2.0.0", DepKind::Development).rename("a-sys-dev"), + ], + &[("default", &["dep:a-sys", "a-sys-dev/bad"])], + ), + ]); + + let deps = vec![dep("dep1")]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_err()); + + let deps = vec![dep("dep2")]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_ok()); + + let deps = vec![ + dep_req("a-sys", "1.0.0"), + dep_req_kind("a-sys", "2.0.0", DepKind::Build).rename("a2"), + ]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_err()); + + let deps = vec![ + dep_req("a-sys", "1.0.0"), + dep_req_kind("a-sys", "2.0.0", DepKind::Development).rename("a2"), + ]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_err()); +} + +#[test] +fn multiple_dep_kinds_and_targets_with_different_packages() { + let reg = registry(vec![ + pkg_dep_with("a", vec![], &[("f", &[])]), + pkg_dep_with("b", vec![], &[("f", &[])]), + pkg_dep_with("c", vec![], &[("g", &[])]), + pkg_dep_with( + "dep1", + vec![ + "a".opt().rename("x").with(&["f"]), + dep_platform("a", "cfg(all())").opt().rename("x"), + dep_kind("b", DepKind::Build).opt().rename("x").with(&["f"]), + ], + &[("default", &["x"])], + ), + pkg_dep_with( + "dep2", + vec![ + "a".opt().rename("x").with(&["f"]), + dep_platform("a", "cfg(all())").opt().rename("x"), + dep_kind("c", DepKind::Build).opt().rename("x").with(&["f"]), + ], + &[("default", &["x"])], + ), + ]); + + let deps = vec!["dep1".with(&["default"])]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_ok()); + + let deps = vec!["dep2".with(&["default"])]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_err()); +} + +#[test] +fn dep_feature_with_shadowing_feature() { + let reg = registry(vec![ + pkg_dep_with("a", vec![], &[("b", &[])]), + pkg_dep_with( + "dep", + vec!["a".opt().rename("aa"), "c".opt()], + &[("default", &["aa/b"]), ("aa", &["c"])], + ), + ]); + + let deps = vec!["dep".with(&["default"])]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_err()); +} + +#[test] +fn dep_feature_not_optional_with_shadowing_feature() { + let reg = registry(vec![ + pkg_dep_with("a", vec![], &[("b", &[])]), + pkg_dep_with( + "dep", + vec!["a".rename("aa"), "c".opt()], + &[("default", &["aa/b"]), ("aa", &["c"])], + ), + ]); + + let deps = vec!["dep".with(&["default"])]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_ok()); +} + +#[test] +fn dep_feature_weak_with_shadowing_feature() { + let reg = registry(vec![ + pkg_dep_with("a", vec![], &[("b", &[])]), + pkg_dep_with( + "dep", + vec!["a".opt().rename("aa"), "c".opt()], + &[("default", &["aa?/b"]), ("aa", &["c"])], + ), + ]); + + let deps = vec!["dep".with(&["default"])]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_ok()); +} + +#[test] +fn dep_feature_duplicate_with_shadowing_feature() { + let reg = registry(vec![ + pkg_dep_with("a", vec![], &[("b", &[])]), + pkg_dep_with( + "dep", + vec![ + "a".opt().rename("aa"), + dep_kind("a", DepKind::Build).rename("aa"), + "c".opt(), + ], + &[("default", &["aa/b"]), ("aa", &["c"])], + ), + ]); + + let deps = vec!["dep".with(&["default"])]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_err()); +} + +#[test] +fn optional_dep_features() { + let reg = registry(vec![ + pkg_dep("a", vec!["bad".opt()]), + pkg_dep("b", vec!["a".opt().with(&["bad"])]), + pkg_dep("dep", vec![dep("a"), dep("b")]), + ]); + + let deps = vec![dep("dep")]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_ok()); +} + +#[test] +fn optional_dep_features_with_rename() { + let reg = registry(vec![ + pkg("x1"), + pkg_dep("a", vec!["x1".opt(), "x2".opt(), "x3".opt()]), + pkg_dep( + "dep1", + vec![ + "a".opt().with(&["x1"]), + dep_kind("a", DepKind::Build).opt().with(&["x2"]), + ], + ), + pkg_dep( + "dep2", + vec![ + "a".opt().with(&["x1"]), + "a".opt().rename("a2").with(&["x3"]), + ], + ), + ]); + + let deps = vec!["dep1".with(&["a"])]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_err()); + + let deps = vec!["dep2".with(&["a"])]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_ok()); +} + +#[test] +fn optional_weak_dep_features() { + let reg = registry(vec![ + pkg_dep("a", vec!["bad".opt()]), + pkg_dep("b", vec![dep("a")]), + pkg_dep_with("dep", vec!["a".opt(), dep("b")], &[("f", &["a?/bad"])]), + ]); + + let deps = vec!["dep".with(&["f"])]; + + // Weak dependencies are not supported yet in the dependency resolver assert!(resolve(deps.clone(), ®).is_err()); assert!(SatResolver::new(®).sat_resolve(&deps)); } + +#[test] +fn default_feature_multiple_major_versions() { + let reg = registry(vec![ + pkg_dep_with(("a", "0.2.0"), vec![], &[("default", &[])]), + pkg(("a", "0.3.0")), + pkg_dep_with(("a", "0.4.0"), vec![], &[("default", &[])]), + pkg_dep( + "dep1", + vec![ + dep_req("a", ">=0.2, <0.4").with_default(), + dep_req("a", "0.2").rename("a2").with(&[]), + ], + ), + pkg_dep( + "dep2", + vec![ + dep_req("a", ">=0.2, <0.4").with_default(), + dep_req("a", "0.3").rename("a2").with(&[]), + ], + ), + pkg_dep( + "dep3", + vec![ + dep_req("a", ">=0.2, <0.4").with_default(), + dep_req("a", "0.2").rename("a1").with(&[]), + dep_req("a", "0.3").rename("a2").with(&[]), + ], + ), + pkg_dep("dep4", vec![dep_req("a", ">=0.2, <0.4").with_default()]), + pkg_dep("dep5", vec![dep_req("a", ">=0.3, <0.5").with_default()]), + ]); + + let deps = vec![dep("dep1")]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_ok()); + + let deps = vec![dep("dep2")]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_ok()); + + let deps = vec![dep("dep3")]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_ok()); + + let deps = vec![dep("dep4")]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_ok()); + + let deps = vec![dep("dep5")]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_ok()); +} From dedf251c279b14e656e79a3db15187ebcdf88a9c Mon Sep 17 00:00:00 2001 From: x-hgg-x <39058530+x-hgg-x@users.noreply.github.com> Date: Sun, 29 Sep 2024 21:58:22 +0200 Subject: [PATCH 4/4] test: add tests from pubgrub --- Cargo.lock | 2 +- crates/resolver-tests/tests/pubgrub.rs | 452 +++++++++++++++++++++++++ 2 files changed, 453 insertions(+), 1 deletion(-) create mode 100644 crates/resolver-tests/tests/pubgrub.rs diff --git a/Cargo.lock b/Cargo.lock index 2b1f206ed8c..ba6d40605d6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2936,7 +2936,7 @@ name = "resolver-tests" version = "0.0.0" dependencies = [ "cargo", - "cargo-platform 0.1.8", + "cargo-platform 0.1.9", "cargo-util", "cargo-util-schemas", "proptest", diff --git a/crates/resolver-tests/tests/pubgrub.rs b/crates/resolver-tests/tests/pubgrub.rs new file mode 100644 index 00000000000..fec5a8757bc --- /dev/null +++ b/crates/resolver-tests/tests/pubgrub.rs @@ -0,0 +1,452 @@ +use cargo::core::{dependency::DepKind, Dependency}; + +use resolver_tests::{ + helpers::{ + dep, dep_kind, dep_req, dep_req_kind, pkg, pkg_dep, pkg_dep_link, pkg_dep_with, + pkg_id_source, registry, ToDep, + }, + pkg, resolve, resolve_and_validated, resolve_and_validated_raw, + sat::SatResolver, +}; + +#[test] +fn test_01_renamed_package() { + let reg = registry(vec![ + pkg_dep_with( + "a", + vec!["b".opt().rename("b_package")], + &[("default", &["b_package"])], + ), + pkg("b"), + ]); + + let deps = vec!["a".with(&["default"])]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_ok()); +} + +#[test] +fn test_02_renamed_package_no_shadowing() { + let reg = registry(vec![ + pkg("url"), + pkg_dep("wasmi", vec!["wasmparser-nostd".rename("wasmparser")]), + pkg_dep("wasmparser", vec!["url".to_dep()]), + ]); + + let deps = vec![dep("wasmi")]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_err()); +} + +#[test] +fn test_03_prerelease_semver() { + let reg = registry(vec![ + pkg!("parking_lot_core" => [dep_req("smallvec", "^1.6.1")]), + pkg(("smallvec", "2.0.0-alpha.3")), + pkg_dep_with( + ("tokio", "1.35.1"), + vec!["parking_lot_core".opt()], + &[("default", &["parking_lot_core"])], + ), + ]); + + let deps = vec!["tokio".with(&["default"])]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_err()); +} + +#[test] +fn test_04_cyclic_features() { + let reg = registry(vec![pkg_dep_with( + "windows", + vec![], + &[ + ("Win32", &["Win32_Foundation"]), + ("Win32_Foundation", &["Win32"]), + ], + )]); + + let deps = vec!["windows".with(&["Win32_Foundation"])]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_ok()); +} + +#[test] +fn test_05_cyclic_optional_dependencies() { + let reg = registry(vec![ + pkg_dep("async-global-executor", vec!["io-lifetimes".opt()]), + pkg_dep( + "io-lifetimes", + vec!["test".with(&["async-global-executor"])], + ), + pkg_dep_with( + "test", + vec!["async-global-executor".opt().with(&["io-lifetimes"])], + &[], + ), + ]); + + let deps = vec![dep("test")]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_ok()); +} + +#[test] +fn test_06_cyclic_dependencies() { + let reg = registry(vec![ + pkg(("a", "1.0.0")), + pkg_dep(("a", "1.0.1"), vec![dep("dep")]), + pkg_dep("dep", vec![dep("a")]), + ]); + + let deps = vec![dep("dep")]; + + // Cyclic dependencies are not checked in the SAT resolver + assert!(resolve(deps.clone(), ®).is_err()); + assert!(SatResolver::new(®).sat_resolve(&deps)); +} + +#[test] +fn test_07_self_dependency() { + let reg = registry(vec![pkg_dep("dep", vec![dep("dep")])]); + + let deps = vec![dep("dep")]; + + // Cyclic dependencies are not checked in the SAT resolver + assert!(resolve(deps.clone(), ®).is_err()); + assert!(SatResolver::new(®).sat_resolve(&deps)); +} + +#[test] +fn test_08_activated_optional_self_dependency() { + let reg = registry(vec![pkg_dep("a", vec!["a".opt()])]); + + let deps = vec!["a".with(&["a"])]; + + // Cyclic dependencies are not checked in the SAT resolver + assert!(resolve(deps.clone(), ®).is_err()); + assert!(SatResolver::new(®).sat_resolve(&deps)); +} + +#[test] +fn test_09_build_dependency_with_same_name() { + let reg = registry(vec![ + pkg("memchr"), + pkg_dep_with( + ("regex", "1.4.6"), + vec!["memchr".opt()], + &[("default", &["memchr"])], + ), + pkg_dep("sv-parser", vec!["regex".with(&["default"])]), + pkg_dep( + "svlint", + vec![ + dep_req("regex", "^1.5"), + dep_req_kind("regex", "^1", DepKind::Build), + dep("sv-parser"), + ], + ), + ]); + + let deps = vec![dep("svlint")]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_err()); +} + +#[test] +fn test_10_root_dev_dependency_with_same_name() { + let reg = registry(vec![pkg(("root", "1.0.1"))]); + + let deps = vec![dep_req_kind("root", "=1.0.1", DepKind::Development).rename("root101")]; + let source = pkg_id_source("root", "https://root.example.com"); + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated_raw(deps, ®, source, &mut sat_resolver).is_ok()); +} + +#[test] +fn test_11_dev_dependency() { + let reg = registry(vec![pkg_dep_with( + "burn-core", + vec![dep_kind("burn-ndarray", DepKind::Development)], + &[("default", &["burn-ndarray/std"])], + )]); + + let deps = vec!["burn-core".with(&["default"])]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_ok()); +} + +#[test] +fn test_12_weak_dependencies() { + let reg = registry(vec![ + pkg_dep_with("borsh", vec!["borsh-derive".opt()], &[("std", &[])]), + pkg_dep_with( + "rust_decimal", + vec!["borsh".opt().with(&["borsh-derive"])], + &[("default", &["borsh?/std"])], + ), + ]); + + let deps = vec!["rust_decimal".with(&["default"])]; + + // Weak dependencies are not supported yet in the dependency resolver + assert!(resolve(deps.clone(), ®).is_err()); + assert!(SatResolver::new(®).sat_resolve(&deps)); +} + +#[test] +fn test_13_weak_dependencies() { + let reg = registry(vec![ + pkg_dep_with("memchr", vec!["std".opt()], &[("std", &["dep:std"])]), + pkg_dep_with( + "winnow", + vec!["memchr".opt()], + &[("default", &["memchr?/std"]), ("simd", &["dep:memchr"])], + ), + ]); + + let deps = vec!["winnow".with(&["default"])]; + + // Weak dependencies are not supported yet in the dependency resolver + assert!(resolve(deps.clone(), ®).is_err()); + assert!(SatResolver::new(®).sat_resolve(&deps)); +} + +#[test] +fn test_14_weak_dependencies() { + let reg = registry(vec![ + pkg_dep("a", vec![dep("bad")]), + pkg_dep_with("b", vec!["a".opt()], &[("perf-literal", &["dep:a"])]), + pkg_dep_with( + "c", + vec!["b".opt()], + &[ + ("perf-literal", &["b?/perf-literal"]), + ("perf-literal-multisubstring", &["dep:b"]), + ], + ), + pkg_dep_with("dep", vec![dep("c")], &[("default", &["c/perf-literal"])]), + ]); + + let deps = vec!["dep".with(&["default"])]; + + // Weak dependencies are not supported yet in the dependency resolver + assert!(resolve(deps.clone(), ®).is_err()); + assert!(SatResolver::new(®).sat_resolve(&deps)); +} + +#[test] +fn test_15_duplicate_sys_crate() { + let reg = registry(vec![ + pkg_dep_link("js", "js", vec![]), + pkg_dep_link("dep", "js", vec![dep("js")]), + ]); + + let deps = vec![dep("dep")]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_err()); +} + +#[test] +fn test_16_missing_optional_dependency() { + let reg = registry(vec![ + pkg_dep("b", vec!["c".opt()]), + pkg_dep_with("dep", vec![dep("b")], &[("d", &["b/c"])]), + ]); + + let deps = vec!["dep".with(&["d"])]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_err()); +} + +#[test] +fn test_17_feature_shadowing_missing_optional_dependency() { + let reg = registry(vec![pkg_dep_with( + "rustix", + vec!["alloc".opt()], + &[ + ("alloc", &[]), + ("default", &["alloc"]), + ("rustc-dep-of-std", &["dep:alloc"]), + ], + )]); + + let deps = vec!["rustix".with(&["default"])]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_ok()); +} + +#[test] +fn test_18_feature_shadowing_activated_optional_dependency() { + let reg = registry(vec![ + pkg_dep("alloc", vec![dep("bad")]), + pkg_dep_with( + "rustix", + vec!["alloc".opt()], + &[ + ("alloc", &[]), + ("default", &["dep:alloc"]), + ("rustc-dep-of-std", &["alloc"]), + ], + ), + ]); + + let deps = vec!["rustix".with(&["default"])]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_err()); +} + +#[test] +fn test_19_same_dep_twice_feature_unification() { + let reg = registry(vec![ + pkg_dep_with( + "iced", + vec!["iced_wgpu".opt(), "iced_wgpu".opt().with(&["webgl"])], + &[("default", &["iced_wgpu"])], + ), + pkg_dep_with("iced_wgpu", vec![], &[("webgl", &[])]), + ]); + + let deps = vec!["iced".with(&["default"])]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_ok()); +} + +#[test] +fn test_20_no_implicit_feature() { + let reg = registry(vec![ + pkg("c"), + pkg_dep_with("ureq", vec!["c".opt()], &[("cookies", &["dep:c"])]), + pkg_dep_with("dep", vec![dep("ureq")], &[("cookies", &["ureq/c"])]), + ]); + + let deps = vec!["dep".with(&["cookies"])]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_err()); +} + +#[test] +fn test_21_implicit_feature() { + let reg = registry(vec![ + pkg("c"), + pkg_dep("ureq", vec!["c".opt()]), + pkg_dep_with("dep", vec![dep("ureq")], &[("cookies", &["ureq/c"])]), + ]); + + let deps = vec!["dep".with(&["cookies"])]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_ok()); +} + +#[test] +fn test_22_missing_explicit_default_feature() { + let reg = registry(vec![ + pkg_dep_with( + "fuel-tx", + vec![dep("serde"), "serde_json".opt()], + &[("default", &["serde/default"]), ("serde", &["serde_json"])], + ), + pkg("serde"), + ]); + + let deps = vec!["fuel-tx".with(&["default"])]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_err()); +} + +#[test] +fn test_23_no_need_for_explicit_default_feature() { + let reg = registry(vec![ + pkg("a"), + pkg_dep_with( + "b", + vec!["a".with_default()], + &[("default", &["std"]), ("std", &[])], + ), + ]); + + let deps = vec!["b".with(&["default"])]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_ok()); +} + +#[test] +fn test_24_dep_feature() { + let reg = registry(vec![ + pkg_dep_with("proc-macro2", vec![], &[("proc-macro", &[])]), + pkg_dep_with( + "syn", + vec![dep("proc-macro2")], + &[("proc-macro", &["proc-macro2/proc-macro"])], + ), + pkg_dep("serde_derive", vec!["syn".with(&["proc-macro"])]), + ]); + + let deps = vec![dep("serde_derive")]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_ok()); +} + +#[test] +fn test_25_dep_feature() { + let reg = registry(vec![ + pkg_dep_with("proc-macro2", vec![], &[("proc-macro", &[])]), + pkg_dep_with( + "syn", + vec![dep("proc-macro2")], + &[("proc-macro", &["proc-macro2/proc-macro"])], + ), + ]); + + let deps = vec!["syn".with(&["proc-macro"])]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_ok()); +} + +#[test] +fn test_26_implicit_feature_with_dep_feature() { + let reg = registry(vec![ + pkg_dep_with("quote", vec![], &[("proc-macro", &[])]), + pkg_dep_with( + "syn", + vec!["quote".opt()], + &[("default", &["quote", "quote/proc-macro"])], + ), + ]); + + let deps = vec!["syn".with(&["default"])]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_ok()); +} + +#[test] +fn test_27_dep_feature_activating_shadowing_feature() { + let reg = registry(vec![ + pkg_dep_with( + "a", + vec!["b".opt(), "x".opt()], + &[("b", &["x"]), ("default", &["b/native"])], + ), + pkg_dep_with("b", vec![], &[("native", &[])]), + ]); + + let deps = vec!["a".with(&["default"])]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_err()); +} + +#[test] +fn test_28_dep_feature_not_activating_shadowing_feature() { + let reg = registry(vec![ + pkg_dep_with( + "fuel-tx", + vec![dep("serde"), "serde_json".opt()], + &[("default", &["serde/default"]), ("serde", &["serde_json"])], + ), + pkg_dep_with("serde", vec![], &[("default", &[])]), + ]); + + let deps = vec!["fuel-tx".with(&["default"])]; + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(deps, ®, &mut sat_resolver).is_ok()); +}