diff --git a/Cargo.lock b/Cargo.lock index b55e5e42f027..b646c193227b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2998,6 +2998,7 @@ dependencies = [ "cargo", "cargo-util", "cargo-util-schemas", + "indexmap", "proptest", "varisat", ] diff --git a/crates/resolver-tests/Cargo.toml b/crates/resolver-tests/Cargo.toml index 44f906900516..5a61ee5cda35 100644 --- a/crates/resolver-tests/Cargo.toml +++ b/crates/resolver-tests/Cargo.toml @@ -8,6 +8,7 @@ publish = false cargo.workspace = true cargo-util-schemas.workspace = true cargo-util.workspace = true +indexmap.workspace = true proptest.workspace = true varisat.workspace = true diff --git a/crates/resolver-tests/src/lib.rs b/crates/resolver-tests/src/lib.rs index 23903ed38733..88b0ce5929d9 100644 --- a/crates/resolver-tests/src/lib.rs +++ b/crates/resolver-tests/src/lib.rs @@ -2,7 +2,7 @@ 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; @@ -13,14 +13,16 @@ use std::time::Instant; use cargo::core::dependency::DepKind; use cargo::core::resolver::features::RequestedFeatures; use cargo::core::resolver::{self, CliFeatures, ResolveOpts, VersionOrdering, VersionPreferences}; -use cargo::core::Resolve; 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 indexmap::IndexMap; use proptest::collection::{btree_map, vec}; use proptest::prelude::*; use proptest::sample::Index; @@ -220,7 +222,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 { @@ -245,27 +247,168 @@ 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: &Summary, + by_name: &HashMap>, +) -> IndexMap> { + 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_feature_value( + solver: &mut varisat::Solver<'_>, + var_for_is_packages_used: &HashMap, + var_for_is_packages_features_used: &HashMap>, + feature_value: &FeatureValue, + bound_var: varisat::Var, + pkg_feature_var_map: &HashMap, + compatible_dep_summaries_by_name_in_toml: &IndexMap>, +) { + match *feature_value { + FeatureValue::Feature(other_feature_name) => { + solver.add_clause(&[ + bound_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([bound_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(&[ + bound_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([bound_var.negative()]) + .collect::>(); + + solver.add_clause(&dep_clause); + } + } + } +} + +fn add_pkg_clauses( + solver: &mut varisat::Solver<'_>, + var_for_is_packages_used: &HashMap, + var_for_is_packages_features_used: &HashMap>, + pkg: &Summary, + pkg_feature_var_map: &HashMap, + compatible_dep_summaries_by_name_in_toml: &IndexMap>, +) { + // add clauses for package features + for (&feature_name, feature_values) in pkg.features() { + for feature_value in feature_values { + process_feature_value( + solver, + var_for_is_packages_used, + var_for_is_packages_features_used, + feature_value, + pkg_feature_var_map[&feature_name], + pkg_feature_var_map, + compatible_dep_summaries_by_name_in_toml, + ); + } + } + + for (dep, compatible_dep_summaries) in pkg + .dependencies() + .iter() + .zip(compatible_dep_summaries_by_name_in_toml.values()) + { + // add clauses for package dep 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(&[ + var_for_is_packages_used[&pkg.package_id()].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 each of their non-optional `deps` to be satisfied + if !dep.is_optional() { + let dep_clause = compatible_dep_summaries + .iter() + .map(|d| var_for_is_packages_used[&d.package_id()].positive()) + .chain([var_for_is_packages_used[&pkg.package_id()].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 does not optimize for the newer version, /// so the selected packages may not match the real resolver. @@ -274,22 +417,47 @@ pub struct SatResolver(Rc>); struct SatResolverInner { 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 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(), cnf.new_var())) - .collect(); + .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 cnf, + &mut solver, registry .iter() .map(|s| (s.links(), var_for_is_packages_used[&s.package_id()])) @@ -298,47 +466,30 @@ impl SatResolver { // 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 { + add_pkg_clauses( + &mut solver, + &var_for_is_packages_used, + &var_for_is_packages_features_used, + pkg, + &var_for_is_packages_features_used[&pkg.package_id()], + &find_compatible_dep_summaries_by_name_in_toml(pkg, &by_name), + ); } - let mut solver = varisat::Solver::new(); - solver.add_formula(&cnf); - - // 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() @@ -346,62 +497,118 @@ impl SatResolver { SatResolver(Rc::new(RefCell::new(SatResolverInner { solver, + old_root_vars: Vec::new(), var_for_is_packages_used, + var_for_is_packages_features_used, by_name, }))) } - pub fn sat_resolve(&self, root_summary: &Summary, _root_cli_features: &CliFeatures) -> bool { - let mut s = self.0.borrow_mut(); + pub fn sat_resolve(&self, root_summary: &Summary, root_cli_features: &CliFeatures) -> bool { + let SatResolverInner { + solver, + old_root_vars, + var_for_is_packages_used, + var_for_is_packages_features_used, + by_name, + } = &mut *self.0.borrow_mut(); + let mut assumption = vec![]; - let mut this_call = None; - - // the starting `deps` need to be satisfied - for dep in root_summary.dependencies().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); - } + + let root_pkg = root_summary.package_id(); + + // root package is always used + let root_var = solver.new_var(); + assumption.push(root_var.positive()); + + // define new vars for each root feature + let var_for_root_features_used: HashMap = root_summary + .features() + .keys() + .map(|&f| (f, solver.new_var())) + .collect(); + + for (_, root_feature_var) in &var_for_root_features_used { + solver.add_clause(&[root_feature_var.negative(), root_var.positive()]); } - s.solver.assume(&assumption); + // replace previous root package + var_for_is_packages_used.insert(root_pkg, root_var); + var_for_is_packages_features_used.insert(root_pkg, var_for_root_features_used); - s.solver + let root_feature_var_map = &var_for_is_packages_features_used[&root_pkg]; + + // root vars from previous runs are deactivated + assumption.extend(old_root_vars.iter().map(|v| v.negative())); + old_root_vars.push(root_var); + + let compatible_dep_summaries_by_name_in_toml = + find_compatible_dep_summaries_by_name_in_toml(root_summary, &by_name); + + add_pkg_clauses( + solver, + var_for_is_packages_used, + var_for_is_packages_features_used, + root_summary, + root_feature_var_map, + &compatible_dep_summaries_by_name_in_toml, + ); + + // add assumptions and clauses for activated cli features + if root_cli_features.all_features { + assumption.extend(root_feature_var_map.values().map(|v| v.positive())); + } + if root_cli_features.uses_default_features + && root_summary.features().contains_key(&*INTERNED_DEFAULT) + { + assumption.push(root_feature_var_map[&*INTERNED_DEFAULT].positive()); + } + for feature_value in &*root_cli_features.features { + process_feature_value( + solver, + var_for_is_packages_used, + var_for_is_packages_features_used, + feature_value, + root_var, + root_feature_var_map, + &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(); + + let mut root_pkg = None; for p in pids { - if p.name().as_str() != "root" && !s.var_for_is_packages_used.contains_key(p) { + if p.name() == "root" { + root_pkg = Some(p); + } else if !s.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(); + let Some(root_pkg) = root_pkg else { + return false; + }; + + // remove previous root package + s.var_for_is_packages_used.remove(&root_pkg); + s.var_for_is_packages_features_used.remove(&root_pkg); + + // root vars from previous runs are deactivated + let assumption = (s.old_root_vars.iter().map(|v| v.negative())) + .chain( + s.var_for_is_packages_used + .iter() + .map(|(p, v)| v.lit(pids.contains(p))), + ) + .collect::>(); s.solver.assume(&assumption); @@ -411,22 +618,62 @@ impl SatResolver { } fn used_packages(&self) -> Option { - self.0.borrow().solver.model().map(|lits| { + let s = self.0.borrow(); + s.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 s.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 &s.var_for_is_packages_features_used { + for (&f, v) in map { + if lits.contains(v) { + used_packages.entry(p).or_default().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 }) } + + #[cfg(test)] + fn reset_solver_lits(&self) { + let mut s = self.0.borrow_mut(); + + let all_negative = s + .old_root_vars + .iter() + .chain(s.var_for_is_packages_used.values()) + .chain( + s.var_for_is_packages_features_used + .values() + .flat_map(|x| x.values()), + ) + .map(|v| v.negative()) + .collect::>(); + + s.solver.assume(&all_negative); + + s.solver + .solve() + .expect("docs say it can't error in default config"); + } } pub trait ToDep { @@ -932,3 +1179,121 @@ pub fn assert_same(a: &[A], b: &[A]) { assert_eq!(a.len(), b.len(), "not equal\n{a:?}\n{b:?}"); assert_contains(b, a); } + +#[cfg(test)] +mod sat_test { + use super::*; + + #[test] + fn test_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"])]), + ]); + + let root_summary = pkg_dep_with( + "root", + vec![ + "image".to_dep_with(&["b"]), + "log".to_opt_dep(), + "rgb".to_opt_dep(), + ], + &[ + ("default", &["log", "image/default"]), + ("man", &["rgb?/man"]), + ], + ); + + let checks = [ + ( + &CliFeatures::new_all(true), + r#"used: + a v1.0.0 (registry `https://example.com/`) + b v1.0.0 (registry `https://example.com/`) + image v1.0.0 (registry `https://example.com/`) + + a + + b + + default + jpg v1.0.0 (registry `https://example.com/`) + log v1.0.0 (registry `https://example.com/`) + man v1.0.0 (registry `https://example.com/`) + rgb v1.0.0 (registry `https://example.com/`) + + man + root v1.0.0 (registry `https://example.com/`) + + default + + log + + man + + rgb +"#, + ), + ( + &CliFeatures::from_command_line(&[], false, true).unwrap(), + r#"used: + a v1.0.0 (registry `https://example.com/`) + b v1.0.0 (registry `https://example.com/`) + image v1.0.0 (registry `https://example.com/`) + + a + + b + + default + jpg v1.0.0 (registry `https://example.com/`) + log v1.0.0 (registry `https://example.com/`) + root v1.0.0 (registry `https://example.com/`) + + default + + log +"#, + ), + ( + &CliFeatures::from_command_line(&[], false, false).unwrap(), + r#"used: + b v1.0.0 (registry `https://example.com/`) + image v1.0.0 (registry `https://example.com/`) + + b + jpg v1.0.0 (registry `https://example.com/`) + root v1.0.0 (registry `https://example.com/`) +"#, + ), + ( + &CliFeatures::from_command_line(&["man".into()], false, false).unwrap(), + r#"used: + b v1.0.0 (registry `https://example.com/`) + image v1.0.0 (registry `https://example.com/`) + + b + jpg v1.0.0 (registry `https://example.com/`) + root v1.0.0 (registry `https://example.com/`) + + man +"#, + ), + ( + &CliFeatures::from_command_line(&["man,rgb".into()], false, false).unwrap(), + r#"used: + b v1.0.0 (registry `https://example.com/`) + image v1.0.0 (registry `https://example.com/`) + + b + jpg v1.0.0 (registry `https://example.com/`) + man v1.0.0 (registry `https://example.com/`) + rgb v1.0.0 (registry `https://example.com/`) + + man + root v1.0.0 (registry `https://example.com/`) + + man + + rgb +"#, + ), + ]; + + let sat_resolver = SatResolver::new(®); + for (cli_features, output) in checks { + assert!(sat_resolver.sat_resolve(&root_summary, cli_features)); + assert_eq!(sat_resolver.used_packages().unwrap(), output); + sat_resolver.reset_solver_lits(); + } + } +}