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.

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

Refactor resolve Method #7186

Merged
merged 2 commits into from
Jul 29, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions crates/resolver-tests/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use std::rc::Rc;
use std::time::Instant;

use cargo::core::dependency::Kind;
use cargo::core::resolver::{self, Method};
use cargo::core::resolver::{self, ResolveOpts};
use cargo::core::source::{GitReference, SourceId};
use cargo::core::Resolve;
use cargo::core::{Dependency, PackageId, Registry, Summary};
Expand Down Expand Up @@ -175,10 +175,10 @@ pub fn resolve_with_config_raw(
false,
)
.unwrap();
let method = Method::Everything;
let opts = ResolveOpts::everything();
let start = Instant::now();
let resolve = resolver::resolve(
&[(summary, method)],
&[(summary, opts)],
&[],
&mut registry,
&HashSet::new(),
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/core/compiler/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
//! The directory layout is a little tricky at times, hence a separate file to
//! house this logic. The current layout looks like this:
//!
//! ```ignore
//! ```text
//! # This is the root directory for all output, the top-level package
//! # places all of its output here.
//! target/
Expand Down
30 changes: 13 additions & 17 deletions src/cargo/core/resolver/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use crate::util::CargoResult;
use crate::util::Graph;

use super::dep_cache::RegistryQueryer;
use super::types::{ConflictMap, FeaturesSet, Method};
use super::types::{ConflictMap, FeaturesSet, ResolveOpts};

pub use super::encode::Metadata;
pub use super::encode::{EncodableDependency, EncodablePackageId, EncodableResolve};
Expand Down Expand Up @@ -103,11 +103,11 @@ impl Context {
/// cased `summary` to get activated. This may not be present for the root
/// crate, for example.
///
/// Returns `true` if this summary with the given method is already activated.
/// Returns `true` if this summary with the given features is already activated.
pub fn flag_activated(
&mut self,
summary: &Summary,
method: &Method,
opts: &ResolveOpts,
parent: Option<(&Summary, &Dependency)>,
) -> CargoResult<bool> {
let id = summary.package_id();
Expand Down Expand Up @@ -158,25 +158,21 @@ impl Context {
}
}
debug!("checking if {} is already activated", summary.package_id());
let (features, use_default) = match method {
Method::Everything
| Method::Required {
all_features: true, ..
} => return Ok(false),
Method::Required {
features,
uses_default_features,
..
} => (features, uses_default_features),
};
if opts.all_features {
return Ok(false);
}

let has_default_feature = summary.features().contains_key("default");
Ok(match self.resolve_features.get(&id) {
Some(prev) => {
features.is_subset(prev)
&& (!use_default || prev.contains("default") || !has_default_feature)
opts.features.is_subset(prev)
&& (!opts.uses_default_features
|| prev.contains("default")
|| !has_default_feature)
}
None => {
opts.features.is_empty() && (!opts.uses_default_features || !has_default_feature)
}
None => features.is_empty() && (!use_default || !has_default_feature),
})
}

Expand Down
75 changes: 26 additions & 49 deletions src/cargo/core/resolver/dep_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use crate::core::{Dependency, FeatureValue, PackageId, PackageIdSpec, Registry,
use crate::util::errors::CargoResult;

use crate::core::resolver::types::{ConflictReason, DepInfo, FeaturesSet};
use crate::core::resolver::{ActivateResult, Method};
use crate::core::resolver::{ActivateResult, ResolveOpts};

pub struct RegistryQueryer<'a> {
pub registry: &'a mut (dyn Registry + 'a),
Expand All @@ -34,7 +34,7 @@ pub struct RegistryQueryer<'a> {
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),
(Option<PackageId>, Summary, ResolveOpts),
Rc<(HashSet<InternedString>, Rc<Vec<DepInfo>>)>,
>,
/// all the cases we ended up using a supplied replacement
Expand Down Expand Up @@ -192,28 +192,28 @@ impl<'a> RegistryQueryer<'a> {
}

