test(resolver-tests): Remove public dep support from SAT resolver

Author: epage

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<varisat::Var> = 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<PackageId, varisat::Var> = 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<PackageId>> = HashMap::new();
 
         for p in registry.iter() {
@@ -285,119 +267,22 @@ impl SatResolve {
 
         let empty_vec = vec![];
 
-        let mut graph: Graph<PackageId, ()> = Graph::new();
-
-        let mut version_selected_for: HashMap<
-            PackageId,
-            HashMap<Dependency, HashMap<_, varisat::Var>>,
-        > = 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<varisat::Lit>> = HashMap::new();
-                for &m in by_name
+                let mut matches: Vec<varisat::Lit> = 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,
                     ))
                 }