diff --git a/src/cargo/core/resolver/context.rs b/src/cargo/core/resolver/context.rs index 8ff76017fe0..9eaf75c7867 100644 --- a/src/cargo/core/resolver/context.rs +++ b/src/cargo/core/resolver/context.rs @@ -9,6 +9,7 @@ use log::debug; use crate::core::interning::InternedString; use crate::core::{Dependency, FeatureValue, PackageId, SourceId, Summary}; +use crate::util::config::Config; use crate::util::CargoResult; use crate::util::Graph; @@ -217,7 +218,7 @@ impl Context { parent: Option<&Summary>, s: &'b Summary, method: &'b Method<'_>, - config: Option<&crate::util::config::Config>, + config: Option<&Config>, ) -> ActivateResult)>> { let dev_deps = match *method { Method::Everything => true, @@ -329,14 +330,16 @@ impl Context { } pub fn graph(&self) -> Graph> { - let mut graph = Graph::new(); + let mut graph: Graph> = Graph::new(); self.activations .values() .for_each(|(r, _)| graph.add(r.package_id())); for i in self.parents.iter() { graph.add(*i); for (o, e) in self.parents.edges(i) { - *graph.link(*o, *i) = e.to_vec(); + let old_link = graph.link(*o, *i); + debug_assert!(old_link.is_empty()); + *old_link = e.to_vec(); } } graph diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index c58b30b4da4..91fff9f9c2b 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -1088,7 +1088,11 @@ fn check_duplicate_pkgs_in_lockfile(resolve: &Resolve) -> CargoResult<()> { Ok(()) } -/// re-run all used resolve_features so it can print warnings +/// Re-run all used `resolve_features` so it can print warnings. +/// Within the resolution loop we do not pass a `config` to `resolve_features` +/// this tells it not to print warnings. Hear we do pass `config` triggering it to print them. +/// This is done as the resolution is NP-Hard so the loop can potentially call `resolve features` +/// an exponential number of times, but this pass is linear in the number of dependencies. fn emit_warnings( cx: &Context, resolve: &Resolve, @@ -1098,22 +1102,26 @@ fn emit_warnings( let mut new_cx = cx.clone(); new_cx.resolve_features = im_rc::HashMap::new(); let mut features_from_dep = HashMap::new(); - for (summery, method) in summaries { + for (summary, method) in summaries { for (dep, features) in new_cx - .resolve_features(None, summery, &method, config) - .expect("can not resolve_features for a required summery") + .resolve_features(None, summary, &method, config) + .expect("can not resolve_features for a required summary") { - features_from_dep.insert((summery.package_id(), dep), features); + features_from_dep.insert((summary.package_id(), dep), features); } } - for summery in resolve.sort().iter().rev().map(|id| { + // crates enable features for their dependencies. + // So by iterating reverse topologically we process all of the parents before each child. + // Then we know all the needed features for each crate. + let topological_sort = resolve.sort(); + for summary in topological_sort.iter().rev().map(|id| { cx.activations .get(&id.as_activations_key()) .expect("id in dependency graph but not in activations") .0 .clone() }) { - for (parent, deps) in cx.parents.edges(&summery.package_id()) { + for (parent, deps) in cx.parents.edges(&summary.package_id()) { for dep in deps.iter() { let features = features_from_dep .remove(&(*parent, dep.clone())) @@ -1125,10 +1133,10 @@ fn emit_warnings( uses_default_features: dep.uses_default_features(), }; for (dep, features) in new_cx - .resolve_features(None, &summery, &method, config) + .resolve_features(None, &summary, &method, config) .expect("can not resolve_features for a used dep") { - features_from_dep.insert((summery.package_id(), dep), features); + features_from_dep.insert((summary.package_id(), dep), features); } } } diff --git a/src/cargo/core/resolver/types.rs b/src/cargo/core/resolver/types.rs index 70b46442d75..098f4f126d4 100644 --- a/src/cargo/core/resolver/types.rs +++ b/src/cargo/core/resolver/types.rs @@ -219,8 +219,21 @@ impl<'a> RegistryQueryer<'a> { debug!("\t{} => {}", dep.package_name(), dep.version_req()); } if let Some(r) = &replace { - self.used_replacements - .insert(summary.package_id(), r.package_id()); + if let Some(old) = self + .used_replacements + .insert(summary.package_id(), r.package_id()) + { + debug_assert_eq!( + r.package_id(), + old, + "we are inconsistent about witch replacement is used for {:?}, \ + one time we used {:?}, \ + now we are adding {:?}.", + summary.package_id(), + old, + r.package_id() + ) + } } candidate.replace = replace; diff --git a/tests/testsuite/resolve.rs b/tests/testsuite/resolve.rs index a1b3277748d..780da0fbec1 100644 --- a/tests/testsuite/resolve.rs +++ b/tests/testsuite/resolve.rs @@ -111,13 +111,13 @@ proptest! { ) { let reg = registry(input.clone()); let mut removed_input = input.clone(); - for (summery_idx, dep_idx) in indexes_to_remove { + for (summary_idx, dep_idx) in indexes_to_remove { if !removed_input.is_empty() { - let summery_idx = summery_idx.index(removed_input.len()); - let deps = removed_input[summery_idx].dependencies(); + let summary_idx = summary_idx.index(removed_input.len()); + let deps = removed_input[summary_idx].dependencies(); if !deps.is_empty() { - let new = remove_dep(&removed_input[summery_idx], dep_idx.index(deps.len())); - removed_input[summery_idx] = new; + let new = remove_dep(&removed_input[summary_idx], dep_idx.index(deps.len())); + removed_input[summary_idx] = new; } } }