diff --git a/crates/resolver-tests/src/lib.rs b/crates/resolver-tests/src/lib.rs index 6d349a69bfc..dce94689eb3 100644 --- a/crates/resolver-tests/src/lib.rs +++ b/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(), diff --git a/src/cargo/core/compiler/layout.rs b/src/cargo/core/compiler/layout.rs index cc48a41ad69..40140e8ce98 100644 --- a/src/cargo/core/compiler/layout.rs +++ b/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/ diff --git a/src/cargo/core/resolver/context.rs b/src/cargo/core/resolver/context.rs index 58a86342db5..27b9a0585eb 100644 --- a/src/cargo/core/resolver/context.rs +++ b/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 { 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), }) } diff --git a/src/cargo/core/resolver/dep_cache.rs b/src/cargo/core/resolver/dep_cache.rs index 58b52a617d5..e20a78a66ae 100644 --- a/src/cargo/core/resolver/dep_cache.rs +++ b/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>>, /// a cache of `Dependency`s that are required for a `Summary` summary_cache: HashMap< - (Option, Summary, Method), + (Option, Summary, ResolveOpts), Rc<(HashSet, Rc>)>, >, /// all the cases we ended up using a supplied replacement @@ -192,20 +192,20 @@ 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, candidate: &Summary, - method: &Method, + opts: &ResolveOpts, ) -> ActivateResult, Rc>)>> { // 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); @@ -213,7 +213,7 @@ impl<'a> RegistryQueryer<'a> { // 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, s: &'b Summary, - method: &'b Method, + opts: &'b ResolveOpts, ) -> ActivateResult<(HashSet, 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> { 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) } diff --git a/src/cargo/core/resolver/encode.rs b/src/cargo/core/resolver/encode.rs index d605b5e0c33..b60b3a20797 100644 --- a/src/cargo/core/resolver/encode.rs +++ b/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>, @@ -123,6 +124,14 @@ struct Patch { pub type Metadata = BTreeMap; 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 { let path_deps = build_path_deps(ws); let mut checksums = HashMap::new(); diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index b7fa8b213fb..4aaa7eeafed 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/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, @@ -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 { 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,7 +583,7 @@ 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( @@ -591,7 +591,7 @@ fn activate( registry: &mut RegistryQueryer<'_>, parent: Option<(&Summary, &Dependency)>, candidate: Summary, - method: Method, + opts: ResolveOpts, ) -> ActivateResult> { 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() { diff --git a/src/cargo/core/resolver/resolve.rs b/src/cargo/core/resolver/resolve.rs index c1353e6299e..9ced48f4d67 100644 --- a/src/cargo/core/resolver/resolve.rs +++ b/src/cargo/core/resolver/resolve.rs @@ -23,15 +23,34 @@ pub struct Resolve { /// from `Cargo.toml`. We need a `Vec` here because the same package /// might be present in both `[dependencies]` and `[build-dependencies]`. graph: Graph>, + /// Replacements from the `[replace]` table. replacements: HashMap, + /// Inverted version of `replacements`. reverse_replacements: HashMap, + /// An empty `HashSet` to avoid creating a new `HashSet` for every package + /// that does not have any features, and to avoid using `Option` to + /// simplify the API. empty_features: HashSet, + /// Features enabled for a given package. features: HashMap>, + /// Checksum for each package. A SHA256 hash of the `.crate` file used to + /// validate the correct crate file is used. This is `None` for sources + /// that do not use `.crate` files, like path or git dependencies. checksums: HashMap>, + /// "Unknown" metadata. This is a collection of extra, unrecognized data + /// found in the `[metadata]` section of `Cargo.lock`, preserved for + /// forwards compatibility. metadata: Metadata, + /// `[patch]` entries that did not match anything, preserved in + /// `Cargo.lock` as the `[[patch.unused]]` table array. Tracking unused + /// patches helps prevent Cargo from being forced to re-update the + /// registry every time it runs, and keeps the resolve in a locked state + /// so it doesn't re-resolve the unused entries. unused_patches: Vec, - // A map from packages to a set of their public dependencies + /// A map from packages to a set of their public dependencies public_dependencies: HashMap>, + /// Version of the `Cargo.lock` format, see + /// `cargo::core::resolver::encode` for more. version: ResolveVersion, } diff --git a/src/cargo/core/resolver/types.rs b/src/cargo/core/resolver/types.rs index 0797403eeba..881869ef16f 100644 --- a/src/cargo/core/resolver/types.rs +++ b/src/cargo/core/resolver/types.rs @@ -99,19 +99,47 @@ impl ResolverProgress { /// optimized comparison operators like `is_subset` at the interfaces. pub type FeaturesSet = Rc>; -#[derive(Clone, Eq, PartialEq, Hash)] -pub enum Method { - Everything, // equivalent to Required { dev_deps: true, all_features: true, .. } - Required { +/// Options for how the resolve should work. +#[derive(Clone, Debug, Eq, PartialEq, Hash)] +pub struct ResolveOpts { + /// Whether or not dev-dependencies should be included. + /// + /// This may be set to `false` by things like `cargo install` or `-Z avoid-dev-deps`. + pub dev_deps: bool, + /// Set of features to enable (`--features=…`). + pub features: FeaturesSet, + /// Indicates *all* features should be enabled (`--all-features`). + pub all_features: bool, + /// Include the `default` feature (`--no-default-features` sets this false). + pub uses_default_features: bool, +} + +impl ResolveOpts { + /// Creates a ResolveOpts that resolves everything. + pub fn everything() -> ResolveOpts { + ResolveOpts { + dev_deps: true, + features: Rc::new(BTreeSet::new()), + all_features: true, + uses_default_features: true, + } + } + + pub fn new( dev_deps: bool, - features: FeaturesSet, + features: &[String], all_features: bool, uses_default_features: bool, - }, -} + ) -> ResolveOpts { + ResolveOpts { + dev_deps, + features: Rc::new(ResolveOpts::split_features(features)), + all_features, + uses_default_features, + } + } -impl Method { - pub fn split_features(features: &[String]) -> BTreeSet { + fn split_features(features: &[String]) -> BTreeSet { features .iter() .flat_map(|s| s.split_whitespace()) diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index cfbc84c5501..c36f10d33c1 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -283,7 +283,7 @@ impl<'cfg> Workspace<'cfg> { .unwrap_or_else(|| Filesystem::new(self.root().join("target"))) } - /// Returns the root [replace] section of this workspace. + /// Returns the root `[replace]` section of this workspace. /// /// This may be from a virtual crate or an actual crate. pub fn root_replace(&self) -> &[(PackageIdSpec, Dependency)] { @@ -293,7 +293,7 @@ impl<'cfg> Workspace<'cfg> { } } - /// Returns the root [patch] section of this workspace. + /// Returns the root `[patch]` section of this workspace. /// /// This may be from a virtual crate or an actual crate. pub fn root_patch(&self) -> &HashMap> { diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 826c3472e8e..0601f1cc12d 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -23,14 +23,13 @@ use std::collections::{BTreeSet, HashMap, HashSet}; use std::iter::FromIterator; use std::path::PathBuf; -use std::rc::Rc; use std::sync::Arc; use crate::core::compiler::{BuildConfig, BuildContext, Compilation, Context}; use crate::core::compiler::{CompileMode, Kind, Unit}; use crate::core::compiler::{DefaultExecutor, Executor, UnitInterner}; use crate::core::profiles::{Profiles, UnitFor}; -use crate::core::resolver::{Method, Resolve}; +use crate::core::resolver::{Resolve, ResolveOpts}; use crate::core::{Package, Target}; use crate::core::{PackageId, PackageIdSpec, TargetKind, Workspace}; use crate::ops; @@ -297,14 +296,9 @@ pub fn compile_ws<'a>( }; let specs = spec.to_package_id_specs(ws)?; - let features = Method::split_features(features); - let method = Method::Required { - dev_deps: ws.require_optional_deps() || filter.need_dev_deps(build_config.mode), - features: Rc::new(features), - all_features, - uses_default_features: !no_default_features, - }; - let resolve = ops::resolve_ws_with_method(ws, method, &specs)?; + let dev_deps = ws.require_optional_deps() || filter.need_dev_deps(build_config.mode); + let opts = ResolveOpts::new(dev_deps, features, all_features, !no_default_features); + let resolve = ops::resolve_ws_with_opts(ws, opts, &specs)?; let (packages, resolve_with_overrides) = resolve; let to_build_ids = specs diff --git a/src/cargo/ops/cargo_doc.rs b/src/cargo/ops/cargo_doc.rs index bb77acf9a69..fb9aa7ec49b 100644 --- a/src/cargo/ops/cargo_doc.rs +++ b/src/cargo/ops/cargo_doc.rs @@ -5,6 +5,7 @@ use std::path::Path; use failure::Fail; use opener; +use crate::core::resolver::ResolveOpts; use crate::core::Workspace; use crate::ops; use crate::util::CargoResult; @@ -21,13 +22,13 @@ pub struct DocOptions<'a> { /// Main method for `cargo doc`. pub fn doc(ws: &Workspace<'_>, options: &DocOptions<'_>) -> CargoResult<()> { let specs = options.compile_opts.spec.to_package_id_specs(ws)?; - let resolve = ops::resolve_ws_precisely( - ws, + let opts = ResolveOpts::new( + /*dev_deps*/ true, &options.compile_opts.features, options.compile_opts.all_features, - options.compile_opts.no_default_features, - &specs, - )?; + !options.compile_opts.no_default_features, + ); + let resolve = ops::resolve_ws_with_opts(ws, opts, &specs)?; let (packages, resolve_with_overrides) = resolve; let ids = specs diff --git a/src/cargo/ops/cargo_generate_lockfile.rs b/src/cargo/ops/cargo_generate_lockfile.rs index 1055e52f713..45c0aa68874 100644 --- a/src/cargo/ops/cargo_generate_lockfile.rs +++ b/src/cargo/ops/cargo_generate_lockfile.rs @@ -4,7 +4,7 @@ use log::debug; use termcolor::Color::{self, Cyan, Green, Red}; use crate::core::registry::PackageRegistry; -use crate::core::resolver::Method; +use crate::core::resolver::ResolveOpts; use crate::core::PackageId; use crate::core::{Resolve, SourceId, Workspace}; use crate::ops; @@ -21,8 +21,15 @@ pub struct UpdateOptions<'a> { pub fn generate_lockfile(ws: &Workspace<'_>) -> CargoResult<()> { let mut registry = PackageRegistry::new(ws.config())?; - let resolve = - ops::resolve_with_previous(&mut registry, ws, Method::Everything, None, None, &[], true)?; + let resolve = ops::resolve_with_previous( + &mut registry, + ws, + ResolveOpts::everything(), + None, + None, + &[], + true, + )?; ops::write_pkg_lockfile(ws, &resolve)?; Ok(()) } @@ -57,7 +64,7 @@ pub fn update_lockfile(ws: &Workspace<'_>, opts: &UpdateOptions<'_>) -> CargoRes ops::resolve_with_previous( &mut registry, ws, - Method::Everything, + ResolveOpts::everything(), None, None, &[], @@ -103,7 +110,7 @@ pub fn update_lockfile(ws: &Workspace<'_>, opts: &UpdateOptions<'_>) -> CargoRes let resolve = ops::resolve_with_previous( &mut registry, ws, - Method::Everything, + ResolveOpts::everything(), Some(&previous_resolve), Some(&to_avoid), &[], diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index 973df18b330..04f9befe33b 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -8,7 +8,7 @@ use tempfile::Builder as TempFileBuilder; use crate::core::compiler::Freshness; use crate::core::compiler::{DefaultExecutor, Executor}; -use crate::core::resolver::Method; +use crate::core::resolver::ResolveOpts; use crate::core::{Edition, PackageId, PackageIdSpec, Source, SourceId, Workspace}; use crate::ops; use crate::ops::common_for_install_and_uninstall::*; @@ -486,10 +486,10 @@ fn check_yanked_install(ws: &Workspace<'_>) -> CargoResult<()> { // It would be best if `source` could be passed in here to avoid a // duplicate "Updating", but since `source` is taken by value, then it // wouldn't be available for `compile_ws`. - let (pkg_set, resolve) = ops::resolve_ws_with_method(ws, Method::Everything, &specs)?; + let (pkg_set, resolve) = ops::resolve_ws_with_opts(ws, ResolveOpts::everything(), &specs)?; let mut sources = pkg_set.sources_mut(); - // Checking the yanked status invovles taking a look at the registry and + // Checking the yanked status involves taking a look at the registry and // maybe updating files, so be sure to lock it here. let _lock = ws.config().acquire_package_cache_lock()?; diff --git a/src/cargo/ops/cargo_output_metadata.rs b/src/cargo/ops/cargo_output_metadata.rs index 2cd93e46ee3..f782414cace 100644 --- a/src/cargo/ops/cargo_output_metadata.rs +++ b/src/cargo/ops/cargo_output_metadata.rs @@ -4,7 +4,7 @@ use std::path::PathBuf; use serde::ser; use serde::Serialize; -use crate::core::resolver::Resolve; +use crate::core::resolver::{Resolve, ResolveOpts}; use crate::core::{Package, PackageId, Workspace}; use crate::ops::{self, Packages}; use crate::util::CargoResult; @@ -50,13 +50,13 @@ fn metadata_no_deps(ws: &Workspace<'_>, _opt: &OutputMetadataOptions) -> CargoRe fn metadata_full(ws: &Workspace<'_>, opt: &OutputMetadataOptions) -> CargoResult { let specs = Packages::All.to_package_id_specs(ws)?; - let (package_set, resolve) = ops::resolve_ws_precisely( - ws, + let opts = ResolveOpts::new( + /*dev_deps*/ true, &opt.features, opt.all_features, - opt.no_default_features, - &specs, - )?; + !opt.no_default_features, + ); + let (package_set, resolve) = ops::resolve_ws_with_opts(ws, opts, &specs)?; let mut packages = HashMap::new(); for pkg in package_set.get_many(package_set.package_ids())? { packages.insert(pkg.package_id(), pkg.clone()); diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs index 2857ab3f1ad..f24332db887 100644 --- a/src/cargo/ops/cargo_package.rs +++ b/src/cargo/ops/cargo_package.rs @@ -14,7 +14,7 @@ use tar::{Archive, Builder, EntryType, Header}; use termcolor::Color; use crate::core::compiler::{BuildConfig, CompileMode, DefaultExecutor, Executor}; -use crate::core::resolver::Method; +use crate::core::resolver::ResolveOpts; use crate::core::Feature; use crate::core::{ Package, PackageId, PackageIdSpec, PackageSet, Resolve, Source, SourceId, Verbosity, Workspace, @@ -152,7 +152,8 @@ fn build_lock(ws: &Workspace<'_>) -> CargoResult { // Regenerate Cargo.lock using the old one as a guide. let specs = vec![PackageIdSpec::from_package_id(new_pkg.package_id())]; let tmp_ws = Workspace::ephemeral(new_pkg, ws.config(), None, true)?; - let (pkg_set, new_resolve) = ops::resolve_ws_with_method(&tmp_ws, Method::Everything, &specs)?; + let (pkg_set, new_resolve) = + ops::resolve_ws_with_opts(&tmp_ws, ResolveOpts::everything(), &specs)?; if let Some(orig_resolve) = orig_resolve { compare_resolve(config, tmp_ws.current()?, &orig_resolve, &new_resolve)?; @@ -558,7 +559,7 @@ fn compare_resolve( } fn check_yanked(config: &Config, pkg_set: &PackageSet<'_>, resolve: &Resolve) -> CargoResult<()> { - // Checking the yanked status invovles taking a look at the registry and + // Checking the yanked status involves taking a look at the registry and // maybe updating files, so be sure to lock it here. let _lock = config.acquire_package_cache_lock()?; diff --git a/src/cargo/ops/mod.rs b/src/cargo/ops/mod.rs index 9e40062b246..304e4d3c3f1 100644 --- a/src/cargo/ops/mod.rs +++ b/src/cargo/ops/mod.rs @@ -23,8 +23,7 @@ pub use self::registry::{http_handle, needs_custom_http_transport, registry_logi pub use self::registry::{modify_owners, yank, OwnersOptions, PublishOpts}; pub use self::registry::{publish, registry_configuration, RegistryConfig}; pub use self::resolve::{ - add_overrides, get_resolved_packages, resolve_with_previous, resolve_ws, resolve_ws_precisely, - resolve_ws_with_method, + add_overrides, get_resolved_packages, resolve_with_previous, resolve_ws, resolve_ws_with_opts, }; pub use self::vendor::{vendor, VendorOptions}; diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs index b660be8b6ca..98e1a70878d 100644 --- a/src/cargo/ops/resolve.rs +++ b/src/cargo/ops/resolve.rs @@ -1,10 +1,22 @@ +//! High-level APIs for executing the resolver. +//! +//! This module provides functions for running the resolver given a workspace. +//! There are roughly 3 main functions: +//! +//! - `resolve_ws`: A simple, high-level function with no options. +//! - `resolve_ws_with_opts`: A medium-level function with options like +//! user-provided features. This is the most appropriate function to use in +//! most cases. +//! - `resolve_with_previous`: A low-level function for running the resolver, +//! providing the most power and flexibility. + use std::collections::HashSet; use std::rc::Rc; use log::{debug, trace}; use crate::core::registry::PackageRegistry; -use crate::core::resolver::{self, Method, Resolve}; +use crate::core::resolver::{self, Resolve, ResolveOpts}; use crate::core::Feature; use crate::core::{PackageId, PackageIdSpec, PackageSet, Source, SourceId, Workspace}; use crate::ops; @@ -21,8 +33,12 @@ version. This may also occur with an optional dependency that is not enabled."; /// Resolves all dependencies for the workspace using the previous /// lock file as a guide if present. /// -/// This function will also write the result of resolution as a new -/// lock file. +/// This function will also write the result of resolution as a new lock file +/// (unless it is an ephemeral workspace such as `cargo install` or `cargo +/// package`). +/// +/// This is a simple interface used by commands like `clean`, `fetch`, and +/// `package`, which don't specify any options or features. pub fn resolve_ws<'a>(ws: &Workspace<'a>) -> CargoResult<(PackageSet<'a>, Resolve)> { let mut registry = PackageRegistry::new(ws.config())?; let resolve = resolve_with_registry(ws, &mut registry)?; @@ -32,30 +48,17 @@ pub fn resolve_ws<'a>(ws: &Workspace<'a>) -> CargoResult<(PackageSet<'a>, Resolv /// Resolves dependencies for some packages of the workspace, /// taking into account `paths` overrides and activated features. -pub fn resolve_ws_precisely<'a>( - ws: &Workspace<'a>, - features: &[String], - all_features: bool, - no_default_features: bool, - specs: &[PackageIdSpec], -) -> CargoResult<(PackageSet<'a>, Resolve)> { - let features = Method::split_features(features); - let method = if all_features { - Method::Everything - } else { - Method::Required { - dev_deps: true, - features: Rc::new(features), - all_features: false, - uses_default_features: !no_default_features, - } - }; - resolve_ws_with_method(ws, method, specs) -} - -pub fn resolve_ws_with_method<'a>( +/// +/// This function will also write the result of resolution as a new lock file +/// (unless `Workspace::require_optional_deps` is false, such as `cargo +/// install` or `-Z avoid-dev-deps`), or it is an ephemeral workspace (`cargo +/// install` or `cargo package`). +/// +/// `specs` may be empty, which indicates it should resolve all workspace +/// members. In this case, `opts.all_features` must be `true`. +pub fn resolve_ws_with_opts<'a>( ws: &Workspace<'a>, - method: Method, + opts: ResolveOpts, specs: &[PackageIdSpec], ) -> CargoResult<(PackageSet<'a>, Resolve)> { let mut registry = PackageRegistry::new(ws.config())?; @@ -67,6 +70,7 @@ pub fn resolve_ws_with_method<'a>( // First, resolve the root_package's *listed* dependencies, as well as // downloading and updating all remotes and such. let resolve = resolve_with_registry(ws, &mut registry)?; + // No need to add patches again, `resolve_with_registry` has done it. add_patches = false; // Second, resolve with precisely what we're doing. Filter out @@ -92,10 +96,10 @@ pub fn resolve_ws_with_method<'a>( ops::load_pkg_lockfile(ws)? }; - let resolved_with_overrides = ops::resolve_with_previous( + let resolved_with_overrides = resolve_with_previous( &mut registry, ws, - method, + opts, resolve.as_ref(), None, specs, @@ -115,7 +119,7 @@ fn resolve_with_registry<'cfg>( let resolve = resolve_with_previous( registry, ws, - Method::Everything, + ResolveOpts::everything(), prev.as_ref(), None, &[], @@ -137,15 +141,26 @@ fn resolve_with_registry<'cfg>( /// /// The previous resolve normally comes from a lock file. This function does not /// read or write lock files from the filesystem. +/// +/// `specs` may be empty, which indicates it should resolve all workspace +/// members. In this case, `opts.all_features` must be `true`. +/// +/// If `register_patches` is true, then entries from the `[patch]` table in +/// the manifest will be added to the given `PackageRegistry`. pub fn resolve_with_previous<'cfg>( registry: &mut PackageRegistry<'cfg>, ws: &Workspace<'cfg>, - method: Method, + opts: ResolveOpts, previous: Option<&Resolve>, to_avoid: Option<&HashSet>, specs: &[PackageIdSpec], register_patches: bool, ) -> CargoResult { + assert!( + !specs.is_empty() || opts.all_features, + "no specs requires all_features" + ); + // We only want one Cargo at a time resolving a crate graph since this can // involve a lot of frobbing of the global caches. let _lock = ws.config().acquire_package_cache_lock()?; @@ -228,85 +243,76 @@ pub fn resolve_with_previous<'cfg>( let mut summaries = Vec::new(); if ws.config().cli_unstable().package_features { let mut members = Vec::new(); - match &method { - Method::Everything => members.extend(ws.members()), - Method::Required { - features, - all_features, - uses_default_features, - .. - } => { - if specs.len() > 1 && !features.is_empty() { - failure::bail!("cannot specify features for more than one package"); - } - members.extend( - ws.members() - .filter(|m| specs.iter().any(|spec| spec.matches(m.package_id()))), - ); - // Edge case: running `cargo build -p foo`, where `foo` is not a member - // of current workspace. Add all packages from workspace to get `foo` - // into the resolution graph. - if members.is_empty() { - if !(features.is_empty() && !all_features && *uses_default_features) { - failure::bail!("cannot specify features for packages outside of workspace"); - } - members.extend(ws.members()); + if specs.is_empty() { + members.extend(ws.members()); + } else { + if specs.len() > 1 && !opts.features.is_empty() { + failure::bail!("cannot specify features for more than one package"); + } + members.extend( + ws.members() + .filter(|m| specs.iter().any(|spec| spec.matches(m.package_id()))), + ); + // Edge case: running `cargo build -p foo`, where `foo` is not a member + // of current workspace. Add all packages from workspace to get `foo` + // into the resolution graph. + if members.is_empty() { + if !(opts.features.is_empty() && !opts.all_features && opts.uses_default_features) { + failure::bail!("cannot specify features for packages outside of workspace"); } + members.extend(ws.members()); + panic!("tested?"); } } for member in members { let summary = registry.lock(member.summary().clone()); - summaries.push((summary, method.clone())) + summaries.push((summary, opts.clone())) } } else { for member in ws.members() { - let method_to_resolve = match method { - // When everything for a workspace we want to be sure to resolve all - // members in the workspace, so propagate the `Method::Everything`. - Method::Everything => Method::Everything, - + let summary_resolve_opts = if specs.is_empty() { + // When resolving the entire workspace, resolve each member + // with all features enabled. + opts.clone() + } else { // If we're not resolving everything though then we're constructing the // exact crate graph we're going to build. Here we don't necessarily // want to keep around all workspace crates as they may not all be // built/tested. // - // Additionally, the `method` specified represents command line + // Additionally, the `opts` specified represents command line // flags, which really only matters for the current package // (determined by the cwd). If other packages are specified (via // `-p`) then the command line flags like features don't apply to // them. // // As a result, if this `member` is the current member of the - // workspace, then we use `method` specified. Otherwise we use a - // base method with no features specified but using default features + // workspace, then we use `opts` specified. Otherwise we use a + // base `opts` with no features specified but using default features // for any other packages specified with `-p`. - Method::Required { - dev_deps, - all_features, - .. - } => { - let base = Method::Required { - dev_deps, - features: Rc::default(), - all_features, - uses_default_features: true, - }; - let member_id = member.package_id(); - match ws.current_opt() { - Some(current) if member_id == current.package_id() => method.clone(), - _ => { - if specs.iter().any(|spec| spec.matches(member_id)) { - base - } else { - continue; + let member_id = member.package_id(); + match ws.current_opt() { + Some(current) if member_id == current.package_id() => opts.clone(), + _ => { + if specs.iter().any(|spec| spec.matches(member_id)) { + // -p for a workspace member that is not the + // "current" one, don't use the local `--features`. + ResolveOpts { + dev_deps: opts.dev_deps, + features: Rc::default(), + all_features: opts.all_features, + uses_default_features: true, } + } else { + // `-p` for non-member, skip. + continue; } } } }; let summary = registry.lock(member.summary().clone()); - summaries.push((summary, method_to_resolve)); + summaries.push((summary, summary_resolve_opts)); } };