diff --git a/crates/resolver-tests/src/lib.rs b/crates/resolver-tests/src/lib.rs index 43dc49dd339..aa9c8a56ce8 100644 --- a/crates/resolver-tests/src/lib.rs +++ b/crates/resolver-tests/src/lib.rs @@ -1,25 +1,24 @@ #![allow(clippy::print_stderr)] -use std::cell::RefCell; use std::cmp::{max, min}; -use std::collections::{BTreeMap, HashMap, HashSet}; +use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; use std::fmt; use std::fmt::Write; -use std::rc::Rc; 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::Resolve; +use cargo::core::FeatureMap; use cargo::core::ResolveVersion; 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_schemas::manifest::RustVersion; use proptest::collection::{btree_map, vec}; use proptest::prelude::*; @@ -28,25 +27,29 @@ use proptest::string::string_regex; use varisat::ExtendFormula; pub fn resolve(deps: Vec, registry: &[Summary]) -> CargoResult> { - resolve_with_global_context(deps, registry, &GlobalContext::default().unwrap()) + Ok( + resolve_with_global_context(deps, registry, &GlobalContext::default().unwrap())? + .into_iter() + .map(|(pkg, _)| pkg) + .collect(), + ) } // Verify that the resolution of cargo resolver can pass the verification of SAT pub fn resolve_and_validated( deps: Vec, registry: &[Summary], - sat_resolve: Option, -) -> CargoResult> { + sat_resolver: &mut SatResolver, +) -> CargoResult)>> { let resolve = resolve_with_global_context_raw(deps.clone(), registry, &GlobalContext::default().unwrap()); match resolve { Err(e) => { - let sat_resolve = sat_resolve.unwrap_or_else(|| SatResolve::new(registry)); - if sat_resolve.sat_resolve(&deps) { + if sat_resolver.sat_resolve(&deps) { panic!( - "the resolve err but the sat_resolve thinks this will work:\n{}", - sat_resolve.use_packages().unwrap() + "`resolve()` returned an error but the sat resolver thinks this will work:\n{}", + sat_resolver.used_packages().unwrap() ); } Err(e) @@ -70,14 +73,13 @@ pub fn resolve_and_validated( })); } } - let out = resolve.sort(); + let out = collect_features(&resolve); assert_eq!(out.len(), used.len()); - let sat_resolve = sat_resolve.unwrap_or_else(|| SatResolve::new(registry)); - if !sat_resolve.sat_is_valid_solution(&out) { + if !sat_resolver.sat_is_valid_solution(&out) { panic!( - "the sat_resolve err but the resolve thinks this will work:\n{:?}", - resolve + "`resolve()` thinks this will work, but the solution is \ + invalid according to the sat resolver:\n{resolve:?}", ); } Ok(out) @@ -85,13 +87,21 @@ pub fn resolve_and_validated( } } +fn collect_features(resolve: &Resolve) -> Vec<(PackageId, Vec)> { + resolve + .sort() + .iter() + .map(|&pkg| (pkg, resolve.features(pkg).to_vec())) + .collect() +} + pub fn resolve_with_global_context( deps: Vec, registry: &[Summary], gctx: &GlobalContext, -) -> CargoResult> { +) -> CargoResult)>> { let resolve = resolve_with_global_context_raw(deps, registry, gctx)?; - Ok(resolve.sort()) + Ok(collect_features(&resolve)) } pub fn resolve_with_global_context_raw( @@ -158,22 +168,26 @@ pub fn resolve_with_global_context_raw( list: registry, used: HashSet::new(), }; - let summary = Summary::new( + + let root_summary = Summary::new( pkg_id("root"), deps, &BTreeMap::new(), None::<&String>, - None::, + None, ) .unwrap(); + let opts = ResolveOpts::everything(); + let start = Instant::now(); let mut version_prefs = VersionPreferences::default(); if gctx.cli_unstable().minimal_versions { version_prefs.version_ordering(VersionOrdering::MinimumVersionsFirst) } + let resolve = resolver::resolve( - &[(summary, opts)], + &[(root_summary, opts)], &[], &mut registry, &version_prefs, @@ -181,8 +195,8 @@ pub fn resolve_with_global_context_raw( Some(gctx), ); - // The largest test in our suite takes less then 30 sec. - // So lets fail the test if we have ben running for two long. + // The largest test in our suite takes less then 30 secs. + // So let's fail the test if we have been running for more than 60 secs. assert!(start.elapsed().as_secs() < 60); resolve } @@ -202,7 +216,7 @@ fn log_bits(x: usize) -> 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 impl varisat::ExtendFormula, vars: &[varisat::Var]) { +fn sat_at_most_one(solver: &mut varisat::Solver<'_>, vars: &[varisat::Var]) { if vars.len() <= 1 { return; } else if vars.len() == 2 { @@ -227,51 +241,201 @@ fn sat_at_most_one(solver: &mut impl varisat::ExtendFormula, vars: &[varisat::Va } fn sat_at_most_one_by_key( - cnf: &mut impl varisat::ExtendFormula, + solver: &mut varisat::Solver<'_>, data: impl Iterator, ) -> HashMap> { - // no two packages with the same links 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) } for key in by_keys.values() { - sat_at_most_one(cnf, key); + 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 big thing that the real resolver does, -/// that this one does not do is work with features and optional dependencies. +/// For the subset of functionality that are currently made by `registry_strategy`, +/// this will find a valid resolution if one exists. /// -/// The SAT library dose not optimize for the newer version, +/// The SAT library does not optimize for the newer version, /// so the selected packages may not match the real resolver. -#[derive(Clone)] -pub struct SatResolve(Rc>); - -struct SatResolveInner { +pub struct SatResolver { solver: varisat::Solver<'static>, + old_root_vars: Vec, var_for_is_packages_used: HashMap, - by_name: HashMap<&'static str, Vec>, + var_for_is_packages_features_used: HashMap>, + by_name: HashMap>, } -impl SatResolve { +impl SatResolver { pub fn new(registry: &[Summary]) -> Self { - let mut cnf = varisat::CnfFormula::new(); + 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: HashMap = registry + 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(), cnf.new_var())) - .collect(); + .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 cnf, + &mut solver, registry .iter() .map(|s| (s.links(), var_for_is_packages_used[&s.package_id()])) @@ -280,128 +444,168 @@ impl SatResolve { // no two semver compatible versions of the same package sat_at_most_one_by_key( - &mut cnf, + &mut solver, var_for_is_packages_used .iter() .map(|(p, &v)| (p.as_activations_key(), v)), ); - let mut by_name: HashMap<&'static str, Vec> = HashMap::new(); + let mut by_name: HashMap> = HashMap::new(); - for p in registry.iter() { - by_name - .entry(p.name().as_str()) - .or_default() - .push(p.package_id()) + for p in registry { + by_name.entry(p.name()).or_default().push(p.clone()) } - let empty_vec = vec![]; - - // Now different versions can conflict, but dependencies might not be selected. - // So we need to add clauses that ensure that if a package is selected for each dependency a version - // that satisfies that dependency is selected. - // - // active packages need each of there `deps` to be satisfied - for p in registry.iter() { - for dep in p.dependencies() { - let mut matches: Vec = by_name - .get(dep.package_name().as_str()) - .unwrap_or(&empty_vec) - .iter() - .filter(|&p| dep.matches_id(*p)) - .map(|p| var_for_is_packages_used[&p].positive()) - .collect(); - // ^ the `dep` is satisfied or `p` is not active - matches.push(var_for_is_packages_used[&p.package_id()].negative()); - cnf.add_clause(&matches); - } - } + 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, + ); - let mut solver = varisat::Solver::new(); - solver.add_formula(&cnf); + 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 dont need to `solve` now. We know that "use nothing" will satisfy all the clauses so far. + // 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"); - SatResolve(Rc::new(RefCell::new(SatResolveInner { + + SatResolver { solver, + old_root_vars: Vec::new(), var_for_is_packages_used, + var_for_is_packages_features_used, by_name, - }))) - } - pub fn sat_resolve(&self, deps: &[Dependency]) -> bool { - let mut s = self.0.borrow_mut(); - let mut assumption = vec![]; - let mut this_call = None; - - // the starting `deps` need to be satisfied - for dep in deps.iter() { - let empty_vec = vec![]; - let matches: Vec = s - .by_name - .get(dep.package_name().as_str()) - .unwrap_or(&empty_vec) - .iter() - .filter(|&p| dep.matches_id(*p)) - .map(|p| s.var_for_is_packages_used[p].positive()) - .collect(); - if matches.is_empty() { - return false; - } else if matches.len() == 1 { - assumption.extend_from_slice(&matches) - } else { - if this_call.is_none() { - let new_var = s.solver.new_var(); - this_call = Some(new_var); - assumption.push(new_var.positive()); - } - let mut matches = matches; - matches.push(this_call.unwrap().negative()); - s.solver.add_clause(&matches); - } } + } + + 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::>(); - s.solver.assume(&assumption); + old_root_vars.push(root_var); - s.solver + 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(&self, pids: &[PackageId]) -> bool { - let mut s = self.0.borrow_mut(); - for p in pids { - if p.name().as_str() != "root" && !s.var_for_is_packages_used.contains_key(p) { + + 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; } } - let assumption: Vec<_> = s - .var_for_is_packages_used - .iter() - .map(|(p, v)| v.lit(pids.contains(p))) - .collect(); - s.solver.assume(&assumption); + // 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::>(); - s.solver + self.solver.assume(&assumption); + + self.solver .solve() .expect("docs say it can't error in default config") } - fn use_packages(&self) -> Option { - self.0.borrow().solver.model().map(|lits| { + + 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 out = String::new(); - out.push_str("used:\n"); - for (p, v) in self.0.borrow().var_for_is_packages_used.iter() { + + let mut used_packages = BTreeMap::>::new(); + for (&p, v) in self.var_for_is_packages_used.iter() { if lits.contains(v) { - writeln!(&mut out, " {}", p).unwrap(); + 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 }) } @@ -409,18 +613,40 @@ impl SatResolve { 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 { @@ -448,8 +674,8 @@ impl, U: AsRef> ToPkgId for (T, U) { #[macro_export] macro_rules! pkg { - ($pkgid:expr => [$($deps:expr),+ $(,)* ]) => ({ - let d: Vec = vec![$($deps.to_dep()),+]; + ($pkgid:expr => [$($deps:expr),* $(,)? ]) => ({ + let d: Vec = vec![$($deps.to_dep()),*]; $crate::pkg_dep($pkgid, d) }); @@ -473,18 +699,29 @@ pub fn pkg(name: T) -> Summary { 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().as_str()) + Some(pkgid.name()) } else { None }; - Summary::new( - name.to_pkgid(), - dep, - &BTreeMap::new(), - link, - None::, - ) - .unwrap() + 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 { @@ -510,7 +747,7 @@ pub fn pkg_loc(name: &str, loc: &str) -> Summary { Vec::new(), &BTreeMap::new(), link, - None::, + None, ) .unwrap() } @@ -519,14 +756,7 @@ 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().map(|a| a.as_str()), - None::, - ) - .unwrap() + Summary::new(sum.package_id(), deps, &BTreeMap::new(), sum.links(), None).unwrap() } pub fn dep(name: &str) -> Dependency { @@ -629,7 +859,7 @@ fn meta_test_deep_pretty_print_registry() { 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")]), + 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")), @@ -654,8 +884,8 @@ fn meta_test_deep_pretty_print_registry() { } /// This generates a random registry index. -/// Unlike vec((Name, Ver, vec((Name, VerRq), ..), ..) -/// This strategy has a high probability of having valid dependencies +/// Unlike `vec((Name, Ver, vec((Name, VerRq), ..), ..)`, +/// this strategy has a high probability of having valid dependencies. pub fn registry_strategy( max_crates: usize, max_versions: usize, diff --git a/crates/resolver-tests/tests/resolve.rs b/crates/resolver-tests/tests/resolve.rs index 2728660b2bf..5ac0a83dcbb 100644 --- a/crates/resolver-tests/tests/resolve.rs +++ b/crates/resolver-tests/tests/resolve.rs @@ -6,9 +6,10 @@ 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_id, - pkg_loc, registry, registry_strategy, remove_dep, resolve, resolve_and_validated, - resolve_with_global_context, PrettyPrintRegistry, SatResolve, ToDep, ToPkgId, + 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, }; use proptest::prelude::*; @@ -40,15 +41,15 @@ proptest! { PrettyPrintRegistry(input) in registry_strategy(50, 20, 60) ) { let reg = registry(input.clone()); - let sat_resolve = SatResolve::new(®); - // there is only a small chance that any one - // crate will be interesting. + 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()))], ®, - Some(sat_resolve.clone()), + &mut sat_resolver, ); } } @@ -75,23 +76,15 @@ proptest! { .unwrap(); let reg = registry(input.clone()); - // there is only a small chance that any one - // crate will be interesting. + + // 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) { - // minimal-versions change what order the candidates - // are tried but not the existence of a solution - let res = resolve( - vec![dep_req(&this.name(), &format!("={}", this.version()))], - ®, - ); - - let mres = resolve_with_global_context( - vec![dep_req(&this.name(), &format!("={}", this.version()))], - ®, - &gctx, - ); + 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(), @@ -124,23 +117,16 @@ proptest! { .unwrap(); let reg = registry(input.clone()); - // there is only a small chance that any one - // crate will be interesting. + + // 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) { - // direct-minimal-versions reduces the number of available solutions, so we verify that - // we do not come up with solutions maximal versions does not - let res = resolve( - vec![dep_req(&this.name(), &format!("={}", this.version()))], - ®, - ); - - let mres = resolve_with_global_context( - vec![dep_req(&this.name(), &format!("={}", this.version()))], - ®, - &gctx, - ); + 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(), @@ -179,8 +165,8 @@ proptest! { } } let removed_reg = registry(removed_input); - // there is only a small chance that any one - // crate will be interesting. + + // 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( @@ -207,8 +193,8 @@ proptest! { indexes_to_unpublish in prop::collection::vec(any::(), ..10) ) { let reg = registry(input.clone()); - // there is only a small chance that any one - // crate will be interesting. + + // 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( @@ -225,6 +211,7 @@ proptest! { .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(); @@ -460,7 +447,10 @@ fn test_resolving_minimum_version_with_transitive_deps() { ®, &gctx, ) - .unwrap(); + .unwrap() + .into_iter() + .map(|(pkg, _)| pkg) + .collect::>(); assert_contains( &res, @@ -659,8 +649,8 @@ fn resolving_with_constrained_sibling_backtrack_parent() { let vsn = format!("1.0.{}", i); reglist.push( pkg!(("bar", vsn.clone()) => [dep_req("backtrack_trap1", "1.0.2"), - dep_req("backtrack_trap2", "1.0.2"), - dep_req("constrained", "1.0.1")]), + dep_req("backtrack_trap2", "1.0.2"), + dep_req("constrained", "1.0.1")]), ); reglist.push(pkg!(("backtrack_trap1", vsn.clone()))); reglist.push(pkg!(("backtrack_trap2", vsn.clone()))); @@ -1227,7 +1217,8 @@ fn off_by_one_bug() { ]; let reg = registry(input); - let _ = resolve_and_validated(vec![dep("f")], ®, None); + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(vec![dep("f")], ®, &mut sat_resolver).is_ok()); } #[test] @@ -1267,7 +1258,8 @@ fn conflict_store_bug() { ]; let reg = registry(input); - let _ = resolve_and_validated(vec![dep("j")], ®, None); + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(vec![dep("j")], ®, &mut sat_resolver).is_err()); } #[test] @@ -1295,7 +1287,8 @@ fn conflict_store_more_then_one_match() { ]), ]; let reg = registry(input); - let _ = resolve_and_validated(vec![dep("nA")], ®, None); + let mut sat_resolver = SatResolver::new(®); + assert!(resolve_and_validated(vec![dep("nA")], ®, &mut sat_resolver).is_err()); } #[test] @@ -1304,7 +1297,7 @@ fn bad_lockfile_from_8249() { 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 deeded for now + 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"), @@ -1320,7 +1313,100 @@ fn bad_lockfile_from_8249() { ]), ]; let reg = registry(input); - let _ = resolve_and_validated(vec![dep("foo")], ®, None); + 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] diff --git a/src/cargo/core/summary.rs b/src/cargo/core/summary.rs index 424328fa6d1..1b07bcb9b4c 100644 --- a/src/cargo/core/summary.rs +++ b/src/cargo/core/summary.rs @@ -216,6 +216,7 @@ fn build_feature_map( .flatten() .filter_map(|fv| fv.explicit_dep_name()) .collect(); + for dep in dependencies { if !dep.is_optional() { continue; @@ -242,7 +243,7 @@ fn build_feature_map( if !is_any_dep { bail!( "feature `{feature}` includes `{fv}` which is neither a dependency \ - nor another feature" + nor another feature" ); } if is_optional_dep {