/// Find out what dependencies will be added by activating `candidate`,
/// with features described in `method`. Then look up in the `registry`
/// with features described in `opts`. Then look up in the `registry`
/// the candidates that will fulfil each of these dependencies, as it is the
/// next obvious question.
pub fn build_deps(
&mut self,
parent: Option<PackageId>,
candidate: &Summary,
method: &Method,
opts: &ResolveOpts,
) -> ActivateResult<Rc<(HashSet<InternedString>, Rc<Vec<DepInfo>>)>> {
// if we have calculated a result before, then we can just return it,
// as it is a "pure" query of its arguments.
if let Some(out) = self
.summary_cache
.get(&(parent, candidate.clone(), method.clone()))
.get(&(parent, candidate.clone(), opts.clone()))
.cloned()
{
return Ok(out);
}
// First, figure out our set of dependencies based on the requested set
// of features. This also calculates what features we're going to enable
// for our own dependencies.
let (used_features, deps) = resolve_features(parent, candidate, method)?;
let (used_features, deps) = resolve_features(parent, candidate, opts)?;

// Next, transform all dependencies into a list of possible candidates
// which can satisfy that dependency.
Expand All @@ -236,7 +236,7 @@ impl<'a> RegistryQueryer<'a> {
// If we succeed we add the result to the cache so we can use it again next time.
// We dont cache the failure cases as they dont impl Clone.
self.summary_cache
.insert((parent, candidate.clone(), method.clone()), out.clone());
.insert((parent, candidate.clone(), opts.clone()), out.clone());

Ok(out)
}
Expand All @@ -247,18 +247,13 @@ impl<'a> RegistryQueryer<'a> {
pub fn resolve_features<'b>(
parent: Option<PackageId>,
s: &'b Summary,
method: &'b Method,
opts: &'b ResolveOpts,
) -> ActivateResult<(HashSet<InternedString>, Vec<(Dependency, FeaturesSet)>)> {
let dev_deps = match *method {
Method::Everything => true,
Method::Required { dev_deps, .. } => dev_deps,
};

// First, filter by dev-dependencies.
let deps = s.dependencies();
let deps = deps.iter().filter(|d| d.is_transitive() || dev_deps);
let deps = deps.iter().filter(|d| d.is_transitive() || opts.dev_deps);

let reqs = build_requirements(s, method)?;
let reqs = build_requirements(s, opts)?;
let mut ret = Vec::new();
let mut used_features = HashSet::new();
let default_dep = (false, BTreeSet::new());
Expand Down Expand Up @@ -336,52 +331,34 @@ pub fn resolve_features<'b>(
Ok((reqs.into_used(), ret))
}

/// Takes requested features for a single package from the input `Method` and
/// Takes requested features for a single package from the input `ResolveOpts` and
/// recurses to find all requested features, dependencies and requested
/// dependency features in a `Requirements` object, returning it to the resolver.
fn build_requirements<'a, 'b: 'a>(
s: &'a Summary,
method: &'b Method,
opts: &'b ResolveOpts,
) -> CargoResult<Requirements<'a>> {
let mut reqs = Requirements::new(s);

