From eea1a7c10532848051713764c76e2f6b9060ff60 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 22 Nov 2023 15:20:10 -0600 Subject: [PATCH] fix(resolver): Remove unused public-deps error handling To implement rust-lang/rfcs#3516, we need to decouple the resolver's behavior from the unstable flag. Since the code path is now dead, I went ahead and removed it. --- crates/resolver-tests/src/lib.rs | 71 -------- crates/resolver-tests/tests/resolve.rs | 225 +------------------------ src/cargo/core/resolver/context.rs | 205 +--------------------- src/cargo/core/resolver/mod.rs | 58 +------ src/cargo/ops/resolve.rs | 4 - 5 files changed, 10 insertions(+), 553 deletions(-) diff --git a/crates/resolver-tests/src/lib.rs b/crates/resolver-tests/src/lib.rs index 9297844998c3..7b7a628315c1 100644 --- a/crates/resolver-tests/src/lib.rs +++ b/crates/resolver-tests/src/lib.rs @@ -201,7 +201,6 @@ pub fn resolve_with_config_raw( &mut registry, &version_prefs, Some(config), - true, ); // The largest test in our suite takes less then 30 sec. @@ -295,13 +294,6 @@ impl SatResolve { ); // 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() { @@ -363,69 +355,6 @@ impl SatResolve { } } - 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); diff --git a/crates/resolver-tests/tests/resolve.rs b/crates/resolver-tests/tests/resolve.rs index dd21502d8fd1..662bad90f47a 100644 --- a/crates/resolver-tests/tests/resolve.rs +++ b/crates/resolver-tests/tests/resolve.rs @@ -6,8 +6,8 @@ use cargo::util::Config; use cargo_util::is_ci; use resolver_tests::{ - assert_contains, assert_same, dep, dep_kind, dep_loc, dep_req, dep_req_kind, loc_names, names, - pkg, pkg_id, pkg_loc, registry, registry_strategy, remove_dep, resolve, resolve_and_validated, + 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_config, PrettyPrintRegistry, SatResolve, ToDep, ToPkgId, }; @@ -287,192 +287,6 @@ proptest! { } } -#[test] -#[should_panic(expected = "pub dep")] // The error handling is not yet implemented. -fn pub_fail() { - let input = vec![ - pkg!(("a", "0.0.4")), - pkg!(("a", "0.0.5")), - pkg!(("e", "0.0.6") => [dep_req_kind("a", "<= 0.0.4", DepKind::Normal, true),]), - pkg!(("kB", "0.0.3") => [dep_req("a", ">= 0.0.5"),dep("e"),]), - ]; - let reg = registry(input); - assert!(resolve_and_validated(vec![dep("kB")], ®, None).is_err()); -} - -#[test] -fn basic_public_dependency() { - let reg = registry(vec![ - pkg!(("A", "0.1.0")), - pkg!(("A", "0.2.0")), - pkg!("B" => [dep_req_kind("A", "0.1", DepKind::Normal, true)]), - pkg!("C" => [dep("A"), dep("B")]), - ]); - - let res = resolve_and_validated(vec![dep("C")], ®, None).unwrap(); - assert_same( - &res, - &names(&[ - ("root", "1.0.0"), - ("C", "1.0.0"), - ("B", "1.0.0"), - ("A", "0.1.0"), - ]), - ); -} - -#[test] -fn public_dependency_filling_in() { - // The resolver has an optimization where if a candidate to resolve a dependency - // has already bean activated then we skip looking at the candidates dependencies. - // However, we have to be careful as the new path may make pub dependencies invalid. - - // Triggering this case requires dependencies to be resolved in a specific order. - // Fuzzing found this unintuitive case, that triggers this unfortunate order of operations: - // 1. `d`'s dep on `c` is resolved - // 2. `d`'s dep on `a` is resolved with `0.1.1` - // 3. `c`'s dep on `b` is resolved with `0.0.2` - // 4. `b`'s dep on `a` is resolved with `0.0.6` no pub dev conflict as `b` is private to `c` - // 5. `d`'s dep on `b` is resolved with `0.0.2` triggering the optimization. - // Do we notice that `d` has a pub dep conflict on `a`? Lets try it and see. - let reg = registry(vec![ - pkg!(("a", "0.0.6")), - pkg!(("a", "0.1.1")), - pkg!(("b", "0.0.0") => [dep("bad")]), - pkg!(("b", "0.0.1") => [dep("bad")]), - pkg!(("b", "0.0.2") => [dep_req_kind("a", "=0.0.6", DepKind::Normal, true)]), - pkg!("c" => [dep_req("b", ">=0.0.1")]), - pkg!("d" => [dep("c"), dep("a"), dep("b")]), - ]); - - let res = resolve_and_validated(vec![dep("d")], ®, None).unwrap(); - assert_same( - &res, - &names(&[ - ("root", "1.0.0"), - ("d", "1.0.0"), - ("c", "1.0.0"), - ("b", "0.0.2"), - ("a", "0.0.6"), - ]), - ); -} - -#[test] -fn public_dependency_filling_in_and_update() { - // The resolver has an optimization where if a candidate to resolve a dependency - // has already bean activated then we skip looking at the candidates dependencies. - // However, we have to be careful as the new path may make pub dependencies invalid. - - // Triggering this case requires dependencies to be resolved in a specific order. - // Fuzzing found this unintuitive case, that triggers this unfortunate order of operations: - // 1. `D`'s dep on `B` is resolved - // 2. `D`'s dep on `C` is resolved - // 3. `B`'s dep on `A` is resolved with `0.0.0` - // 4. `C`'s dep on `B` triggering the optimization. - // So did we add `A 0.0.0` to the deps `C` can see? - // Or are we going to resolve `C`'s dep on `A` with `0.0.2`? - // Lets try it and see. - let reg = registry(vec![ - pkg!(("A", "0.0.0")), - pkg!(("A", "0.0.2")), - pkg!("B" => [dep_req_kind("A", "=0.0.0", DepKind::Normal, true),]), - pkg!("C" => [dep("A"),dep("B")]), - pkg!("D" => [dep("B"),dep("C")]), - ]); - let res = resolve_and_validated(vec![dep("D")], ®, None).unwrap(); - assert_same( - &res, - &names(&[ - ("root", "1.0.0"), - ("D", "1.0.0"), - ("C", "1.0.0"), - ("B", "1.0.0"), - ("A", "0.0.0"), - ]), - ); -} - -#[test] -fn public_dependency_skipping() { - // When backtracking due to a failed dependency, if Cargo is - // trying to be clever and skip irrelevant dependencies, care must - // the effects of pub dep must be accounted for. - let input = vec![ - pkg!(("a", "0.2.0")), - pkg!(("a", "2.0.0")), - pkg!(("b", "0.0.0") => [dep("bad")]), - pkg!(("b", "0.2.1") => [dep_req_kind("a", "0.2.0", DepKind::Normal, true)]), - pkg!("c" => [dep("a"),dep("b")]), - ]; - let reg = registry(input); - - resolve_and_validated(vec![dep("c")], ®, None).unwrap(); -} - -#[test] -fn public_dependency_skipping_in_backtracking() { - // When backtracking due to a failed dependency, if Cargo is - // trying to be clever and skip irrelevant dependencies, care must - // the effects of pub dep must be accounted for. - let input = vec![ - pkg!(("A", "0.0.0") => [dep("bad")]), - pkg!(("A", "0.0.1") => [dep("bad")]), - pkg!(("A", "0.0.2") => [dep("bad")]), - pkg!(("A", "0.0.3") => [dep("bad")]), - pkg!(("A", "0.0.4")), - pkg!(("A", "0.0.5")), - pkg!("B" => [dep_req_kind("A", ">= 0.0.3", DepKind::Normal, true)]), - pkg!("C" => [dep_req("A", "<= 0.0.4"), dep("B")]), - ]; - let reg = registry(input); - - resolve_and_validated(vec![dep("C")], ®, None).unwrap(); -} - -#[test] -fn public_sat_topological_order() { - let input = vec![ - pkg!(("a", "0.0.1")), - pkg!(("a", "0.0.0")), - pkg!(("b", "0.0.1") => [dep_req_kind("a", "= 0.0.1", DepKind::Normal, true),]), - pkg!(("b", "0.0.0") => [dep("bad"),]), - pkg!("A" => [dep_req("a", "= 0.0.0"),dep_req_kind("b", "*", DepKind::Normal, true)]), - ]; - - let reg = registry(input); - assert!(resolve_and_validated(vec![dep("A")], ®, None).is_err()); -} - -#[test] -fn public_sat_unused_makes_things_pub() { - let input = vec![ - pkg!(("a", "0.0.1")), - pkg!(("a", "0.0.0")), - pkg!(("b", "8.0.1") => [dep_req_kind("a", "= 0.0.1", DepKind::Normal, true),]), - pkg!(("b", "8.0.0") => [dep_req("a", "= 0.0.1"),]), - pkg!("c" => [dep_req("b", "= 8.0.0"),dep_req("a", "= 0.0.0"),]), - ]; - let reg = registry(input); - - resolve_and_validated(vec![dep("c")], ®, None).unwrap(); -} - -#[test] -fn public_sat_unused_makes_things_pub_2() { - let input = vec![ - pkg!(("c", "0.0.2")), - pkg!(("c", "0.0.1")), - pkg!(("a-sys", "0.0.2")), - pkg!(("a-sys", "0.0.1") => [dep_req_kind("c", "= 0.0.1", DepKind::Normal, true),]), - pkg!("P" => [dep_req_kind("a-sys", "*", DepKind::Normal, true),dep_req("c", "= 0.0.1"),]), - pkg!("A" => [dep("P"),dep_req("c", "= 0.0.2"),]), - ]; - let reg = registry(input); - - resolve_and_validated(vec![dep("A")], ®, None).unwrap(); -} - #[test] #[should_panic(expected = "assertion failed: !name.is_empty()")] fn test_dependency_with_empty_name() { @@ -1115,41 +929,6 @@ fn resolving_with_constrained_sibling_backtrack_activation() { ); } -#[test] -fn resolving_with_public_constrained_sibling() { - // It makes sense to resolve most-constrained deps first, but - // with that logic the backtrack traps here come between the two - // attempted resolutions of 'constrained'. When backtracking, - // cargo should skip past them and resume resolution once the - // number of activations for 'constrained' changes. - let mut reglist = vec![ - pkg!(("foo", "1.0.0") => [dep_req("bar", "=1.0.0"), - dep_req("backtrack_trap1", "1.0"), - dep_req("backtrack_trap2", "1.0"), - dep_req("constrained", "<=60")]), - pkg!(("bar", "1.0.0") => [dep_req_kind("constrained", ">=60", DepKind::Normal, true)]), - ]; - // Bump these to make the test harder, but you'll also need to - // change the version constraints on `constrained` above. To correctly - // exercise Cargo, the relationship between the values is: - // NUM_CONSTRAINED - vsn < NUM_TRAPS < vsn - // to make sure the traps are resolved between `constrained`. - const NUM_TRAPS: usize = 45; // min 1 - const NUM_CONSTRAINED: usize = 100; // min 1 - for i in 0..NUM_TRAPS { - let vsn = format!("1.0.{}", i); - reglist.push(pkg!(("backtrack_trap1", vsn.clone()))); - reglist.push(pkg!(("backtrack_trap2", vsn.clone()))); - } - for i in 0..NUM_CONSTRAINED { - let vsn = format!("{}.0.0", i); - reglist.push(pkg!(("constrained", vsn.clone()))); - } - let reg = registry(reglist); - - let _ = resolve_and_validated(vec![dep_req("foo", "1")], ®, None); -} - #[test] fn resolving_with_constrained_sibling_transitive_dep_effects() { // When backtracking due to a failed dependency, if Cargo is diff --git a/src/cargo/core/resolver/context.rs b/src/cargo/core/resolver/context.rs index 09b16b39cd99..cfeea209ae13 100644 --- a/src/cargo/core/resolver/context.rs +++ b/src/cargo/core/resolver/context.rs @@ -22,9 +22,6 @@ pub struct Context { pub resolve_features: im_rc::HashMap, /// get the package that will be linking to a native library by its links attribute pub links: im_rc::HashMap, - /// for each package the list of names it can see, - /// then for each name the exact version that name represents and whether the name is public. - pub public_dependency: Option, /// a way to look up for a package in activations what packages required it /// and all of the exact deps that it fulfilled. @@ -74,16 +71,11 @@ impl PackageId { } impl Context { - pub fn new(check_public_visible_dependencies: bool) -> Context { + pub fn new() -> Context { Context { age: 0, resolve_features: im_rc::HashMap::new(), links: im_rc::HashMap::new(), - public_dependency: if check_public_visible_dependencies { - Some(PublicDependency::new()) - } else { - None - }, parents: Graph::new(), activations: im_rc::HashMap::new(), } @@ -192,42 +184,6 @@ impl Context { .and_then(|(s, l)| if s.package_id() == id { Some(*l) } else { None }) } - /// If the conflict reason on the package still applies returns the `ContextAge` when it was added - pub fn still_applies(&self, id: PackageId, reason: &ConflictReason) -> Option { - self.is_active(id).and_then(|mut max| { - match reason { - ConflictReason::PublicDependency(name) => { - if &id == name { - return Some(max); - } - max = std::cmp::max(max, self.is_active(*name)?); - max = std::cmp::max( - max, - self.public_dependency - .as_ref() - .unwrap() - .can_see_item(*name, id)?, - ); - } - ConflictReason::PubliclyExports(name) => { - if &id == name { - return Some(max); - } - max = std::cmp::max(max, self.is_active(*name)?); - max = std::cmp::max( - max, - self.public_dependency - .as_ref() - .unwrap() - .publicly_exports_item(*name, id)?, - ); - } - _ => {} - } - Some(max) - }) - } - /// Checks whether all of `parent` and the keys of `conflicting activations` /// are still active. /// If so returns the `ContextAge` when the newest one was added. @@ -241,8 +197,8 @@ impl Context { max = std::cmp::max(max, self.is_active(parent)?); } - for (id, reason) in conflicting_activations.iter() { - max = std::cmp::max(max, self.still_applies(*id, reason)?); + for id in conflicting_activations.keys() { + max = std::cmp::max(max, self.is_active(*id)?); } Some(max) } @@ -280,158 +236,3 @@ impl Graph> { .map(|(grand, d)| (*grand, d.iter().any(|x| x.is_public()))) } } - -#[derive(Clone, Debug, Default)] -pub struct PublicDependency { - /// For each active package the set of all the names it can see, - /// for each name the exact package that name resolves to, - /// the `ContextAge` when it was first visible, - /// and the `ContextAge` when it was first exported. - inner: im_rc::HashMap< - PackageId, - im_rc::HashMap)>, - >, -} - -impl PublicDependency { - fn new() -> Self { - PublicDependency { - inner: im_rc::HashMap::new(), - } - } - fn publicly_exports(&self, candidate_pid: PackageId) -> Vec { - self.inner - .get(&candidate_pid) // if we have seen it before - .iter() - .flat_map(|x| x.values()) // all the things we have stored - .filter(|x| x.2.is_some()) // as publicly exported - .map(|x| x.0) - .chain(Some(candidate_pid)) // but even if not we know that everything exports itself - .collect() - } - fn publicly_exports_item( - &self, - candidate_pid: PackageId, - target: PackageId, - ) -> Option { - debug_assert_ne!(candidate_pid, target); - let out = self - .inner - .get(&candidate_pid) - .and_then(|names| names.get(&target.name())) - .filter(|(p, _, _)| *p == target) - .and_then(|(_, _, age)| *age); - debug_assert_eq!( - out.is_some(), - self.publicly_exports(candidate_pid).contains(&target) - ); - out - } - pub fn can_see_item(&self, candidate_pid: PackageId, target: PackageId) -> Option { - self.inner - .get(&candidate_pid) - .and_then(|names| names.get(&target.name())) - .filter(|(p, _, _)| *p == target) - .map(|(_, age, _)| *age) - } - pub fn add_edge( - &mut self, - candidate_pid: PackageId, - parent_pid: PackageId, - is_public: bool, - age: ContextAge, - parents: &Graph>, - ) { - // one tricky part is that `candidate_pid` may already be active and - // have public dependencies of its own. So we not only need to mark - // `candidate_pid` as visible to its parents but also all of its existing - // publicly exported dependencies. - for c in self.publicly_exports(candidate_pid) { - // for each (transitive) parent that can newly see `t` - let mut stack = vec![(parent_pid, is_public)]; - while let Some((p, public)) = stack.pop() { - match self.inner.entry(p).or_default().entry(c.name()) { - im_rc::hashmap::Entry::Occupied(mut o) => { - // the (transitive) parent can already see something by `c`s name, it had better be `c`. - assert_eq!(o.get().0, c); - if o.get().2.is_some() { - // The previous time the parent saw `c`, it was a public dependency. - // So all of its parents already know about `c` - // and we can save some time by stopping now. - continue; - } - if public { - // Mark that `c` has now bean seen publicly - let old_age = o.get().1; - o.insert((c, old_age, if public { Some(age) } else { None })); - } - } - im_rc::hashmap::Entry::Vacant(v) => { - // The (transitive) parent does not have anything by `c`s name, - // so we add `c`. - v.insert((c, age, if public { Some(age) } else { None })); - } - } - // if `candidate_pid` was a private dependency of `p` then `p` parents can't see `c` thru `p` - if public { - // if it was public, then we add all of `p`s parents to be checked - stack.extend(parents.parents_of(p)); - } - } - } - } - pub fn can_add_edge( - &self, - b_id: PackageId, - parent: PackageId, - is_public: bool, - parents: &Graph>, - ) -> Result< - (), - ( - ((PackageId, ConflictReason), (PackageId, ConflictReason)), - Option<(PackageId, ConflictReason)>, - ), - > { - // one tricky part is that `candidate_pid` may already be active and - // have public dependencies of its own. So we not only need to check - // `b_id` as visible to its parents but also all of its existing - // publicly exported dependencies. - for t in self.publicly_exports(b_id) { - // for each (transitive) parent that can newly see `t` - let mut stack = vec![(parent, is_public)]; - while let Some((p, public)) = stack.pop() { - // TODO: don't look at the same thing more than once - if let Some(o) = self.inner.get(&p).and_then(|x| x.get(&t.name())) { - if o.0 != t { - // the (transitive) parent can already see a different version by `t`s name. - // So, adding `b` will cause `p` to have a public dependency conflict on `t`. - return Err(( - (o.0, ConflictReason::PublicDependency(p)), // p can see the other version and - (parent, ConflictReason::PublicDependency(p)), // p can see us - )) - .map_err(|e| { - if t == b_id { - (e, None) - } else { - (e, Some((t, ConflictReason::PubliclyExports(b_id)))) - } - }); - } - if o.2.is_some() { - // The previous time the parent saw `t`, it was a public dependency. - // So all of its parents already know about `t` - // and we can save some time by stopping now. - continue; - } - } - // if `b` was a private dependency of `p` then `p` parents can't see `t` thru `p` - if public { - // if it was public, then we add all of `p`s parents to be checked - stack.extend(parents.parents_of(p)); - } - } - } - Ok(()) - } -} diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index ecb6f36e6124..4b12f2cf32cc 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -120,24 +120,12 @@ mod version_prefs; /// /// * `config` - a location to print warnings and such, or `None` if no warnings /// should be printed -/// -/// * `check_public_visible_dependencies` - a flag for whether to enforce the restrictions -/// introduced in the "public & private dependencies" RFC (1977). The current implementation -/// makes sure that there is only one version of each name visible to each package. -/// -/// But there are 2 stable ways to directly depend on different versions of the same name. -/// 1. Use the renamed dependencies functionality -/// 2. Use 'cfg({})' dependencies functionality -/// -/// When we have a decision for how to implement is without breaking existing functionality -/// this flag can be removed. pub fn resolve( summaries: &[(Summary, ResolveOpts)], replacements: &[(PackageIdSpec, Dependency)], registry: &mut dyn Registry, version_prefs: &VersionPreferences, config: Option<&Config>, - check_public_visible_dependencies: bool, ) -> CargoResult { let _p = profile::start("resolving"); let first_version = match config { @@ -148,7 +136,7 @@ pub fn resolve( }; let mut registry = RegistryQueryer::new(registry, replacements, version_prefs); let cx = loop { - let cx = Context::new(check_public_visible_dependencies); + let cx = Context::new(); let cx = activate_deps_loop(cx, &mut registry, summaries, first_version, config)?; if registry.reset_pending() { break cx; @@ -286,12 +274,7 @@ fn activate_deps_loop( let mut backtracked = false; loop { - let next = remaining_candidates.next( - &mut conflicting_activations, - &cx, - &dep, - parent.package_id(), - ); + let next = remaining_candidates.next(&mut conflicting_activations, &cx); let (candidate, has_another) = next.ok_or(()).or_else(|_| { // If we get here then our `remaining_candidates` was just @@ -649,15 +632,6 @@ fn activate( .link(candidate_pid, parent_pid) // and associate dep with that edge .insert(dep.clone()); - if let Some(public_dependency) = cx.public_dependency.as_mut() { - public_dependency.add_edge( - candidate_pid, - parent_pid, - dep.is_public(), - cx.age, - &cx.parents, - ); - } } let activated = cx.flag_activated(&candidate, opts, parent)?; @@ -772,8 +746,6 @@ impl RemainingCandidates { &mut self, conflicting_prev_active: &mut ConflictMap, cx: &Context, - dep: &Dependency, - parent: PackageId, ) -> Option<(Summary, bool)> { for b in self.remaining.by_ref() { let b_id = b.package_id(); @@ -808,23 +780,6 @@ impl RemainingCandidates { continue; } } - // We may still have to reject do to a public dependency conflict. If one of any of our - // ancestors that can see us already knows about a different crate with this name then - // we have to reject this candidate. Additionally this candidate may already have been - // activated and have public dependants of its own, - // all of witch also need to be checked the same way. - if let Some(public_dependency) = cx.public_dependency.as_ref() { - if let Err(((c1, c2), c3)) = - public_dependency.can_add_edge(b_id, parent, dep.is_public(), &cx.parents) - { - conflicting_prev_active.insert(c1.0, c1.1); - conflicting_prev_active.insert(c2.0, c2.1); - if let Some(c3) = c3 { - conflicting_prev_active.insert(c3.0, c3.1); - } - continue; - } - } // Well if we made it this far then we've got a valid dependency. We // want this iterator to be inherently "peekable" so we don't @@ -1001,12 +956,9 @@ fn find_candidate( }; while let Some(mut frame) = backtrack_stack.pop() { - let next = frame.remaining_candidates.next( - &mut frame.conflicting_activations, - &frame.context, - &frame.dep, - frame.parent.package_id(), - ); + let next = frame + .remaining_candidates + .next(&mut frame.conflicting_activations, &frame.context); let Some((candidate, has_another)) = next else { continue; }; diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs index 00d3b1144500..c3ae6b2def84 100644 --- a/src/cargo/ops/resolve.rs +++ b/src/cargo/ops/resolve.rs @@ -64,7 +64,6 @@ use crate::core::resolver::{ self, HasDevUnits, Resolve, ResolveOpts, ResolveVersion, VersionOrdering, VersionPreferences, }; use crate::core::summary::Summary; -use crate::core::Feature; use crate::core::{GitReference, PackageId, PackageIdSpec, PackageSet, SourceId, Workspace}; use crate::ops; use crate::sources::PathSource; @@ -512,9 +511,6 @@ pub fn resolve_with_previous<'cfg>( registry, &version_prefs, Some(ws.config()), - ws.unstable_features() - .require(Feature::public_dependency()) - .is_ok(), )?; let patches: Vec<_> = registry .patches()