From c86b8345291657dc5a9074d7b54d73a959d3f2a7 Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Wed, 15 May 2019 13:08:38 -0400 Subject: [PATCH 1/2] remove Candidate.replace. Look it up if needed. --- src/cargo/core/resolver/dep_cache.rs | 20 +++++++++----------- src/cargo/core/resolver/mod.rs | 5 ++--- src/cargo/core/resolver/types.rs | 1 - 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/src/cargo/core/resolver/dep_cache.rs b/src/cargo/core/resolver/dep_cache.rs index 75f96b7e3d9..b78d57439db 100644 --- a/src/cargo/core/resolver/dep_cache.rs +++ b/src/cargo/core/resolver/dep_cache.rs @@ -38,7 +38,7 @@ pub struct RegistryQueryer<'a> { Rc<(HashSet, Rc>)>, >, /// all the cases we ended up using a supplied replacement - used_replacements: HashMap, + used_replacements: HashMap, } impl<'a> RegistryQueryer<'a> { @@ -60,7 +60,11 @@ impl<'a> RegistryQueryer<'a> { } pub fn used_replacement_for(&self, p: PackageId) -> Option<(PackageId, PackageId)> { - self.used_replacements.get(&p).map(|&r| (p, r)) + self.used_replacements.get(&p).map(|r| (p, r.package_id())) + } + + pub fn replacement_summary(&self, p: PackageId) -> Option<&Summary> { + self.used_replacements.get(&p) } /// Queries the `registry` to return a list of candidates for `dep`. @@ -78,10 +82,7 @@ impl<'a> RegistryQueryer<'a> { self.registry.query( dep, &mut |s| { - ret.push(Candidate { - summary: s, - replace: None, - }); + ret.push(Candidate { summary: s }); }, false, )?; @@ -157,12 +158,9 @@ impl<'a> RegistryQueryer<'a> { for dep in summary.dependencies() { 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(r) = replace { + self.used_replacements.insert(summary.package_id(), r); } - - candidate.replace = replace; } // When we attempt versions for a package we'll want to do so in a diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index f44cfb514cd..8df391a505b 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -183,7 +183,6 @@ fn activate_deps_loop( debug!("initial activation: {}", summary.package_id()); let candidate = Candidate { summary: summary.clone(), - replace: None, }; let res = activate(&mut cx, registry, None, candidate, method.clone()); match res { @@ -659,7 +658,7 @@ fn activate( let activated = cx.flag_activated(&candidate.summary, &method)?; - let candidate = match candidate.replace { + let candidate = match registry.replacement_summary(candidate_pid) { Some(replace) => { if cx.flag_activated(&replace, &method)? && activated { return Ok(None); @@ -669,7 +668,7 @@ fn activate( replace.package_id(), candidate_pid ); - replace + replace.clone() } None => { if activated { diff --git a/src/cargo/core/resolver/types.rs b/src/cargo/core/resolver/types.rs index 28bf903d734..6d9512de841 100644 --- a/src/cargo/core/resolver/types.rs +++ b/src/cargo/core/resolver/types.rs @@ -125,7 +125,6 @@ impl Method { #[derive(Clone)] pub struct Candidate { pub summary: Summary, - pub replace: Option, } #[derive(Clone)] From c1b22c5b8661f5b4e21400b30be0646e86a005ee Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Wed, 15 May 2019 13:21:58 -0400 Subject: [PATCH 2/2] remove Candidate. --- src/cargo/core/resolver/dep_cache.rs | 18 ++++++------ src/cargo/core/resolver/errors.rs | 6 ++-- src/cargo/core/resolver/mod.rs | 41 +++++++++++++--------------- src/cargo/core/resolver/types.rs | 7 +---- 4 files changed, 31 insertions(+), 41 deletions(-) diff --git a/src/cargo/core/resolver/dep_cache.rs b/src/cargo/core/resolver/dep_cache.rs index b78d57439db..a6ae59132d5 100644 --- a/src/cargo/core/resolver/dep_cache.rs +++ b/src/cargo/core/resolver/dep_cache.rs @@ -19,7 +19,7 @@ use crate::core::interning::InternedString; use crate::core::{Dependency, FeatureValue, PackageId, PackageIdSpec, Registry, Summary}; use crate::util::errors::CargoResult; -use crate::core::resolver::types::{Candidate, ConflictReason, DepInfo, FeaturesSet}; +use crate::core::resolver::types::{ConflictReason, DepInfo, FeaturesSet}; use crate::core::resolver::{ActivateResult, Method}; pub struct RegistryQueryer<'a> { @@ -31,7 +31,7 @@ pub struct RegistryQueryer<'a> { /// specify minimum dependency versions to be used. minimal_versions: bool, /// a cache of `Candidate`s that fulfil a `Dependency` - registry_cache: HashMap>>, + registry_cache: HashMap>>, /// a cache of `Dependency`s that are required for a `Summary` summary_cache: HashMap< (Option, Summary, Method), @@ -73,7 +73,7 @@ impl<'a> RegistryQueryer<'a> { /// any candidates are returned which match an override then the override is /// applied by performing a second query for what the override should /// return. - pub fn query(&mut self, dep: &Dependency) -> CargoResult>> { + pub fn query(&mut self, dep: &Dependency) -> CargoResult>> { if let Some(out) = self.registry_cache.get(dep).cloned() { return Ok(out); } @@ -82,13 +82,11 @@ impl<'a> RegistryQueryer<'a> { self.registry.query( dep, &mut |s| { - ret.push(Candidate { summary: s }); + ret.push(s); }, false, )?; - for candidate in ret.iter_mut() { - let summary = &candidate.summary; - + for summary in ret.iter_mut() { let mut potential_matches = self .replacements .iter() @@ -168,12 +166,12 @@ impl<'a> RegistryQueryer<'a> { // prioritized summaries (those in `try_to_use`) and failing that we // list everything from the maximum version to the lowest version. ret.sort_unstable_by(|a, b| { - let a_in_previous = self.try_to_use.contains(&a.summary.package_id()); - let b_in_previous = self.try_to_use.contains(&b.summary.package_id()); + let a_in_previous = self.try_to_use.contains(&a.package_id()); + let b_in_previous = self.try_to_use.contains(&b.package_id()); let previous_cmp = a_in_previous.cmp(&b_in_previous).reverse(); match previous_cmp { Ordering::Equal => { - let cmp = a.summary.version().cmp(b.summary.version()); + let cmp = a.version().cmp(b.version()); if self.minimal_versions { // Lower version ordered first. cmp diff --git a/src/cargo/core/resolver/errors.rs b/src/cargo/core/resolver/errors.rs index b531743d17d..e67ec9ede01 100644 --- a/src/cargo/core/resolver/errors.rs +++ b/src/cargo/core/resolver/errors.rs @@ -7,7 +7,7 @@ use failure::{Error, Fail}; use semver; use super::context::Context; -use super::types::{Candidate, ConflictMap, ConflictReason}; +use super::types::{ConflictMap, ConflictReason}; /// Error during resolution providing a path of `PackageId`s. pub struct ResolveError { @@ -74,7 +74,7 @@ pub(super) fn activation_error( parent: &Summary, dep: &Dependency, conflicting_activations: &ConflictMap, - candidates: &[Candidate], + candidates: &[Summary], config: Option<&Config>, ) -> ResolveError { let to_resolve_err = |err| { @@ -101,7 +101,7 @@ pub(super) fn activation_error( msg.push_str( &candidates .iter() - .map(|v| v.summary.version()) + .map(|v| v.version()) .map(|v| v.to_string()) .collect::>() .join(", "), diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index 8df391a505b..c410b7accca 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -62,7 +62,7 @@ use crate::util::profile; use self::context::{Activations, Context}; use self::dep_cache::RegistryQueryer; -use self::types::{Candidate, ConflictMap, ConflictReason, DepsFrame}; +use self::types::{ConflictMap, ConflictReason, DepsFrame}; use self::types::{FeaturesSet, RcVecIter, RemainingDeps, ResolverProgress}; pub use self::encode::{EncodableDependency, EncodablePackageId, EncodableResolve}; @@ -181,10 +181,7 @@ fn activate_deps_loop( // Activate all the initial summaries to kick off some work. for &(ref summary, ref method) in summaries { debug!("initial activation: {}", summary.package_id()); - let candidate = Candidate { - summary: summary.clone(), - }; - let res = activate(&mut cx, registry, None, candidate, method.clone()); + let res = activate(&mut cx, registry, None, summary.clone(), method.clone()); match res { Ok(Some((frame, _))) => remaining_deps.push(frame), Ok(None) => (), @@ -369,7 +366,7 @@ fn activate_deps_loop( None }; - let pid = candidate.summary.package_id(); + let pid = candidate.package_id(); let method = Method::Required { dev_deps: false, features: Rc::clone(&features), @@ -381,7 +378,7 @@ fn activate_deps_loop( parent.name(), cur, dep.package_name(), - candidate.summary.version() + candidate.version() ); let res = activate(&mut cx, registry, Some((&parent, &dep)), candidate, method); @@ -594,10 +591,10 @@ fn activate( cx: &mut Context, registry: &mut RegistryQueryer<'_>, parent: Option<(&Summary, &Dependency)>, - candidate: Candidate, + candidate: Summary, method: Method, ) -> ActivateResult> { - let candidate_pid = candidate.summary.package_id(); + let candidate_pid = candidate.package_id(); if let Some((parent, dep)) = parent { let parent_pid = parent.package_id(); Rc::make_mut( @@ -656,7 +653,7 @@ fn activate( } } - let activated = cx.flag_activated(&candidate.summary, &method)?; + let activated = cx.flag_activated(&candidate, &method)?; let candidate = match registry.replacement_summary(candidate_pid) { Some(replace) => { @@ -675,7 +672,7 @@ fn activate( return Ok(None); } trace!("activating {}", candidate_pid); - candidate.summary + candidate } }; @@ -726,13 +723,13 @@ struct BacktrackFrame { /// filtered out, and as they are filtered the causes will be added to `conflicting_prev_active`. #[derive(Clone)] struct RemainingCandidates { - remaining: RcVecIter, + remaining: RcVecIter, // This is a inlined peekable generator - has_another: Option, + has_another: Option, } impl RemainingCandidates { - fn new(candidates: &Rc>) -> RemainingCandidates { + fn new(candidates: &Rc>) -> RemainingCandidates { RemainingCandidates { remaining: RcVecIter::new(Rc::clone(candidates)), has_another: None, @@ -761,14 +758,14 @@ impl RemainingCandidates { cx: &Context, dep: &Dependency, parent: PackageId, - ) -> Option<(Candidate, bool)> { + ) -> Option<(Summary, bool)> { 'main: for (_, b) in self.remaining.by_ref() { - let b_id = b.summary.package_id(); + let b_id = b.package_id(); // The `links` key in the manifest dictates that there's only one // package in a dependency graph, globally, with that particular // `links` key. If this candidate links to something that's already // linked to by a different package then we've gotta skip this. - if let Some(link) = b.summary.links() { + if let Some(link) = b.links() { if let Some(&a) = cx.links.get(&link) { if a != b_id { conflicting_prev_active @@ -788,7 +785,7 @@ impl RemainingCandidates { // Here we throw out our candidate if it's *compatible*, yet not // equal, to all previously activated versions. if let Some((a, _)) = cx.activations.get(&b_id.as_activations_key()) { - if *a != b.summary { + if *a != b { conflicting_prev_active .entry(a.package_id()) .or_insert(ConflictReason::Semver); @@ -904,7 +901,7 @@ fn generalize_conflicting( .find( dep, &|id| { - if id == other.summary.package_id() { + if id == other.package_id() { // we are imagining that we used other instead Some(backtrack_critical_age) } else { @@ -913,9 +910,9 @@ fn generalize_conflicting( age < backtrack_critical_age) } }, - Some(other.summary.package_id()), + Some(other.package_id()), ) - .map(|con| (other.summary.package_id(), con)) + .map(|con| (other.package_id(), con)) }) .collect::>>() { @@ -972,7 +969,7 @@ fn find_candidate( parent: &Summary, backtracked: bool, conflicting_activations: &ConflictMap, -) -> Option<(Candidate, bool, BacktrackFrame)> { +) -> Option<(Summary, bool, BacktrackFrame)> { // When we're calling this method we know that `parent` failed to // activate. That means that some dependency failed to get resolved for // whatever reason. Normally, that means that all of those reasons diff --git a/src/cargo/core/resolver/types.rs b/src/cargo/core/resolver/types.rs index 6d9512de841..ea8d3a22c43 100644 --- a/src/cargo/core/resolver/types.rs +++ b/src/cargo/core/resolver/types.rs @@ -122,11 +122,6 @@ impl Method { } } -#[derive(Clone)] -pub struct Candidate { - pub summary: Summary, -} - #[derive(Clone)] pub struct DepsFrame { pub parent: Summary, @@ -230,7 +225,7 @@ impl RemainingDeps { /// Information about the dependencies for a crate, a tuple of: /// /// (dependency info, candidates, features activated) -pub type DepInfo = (Dependency, Rc>, FeaturesSet); +pub type DepInfo = (Dependency, Rc>, FeaturesSet); /// All possible reasons that a package might fail to activate. ///