From 12392f6d9992a3f293c3765551a5467d22fe7fdd Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Wed, 23 Oct 2019 11:24:24 -0700 Subject: [PATCH] Fix profile override warning in a workspace. --- src/cargo/core/compiler/standard_lib.rs | 2 +- src/cargo/core/profiles.rs | 27 +++++++++------------ src/cargo/ops/cargo_compile.rs | 31 +++++++++++++----------- src/cargo/ops/cargo_doc.rs | 7 +++--- src/cargo/ops/cargo_install.rs | 6 ++--- src/cargo/ops/cargo_output_metadata.rs | 9 ++++--- src/cargo/ops/cargo_package.rs | 14 +++++++---- src/cargo/ops/resolve.rs | 24 ++++++++++++++++--- tests/testsuite/profile_overrides.rs | 32 +++++++++++++++++++++++++ 9 files changed, 103 insertions(+), 49 deletions(-) diff --git a/src/cargo/core/compiler/standard_lib.rs b/src/cargo/core/compiler/standard_lib.rs index fc874eba223..bfab07725ba 100644 --- a/src/cargo/core/compiler/standard_lib.rs +++ b/src/cargo/core/compiler/standard_lib.rs @@ -103,7 +103,7 @@ pub fn resolve_std<'cfg>( /*uses_default_features*/ true, ); let resolve = ops::resolve_ws_with_opts(&std_ws, opts, &specs)?; - Ok(resolve) + Ok((resolve.pkg_set, resolve.targeted_resolve)) } /// Generate a list of root `Unit`s for the standard library. diff --git a/src/cargo/core/profiles.rs b/src/cargo/core/profiles.rs index 832d387bf73..f184ee3e44a 100644 --- a/src/cargo/core/profiles.rs +++ b/src/cargo/core/profiles.rs @@ -6,7 +6,7 @@ use serde::Deserialize; use crate::core::compiler::{CompileMode, ProfileKind}; use crate::core::interning::InternedString; -use crate::core::{Feature, Features, PackageId, PackageIdSpec, PackageSet, Shell}; +use crate::core::{Feature, Features, PackageId, PackageIdSpec, Resolve, Shell}; use crate::util::errors::CargoResultExt; use crate::util::toml::{ProfilePackageSpec, StringOrBool, TomlProfile, TomlProfiles, U32OrBool}; use crate::util::{closest_msg, CargoResult, Config}; @@ -429,13 +429,9 @@ impl Profiles { } /// Used to check for overrides for non-existing packages. - pub fn validate_packages( - &self, - shell: &mut Shell, - packages: &PackageSet<'_>, - ) -> CargoResult<()> { + pub fn validate_packages(&self, shell: &mut Shell, resolve: &Resolve) -> CargoResult<()> { for profile in self.by_name.values() { - profile.validate_packages(shell, packages)?; + profile.validate_packages(shell, resolve)?; } Ok(()) } @@ -502,16 +498,16 @@ impl ProfileMaker { profile } - fn validate_packages(&self, shell: &mut Shell, packages: &PackageSet<'_>) -> CargoResult<()> { - self.validate_packages_toml(shell, packages, &self.toml, true)?; - self.validate_packages_toml(shell, packages, &self.config, false)?; + fn validate_packages(&self, shell: &mut Shell, resolve: &Resolve) -> CargoResult<()> { + self.validate_packages_toml(shell, resolve, &self.toml, true)?; + self.validate_packages_toml(shell, resolve, &self.config, false)?; Ok(()) } fn validate_packages_toml( &self, shell: &mut Shell, - packages: &PackageSet<'_>, + resolve: &Resolve, toml: &Option, warn_unmatched: bool, ) -> CargoResult<()> { @@ -525,7 +521,7 @@ impl ProfileMaker { }; // Verify that a package doesn't match multiple spec overrides. let mut found = HashSet::new(); - for pkg_id in packages.package_ids() { + for pkg_id in resolve.iter() { let matches: Vec<&PackageIdSpec> = overrides .keys() .filter_map(|key| match *key { @@ -575,8 +571,8 @@ impl ProfileMaker { }); for spec in missing_specs { // See if there is an exact name match. - let name_matches: Vec = packages - .package_ids() + let name_matches: Vec = resolve + .iter() .filter_map(|pkg_id| { if pkg_id.name() == spec.name() { Some(pkg_id.to_string()) @@ -586,8 +582,7 @@ impl ProfileMaker { }) .collect(); if name_matches.is_empty() { - let suggestion = - closest_msg(&spec.name(), packages.package_ids(), |p| p.name().as_str()); + let suggestion = closest_msg(&spec.name(), resolve.iter(), |p| p.name().as_str()); shell.warn(format!( "package profile spec `{}` did not match any packages{}", spec, suggestion diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index b14557baf2d..180f9dd55d5 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -38,6 +38,7 @@ use crate::core::resolver::{Resolve, ResolveOpts}; use crate::core::{LibKind, Package, PackageSet, Target}; use crate::core::{PackageId, PackageIdSpec, TargetKind, Workspace}; use crate::ops; +use crate::ops::resolve::WorkspaceResolve; use crate::util::config::Config; use crate::util::{closest_msg, profile, CargoResult}; @@ -303,7 +304,11 @@ pub fn compile_ws<'a>( 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 (mut packages, resolve_with_overrides) = resolve; + let WorkspaceResolve { + mut pkg_set, + workspace_resolve, + targeted_resolve: resolve, + } = resolve; let std_resolve = if let Some(crates) = &config.cli_unstable().build_std { if build_config.build_plan { @@ -319,7 +324,7 @@ pub fn compile_ws<'a>( } let (mut std_package_set, std_resolve) = standard_lib::resolve_std(ws, crates)?; remove_dylib_crate_type(&mut std_package_set)?; - packages.add_set(std_package_set); + pkg_set.add_set(std_package_set); Some(std_resolve) } else { None @@ -330,12 +335,12 @@ pub fn compile_ws<'a>( // Vec to a Vec<&PackageId>. let to_build_ids = specs .iter() - .map(|s| s.query(resolve_with_overrides.iter())) + .map(|s| s.query(resolve.iter())) .collect::>>()?; // Now get the `Package` for each `PackageId`. This may trigger a download // if the user specified `-p` for a dependency that is not downloaded. // Dependencies will be downloaded during build_unit_dependencies. - let mut to_builds = packages.get_many(to_build_ids)?; + let mut to_builds = pkg_set.get_many(to_build_ids)?; // The ordering here affects some error messages coming out of cargo, so // let's be test and CLI friendly by always printing in the same order if @@ -370,12 +375,15 @@ pub fn compile_ws<'a>( ); } - profiles.validate_packages(&mut config.shell(), &packages)?; + profiles.validate_packages( + &mut config.shell(), + workspace_resolve.as_ref().unwrap_or(&resolve), + )?; let interner = UnitInterner::new(); let mut bcx = BuildContext::new( ws, - &packages, + &pkg_set, config, build_config, profiles, @@ -388,7 +396,7 @@ pub fn compile_ws<'a>( &to_builds, filter, build_config.requested_kind, - &resolve_with_overrides, + &resolve, &bcx, )?; @@ -434,13 +442,8 @@ pub fn compile_ws<'a>( } } - let unit_dependencies = build_unit_dependencies( - &bcx, - &resolve_with_overrides, - std_resolve.as_ref(), - &units, - &std_roots, - )?; + let unit_dependencies = + build_unit_dependencies(&bcx, &resolve, std_resolve.as_ref(), &units, &std_roots)?; let ret = { let _p = profile::start("compiling"); diff --git a/src/cargo/ops/cargo_doc.rs b/src/cargo/ops/cargo_doc.rs index c93f586b06f..6b75e310bef 100644 --- a/src/cargo/ops/cargo_doc.rs +++ b/src/cargo/ops/cargo_doc.rs @@ -24,14 +24,13 @@ pub fn doc(ws: &Workspace<'_>, options: &DocOptions<'_>) -> CargoResult<()> { options.compile_opts.all_features, !options.compile_opts.no_default_features, ); - let resolve = ops::resolve_ws_with_opts(ws, opts, &specs)?; - let (packages, resolve_with_overrides) = resolve; + let ws_resolve = ops::resolve_ws_with_opts(ws, opts, &specs)?; let ids = specs .iter() - .map(|s| s.query(resolve_with_overrides.iter())) + .map(|s| s.query(ws_resolve.targeted_resolve.iter())) .collect::>>()?; - let pkgs = packages.get_many(ids)?; + let pkgs = ws_resolve.pkg_set.get_many(ids)?; let mut lib_names = HashMap::new(); let mut bin_names = HashMap::new(); diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index c1bce3ef2e0..a5ebba0262b 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -491,14 +491,14 @@ 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_opts(ws, ResolveOpts::everything(), &specs)?; - let mut sources = pkg_set.sources_mut(); + let ws_resolve = ops::resolve_ws_with_opts(ws, ResolveOpts::everything(), &specs)?; + let mut sources = ws_resolve.pkg_set.sources_mut(); // 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()?; - for pkg_id in resolve.iter() { + for pkg_id in ws_resolve.targeted_resolve.iter() { if let Some(source) = sources.get_mut(pkg_id.source_id()) { if source.is_yanked(pkg_id)? { ws.config().shell().warn(format!( diff --git a/src/cargo/ops/cargo_output_metadata.rs b/src/cargo/ops/cargo_output_metadata.rs index f782414cace..8c60712283f 100644 --- a/src/cargo/ops/cargo_output_metadata.rs +++ b/src/cargo/ops/cargo_output_metadata.rs @@ -56,9 +56,12 @@ fn metadata_full(ws: &Workspace<'_>, opt: &OutputMetadataOptions) -> CargoResult opt.all_features, !opt.no_default_features, ); - let (package_set, resolve) = ops::resolve_ws_with_opts(ws, opts, &specs)?; + let ws_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())? { + for pkg in ws_resolve + .pkg_set + .get_many(ws_resolve.pkg_set.package_ids())? + { packages.insert(pkg.package_id(), pkg.clone()); } @@ -66,7 +69,7 @@ fn metadata_full(ws: &Workspace<'_>, opt: &OutputMetadataOptions) -> CargoResult packages: packages.values().map(|p| (*p).clone()).collect(), workspace_members: ws.members().map(|pkg| pkg.package_id()).collect(), resolve: Some(MetadataResolve { - resolve: (packages, resolve), + resolve: (packages, ws_resolve.targeted_resolve), root: ws.current_opt().map(|pkg| pkg.package_id()), }), target_directory: ws.target_dir().into_path_unlocked(), diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs index e965db04fe6..a1b9a5f63ad 100644 --- a/src/cargo/ops/cargo_package.rs +++ b/src/cargo/ops/cargo_package.rs @@ -155,15 +155,19 @@ 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_opts(&tmp_ws, ResolveOpts::everything(), &specs)?; + let 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)?; + compare_resolve( + config, + tmp_ws.current()?, + &orig_resolve, + &new_resolve.targeted_resolve, + )?; } - check_yanked(config, &pkg_set, &new_resolve)?; + check_yanked(config, &new_resolve.pkg_set, &new_resolve.targeted_resolve)?; - ops::resolve_to_string(&tmp_ws, &new_resolve) + ops::resolve_to_string(&tmp_ws, &new_resolve.targeted_resolve) } // Checks that the package has some piece of metadata that a human can diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs index bd243afff6e..a46440cb2e2 100644 --- a/src/cargo/ops/resolve.rs +++ b/src/cargo/ops/resolve.rs @@ -24,6 +24,20 @@ use crate::sources::PathSource; use crate::util::errors::{CargoResult, CargoResultExt}; use crate::util::profile; +/// Result for `resolve_ws_with_opts`. +pub struct WorkspaceResolve<'a> { + /// Packages to be downloaded. + pub pkg_set: PackageSet<'a>, + /// The resolve for the entire workspace. + /// + /// This may be `None` for things like `cargo install` and `-Zavoid-dev-deps`. + /// This does not include `paths` overrides. + pub workspace_resolve: Option, + /// The narrowed resolve, with the specific features enabled, and only the + /// given package specs requested. + pub targeted_resolve: Resolve, +} + const UNUSED_PATCH_WARNING: &str = "\ Check that the patched package version and available features are compatible with the dependency requirements. If the patch has a different version from @@ -60,7 +74,7 @@ pub fn resolve_ws_with_opts<'a>( ws: &Workspace<'a>, opts: ResolveOpts, specs: &[PackageIdSpec], -) -> CargoResult<(PackageSet<'a>, Resolve)> { +) -> CargoResult> { let mut registry = PackageRegistry::new(ws.config())?; let mut add_patches = true; @@ -106,9 +120,13 @@ pub fn resolve_ws_with_opts<'a>( add_patches, )?; - let packages = get_resolved_packages(&resolved_with_overrides, registry)?; + let pkg_set = get_resolved_packages(&resolved_with_overrides, registry)?; - Ok((packages, resolved_with_overrides)) + Ok(WorkspaceResolve { + pkg_set, + workspace_resolve: resolve, + targeted_resolve: resolved_with_overrides, + }) } fn resolve_with_registry<'cfg>( diff --git a/tests/testsuite/profile_overrides.rs b/tests/testsuite/profile_overrides.rs index e0465dc9bc6..da197fcd419 100644 --- a/tests/testsuite/profile_overrides.rs +++ b/tests/testsuite/profile_overrides.rs @@ -512,3 +512,35 @@ fn override_package_rename() { ") .run(); } + +#[cargo_test] +fn no_warning_ws() { + // https://github.com/rust-lang/cargo/issues/7378, avoid warnings in a workspace. + let p = project() + .file( + "Cargo.toml", + r#" + cargo-features = ["profile-overrides"] + [workspace] + members = ["a", "b"] + + [profile.dev.package.a] + codegen-units = 3 + "#, + ) + .file("a/Cargo.toml", &basic_manifest("a", "0.1.0")) + .file("a/src/lib.rs", "") + .file("b/Cargo.toml", &basic_manifest("b", "0.1.0")) + .file("b/src/lib.rs", "") + .build(); + + p.cargo("build -p b") + .masquerade_as_nightly_cargo() + .with_stderr( + "\ +[COMPILING] b [..] +[FINISHED] [..] +", + ) + .run(); +}