Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove Candidate #6946

Merged
merged 2 commits into from
May 15, 2019
Merged

Remove Candidate #6946

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c86b834
remove Candidate.replace. Look it up if needed.
Eh2406 May 15, 2019
c1b22c5
remove Candidate.
Eh2406 May 15, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 16 additions & 20 deletions src/cargo/core/resolver/dep_cache.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand All @@ -31,14 +31,14 @@ 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<Dependency, Rc<Vec<Candidate>>>,
registry_cache: HashMap<Dependency, Rc<Vec<Summary>>>,
/// a cache of `Dependency`s that are required for a `Summary`
summary_cache: HashMap<
(Option<PackageId>, Summary, Method),
Rc<(HashSet<InternedString>, Rc<Vec<DepInfo>>)>,
>,
/// all the cases we ended up using a supplied replacement
used_replacements: HashMap<PackageId, PackageId>,
used_replacements: HashMap<PackageId, Summary>,
}

impl<'a> RegistryQueryer<'a> {
Expand All @@ -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`.
Expand All @@ -69,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<Rc<Vec<Candidate>>> {
pub fn query(&mut self, dep: &Dependency) -> CargoResult<Rc<Vec<Summary>>> {
if let Some(out) = self.registry_cache.get(dep).cloned() {
return Ok(out);
}
Expand All @@ -78,16 +82,11 @@ impl<'a> RegistryQueryer<'a> {
self.registry.query(
dep,
&mut |s| {
ret.push(Candidate {
summary: s,
replace: None,
});
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()
Expand Down Expand Up @@ -157,25 +156,22 @@ 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
// sorted fashion to pick the "best candidates" first. Currently we try
// 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
Expand Down
6 changes: 3 additions & 3 deletions src/cargo/core/resolver/errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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| {
Expand All @@ -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::<Vec<_>>()
.join(", "),
Expand Down
46 changes: 21 additions & 25 deletions src/cargo/core/resolver/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -181,11 +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(),
replace: None,
};
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) => (),
Expand Down Expand Up @@ -370,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),
Expand All @@ -382,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);

Expand Down Expand Up @@ -595,10 +591,10 @@ fn activate(
cx: &mut Context,
registry: &mut RegistryQueryer<'_>,
parent: Option<(&Summary, &Dependency)>,
candidate: Candidate,
candidate: Summary,
method: Method,
) -> ActivateResult<Option<(DepsFrame, Duration)>> {
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(
Expand Down Expand Up @@ -657,9 +653,9 @@ fn activate(
}
}

let activated = cx.flag_activated(&candidate.summary, &method)?;
let activated = cx.flag_activated(&candidate, &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);
Expand All @@ -669,14 +665,14 @@ fn activate(
replace.package_id(),
candidate_pid
);
replace
replace.clone()
}
None => {
if activated {
return Ok(None);
}
trace!("activating {}", candidate_pid);
candidate.summary
candidate
}
};

Expand Down Expand Up @@ -727,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<Candidate>,
remaining: RcVecIter<Summary>,
// This is a inlined peekable generator
has_another: Option<Candidate>,
has_another: Option<Summary>,
}

impl RemainingCandidates {
fn new(candidates: &Rc<Vec<Candidate>>) -> RemainingCandidates {
fn new(candidates: &Rc<Vec<Summary>>) -> RemainingCandidates {
RemainingCandidates {
remaining: RcVecIter::new(Rc::clone(candidates)),
has_another: None,
Expand Down Expand Up @@ -762,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
Expand All @@ -789,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);
Expand Down Expand Up @@ -905,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 {
Expand All @@ -914,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::<Option<Vec<(PackageId, &ConflictMap)>>>()
{
Expand Down Expand Up @@ -973,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
Expand Down
8 changes: 1 addition & 7 deletions src/cargo/core/resolver/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,12 +122,6 @@ impl Method {
}
}

#[derive(Clone)]
pub struct Candidate {
pub summary: Summary,
pub replace: Option<Summary>,
}

#[derive(Clone)]
pub struct DepsFrame {
pub parent: Summary,
Expand Down Expand Up @@ -231,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<Vec<Candidate>>, FeaturesSet);
pub type DepInfo = (Dependency, Rc<Vec<Summary>>, FeaturesSet);

/// All possible reasons that a package might fail to activate.
///
Expand Down