match method {
Method::Everything
| Method::Required {
all_features: true, ..
} => {
for key in s.features().keys() {
reqs.require_feature(*key)?;
}
for dep in s.dependencies().iter().filter(|d| d.is_optional()) {
reqs.require_dependency(dep.name_in_toml());
}
if opts.all_features {
for key in s.features().keys() {
reqs.require_feature(*key)?;
}
Method::Required {
all_features: false,
features: requested,
..
} => {
for &f in requested.iter() {
reqs.require_value(&FeatureValue::new(f, s))?;
}
for dep in s.dependencies().iter().filter(|d| d.is_optional()) {
reqs.require_dependency(dep.name_in_toml());
}
} else {
for &f in opts.features.iter() {
reqs.require_value(&FeatureValue::new(f, s))?;
}
}
match *method {
Method::Everything
| Method::Required {
uses_default_features: true,
..
} => {
if s.features().contains_key("default") {
reqs.require_feature(InternedString::new("default"))?;
}

if opts.uses_default_features {
if s.features().contains_key("default") {
reqs.require_feature(InternedString::new("default"))?;
}
Method::Required {
uses_default_features: false,
..
} => {}
}

Ok(reqs)
}

Expand Down
9 changes: 9 additions & 0 deletions src/cargo/core/resolver/encode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ use crate::util::{internal, Graph};

use super::{Resolve, ResolveVersion};

/// The `Cargo.lock` structure.
#[derive(Serialize, Deserialize, Debug)]
pub struct EncodableResolve {
package: Option<Vec<EncodableDependency>>,
Expand All @@ -123,6 +124,14 @@ struct Patch {
pub type Metadata = BTreeMap<String, String>;

impl EncodableResolve {
/// Convert a `Cargo.lock` to a Resolve.
///
/// Note that this `Resolve` is not "complete". For example, the
/// dependencies do not know the difference between regular/dev/build
/// dependencies, so they are not filled in. It also does not include
/// `features`. Care should be taken when using this Resolve. One of the
/// primary uses is to be used with `resolve_with_previous` to guide the
/// resolver to create a complete Resolve.
pub fn into_resolve(self, ws: &Workspace<'_>) -> CargoResult<Resolve> {
let path_deps = build_path_deps(ws);
let mut checksums = HashMap::new();
Expand Down
24 changes: 12 additions & 12 deletions src/cargo/core/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ pub use self::encode::Metadata;
pub use self::encode::{EncodableDependency, EncodablePackageId, EncodableResolve};
pub use self::errors::{ActivateError, ActivateResult, ResolveError};
pub use self::resolve::{Resolve, ResolveVersion};
pub use self::types::Method;
pub use self::types::ResolveOpts;

mod conflict_cache;
mod context;
Expand Down Expand Up @@ -120,7 +120,7 @@ mod types;
/// When we have a decision for how to implement is without breaking existing functionality
/// this flag can be removed.
pub fn resolve(
summaries: &[(Summary, Method)],
summaries: &[(Summary, ResolveOpts)],
replacements: &[(PackageIdSpec, Dependency)],
registry: &mut dyn Registry,
try_to_use: &HashSet<PackageId>,
Expand Down Expand Up @@ -169,7 +169,7 @@ pub fn resolve(
fn activate_deps_loop(
mut cx: Context,
registry: &mut RegistryQueryer<'_>,
summaries: &[(Summary, Method)],
summaries: &[(Summary, ResolveOpts)],
config: Option<&Config>,
) -> CargoResult<Context> {
let mut backtrack_stack = Vec::new();
Expand All @@ -180,9 +180,9 @@ fn activate_deps_loop(
let mut past_conflicting_activations = conflict_cache::ConflictCache::new();

// Activate all the initial summaries to kick off some work.
for &(ref summary, ref method) in summaries {
for &(ref summary, ref opts) in summaries {
debug!("initial activation: {}", summary.package_id());
let res = activate(&mut cx, registry, None, summary.clone(), method.clone());
let res = activate(&mut cx, registry, None, summary.clone(), opts.clone());
match res {
Ok(Some((frame, _))) => remaining_deps.push(frame),
Ok(None) => (),
Expand Down Expand Up @@ -366,7 +366,7 @@ fn activate_deps_loop(
};

let pid = candidate.package_id();
let method = Method::Required {
let opts = ResolveOpts {
dev_deps: false,
features: Rc::clone(&features),
all_features: false,
Expand All @@ -379,7 +379,7 @@ fn activate_deps_loop(
dep.package_name(),
candidate.version()
);
let res = activate(&mut cx, registry, Some((&parent, &dep)), candidate, method);
let res = activate(&mut cx, registry, Some((&parent, &dep)), candidate, opts);

let successfully_activated = match res {
// Success! We've now activated our `candidate` in our context
Expand Down Expand Up @@ -583,15 +583,15 @@ fn activate_deps_loop(
/// Attempts to activate the summary `candidate` in the context `cx`.
///
/// This function will pull dependency summaries from the registry provided, and
/// the dependencies of the package will be determined by the `method` provided.
/// the dependencies of the package will be determined by the `opts` provided.
/// If `candidate` was activated, this function returns the dependency frame to
/// iterate through next.
fn activate(
cx: &mut Context,
registry: &mut RegistryQueryer<'_>,
parent: Option<(&Summary, &Dependency)>,
candidate: Summary,
method: Method,
opts: ResolveOpts,
) -> ActivateResult<Option<(DepsFrame, Duration)>> {
let candidate_pid = candidate.package_id();
if let Some((parent, dep)) = parent {
Expand Down Expand Up @@ -652,7 +652,7 @@ fn activate(
}
}

let activated = cx.flag_activated(&candidate, &method, parent)?;
let activated = cx.flag_activated(&candidate, &opts, parent)?;

let candidate = match registry.replacement_summary(candidate_pid) {
Some(replace) => {
Expand All @@ -661,7 +661,7 @@ fn activate(
// does. TBH it basically cause panics in the test suite if
// `parent` is passed through here and `[replace]` is otherwise
// on life support so it's not critical to fix bugs anyway per se.
if cx.flag_activated(replace, &method, None)? && activated {
if cx.flag_activated(replace, &opts, None)? && activated {
return Ok(None);
}
trace!(
Expand All @@ -682,7 +682,7 @@ fn activate(

let now = Instant::now();
let (used_features, deps) =
&*registry.build_deps(parent.map(|p| p.0.package_id()), &candidate, &method)?;
&*registry.build_deps(parent.map(|p| p.0.package_id()), &candidate, &opts)?;

// Record what list of features is active for this package.
if !used_features.is_empty() {
Expand Down
Loading