Refactor resolve Method

Author: ehuss

crates/resolver-tests/src/lib.rs

@@ -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};
@@ -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(),

src/cargo/core/compiler/layout.rs

@@ -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/

src/cargo/core/resolver/context.rs

@@ -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};
@@ -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();
@@ -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),
         })
     }
 

src/cargo/core/resolver/dep_cache.rs

@@ -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),
@@ -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
@@ -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.
@@ -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)
     }
@@ -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());
@@ -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)
 }
 

src/cargo/core/resolver/encode.rs

@@ -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>>,
@@ -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();

src/cargo/core/resolver/mod.rs

@@ -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;
@@ -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>,
@@ -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();
@@ -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) => (),
@@ -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,
@@ -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
@@ -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 {
@@ -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) => {
@@ -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!(
@@ -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() {