From 3b4f3b0bf0f2737dd48dcb19b9fa74d4fd500d9a Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 29 Nov 2023 12:17:26 -0600 Subject: [PATCH] test(resolver-tests): Remove public dep support from SAT resolver --- crates/resolver-tests/src/lib.rs | 131 ++----------------------------- 1 file changed, 8 insertions(+), 123 deletions(-) diff --git a/crates/resolver-tests/src/lib.rs b/crates/resolver-tests/src/lib.rs index 2e653add749f..56757705680a 100644 --- a/crates/resolver-tests/src/lib.rs +++ b/crates/resolver-tests/src/lib.rs @@ -18,7 +18,7 @@ use cargo::core::{Dependency, PackageId, Registry, Summary}; use cargo::core::{GitReference, SourceId}; use cargo::sources::source::QueryKind; use cargo::sources::IndexSummary; -use cargo::util::{CargoResult, Config, Graph, IntoUrl, RustVersion}; +use cargo::util::{CargoResult, Config, IntoUrl, RustVersion}; use proptest::collection::{btree_map, vec}; use proptest::prelude::*; @@ -195,17 +195,6 @@ fn log_bits(x: usize) -> usize { } fn sat_at_most_one(solver: &mut impl varisat::ExtendFormula, 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; - } // 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(); @@ -254,7 +243,8 @@ impl SatResolve { let mut cnf = varisat::CnfFormula::new(); let var_for_is_packages_used: HashMap = registry .iter() - .map(|s| (s.package_id(), cnf.new_var())) + .map(|s| s.package_id()) + .zip(cnf.new_var_iter(registry.len())) .collect(); // no two packages with the same links set @@ -266,14 +256,6 @@ impl SatResolve { .filter(|(l, _)| l.is_some()), ); - // no two semver compatible versions of the same package - let by_activations_keys = sat_at_most_one_by_key( - &mut cnf, - var_for_is_packages_used - .iter() - .map(|(p, &v)| (p.as_activations_key(), v)), - ); - let mut by_name: HashMap<&'static str, Vec> = HashMap::new(); for p in registry.iter() { @@ -285,119 +267,22 @@ impl SatResolve { let empty_vec = vec![]; - let mut graph: Graph = Graph::new(); - - let mut version_selected_for: HashMap< - PackageId, - HashMap>, - > = HashMap::new(); // active packages need each of there `deps` to be satisfied for p in registry.iter() { - graph.add(p.package_id()); for dep in p.dependencies() { - // This can more easily be written as: - // !is_active(p) or one of the things that match dep is_active - // All the complexity, from here to the end, is to support public and private dependencies! - let mut by_key: HashMap<_, Vec> = HashMap::new(); - for &m in by_name + let mut matches: Vec = by_name .get(dep.package_name().as_str()) .unwrap_or(&empty_vec) .iter() .filter(|&p| dep.matches_id(*p)) - { - graph.link(p.package_id(), m); - by_key - .entry(m.as_activations_key()) - .or_default() - .push(var_for_is_packages_used[&m].positive()); - } - let keys: HashMap<_, _> = by_key.keys().map(|&k| (k, cnf.new_var())).collect(); - - // if `p` is active then we need to select one of the keys - let matches: Vec<_> = keys - .values() - .map(|v| v.positive()) - .chain(Some(var_for_is_packages_used[&p.package_id()].negative())) + .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); - - // if a key is active then we need to select one of the versions - for (key, vars) in by_key.iter() { - let mut matches = vars.clone(); - matches.push(keys[key].negative()); - cnf.add_clause(&matches); - } - - version_selected_for - .entry(p.package_id()) - .or_default() - .insert(dep.clone(), keys); - } - } - - let topological_order = graph.sort(); - - // we already ensure there is only one version for each `activations_key` so we can think of - // `publicly_exports` as being in terms of a set of `activations_key`s - let mut publicly_exports: HashMap<_, HashMap<_, varisat::Var>> = HashMap::new(); - - for &key in by_activations_keys.keys() { - // everything publicly depends on itself - let var = publicly_exports - .entry(key) - .or_default() - .entry(key) - .or_insert_with(|| cnf.new_var()); - cnf.add_clause(&[var.positive()]); - } - - // if a `dep` is public then `p` `publicly_exports` all the things that the selected version `publicly_exports` - for &p in topological_order.iter() { - if let Some(deps) = version_selected_for.get(&p) { - let mut p_exports = publicly_exports.remove(&p.as_activations_key()).unwrap(); - for (_, versions) in deps.iter().filter(|(d, _)| d.is_public()) { - for (ver, sel) in versions { - for (&export_pid, &export_var) in publicly_exports[ver].iter() { - let our_var = - p_exports.entry(export_pid).or_insert_with(|| cnf.new_var()); - cnf.add_clause(&[ - sel.negative(), - export_var.negative(), - our_var.positive(), - ]); - } - } - } - publicly_exports.insert(p.as_activations_key(), p_exports); } } - // we already ensure there is only one version for each `activations_key` so we can think of - // `can_see` as being in terms of a set of `activations_key`s - // and if `p` `publicly_exports` `export` then it `can_see` `export` - let mut can_see: HashMap<_, HashMap<_, varisat::Var>> = HashMap::new(); - - // if `p` has a `dep` that selected `ver` then it `can_see` all the things that the selected version `publicly_exports` - for (&p, deps) in version_selected_for.iter() { - let p_can_see = can_see.entry(p).or_default(); - for (_, versions) in deps.iter() { - for (&ver, sel) in versions { - for (&export_pid, &export_var) in publicly_exports[&ver].iter() { - let our_var = p_can_see.entry(export_pid).or_insert_with(|| cnf.new_var()); - cnf.add_clause(&[ - sel.negative(), - export_var.negative(), - our_var.positive(), - ]); - } - } - } - } - - // a package `can_see` only one version by each name - for (_, see) in can_see.iter() { - sat_at_most_one_by_key(&mut cnf, see.iter().map(|((name, _, _), &v)| (name, v))); - } let mut solver = varisat::Solver::new(); solver.add_formula(&cnf); @@ -840,7 +725,7 @@ pub fn registry_strategy( // => DepKind::Development, // Development has no impact so don't gen _ => panic!("bad index for DepKind"), }, - p && k == 0, + p, )) }