From 77ee608de3404810d437fa83340de8887685ebbc Mon Sep 17 00:00:00 2001 From: Eric Huss <eric@huss.org> Date: Thu, 26 Dec 2019 19:59:19 -0800 Subject: [PATCH 1/2] Add named config profiles. --- src/bin/cargo/commands/bench.rs | 8 +- src/bin/cargo/commands/clean.rs | 2 +- src/bin/cargo/commands/install.rs | 4 +- src/bin/cargo/commands/test.rs | 7 +- src/cargo/core/compiler/build_config.rs | 32 +- src/cargo/core/compiler/build_context/mod.rs | 4 +- src/cargo/core/compiler/context/mod.rs | 3 +- src/cargo/core/compiler/job_queue.rs | 9 +- src/cargo/core/compiler/mod.rs | 2 +- src/cargo/core/compiler/standard_lib.rs | 4 +- src/cargo/core/compiler/timings.rs | 2 +- src/cargo/core/compiler/unit_dependencies.rs | 1 - src/cargo/core/interning.rs | 12 + src/cargo/core/manifest.rs | 19 +- src/cargo/core/profiles.rs | 748 ++++++++++-------- src/cargo/core/workspace.rs | 13 +- src/cargo/ops/cargo_clean.rs | 36 +- src/cargo/ops/cargo_compile.rs | 23 +- .../ops/common_for_install_and_uninstall.rs | 6 +- src/cargo/util/command_prelude.rs | 34 +- src/cargo/util/config/de.rs | 10 +- src/cargo/util/config/mod.rs | 24 - src/cargo/util/toml/mod.rs | 75 +- tests/testsuite/config.rs | 17 +- tests/testsuite/profile_config.rs | 175 +++- tests/testsuite/profile_custom.rs | 59 +- tests/testsuite/profile_overrides.rs | 52 +- 27 files changed, 753 insertions(+), 628 deletions(-) diff --git a/src/bin/cargo/commands/bench.rs b/src/bin/cargo/commands/bench.rs index 306d9cd793b..b84c8160be3 100644 --- a/src/bin/cargo/commands/bench.rs +++ b/src/bin/cargo/commands/bench.rs @@ -1,5 +1,4 @@ use crate::command_prelude::*; - use cargo::ops::{self, TestOptions}; pub fn cli() -> App { @@ -80,11 +79,8 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult { ProfileChecking::Checked, )?; - compile_opts.build_config.profile_kind = args.get_profile_kind( - config, - ProfileKind::Custom("bench".to_owned()), - ProfileChecking::Checked, - )?; + compile_opts.build_config.requested_profile = + args.get_profile_name(config, "bench", ProfileChecking::Checked)?; let ops = TestOptions { no_run: args.is_present("no-run"), diff --git a/src/bin/cargo/commands/clean.rs b/src/bin/cargo/commands/clean.rs index 0cce5725767..22ceece0d68 100644 --- a/src/bin/cargo/commands/clean.rs +++ b/src/bin/cargo/commands/clean.rs @@ -29,7 +29,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult { config, spec: values(args, "package"), target: args.target(), - profile_kind: args.get_profile_kind(config, ProfileKind::Dev, ProfileChecking::Checked)?, + requested_profile: args.get_profile_name(config, "dev", ProfileChecking::Checked)?, profile_specified: args.is_present("profile") || args.is_present("release"), doc: args.is_present("doc"), }; diff --git a/src/bin/cargo/commands/install.rs b/src/bin/cargo/commands/install.rs index beb3287e4c9..a9a8ea6b494 100644 --- a/src/bin/cargo/commands/install.rs +++ b/src/bin/cargo/commands/install.rs @@ -116,8 +116,8 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult { ProfileChecking::Checked, )?; - compile_opts.build_config.profile_kind = - args.get_profile_kind(config, ProfileKind::Release, ProfileChecking::Checked)?; + compile_opts.build_config.requested_profile = + args.get_profile_name(config, "release", ProfileChecking::Checked)?; let krates = args .values_of("crate") diff --git a/src/bin/cargo/commands/test.rs b/src/bin/cargo/commands/test.rs index d0e879c1c0b..932cf308a2f 100644 --- a/src/bin/cargo/commands/test.rs +++ b/src/bin/cargo/commands/test.rs @@ -108,11 +108,8 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult { ProfileChecking::Checked, )?; - compile_opts.build_config.profile_kind = args.get_profile_kind( - config, - ProfileKind::Custom("test".to_owned()), - ProfileChecking::Checked, - )?; + compile_opts.build_config.requested_profile = + args.get_profile_name(config, "test", ProfileChecking::Checked)?; // `TESTNAME` is actually an argument of the test binary, but it's // important, so we explicitly mention it and reconfigure. diff --git a/src/cargo/core/compiler/build_config.rs b/src/cargo/core/compiler/build_config.rs index 2fe68bda426..c5c37e611b6 100644 --- a/src/cargo/core/compiler/build_config.rs +++ b/src/cargo/core/compiler/build_config.rs @@ -1,27 +1,9 @@ -use std::cell::RefCell; - -use serde::ser; - use crate::core::compiler::{CompileKind, CompileTarget}; +use crate::core::interning::InternedString; use crate::util::ProcessBuilder; use crate::util::{CargoResult, Config, RustfixDiagnosticServer}; - -#[derive(Debug, Clone)] -pub enum ProfileKind { - Dev, - Release, - Custom(String), -} - -impl ProfileKind { - pub fn name(&self) -> &str { - match self { - ProfileKind::Dev => "dev", - ProfileKind::Release => "release", - ProfileKind::Custom(name) => name, - } - } -} +use serde::ser; +use std::cell::RefCell; /// Configuration information for a rustc build. #[derive(Debug)] @@ -31,7 +13,7 @@ pub struct BuildConfig { /// Number of rustc jobs to run in parallel. pub jobs: u32, /// Build profile - pub profile_kind: ProfileKind, + pub requested_profile: InternedString, /// The mode we are compiling in. pub mode: CompileMode, /// `true` to print stdout in JSON format (for machine reading). @@ -92,7 +74,7 @@ impl BuildConfig { Ok(BuildConfig { requested_kind, jobs, - profile_kind: ProfileKind::Dev, + requested_profile: InternedString::new("dev"), mode, message_format: MessageFormat::Human, force_rebuild: false, @@ -111,10 +93,6 @@ impl BuildConfig { } } - pub fn profile_name(&self) -> &str { - self.profile_kind.name() - } - pub fn test(&self) -> bool { self.mode == CompileMode::Test || self.mode == CompileMode::Bench } diff --git a/src/cargo/core/compiler/build_context/mod.rs b/src/cargo/core/compiler/build_context/mod.rs index 88566a0c67c..ee39248f51b 100644 --- a/src/cargo/core/compiler/build_context/mod.rs +++ b/src/cargo/core/compiler/build_context/mod.rs @@ -26,7 +26,7 @@ pub struct BuildContext<'a, 'cfg> { pub ws: &'a Workspace<'cfg>, /// The cargo configuration. pub config: &'cfg Config, - pub profiles: &'a Profiles, + pub profiles: Profiles, pub build_config: &'a BuildConfig, /// Extra compiler args for either `rustc` or `rustdoc`. pub extra_compiler_args: HashMap<Unit<'a>, Vec<String>>, @@ -58,7 +58,7 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> { packages: &'a PackageSet<'cfg>, config: &'cfg Config, build_config: &'a BuildConfig, - profiles: &'a Profiles, + profiles: Profiles, units: &'a UnitInterner<'a>, extra_compiler_args: HashMap<Unit<'a>, Vec<String>>, ) -> CargoResult<BuildContext<'a, 'cfg>> { diff --git a/src/cargo/core/compiler/context/mod.rs b/src/cargo/core/compiler/context/mod.rs index 27b8ca691eb..652acaba869 100644 --- a/src/cargo/core/compiler/context/mod.rs +++ b/src/cargo/core/compiler/context/mod.rs @@ -281,8 +281,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> { export_dir: Option<PathBuf>, units: &[Unit<'a>], ) -> CargoResult<()> { - let profile_kind = &self.bcx.build_config.profile_kind; - let dest = self.bcx.profiles.get_dir_name(profile_kind); + let dest = self.bcx.profiles.get_dir_name(); let host_layout = Layout::new(self.bcx.ws, None, &dest)?; let mut targets = HashMap::new(); if let CompileKind::Target(target) = self.bcx.build_config.requested_kind { diff --git a/src/cargo/core/compiler/job_queue.rs b/src/cargo/core/compiler/job_queue.rs index b958770546b..bdde1297237 100644 --- a/src/cargo/core/compiler/job_queue.rs +++ b/src/cargo/core/compiler/job_queue.rs @@ -19,7 +19,6 @@ use super::job::{ }; use super::timings::Timings; use super::{BuildContext, BuildPlan, CompileMode, Context, Unit}; -use crate::core::compiler::ProfileKind; use crate::core::{PackageId, TargetKind}; use crate::handle_error; use crate::util; @@ -44,7 +43,6 @@ pub struct JobQueue<'a, 'cfg> { progress: Progress<'cfg>, next_id: u32, timings: Timings<'a, 'cfg>, - profile_kind: ProfileKind, } pub struct JobState<'a> { @@ -148,7 +146,6 @@ impl<'a, 'cfg> JobQueue<'a, 'cfg> { progress, next_id: 0, timings, - profile_kind: bcx.build_config.profile_kind.clone(), } } @@ -415,7 +412,7 @@ impl<'a, 'cfg> JobQueue<'a, 'cfg> { } self.progress.clear(); - let build_type = self.profile_kind.name(); + let profile_name = cx.bcx.build_config.requested_profile; // NOTE: this may be a bit inaccurate, since this may not display the // profile for what was actually built. Profile overrides can change // these settings, and in some cases different targets are built with @@ -423,7 +420,7 @@ impl<'a, 'cfg> JobQueue<'a, 'cfg> { // list of Units built, and maybe display a list of the different // profiles used. However, to keep it simple and compatible with old // behavior, we just display what the base profile is. - let profile = cx.bcx.profiles.base_profile(&self.profile_kind)?; + let profile = cx.bcx.profiles.base_profile(); let mut opt_type = String::from(if profile.opt_level.as_str() == "0" { "unoptimized" } else { @@ -440,7 +437,7 @@ impl<'a, 'cfg> JobQueue<'a, 'cfg> { } else if self.queue.is_empty() && queue.is_empty() { let message = format!( "{} [{}] target(s) in {}", - build_type, opt_type, time_elapsed + profile_name, opt_type, time_elapsed ); if !cx.bcx.build_config.build_plan { cx.bcx.config.shell().status("Finished", message)?; diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index c5aea7610ed..29243049a22 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -27,7 +27,7 @@ use anyhow::Error; use lazycell::LazyCell; use log::debug; -pub use self::build_config::{BuildConfig, CompileMode, MessageFormat, ProfileKind}; +pub use self::build_config::{BuildConfig, CompileMode, MessageFormat}; pub use self::build_context::{BuildContext, FileFlavor, TargetInfo}; use self::build_plan::BuildPlan; pub use self::compilation::{Compilation, Doctest}; diff --git a/src/cargo/core/compiler/standard_lib.rs b/src/cargo/core/compiler/standard_lib.rs index 2881f4dd843..112b2f7b9fa 100644 --- a/src/cargo/core/compiler/standard_lib.rs +++ b/src/cargo/core/compiler/standard_lib.rs @@ -66,8 +66,7 @@ pub fn resolve_std<'cfg>( /*replace*/ Vec::new(), patch, ws_config, - // Profiles are not used here, but we need something to pass in. - ws.profiles().clone(), + /*profiles*/ None, crate::core::Features::default(), ); @@ -139,7 +138,6 @@ pub fn generate_std_roots<'a>( /*is_member*/ false, unit_for, mode, - bcx.build_config.profile_kind.clone(), ); let features = std_resolve.features_sorted(pkg.package_id()); Ok(bcx.units.intern( diff --git a/src/cargo/core/compiler/timings.rs b/src/cargo/core/compiler/timings.rs index 81ead5f353a..a205b658642 100644 --- a/src/cargo/core/compiler/timings.rs +++ b/src/cargo/core/compiler/timings.rs @@ -116,7 +116,7 @@ impl<'a, 'cfg> Timings<'a, 'cfg> { }) .collect(); let start_str = humantime::format_rfc3339_seconds(SystemTime::now()).to_string(); - let profile = bcx.build_config.profile_kind.name().to_owned(); + let profile = bcx.build_config.requested_profile.to_string(); Timings { config: bcx.config, diff --git a/src/cargo/core/compiler/unit_dependencies.rs b/src/cargo/core/compiler/unit_dependencies.rs index 5c1f5466ff2..48095cdb16c 100644 --- a/src/cargo/core/compiler/unit_dependencies.rs +++ b/src/cargo/core/compiler/unit_dependencies.rs @@ -546,7 +546,6 @@ fn new_unit_dep<'a>( state.bcx.ws.is_member(pkg), unit_for, mode, - state.bcx.build_config.profile_kind.clone(), ); new_unit_dep_with_profile(state, parent, pkg, target, unit_for, kind, mode, profile) } diff --git a/src/cargo/core/interning.rs b/src/cargo/core/interning.rs index 22935f5ecf9..942869ac70b 100644 --- a/src/cargo/core/interning.rs +++ b/src/cargo/core/interning.rs @@ -48,6 +48,18 @@ impl PartialEq for InternedString { } } +impl PartialEq<str> for InternedString { + fn eq(&self, other: &str) -> bool { + *self == other + } +} + +impl<'a> PartialEq<&'a str> for InternedString { + fn eq(&self, other: &&str) -> bool { + **self == **other + } +} + impl Eq for InternedString {} impl InternedString { diff --git a/src/cargo/core/manifest.rs b/src/cargo/core/manifest.rs index bf307d1290a..9c30afd50cc 100644 --- a/src/cargo/core/manifest.rs +++ b/src/cargo/core/manifest.rs @@ -10,11 +10,10 @@ use serde::Serialize; use url::Url; use crate::core::interning::InternedString; -use crate::core::profiles::Profiles; use crate::core::{Dependency, PackageId, PackageIdSpec, SourceId, Summary}; use crate::core::{Edition, Feature, Features, WorkspaceConfig}; use crate::util::errors::*; -use crate::util::toml::TomlManifest; +use crate::util::toml::{TomlManifest, TomlProfiles}; use crate::util::{short_hash, Config, Filesystem}; pub enum EitherManifest { @@ -33,7 +32,7 @@ pub struct Manifest { include: Vec<String>, metadata: ManifestMetadata, custom_metadata: Option<toml::Value>, - profiles: Profiles, + profiles: Option<TomlProfiles>, publish: Option<Vec<String>>, publish_lockfile: bool, replace: Vec<(PackageIdSpec, Dependency)>, @@ -64,7 +63,7 @@ pub struct VirtualManifest { replace: Vec<(PackageIdSpec, Dependency)>, patch: HashMap<Url, Vec<Dependency>>, workspace: WorkspaceConfig, - profiles: Profiles, + profiles: Option<TomlProfiles>, warnings: Warnings, features: Features, } @@ -399,7 +398,7 @@ impl Manifest { links: Option<String>, metadata: ManifestMetadata, custom_metadata: Option<toml::Value>, - profiles: Profiles, + profiles: Option<TomlProfiles>, publish: Option<Vec<String>>, publish_lockfile: bool, replace: Vec<(PackageIdSpec, Dependency)>, @@ -475,8 +474,8 @@ impl Manifest { pub fn warnings(&self) -> &Warnings { &self.warnings } - pub fn profiles(&self) -> &Profiles { - &self.profiles + pub fn profiles(&self) -> Option<&TomlProfiles> { + self.profiles.as_ref() } pub fn publish(&self) -> &Option<Vec<String>> { &self.publish @@ -563,7 +562,7 @@ impl VirtualManifest { replace: Vec<(PackageIdSpec, Dependency)>, patch: HashMap<Url, Vec<Dependency>>, workspace: WorkspaceConfig, - profiles: Profiles, + profiles: Option<TomlProfiles>, features: Features, ) -> VirtualManifest { VirtualManifest { @@ -588,8 +587,8 @@ impl VirtualManifest { &self.workspace } - pub fn profiles(&self) -> &Profiles { - &self.profiles + pub fn profiles(&self) -> Option<&TomlProfiles> { + self.profiles.as_ref() } pub fn warnings_mut(&mut self) -> &mut Warnings { diff --git a/src/cargo/core/profiles.rs b/src/cargo/core/profiles.rs index 0be1c19c35d..7494e8de645 100644 --- a/src/cargo/core/profiles.rs +++ b/src/cargo/core/profiles.rs @@ -1,45 +1,42 @@ -use std::collections::HashSet; -use std::collections::{BTreeMap, HashMap}; -use std::{cmp, env, fmt, hash}; - -use serde::Deserialize; - -use crate::core::compiler::{CompileMode, ProfileKind}; +use crate::core::compiler::CompileMode; use crate::core::interning::InternedString; 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}; +use crate::util::{closest_msg, config, CargoResult, Config}; +use anyhow::bail; +use std::collections::{BTreeMap, HashMap, HashSet}; +use std::{cmp, env, fmt, hash}; -/// Collection of all user profiles. +/// Collection of all profiles. #[derive(Clone, Debug)] pub struct Profiles { /// Incremental compilation can be overridden globally via: /// - `CARGO_INCREMENTAL` environment variable. /// - `build.incremental` config value. incremental: Option<bool>, - dir_names: HashMap<String, String>, - by_name: HashMap<String, ProfileMaker>, + /// Map of profile name to directory name for that profile. + dir_names: HashMap<InternedString, InternedString>, + /// The profile makers. Key is the profile name. + by_name: HashMap<InternedString, ProfileMaker>, + /// Whether or not unstable "named" profiles are enabled. named_profiles_enabled: bool, + /// The profile the user requested to use. + requested_profile: InternedString, } impl Profiles { pub fn new( profiles: Option<&TomlProfiles>, config: &Config, + requested_profile: InternedString, features: &Features, - warnings: &mut Vec<String>, ) -> CargoResult<Profiles> { - if let Some(profiles) = profiles { - profiles.validate(features, warnings)?; - } - - let config_profiles = config.profiles()?; - let incremental = match env::var_os("CARGO_INCREMENTAL") { Some(v) => Some(v == "1"), None => config.build_config()?.incremental, }; + let mut profiles = merge_config_profiles(profiles, config, requested_profile, features)?; if !features.is_enabled(Feature::named_profiles()) { let mut profile_makers = Profiles { @@ -47,72 +44,49 @@ impl Profiles { named_profiles_enabled: false, dir_names: Self::predefined_dir_names(), by_name: HashMap::new(), + requested_profile, }; profile_makers.by_name.insert( - "dev".to_owned(), - ProfileMaker { - toml: profiles.and_then(|p| p.get("dev").cloned()), - inherits: vec![], - config: config_profiles.dev.clone(), - default: Profile::default_dev(), - }, + InternedString::new("dev"), + ProfileMaker::new(Profile::default_dev(), profiles.remove("dev")), ); profile_makers .dir_names - .insert("dev".to_owned(), "debug".to_owned()); + .insert(InternedString::new("dev"), InternedString::new("debug")); profile_makers.by_name.insert( - "release".to_owned(), - ProfileMaker { - toml: profiles.and_then(|p| p.get("release").cloned()), - inherits: vec![], - config: config_profiles.release.clone(), - default: Profile::default_release(), - }, + InternedString::new("release"), + ProfileMaker::new(Profile::default_release(), profiles.remove("release")), + ); + profile_makers.dir_names.insert( + InternedString::new("release"), + InternedString::new("release"), ); - profile_makers - .dir_names - .insert("release".to_owned(), "release".to_owned()); profile_makers.by_name.insert( - "test".to_owned(), - ProfileMaker { - toml: profiles.and_then(|p| p.get("test").cloned()), - inherits: vec![], - config: None, - default: Profile::default_test(), - }, + InternedString::new("test"), + ProfileMaker::new(Profile::default_test(), profiles.remove("test")), ); profile_makers .dir_names - .insert("test".to_owned(), "debug".to_owned()); + .insert(InternedString::new("test"), InternedString::new("debug")); profile_makers.by_name.insert( - "bench".to_owned(), - ProfileMaker { - toml: profiles.and_then(|p| p.get("bench").cloned()), - inherits: vec![], - config: None, - default: Profile::default_bench(), - }, + InternedString::new("bench"), + ProfileMaker::new(Profile::default_bench(), profiles.remove("bench")), ); profile_makers .dir_names - .insert("bench".to_owned(), "release".to_owned()); + .insert(InternedString::new("bench"), InternedString::new("release")); profile_makers.by_name.insert( - "doc".to_owned(), - ProfileMaker { - toml: profiles.and_then(|p| p.get("doc").cloned()), - inherits: vec![], - config: None, - default: Profile::default_doc(), - }, + InternedString::new("doc"), + ProfileMaker::new(Profile::default_doc(), profiles.remove("doc")), ); profile_makers .dir_names - .insert("doc".to_owned(), "debug".to_owned()); + .insert(InternedString::new("doc"), InternedString::new("debug")); return Ok(profile_makers); } @@ -122,20 +96,15 @@ impl Profiles { named_profiles_enabled: true, dir_names: Self::predefined_dir_names(), by_name: HashMap::new(), + requested_profile, }; - Self::add_root_profiles(&mut profile_makers, profiles, config_profiles); - - let mut profiles = if let Some(profiles) = profiles { - profiles.get_all().clone() - } else { - BTreeMap::new() - }; + Self::add_root_profiles(&mut profile_makers, &profiles)?; - // Merge with predefined profiles + // Merge with predefined profiles. use std::collections::btree_map::Entry; for (predef_name, mut predef_prof) in Self::predefined_profiles().into_iter() { - match profiles.entry(predef_name.to_owned()) { + match profiles.entry(InternedString::new(predef_name)) { Entry::Vacant(vac) => { vac.insert(predef_prof); } @@ -148,159 +117,166 @@ impl Profiles { } } - profile_makers.process_customs(&profiles)?; - + for (name, profile) in &profiles { + profile_makers.add_maker(*name, profile, &profiles)?; + } + // Verify that the requested profile is defined *somewhere*. + // This simplifies the API (no need for CargoResult), and enforces + // assumptions about how config profiles are loaded. + profile_makers.get_profile_maker(requested_profile)?; Ok(profile_makers) } - fn predefined_dir_names() -> HashMap<String, String> { + /// Returns the hard-coded directory names for built-in profiles. + fn predefined_dir_names() -> HashMap<InternedString, InternedString> { let mut dir_names = HashMap::new(); - dir_names.insert("dev".to_owned(), "debug".to_owned()); - dir_names.insert("check".to_owned(), "debug".to_owned()); - dir_names.insert("test".to_owned(), "debug".to_owned()); - dir_names.insert("bench".to_owned(), "release".to_owned()); + dir_names.insert(InternedString::new("dev"), InternedString::new("debug")); + dir_names.insert(InternedString::new("check"), InternedString::new("debug")); + dir_names.insert(InternedString::new("test"), InternedString::new("debug")); + dir_names.insert(InternedString::new("bench"), InternedString::new("release")); dir_names } + /// Initialize `by_name` with the two "root" profiles, `dev`, and + /// `release` given the user's definition. fn add_root_profiles( profile_makers: &mut Profiles, - profiles: Option<&TomlProfiles>, - config_profiles: &ConfigProfiles, - ) { - let profile_name = "dev"; + profiles: &BTreeMap<InternedString, TomlProfile>, + ) -> CargoResult<()> { profile_makers.by_name.insert( - profile_name.to_owned(), - ProfileMaker { - default: Profile::default_dev(), - toml: profiles.and_then(|p| p.get(profile_name).cloned()), - config: config_profiles.dev.clone(), - inherits: vec![], - }, + InternedString::new("dev"), + ProfileMaker::new(Profile::default_dev(), profiles.get("dev").cloned()), ); - let profile_name = "release"; profile_makers.by_name.insert( - profile_name.to_owned(), - ProfileMaker { - default: Profile::default_release(), - toml: profiles.and_then(|p| p.get(profile_name).cloned()), - config: config_profiles.release.clone(), - inherits: vec![], - }, + InternedString::new("release"), + ProfileMaker::new(Profile::default_release(), profiles.get("release").cloned()), ); + Ok(()) } + /// Returns the built-in profiles (not including dev/release, which are + /// "root" profiles). fn predefined_profiles() -> Vec<(&'static str, TomlProfile)> { vec![ ( "bench", TomlProfile { - inherits: Some(String::from("release")), + inherits: Some(InternedString::new("release")), ..TomlProfile::default() }, ), ( "test", TomlProfile { - inherits: Some(String::from("dev")), + inherits: Some(InternedString::new("dev")), ..TomlProfile::default() }, ), ( "check", TomlProfile { - inherits: Some(String::from("dev")), + inherits: Some(InternedString::new("dev")), ..TomlProfile::default() }, ), ( "doc", TomlProfile { - inherits: Some(String::from("dev")), + inherits: Some(InternedString::new("dev")), ..TomlProfile::default() }, ), ] } - fn process_customs(&mut self, profiles: &BTreeMap<String, TomlProfile>) -> CargoResult<()> { - for (name, profile) in profiles { - let mut set = HashSet::new(); - let mut result = Vec::new(); - - set.insert(name.as_str().to_owned()); - match &profile.dir_name { - None => {} - Some(dir_name) => { - self.dir_names.insert(name.clone(), dir_name.to_owned()); - } + /// Creates a `ProfileMaker`, and inserts it into `self.by_name`. + fn add_maker( + &mut self, + name: InternedString, + profile: &TomlProfile, + profiles: &BTreeMap<InternedString, TomlProfile>, + ) -> CargoResult<()> { + match &profile.dir_name { + None => {} + Some(dir_name) => { + self.dir_names.insert(name, dir_name.to_owned()); } - match name.as_str() { - "dev" | "release" => { - match &profile.inherits { - None => {} - Some(_) => { - anyhow::bail!( - "An 'inherits' must not specified root profile '{}'", - name - ); - } - } - continue; - } - _ => {} - }; - - let mut maker = self.process_chain(name, profile, &mut set, &mut result, profiles)?; - result.reverse(); - maker.inherits = result; + } - self.by_name.insert(name.as_str().to_owned(), maker); + // dev/release are "roots" and don't inherit. + if name == "dev" || name == "release" { + if profile.inherits.is_some() { + bail!( + "`inherits` must not be specified in root profile `{}`", + name + ); + } + // Already inserted from `add_root_profiles`, no need to do anything. + return Ok(()); } + // Keep track for inherits cycles. + let mut set = HashSet::new(); + set.insert(name); + let maker = self.process_chain(name, profile, &mut set, profiles)?; + self.by_name.insert(name, maker); Ok(()) } + /// Build a `ProfileMaker` by recursively following the `inherits` setting. + /// + /// * `name`: The name of the profile being processed. + /// * `profile`: The TOML profile being processed. + /// * `set`: Set of profiles that have been visited, used to detect cycles. + /// * `profiles`: Map of all TOML profiles. + /// + /// Returns a `ProfileMaker` to be used for the given named profile. fn process_chain( &mut self, - name: &str, + name: InternedString, profile: &TomlProfile, - set: &mut HashSet<String>, - result: &mut Vec<TomlProfile>, - profiles: &BTreeMap<String, TomlProfile>, + set: &mut HashSet<InternedString>, + profiles: &BTreeMap<InternedString, TomlProfile>, ) -> CargoResult<ProfileMaker> { - result.push(profile.clone()); - match profile.inherits.as_ref().map(|x| x.as_str()) { - Some(name @ "dev") | Some(name @ "release") => { - // These are the root profiles - Ok(self.by_name.get(name).unwrap().clone()) + let mut maker = match profile.inherits { + Some(inherits_name) if inherits_name == "dev" || inherits_name == "release" => { + // These are the root profiles added in `add_root_profiles`. + self.get_profile_maker(inherits_name).unwrap().clone() } - Some(name) => { - let name = name.to_owned(); - if set.get(&name).is_some() { - anyhow::bail!( - "Inheritance loop of profiles cycles with profile '{}'", - name + Some(inherits_name) => { + if !set.insert(inherits_name) { + bail!( + "profile inheritance loop detected with profile `{}` inheriting `{}`", + name, + inherits_name ); } - set.insert(name.clone()); - match profiles.get(&name) { + match profiles.get(&inherits_name) { None => { - anyhow::bail!("Profile '{}' not found in Cargo.toml", name); + bail!( + "profile `{}` inherits from `{}`, but that profile is not defined", + name, + inherits_name + ); } - Some(parent) => self.process_chain(&name, parent, set, result, profiles), + Some(parent) => self.process_chain(inherits_name, parent, set, profiles)?, } } None => { - anyhow::bail!( - "An 'inherits' directive is needed for all \ - profiles that are not 'dev' or 'release'. Here \ - it is missing from '{}'", + bail!( + "profile `{}` is missing an `inherits` directive \ + (`inherits` is required for all profiles except `dev` or `release`)", name ); } - } + }; + match &mut maker.toml { + Some(toml) => toml.merge(profile), + None => maker.toml = Some(profile.clone()), + }; + Ok(maker) } /// Retrieves the profile for a target. @@ -312,25 +288,29 @@ impl Profiles { is_member: bool, unit_for: UnitFor, mode: CompileMode, - profile_kind: ProfileKind, ) -> Profile { let (profile_name, inherits) = if !self.named_profiles_enabled { // With the feature disabled, we degrade `--profile` back to the // `--release` and `--debug` predicates, and convert back from // ProfileKind::Custom instantiation. - let release = match profile_kind { - ProfileKind::Release => true, - ProfileKind::Custom(ref s) if s == "bench" => true, + let release = match self.requested_profile.as_str() { + "release" | "bench" => true, _ => false, }; match mode { CompileMode::Test | CompileMode::Bench => { if release { - ("bench", Some("release")) + ( + InternedString::new("bench"), + Some(InternedString::new("release")), + ) } else { - ("test", Some("dev")) + ( + InternedString::new("test"), + Some(InternedString::new("dev")), + ) } } CompileMode::Build @@ -342,20 +322,17 @@ impl Profiles { // ancestor's profile. However, `cargo clean -p` can hit this // path. if release { - ("release", None) + (InternedString::new("release"), None) } else { - ("dev", None) + (InternedString::new("dev"), None) } } - CompileMode::Doc { .. } => ("doc", None), + CompileMode::Doc { .. } => (InternedString::new("doc"), None), } } else { - (profile_kind.name(), None) - }; - let maker = match self.by_name.get(profile_name) { - None => panic!("Profile {} undefined", profile_name), - Some(r) => r, + (self.requested_profile, None) }; + let maker = self.get_profile_maker(profile_name).unwrap(); let mut profile = maker.get_profile(Some(pkg_id), is_member, unit_for); // Dealing with `panic=abort` and `panic=unwind` requires some special @@ -365,7 +342,8 @@ impl Profiles { PanicSetting::ReadProfile => {} PanicSetting::Inherit => { if let Some(inherits) = inherits { - let maker = self.by_name.get(inherits).unwrap(); + // TODO: Fixme, broken with named profiles. + let maker = self.get_profile_maker(inherits).unwrap(); profile.panic = maker.get_profile(Some(pkg_id), is_member, unit_for).panic; } } @@ -384,6 +362,7 @@ impl Profiles { if !pkg_id.source_id().is_path() { profile.incremental = false; } + profile.name = profile_name; profile } @@ -403,38 +382,55 @@ impl Profiles { /// This returns the base profile. This is currently used for the /// `[Finished]` line. It is not entirely accurate, since it doesn't /// select for the package that was actually built. - pub fn base_profile(&self, profile_kind: &ProfileKind) -> CargoResult<Profile> { + pub fn base_profile(&self) -> Profile { let profile_name = if !self.named_profiles_enabled { - match profile_kind { - ProfileKind::Release => "release", - ProfileKind::Custom(ref s) if s == "bench" => "bench", - _ => "dev", + match self.requested_profile.as_str() { + "release" | "bench" => self.requested_profile, + _ => InternedString::new("dev"), } } else { - profile_kind.name() + self.requested_profile }; - match self.by_name.get(profile_name) { - None => anyhow::bail!("Profile `{}` undefined", profile_kind.name()), - Some(r) => Ok(r.get_profile(None, true, UnitFor::new_normal())), - } + let maker = self.get_profile_maker(profile_name).unwrap(); + maker.get_profile(None, true, UnitFor::new_normal()) } - pub fn get_dir_name(&self, profile_kind: &ProfileKind) -> String { - let dest = profile_kind.name(); - match self.dir_names.get(dest) { - None => dest.to_owned(), - Some(s) => s.clone(), - } + /// Gets the directory name for a profile, like `debug` or `release`. + pub fn get_dir_name(&self) -> InternedString { + *self + .dir_names + .get(&self.requested_profile) + .unwrap_or(&self.requested_profile) } /// Used to check for overrides for non-existing packages. - pub fn validate_packages(&self, shell: &mut Shell, resolve: &Resolve) -> CargoResult<()> { - for profile in self.by_name.values() { - profile.validate_packages(shell, resolve)?; + pub fn validate_packages( + &self, + profiles: Option<&TomlProfiles>, + shell: &mut Shell, + resolve: &Resolve, + ) -> CargoResult<()> { + for (name, profile) in &self.by_name { + let found = validate_packages_unique(resolve, name, &profile.toml)?; + // We intentionally do not validate unmatched packages for config + // profiles, in case they are defined in a central location. This + // iterates over the manifest profiles only. + if let Some(profiles) = profiles { + if let Some(toml_profile) = profiles.get(name) { + validate_packages_unmatched(shell, resolve, name, toml_profile, &found)?; + } + } } Ok(()) } + + /// Returns the profile maker for the given profile name. + fn get_profile_maker(&self, name: InternedString) -> CargoResult<&ProfileMaker> { + self.by_name + .get(&name) + .ok_or_else(|| anyhow::format_err!("profile `{}` is not defined", name)) + } } /// An object used for handling the profile hierarchy. @@ -451,18 +447,19 @@ impl Profiles { struct ProfileMaker { /// The starting, hard-coded defaults for the profile. default: Profile, - /// The profile from the `Cargo.toml` manifest. + /// The TOML profile defined in `Cargo.toml` or config. toml: Option<TomlProfile>, - - /// Profiles from which we inherit, in the order from which - /// we inherit. - inherits: Vec<TomlProfile>, - - /// Profile loaded from `.cargo/config` files. - config: Option<TomlProfile>, } impl ProfileMaker { + /// Creates a new `ProfileMaker`. + /// + /// Note that this does not process `inherits`, the caller is responsible for that. + fn new(default: Profile, toml: Option<TomlProfile>) -> ProfileMaker { + ProfileMaker { default, toml } + } + + /// Generates a new `Profile`. fn get_profile( &self, pkg_id: Option<PackageId>, @@ -470,136 +467,15 @@ impl ProfileMaker { unit_for: UnitFor, ) -> Profile { let mut profile = self.default; - - let mut tomls = vec![]; - if let Some(ref toml) = self.toml { - tomls.push(toml); - } - for toml in &self.inherits { - tomls.push(toml); - } - - // First merge the profiles - for toml in &tomls { + if let Some(toml) = &self.toml { merge_profile(&mut profile, toml); - } - - // Then their overrides - for toml in &tomls { merge_toml_overrides(pkg_id, is_member, unit_for, &mut profile, toml); } - - // '.cargo/config' can still overrides everything we had so far. - if let Some(ref toml) = self.config { - merge_profile(&mut profile, toml); - merge_toml_overrides(pkg_id, is_member, unit_for, &mut profile, toml); - } - profile } - - 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, - resolve: &Resolve, - toml: &Option<TomlProfile>, - warn_unmatched: bool, - ) -> CargoResult<()> { - let toml = match *toml { - Some(ref toml) => toml, - None => return Ok(()), - }; - let overrides = match toml.package.as_ref().or_else(|| toml.overrides.as_ref()) { - Some(overrides) => overrides, - None => return Ok(()), - }; - // Verify that a package doesn't match multiple spec overrides. - let mut found = HashSet::new(); - for pkg_id in resolve.iter() { - let matches: Vec<&PackageIdSpec> = overrides - .keys() - .filter_map(|key| match *key { - ProfilePackageSpec::All => None, - ProfilePackageSpec::Spec(ref spec) => { - if spec.matches(pkg_id) { - Some(spec) - } else { - None - } - } - }) - .collect(); - match matches.len() { - 0 => {} - 1 => { - found.insert(matches[0].clone()); - } - _ => { - let specs = matches - .iter() - .map(|spec| spec.to_string()) - .collect::<Vec<_>>() - .join(", "); - anyhow::bail!( - "multiple package overrides in profile `{}` match package `{}`\n\ - found package specs: {}", - self.default.name, - pkg_id, - specs - ); - } - } - } - - if !warn_unmatched { - return Ok(()); - } - // Verify every override matches at least one package. - let missing_specs = overrides.keys().filter_map(|key| { - if let ProfilePackageSpec::Spec(ref spec) = *key { - if !found.contains(spec) { - return Some(spec); - } - } - None - }); - for spec in missing_specs { - // See if there is an exact name match. - let name_matches: Vec<String> = resolve - .iter() - .filter_map(|pkg_id| { - if pkg_id.name() == spec.name() { - Some(pkg_id.to_string()) - } else { - None - } - }) - .collect(); - if name_matches.is_empty() { - 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 - ))?; - } else { - shell.warn(format!( - "version or URL in package profile spec `{}` does not \ - match any of the packages: {}", - spec, - name_matches.join(", ") - ))?; - } - } - Ok(()) - } } +/// Merge package and build overrides from the given TOML profile into the given `Profile`. fn merge_toml_overrides( pkg_id: Option<PackageId>, is_member: bool, @@ -612,7 +488,7 @@ fn merge_toml_overrides( merge_profile(profile, build_override); } } - if let Some(overrides) = toml.package.as_ref().or_else(|| toml.overrides.as_ref()) { + if let Some(overrides) = toml.package.as_ref() { if !is_member { if let Some(all) = overrides.get(&ProfilePackageSpec::All) { merge_profile(profile, all); @@ -645,6 +521,9 @@ fn merge_toml_overrides( } } +/// Merge the given TOML profile into the given `Profile`. +/// +/// Does not merge overrides (see `merge_toml_overrides`). fn merge_profile(profile: &mut Profile, toml: &TomlProfile) { if let Some(ref opt_level) = toml.opt_level { profile.opt_level = InternedString::new(&opt_level.0); @@ -685,6 +564,11 @@ fn merge_profile(profile: &mut Profile, toml: &TomlProfile) { } } +/// The root profile (dev/release). +/// +/// This is currently only used for the `PROFILE` env var for build scripts +/// for backwards compatibility. We should probably deprecate `PROFILE` and +/// encourage using things like `DEBUG` and `OPT_LEVEL` instead. #[derive(Clone, Copy, Eq, PartialOrd, Ord, PartialEq, Debug)] pub enum ProfileRoot { Release, @@ -695,7 +579,7 @@ pub enum ProfileRoot { /// target. #[derive(Clone, Copy, Eq, PartialOrd, Ord)] pub struct Profile { - pub name: &'static str, + pub name: InternedString, pub opt_level: InternedString, pub root: ProfileRoot, pub lto: Lto, @@ -712,7 +596,7 @@ pub struct Profile { impl Default for Profile { fn default() -> Profile { Profile { - name: "", + name: InternedString::new(""), opt_level: InternedString::new("0"), root: ProfileRoot::Debug, lto: Lto::Bool(false), @@ -730,7 +614,7 @@ impl Default for Profile { compact_debug! { impl fmt::Debug for Profile { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - let (default, default_name) = match self.name { + let (default, default_name) = match self.name.as_str() { "dev" => (Profile::default_dev(), "default_dev()"), "release" => (Profile::default_release(), "default_release()"), _ => (Profile::default(), "default()"), @@ -776,7 +660,7 @@ impl cmp::PartialEq for Profile { impl Profile { fn default_dev() -> Profile { Profile { - name: "dev", + name: InternedString::new("dev"), root: ProfileRoot::Debug, debuginfo: Some(2), debug_assertions: true, @@ -788,7 +672,7 @@ impl Profile { fn default_release() -> Profile { Profile { - name: "release", + name: InternedString::new("release"), root: ProfileRoot::Release, opt_level: InternedString::new("3"), ..Profile::default() @@ -799,21 +683,21 @@ impl Profile { fn default_test() -> Profile { Profile { - name: "test", + name: InternedString::new("test"), ..Profile::default_dev() } } fn default_bench() -> Profile { Profile { - name: "bench", + name: InternedString::new("bench"), ..Profile::default_release() } } fn default_doc() -> Profile { Profile { - name: "doc", + name: InternedString::new("doc"), ..Profile::default_dev() } } @@ -1009,27 +893,199 @@ impl UnitFor { } } -/// Profiles loaded from `.cargo/config` files. -#[derive(Clone, Debug, Deserialize, Default)] -pub struct ConfigProfiles { - dev: Option<TomlProfile>, - release: Option<TomlProfile>, +/// Takes the manifest profiles, and overlays the config profiles on-top. +/// +/// Returns a new copy of the profile map with all the mergers complete. +fn merge_config_profiles( + profiles: Option<&TomlProfiles>, + config: &Config, + requested_profile: InternedString, + features: &Features, +) -> CargoResult<BTreeMap<InternedString, TomlProfile>> { + let mut profiles = match profiles { + Some(profiles) => profiles.get_all().clone(), + None => BTreeMap::new(), + }; + // List of profile names to check if defined in config only. + let mut check_to_add = vec![requested_profile]; + // Flag so -Zconfig-profile warning is only printed once. + let mut unstable_warned = false; + // Merge config onto manifest profiles. + for (name, profile) in &mut profiles { + if let Some(config_profile) = + get_config_profile(name, config, features, &mut unstable_warned)? + { + profile.merge(&config_profile); + } + if let Some(inherits) = &profile.inherits { + check_to_add.push(*inherits); + } + } + // Add config-only profiles. + // Need to iterate repeatedly to get all the inherits values. + let mut current = Vec::new(); + while !check_to_add.is_empty() { + std::mem::swap(&mut current, &mut check_to_add); + for name in current.drain(..) { + if !profiles.contains_key(&name) { + if let Some(config_profile) = + get_config_profile(&name, config, features, &mut unstable_warned)? + { + if let Some(inherits) = &config_profile.inherits { + check_to_add.push(*inherits); + } + profiles.insert(name, config_profile); + } + } + } + } + Ok(profiles) +} + +/// Helper for fetching a profile from config. +fn get_config_profile( + name: &str, + config: &Config, + features: &Features, + unstable_warned: &mut bool, +) -> CargoResult<Option<TomlProfile>> { + let profile: Option<config::Value<TomlProfile>> = config.get(&format!("profile.{}", name))?; + let profile = match profile { + Some(profile) => profile, + None => return Ok(None), + }; + if !*unstable_warned && !config.cli_unstable().config_profile { + config.shell().warn(format!( + "config profiles require the `-Z config-profile` command-line option (found profile `{}` in {})", + name, profile.definition))?; + *unstable_warned = true; + return Ok(None); + } + let mut warnings = Vec::new(); + profile + .val + .validate(name, features, &mut warnings) + .chain_err(|| { + anyhow::format_err!( + "config profile `{}` is not valid (defined in `{}`)", + name, + profile.definition + ) + })?; + for warning in warnings { + config.shell().warn(warning)?; + } + Ok(Some(profile.val)) } -impl ConfigProfiles { - pub fn validate(&self, features: &Features, warnings: &mut Vec<String>) -> CargoResult<()> { - if let Some(ref profile) = self.dev { - profile - .validate("dev", features, warnings) - .chain_err(|| anyhow::format_err!("config profile `profile.dev` is not valid"))?; +/// Validate that a package does not match multiple package override specs. +/// +/// For example `[profile.dev.package.bar]` and `[profile.dev.package."bar:0.5.0"]` +/// would both match `bar:0.5.0` which would be ambiguous. +fn validate_packages_unique( + resolve: &Resolve, + name: &str, + toml: &Option<TomlProfile>, +) -> CargoResult<HashSet<PackageIdSpec>> { + let toml = match toml { + Some(ref toml) => toml, + None => return Ok(HashSet::new()), + }; + let overrides = match toml.package.as_ref() { + Some(overrides) => overrides, + None => return Ok(HashSet::new()), + }; + // Verify that a package doesn't match multiple spec overrides. + let mut found = HashSet::new(); + for pkg_id in resolve.iter() { + let matches: Vec<&PackageIdSpec> = overrides + .keys() + .filter_map(|key| match *key { + ProfilePackageSpec::All => None, + ProfilePackageSpec::Spec(ref spec) => { + if spec.matches(pkg_id) { + Some(spec) + } else { + None + } + } + }) + .collect(); + match matches.len() { + 0 => {} + 1 => { + found.insert(matches[0].clone()); + } + _ => { + let specs = matches + .iter() + .map(|spec| spec.to_string()) + .collect::<Vec<_>>() + .join(", "); + bail!( + "multiple package overrides in profile `{}` match package `{}`\n\ + found package specs: {}", + name, + pkg_id, + specs + ); + } } - if let Some(ref profile) = self.release { - profile - .validate("release", features, warnings) - .chain_err(|| { - anyhow::format_err!("config profile `profile.release` is not valid") - })?; + } + Ok(found) +} + +/// Check for any profile override specs that do not match any known packages. +/// +/// This helps check for typos and mistakes. +fn validate_packages_unmatched( + shell: &mut Shell, + resolve: &Resolve, + name: &str, + toml: &TomlProfile, + found: &HashSet<PackageIdSpec>, +) -> CargoResult<()> { + let overrides = match toml.package.as_ref() { + Some(overrides) => overrides, + None => return Ok(()), + }; + + // Verify every override matches at least one package. + let missing_specs = overrides.keys().filter_map(|key| { + if let ProfilePackageSpec::Spec(ref spec) = *key { + if !found.contains(spec) { + return Some(spec); + } + } + None + }); + for spec in missing_specs { + // See if there is an exact name match. + let name_matches: Vec<String> = resolve + .iter() + .filter_map(|pkg_id| { + if pkg_id.name() == spec.name() { + Some(pkg_id.to_string()) + } else { + None + } + }) + .collect(); + if name_matches.is_empty() { + let suggestion = closest_msg(&spec.name(), resolve.iter(), |p| p.name().as_str()); + shell.warn(format!( + "profile package spec `{}` in profile `{}` did not match any packages{}", + spec, name, suggestion + ))?; + } else { + shell.warn(format!( + "profile package spec `{}` in profile `{}` \ + has a version or URL that does not match any of the packages: {}", + spec, + name, + name_matches.join(", ") + ))?; } - Ok(()) } + Ok(()) } diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index c46a223cd5c..b34ea9acaf8 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -9,7 +9,6 @@ use log::debug; use url::Url; use crate::core::features::Features; -use crate::core::profiles::Profiles; use crate::core::registry::PackageRegistry; use crate::core::{Dependency, PackageId, PackageIdSpec}; use crate::core::{EitherManifest, Package, SourceId, VirtualManifest}; @@ -17,7 +16,7 @@ use crate::ops; use crate::sources::PathSource; use crate::util::errors::{CargoResult, CargoResultExt, ManifestError}; use crate::util::paths; -use crate::util::toml::read_manifest; +use crate::util::toml::{read_manifest, TomlProfiles}; use crate::util::{Config, Filesystem}; /// The core abstraction in Cargo for working with a workspace of crates. @@ -273,7 +272,7 @@ impl<'cfg> Workspace<'cfg> { self.config } - pub fn profiles(&self) -> &Profiles { + pub fn profiles(&self) -> Option<&TomlProfiles> { match self.root_maybe() { MaybePackage::Package(p) => p.manifest().profiles(), MaybePackage::Virtual(vm) => vm.profiles(), @@ -583,14 +582,6 @@ impl<'cfg> Workspace<'cfg> { /// 2. All workspace members agree on this one root as the root. /// 3. The current crate is a member of this workspace. fn validate(&mut self) -> CargoResult<()> { - // Validate config profiles only once per workspace. - let features = self.features(); - let mut warnings = Vec::new(); - self.config.profiles()?.validate(features, &mut warnings)?; - for warning in warnings { - self.config.shell().warn(&warning)?; - } - // The rest of the checks require a VirtualManifest or multiple members. if self.root_manifest.is_none() { return Ok(()); diff --git a/src/cargo/ops/cargo_clean.rs b/src/cargo/ops/cargo_clean.rs index 6ceef99d514..88be9d4070e 100644 --- a/src/cargo/ops/cargo_clean.rs +++ b/src/cargo/ops/cargo_clean.rs @@ -1,13 +1,12 @@ +use crate::core::InternedString; use std::collections::HashMap; use std::fs; use std::path::Path; use crate::core::compiler::unit_dependencies; use crate::core::compiler::UnitInterner; -use crate::core::compiler::{ - BuildConfig, BuildContext, CompileKind, CompileMode, Context, ProfileKind, -}; -use crate::core::profiles::UnitFor; +use crate::core::compiler::{BuildConfig, BuildContext, CompileKind, CompileMode, Context}; +use crate::core::profiles::{Profiles, UnitFor}; use crate::core::Workspace; use crate::ops; use crate::util::errors::{CargoResult, CargoResultExt}; @@ -23,7 +22,7 @@ pub struct CleanOptions<'a> { /// Whether to clean the release directory pub profile_specified: bool, /// Whether to clean the directory of a certain build profile - pub profile_kind: ProfileKind, + pub requested_profile: InternedString, /// Whether to just clean the doc directory pub doc: bool, } @@ -39,16 +38,13 @@ pub fn clean(ws: &Workspace<'_>, opts: &CleanOptions<'_>) -> CargoResult<()> { return rm_rf(&target_dir.into_path_unlocked(), config); } - let profiles = ws.profiles(); - - // Check for whether the profile is defined. - let _ = profiles.base_profile(&opts.profile_kind)?; + let profiles = Profiles::new(ws.profiles(), config, opts.requested_profile, ws.features())?; if opts.profile_specified { // After parsing profiles we know the dir-name of the profile, if a profile // was passed from the command line. If so, delete only the directory of // that profile. - let dir_name = profiles.get_dir_name(&opts.profile_kind); + let dir_name = profiles.get_dir_name(); target_dir = target_dir.join(dir_name); } @@ -64,8 +60,7 @@ pub fn clean(ws: &Workspace<'_>, opts: &CleanOptions<'_>) -> CargoResult<()> { let interner = UnitInterner::new(); let mut build_config = BuildConfig::new(config, Some(1), &opts.target, CompileMode::Build)?; - let profile_kind = opts.profile_kind.clone(); - build_config.profile_kind = profile_kind.clone(); + build_config.requested_profile = opts.requested_profile; let bcx = BuildContext::new( ws, &packages, @@ -88,20 +83,19 @@ pub fn clean(ws: &Workspace<'_>, opts: &CleanOptions<'_>) -> CargoResult<()> { for mode in CompileMode::all_modes() { for unit_for in UnitFor::all_values() { let profile = if mode.is_run_custom_build() { - profiles.get_profile_run_custom_build(&profiles.get_profile( - pkg.package_id(), - ws.is_member(pkg), - *unit_for, - CompileMode::Build, - profile_kind.clone(), - )) + bcx.profiles + .get_profile_run_custom_build(&bcx.profiles.get_profile( + pkg.package_id(), + ws.is_member(pkg), + *unit_for, + CompileMode::Build, + )) } else { - profiles.get_profile( + bcx.profiles.get_profile( pkg.package_id(), ws.is_member(pkg), *unit_for, *mode, - profile_kind.clone(), ) }; let features = resolve.features_sorted(pkg.package_id()); diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 0f9a871bd3a..04ef8729856 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -300,10 +300,12 @@ pub fn compile_ws<'a>( } } - let profiles = ws.profiles(); - - // Early check for whether the profile is defined. - let _ = profiles.base_profile(&build_config.profile_kind)?; + let profiles = Profiles::new( + ws.profiles(), + config, + build_config.requested_profile, + ws.features(), + )?; let specs = spec.to_package_id_specs(ws)?; let dev_deps = ws.require_optional_deps() || filter.need_dev_deps(build_config.mode); @@ -381,6 +383,7 @@ pub fn compile_ws<'a>( } profiles.validate_packages( + ws.profiles(), &mut config.shell(), workspace_resolve.as_ref().unwrap_or(&resolve), )?; @@ -397,7 +400,6 @@ pub fn compile_ws<'a>( )?; let units = generate_targets( ws, - profiles, &to_builds, filter, build_config.requested_kind, @@ -652,7 +654,6 @@ struct Proposal<'a> { /// compile. Dependencies for these targets are computed later in `unit_dependencies`. fn generate_targets<'a>( ws: &Workspace<'_>, - profiles: &Profiles, packages: &[&'a Package], filter: &CompileFilter, default_arch_kind: CompileKind, @@ -716,13 +717,9 @@ fn generate_targets<'a>( _ => target_mode, }; let kind = default_arch_kind.for_target(target); - let profile = profiles.get_profile( - pkg.package_id(), - ws.is_member(pkg), - unit_for, - target_mode, - bcx.build_config.profile_kind.clone(), - ); + let profile = + bcx.profiles + .get_profile(pkg.package_id(), ws.is_member(pkg), unit_for, target_mode); let features = resolve.features_sorted(pkg.package_id()); bcx.units.intern( pkg, diff --git a/src/cargo/ops/common_for_install_and_uninstall.rs b/src/cargo/ops/common_for_install_and_uninstall.rs index 4c03fd5803f..18a882eeb62 100644 --- a/src/cargo/ops/common_for_install_and_uninstall.rs +++ b/src/cargo/ops/common_for_install_and_uninstall.rs @@ -427,7 +427,7 @@ impl CrateListingV2 { info.features = feature_set(&opts.features); info.all_features = opts.all_features; info.no_default_features = opts.no_default_features; - info.profile = opts.build_config.profile_name().to_string(); + info.profile = opts.build_config.requested_profile.to_string(); info.target = Some(target.to_string()); info.rustc = Some(rustc.to_string()); } else { @@ -439,7 +439,7 @@ impl CrateListingV2 { features: feature_set(&opts.features), all_features: opts.all_features, no_default_features: opts.no_default_features, - profile: opts.build_config.profile_name().to_string(), + profile: opts.build_config.requested_profile.to_string(), target: Some(target.to_string()), rustc: Some(rustc.to_string()), other: BTreeMap::new(), @@ -499,7 +499,7 @@ impl InstallInfo { self.features == feature_set(&opts.features) && self.all_features == opts.all_features && self.no_default_features == opts.no_default_features - && self.profile == opts.build_config.profile_name() + && self.profile.as_str() == opts.build_config.requested_profile.as_str() && (self.target.is_none() || self.target.as_ref().map(|t| t.as_ref()) == Some(target)) && &self.bins == exes } diff --git a/src/cargo/util/command_prelude.rs b/src/cargo/util/command_prelude.rs index 1a93e203a3d..a3477b4575a 100644 --- a/src/cargo/util/command_prelude.rs +++ b/src/cargo/util/command_prelude.rs @@ -1,4 +1,5 @@ use crate::core::compiler::{BuildConfig, MessageFormat}; +use crate::core::InternedString; use crate::core::Workspace; use crate::ops::{CompileFilter, CompileOptions, NewOptions, Packages, VersionControl}; use crate::sources::CRATES_IO_REGISTRY; @@ -15,7 +16,7 @@ use std::ffi::{OsStr, OsString}; use std::fs; use std::path::PathBuf; -pub use crate::core::compiler::{CompileMode, ProfileKind}; +pub use crate::core::compiler::CompileMode; pub use crate::{CliError, CliResult, Config}; pub use clap::{AppSettings, Arg, ArgMatches}; @@ -307,19 +308,17 @@ pub trait ArgMatchesExt { self._value_of("target").map(|s| s.to_string()) } - fn get_profile_kind( + fn get_profile_name( &self, config: &Config, - default: ProfileKind, + default: &str, profile_checking: ProfileChecking, - ) -> CargoResult<ProfileKind> { + ) -> CargoResult<InternedString> { let specified_profile = match self._value_of("profile") { None => None, - Some("dev") => Some(ProfileKind::Dev), - Some("release") => Some(ProfileKind::Release), Some(name) => { TomlProfile::validate_name(name, "profile name")?; - Some(ProfileKind::Custom(name.to_string())) + Some(InternedString::new(name)) } }; @@ -334,24 +333,28 @@ pub trait ArgMatchesExt { if self._is_present("release") { if !config.cli_unstable().unstable_options { - Ok(ProfileKind::Release) + Ok(InternedString::new("release")) } else { match specified_profile { - None | Some(ProfileKind::Release) => Ok(ProfileKind::Release), - _ => anyhow::bail!("Conflicting usage of --profile and --release"), + Some(name) if name != "release" => { + anyhow::bail!("Conflicting usage of --profile and --release") + } + _ => Ok(InternedString::new("release")), } } } else if self._is_present("debug") { if !config.cli_unstable().unstable_options { - Ok(ProfileKind::Dev) + Ok(InternedString::new("dev")) } else { match specified_profile { - None | Some(ProfileKind::Dev) => Ok(ProfileKind::Dev), - _ => anyhow::bail!("Conflicting usage of --profile and --debug"), + Some(name) if name != "dev" => { + anyhow::bail!("Conflicting usage of --profile and --debug") + } + _ => Ok(InternedString::new("dev")), } } } else { - Ok(specified_profile.unwrap_or(default)) + Ok(specified_profile.unwrap_or_else(|| InternedString::new(default))) } } @@ -433,8 +436,7 @@ pub trait ArgMatchesExt { let mut build_config = BuildConfig::new(config, self.jobs()?, &self.target(), mode)?; build_config.message_format = message_format.unwrap_or(MessageFormat::Human); - build_config.profile_kind = - self.get_profile_kind(config, ProfileKind::Dev, profile_checking)?; + build_config.requested_profile = self.get_profile_name(config, "dev", profile_checking)?; build_config.build_plan = self._is_present("build-plan"); if build_config.build_plan { config diff --git a/src/cargo/util/config/de.rs b/src/cargo/util/config/de.rs index 8cd3e5ff04e..32e8ee6d93a 100644 --- a/src/cargo/util/config/de.rs +++ b/src/cargo/util/config/de.rs @@ -425,13 +425,11 @@ impl<'config> ValueDeserializer<'config> { cv.definition().clone() } } - (true, None) => env_def, (false, Some(cv)) => cv.definition().clone(), - (false, None) => { - return Err( - anyhow::format_err!("failed to find definition of `{}`", de.key).into(), - ) - } + // Assume it is an environment, even if the key is not set. + // This can happen for intermediate tables, like + // CARGO_FOO_BAR_* where `CARGO_FOO_BAR` is not set. + (_, None) => env_def, } }; Ok(ValueDeserializer { diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index 08c5ccb8b21..4dca35e49aa 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -70,7 +70,6 @@ use serde::Deserialize; use url::Url; use self::ConfigValue as CV; -use crate::core::profiles::ConfigProfiles; use crate::core::shell::Verbosity; use crate::core::{nightly_features_allowed, CliUnstable, Shell, SourceId, Workspace}; use crate::ops; @@ -163,8 +162,6 @@ pub struct Config { target_dir: Option<Filesystem>, /// Environment variables, separated to assist testing. env: HashMap<String, String>, - /// Profiles loaded from config. - profiles: LazyCell<ConfigProfiles>, /// Tracks which sources have been updated to avoid multiple updates. updated_sources: LazyCell<RefCell<HashSet<SourceId>>>, /// Lock, if held, of the global package cache along with the number of @@ -238,7 +235,6 @@ impl Config { creation_time: Instant::now(), target_dir: None, env, - profiles: LazyCell::new(), updated_sources: LazyCell::new(), package_cache_lock: RefCell::new(None), http_config: LazyCell::new(), @@ -372,26 +368,6 @@ impl Config { .map(AsRef::as_ref) } - /// Gets profiles defined in config files. - pub fn profiles(&self) -> CargoResult<&ConfigProfiles> { - self.profiles.try_borrow_with(|| { - let ocp = self.get::<Option<ConfigProfiles>>("profile")?; - if let Some(config_profiles) = ocp { - // Warn if config profiles without CLI option. - if !self.cli_unstable().config_profile { - self.shell().warn( - "profiles in config files require `-Z config-profile` \ - command-line option", - )?; - return Ok(ConfigProfiles::default()); - } - Ok(config_profiles) - } else { - Ok(ConfigProfiles::default()) - } - }) - } - /// Which package sources have been updated, used to ensure it is only done once. pub fn updated_sources(&self) -> RefMut<'_, HashSet<SourceId>> { self.updated_sources diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index f59691d5b73..7aef4cbc010 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -16,7 +16,6 @@ use url::Url; use crate::core::dependency::DepKind; use crate::core::manifest::{LibKind, ManifestMetadata, TargetSourcePath, Warnings}; -use crate::core::profiles::Profiles; use crate::core::{Dependency, InternedString, Manifest, PackageId, Summary, Target}; use crate::core::{Edition, EitherManifest, Feature, Features, VirtualManifest}; use crate::core::{GitReference, PackageIdSpec, SourceId, WorkspaceConfig, WorkspaceRootConfig}; @@ -270,23 +269,19 @@ pub struct TomlManifest { } #[derive(Deserialize, Serialize, Clone, Debug, Default)] -pub struct TomlProfiles(BTreeMap<String, TomlProfile>); +pub struct TomlProfiles(BTreeMap<InternedString, TomlProfile>); impl TomlProfiles { - pub fn get_all(&self) -> &BTreeMap<String, TomlProfile> { + pub fn get_all(&self) -> &BTreeMap<InternedString, TomlProfile> { &self.0 } - pub fn get(&self, name: &'static str) -> Option<&TomlProfile> { - self.0.get(&String::from(name)) + pub fn get(&self, name: &str) -> Option<&TomlProfile> { + self.0.get(name) } pub fn validate(&self, features: &Features, warnings: &mut Vec<String>) -> CargoResult<()> { for (name, profile) in &self.0 { - if name == "debug" { - warnings.push("use `[profile.dev]` to configure debug builds".to_string()); - } - profile.validate(name, features, warnings)?; } Ok(()) @@ -408,13 +403,10 @@ pub struct TomlProfile { pub panic: Option<String>, pub overflow_checks: Option<bool>, pub incremental: Option<bool>, - // `overrides` has been renamed to `package`, this should be removed when - // stabilized. - pub overrides: Option<BTreeMap<ProfilePackageSpec, TomlProfile>>, pub package: Option<BTreeMap<ProfilePackageSpec, TomlProfile>>, pub build_override: Option<Box<TomlProfile>>, - pub dir_name: Option<String>, - pub inherits: Option<String>, + pub dir_name: Option<InternedString>, + pub inherits: Option<InternedString>, } #[derive(Clone, Debug, PartialEq, Eq, Ord, PartialOrd, Hash)] @@ -458,21 +450,14 @@ impl TomlProfile { features: &Features, warnings: &mut Vec<String>, ) -> CargoResult<()> { + if name == "debug" { + warnings.push("use `[profile.dev]` to configure debug builds".to_string()); + } + if let Some(ref profile) = self.build_override { features.require(Feature::profile_overrides())?; profile.validate_override("build-override")?; } - if let Some(ref override_map) = self.overrides { - warnings.push( - "profile key `overrides` has been renamed to `package`, \ - please update the manifest to the new key name" - .to_string(), - ); - features.require(Feature::profile_overrides())?; - for profile in override_map.values() { - profile.validate_override("package")?; - } - } if let Some(ref packages) = self.package { features.require(Feature::profile_overrides())?; for profile in packages.values() { @@ -570,7 +555,7 @@ impl TomlProfile { } fn validate_override(&self, which: &str) -> CargoResult<()> { - if self.overrides.is_some() || self.package.is_some() { + if self.package.is_some() { bail!("package-specific profiles cannot be nested"); } if self.build_override.is_some() { @@ -588,6 +573,7 @@ impl TomlProfile { Ok(()) } + /// Overwrite self's values with the given profile. pub fn merge(&mut self, profile: &TomlProfile) { if let Some(v) = &profile.opt_level { self.opt_level = Some(v.clone()); @@ -625,16 +611,27 @@ impl TomlProfile { self.incremental = Some(v); } - if let Some(v) = &profile.overrides { - self.overrides = Some(v.clone()); - } - - if let Some(v) = &profile.package { - self.package = Some(v.clone()); + if let Some(other_package) = &profile.package { + match &mut self.package { + Some(self_package) => { + for (spec, other_pkg_profile) in other_package { + match self_package.get_mut(spec) { + Some(p) => p.merge(other_pkg_profile), + None => { + self_package.insert(spec.clone(), other_pkg_profile.clone()); + } + } + } + } + None => self.package = Some(other_package.clone()), + } } - if let Some(v) = &profile.build_override { - self.build_override = Some(v.clone()); + if let Some(other_bo) = &profile.build_override { + match &mut self.build_override { + Some(self_bo) => self_bo.merge(other_bo), + None => self.build_override = Some(other_bo.clone()), + } } if let Some(v) = &profile.inherits { @@ -1173,7 +1170,10 @@ impl TomlManifest { `[workspace]`, only one can be specified" ), }; - let profiles = Profiles::new(me.profile.as_ref(), config, &features, &mut warnings)?; + let profiles = me.profile.clone(); + if let Some(profiles) = &profiles { + profiles.validate(&features, &mut warnings)?; + } let publish = match project.publish { Some(VecStringOrBool::VecString(ref vecstring)) => Some(vecstring.clone()), Some(VecStringOrBool::Bool(false)) => Some(vec![]), @@ -1321,7 +1321,10 @@ impl TomlManifest { }; (me.replace(&mut cx)?, me.patch(&mut cx)?) }; - let profiles = Profiles::new(me.profile.as_ref(), config, &features, &mut warnings)?; + let profiles = me.profile.clone(); + if let Some(profiles) = &profiles { + profiles.validate(&features, &mut warnings)?; + } let workspace_config = match me.workspace { Some(ref config) => WorkspaceConfig::Root(WorkspaceRootConfig::new( root, diff --git a/tests/testsuite/config.rs b/tests/testsuite/config.rs index 788cdba9a5f..fdbc9339bca 100644 --- a/tests/testsuite/config.rs +++ b/tests/testsuite/config.rs @@ -1,5 +1,11 @@ //! Tests for config settings. +use cargo::core::{enable_nightly_features, InternedString, Shell}; +use cargo::util::config::{self, Config, SslVersionConfig, StringList}; +use cargo::util::toml::{self, VecStringOrBool as VSOB}; +use cargo::CargoResult; +use cargo_test_support::{normalized_lines_match, paths, project, t}; +use serde::Deserialize; use std::borrow::Borrow; use std::collections::{BTreeMap, HashMap}; use std::fs; @@ -7,13 +13,6 @@ use std::io; use std::os; use std::path::{Path, PathBuf}; -use cargo::core::{enable_nightly_features, Shell}; -use cargo::util::config::{self, Config, SslVersionConfig, StringList}; -use cargo::util::toml::{self, VecStringOrBool as VSOB}; -use cargo::CargoResult; -use cargo_test_support::{normalized_lines_match, paths, project, t}; -use serde::Deserialize; - /// Helper for constructing a `Config` object. pub struct ConfigBuilder { env: HashMap<String, String>, @@ -424,8 +423,8 @@ lto = false p, toml::TomlProfile { lto: Some(toml::StringOrBool::Bool(false)), - dir_name: Some("without-lto".to_string()), - inherits: Some("dev".to_string()), + dir_name: Some(InternedString::new("without-lto")), + inherits: Some(InternedString::new("dev")), ..Default::default() } ); diff --git a/tests/testsuite/profile_config.rs b/tests/testsuite/profile_config.rs index 1091458eaa2..685cc3facda 100644 --- a/tests/testsuite/profile_config.rs +++ b/tests/testsuite/profile_config.rs @@ -1,5 +1,6 @@ //! Tests for profiles defined in config files. +use cargo_test_support::paths::CargoPathExt; use cargo_test_support::{basic_lib_manifest, paths, project}; #[cargo_test] @@ -19,13 +20,44 @@ fn profile_config_gated() { p.cargo("build -v") .with_stderr_contains( "\ -[WARNING] profiles in config files require `-Z config-profile` command-line option +[WARNING] config profiles require the `-Z config-profile` command-line option \ + (found profile `dev` in [..]/foo/.cargo/config) ", ) .with_stderr_contains("[..]-C debuginfo=2[..]") .run(); } +#[cargo_test] +fn named_profile_gated() { + // Named profile in config requires enabling in Cargo.toml. + let p = project() + .file("src/lib.rs", "") + .file( + ".cargo/config", + r#" + [profile.foo] + inherits = 'dev' + opt-level = 1 + "#, + ) + .build(); + p.cargo("build --profile foo -Zunstable-options -Zconfig-profile") + .masquerade_as_nightly_cargo() + .with_stderr( + "\ +[ERROR] config profile `foo` is not valid (defined in `[..]/foo/.cargo/config`) + +Caused by: + feature `named-profiles` is required + +consider adding `cargo-features = [\"named-profiles\"]` to the manifest +", + ) + .with_status(101) + .run(); +} + #[cargo_test] fn profile_config_validate_warnings() { let p = project() @@ -56,8 +88,6 @@ fn profile_config_validate_warnings() { .masquerade_as_nightly_cargo() .with_stderr_unordered( "\ -[WARNING] unused config key `profile.asdf` in `[..].cargo/config` -[WARNING] unused config key `profile.test` in `[..].cargo/config` [WARNING] unused config key `profile.dev.bad-key` in `[..].cargo/config` [WARNING] unused config key `profile.dev.package.bar.bad-key-bar` in `[..].cargo/config` [WARNING] unused config key `profile.dev.build-override.bad-key-bo` in `[..].cargo/config` @@ -70,6 +100,7 @@ fn profile_config_validate_warnings() { #[cargo_test] fn profile_config_error_paths() { + // Errors in config show where the error is located. let p = project() .file("Cargo.toml", &basic_lib_manifest("foo")) .file("src/lib.rs", "") @@ -94,10 +125,10 @@ fn profile_config_error_paths() { .with_status(101) .with_stderr( "\ -[ERROR] failed to parse manifest at `[CWD]/Cargo.toml` - -Caused by: - error in [..].cargo/config: `profile.dev.rpath` expected true/false, but found a string +[ERROR] error in [..]/foo/.cargo/config: \ + could not load config key `profile.dev`: \ + error in [..]/home/.cargo/config: \ + `profile.dev.rpath` expected true/false, but found a string ", ) .run(); @@ -122,7 +153,7 @@ fn profile_config_validate_errors() { .with_status(101) .with_stderr( "\ -[ERROR] config profile `profile.dev` is not valid +[ERROR] config profile `dev` is not valid (defined in `[..]/foo/.cargo/config`) Caused by: `panic` may not be specified in a `package` profile @@ -150,10 +181,10 @@ fn profile_config_syntax_errors() { .with_status(101) .with_stderr( "\ -[ERROR] failed to parse manifest at [..] - -Caused by: - error in [..].cargo/config: `profile.dev.codegen-units` expected an integer, but found a string +[ERROR] error in [..]/foo/.cargo/config: \ + could not load config key `profile.dev`: \ + error in [..]/foo/.cargo/config: \ + `profile.dev.codegen-units` expected an integer, but found a string ", ) .run(); @@ -337,3 +368,123 @@ fn profile_config_mixed_types() { .with_stderr_contains("[..]-C opt-level=3 [..]") .run(); } + +#[cargo_test] +fn named_config_profile() { + // Exercises config named profies. + // foo -> middle -> bar -> dev + // middle exists in Cargo.toml, the others in .cargo/config + use super::config::ConfigBuilder; + use cargo::core::compiler::CompileMode; + use cargo::core::enable_nightly_features; + use cargo::core::features::Features; + use cargo::core::profiles::{Profiles, UnitFor}; + use cargo::core::{InternedString, PackageId}; + use cargo::util::toml::TomlProfiles; + use std::fs; + enable_nightly_features(); + paths::root().join(".cargo").mkdir_p(); + fs::write( + paths::root().join(".cargo/config"), + r#" + [profile.foo] + inherits = "middle" + codegen-units = 2 + [profile.foo.build-override] + codegen-units = 6 + [profile.foo.package.dep] + codegen-units = 7 + + [profile.middle] + inherits = "bar" + codegen-units = 3 + + [profile.bar] + inherits = "dev" + codegen-units = 4 + debug = 1 + "#, + ) + .unwrap(); + let config = ConfigBuilder::new().unstable_flag("config-profile").build(); + let mut warnings = Vec::new(); + let features = Features::new(&["named-profiles".to_string()], &mut warnings).unwrap(); + assert_eq!(warnings.len(), 0); + let profile_name = InternedString::new("foo"); + let toml = r#" + [profile.middle] + inherits = "bar" + codegen-units = 1 + opt-level = 1 + [profile.middle.package.dep] + overflow-checks = false + + [profile.foo.build-override] + codegen-units = 5 + debug-assertions = false + [profile.foo.package.dep] + codegen-units = 8 + "#; + #[derive(serde::Deserialize)] + struct TomlManifest { + profile: Option<TomlProfiles>, + } + let manifest: TomlManifest = toml::from_str(toml).unwrap(); + let profiles = + Profiles::new(manifest.profile.as_ref(), &config, profile_name, &features).unwrap(); + + let crates_io = cargo::core::source::SourceId::crates_io(&config).unwrap(); + let a_pkg = PackageId::new("a", "0.1.0", crates_io).unwrap(); + let dep_pkg = PackageId::new("dep", "0.1.0", crates_io).unwrap(); + + // normal package + let p = profiles.get_profile(a_pkg, true, UnitFor::new_normal(), CompileMode::Build); + assert_eq!(p.name, "foo"); + assert_eq!(p.codegen_units, Some(2)); // "foo" from config + assert_eq!(p.opt_level, "1"); // "middle" from manifest + assert_eq!(p.debuginfo, Some(1)); // "bar" from config + assert_eq!(p.debug_assertions, true); // "dev" built-in (ignore build-override) + assert_eq!(p.overflow_checks, true); // "dev" built-in (ignore package override) + + // build-override + let bo = profiles.get_profile(a_pkg, true, UnitFor::new_build(), CompileMode::Build); + assert_eq!(bo.name, "foo"); + assert_eq!(bo.codegen_units, Some(6)); // "foo" build override from config + assert_eq!(bo.opt_level, "1"); // SAME as normal + assert_eq!(bo.debuginfo, Some(1)); // SAME as normal + assert_eq!(bo.debug_assertions, false); // "foo" build override from manifest + assert_eq!(bo.overflow_checks, true); // SAME as normal + + // package overrides + let po = profiles.get_profile(dep_pkg, false, UnitFor::new_normal(), CompileMode::Build); + assert_eq!(po.name, "foo"); + assert_eq!(po.codegen_units, Some(7)); // "foo" package override from config + assert_eq!(po.opt_level, "1"); // SAME as normal + assert_eq!(po.debuginfo, Some(1)); // SAME as normal + assert_eq!(po.debug_assertions, true); // SAME as normal + assert_eq!(po.overflow_checks, false); // "middle" package override from manifest +} + +#[cargo_test] +fn named_env_profile() { + // Environment variables used to define a named profile. + let p = project() + .file( + "Cargo.toml", + r#" + cargo-features = ["named-profiles"] + [package] + name = "foo" + version = "0.1.0" + "#, + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("build -v -Zconfig-profile -Zunstable-options --profile=other") + .masquerade_as_nightly_cargo() + .env("CARGO_PROFILE_OTHER_CODEGEN_UNITS", "1") + .env("CARGO_PROFILE_OTHER_INHERITS", "dev") + .with_stderr_contains("[..]-C codegen-units=1 [..]") + .run(); +} diff --git a/tests/testsuite/profile_custom.rs b/tests/testsuite/profile_custom.rs index 740bee2b7c9..3e88f736376 100644 --- a/tests/testsuite/profile_custom.rs +++ b/tests/testsuite/profile_custom.rs @@ -27,10 +27,7 @@ fn inherits_on_release() { .with_status(101) .with_stderr( "\ -[ERROR] failed to parse manifest at [..] - -Caused by: - An 'inherits' must not specified root profile 'release' +[ERROR] `inherits` must not be specified in root profile `release` ", ) .run(); @@ -61,10 +58,9 @@ fn missing_inherits() { .with_status(101) .with_stderr( "\ -[ERROR] failed to parse manifest at [..] - -Caused by: - An 'inherits' directive is needed for all profiles that are not 'dev' or 'release'. Here it is missing from 'release-lto'", +[ERROR] profile `release-lto` is missing an `inherits` directive \ + (`inherits` is required for all profiles except `dev` or `release`) +", ) .run(); } @@ -198,10 +194,8 @@ fn non_existent_inherits() { .with_status(101) .with_stderr( "\ -[ERROR] failed to parse manifest at [..] - -Caused by: - Profile 'non-existent' not found in Cargo.toml", +[ERROR] profile `release-lto` inherits from `non-existent`, but that profile is not defined +", ) .run(); } @@ -232,10 +226,8 @@ fn self_inherits() { .with_status(101) .with_stderr( "\ -[ERROR] failed to parse manifest at [..] - -Caused by: - Inheritance loop of profiles cycles with profile 'release-lto'", +[ERROR] profile inheritance loop detected with profile `release-lto` inheriting `release-lto` +", ) .run(); } @@ -270,10 +262,8 @@ fn inherits_loop() { .with_status(101) .with_stderr( "\ -[ERROR] failed to parse manifest at [..] - -Caused by: - Inheritance loop of profiles cycles with profile 'release-lto'", +[ERROR] profile inheritance loop detected with profile `release-lto2` inheriting `release-lto` +", ) .run(); } @@ -477,3 +467,32 @@ fn clean_custom_dirname() { assert!(p.build_dir().join("debug").is_dir()); assert!(!p.build_dir().join("other").is_dir()); } + +#[cargo_test] +fn unknown_profile() { + let p = project() + .file( + "Cargo.toml", + r#" + cargo-features = ["named-profiles"] + + [package] + name = "foo" + version = "0.0.1" + "#, + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("build --profile alpha -Zunstable-options") + .masquerade_as_nightly_cargo() + .with_stderr("[ERROR] profile `alpha` is not defined") + .with_status(101) + .run(); + // Clean has a separate code path, need to check it too. + p.cargo("clean --profile alpha -Zunstable-options") + .masquerade_as_nightly_cargo() + .with_stderr("[ERROR] profile `alpha` is not defined") + .with_status(101) + .run(); +} diff --git a/tests/testsuite/profile_overrides.rs b/tests/testsuite/profile_overrides.rs index 3ed8ac021ae..59766187ae5 100644 --- a/tests/testsuite/profile_overrides.rs +++ b/tests/testsuite/profile_overrides.rs @@ -68,13 +68,16 @@ fn profile_override_warnings() { .file("bar/src/lib.rs", "") .build(); - p.cargo("build").with_stderr_contains( + p.cargo("build") + .with_stderr_contains( "\ -[WARNING] version or URL in package profile spec `bar:1.2.3` does not match any of the packages: bar v0.5.0 ([..]) -[WARNING] package profile spec `bart` did not match any packages +[WARNING] profile package spec `bar:1.2.3` in profile `dev` \ + has a version or URL that does not match any of the packages: \ + bar v0.5.0 ([..]/foo/bar) +[WARNING] profile package spec `bart` in profile `dev` did not match any packages <tab>Did you mean `bar`? -[WARNING] package profile spec `no-suggestion` did not match any packages +[WARNING] profile package spec `no-suggestion` in profile `dev` did not match any packages [COMPILING] [..] ", ) @@ -96,10 +99,7 @@ fn profile_override_bad_settings() { "rpath = true", "`rpath` may not be specified in a `package` profile", ), - ( - "overrides = {}", - "package-specific profiles cannot be nested", - ), + ("package = {}", "package-specific profiles cannot be nested"), ]; for &(snippet, expected) in bad_values.iter() { let p = project() @@ -394,42 +394,6 @@ fn override_proc_macro() { .run(); } -#[cargo_test] -fn override_package_rename() { - // backwards-compatibility test - let p = project() - .file( - "Cargo.toml", - r#" - [package] - name = "foo" - version = "0.1.0" - - [dependencies] - bar = {path = "bar"} - - [profile.dev] - opt-level = 1 - - [profile.dev.overrides.bar] - opt-level = 3 - "#, - ) - .file("src/lib.rs", "") - .file("bar/Cargo.toml", &basic_lib_manifest("bar")) - .file("bar/src/lib.rs", "") - .build(); - - p.cargo("check") - .with_stderr("\ -[WARNING] profile key `overrides` has been renamed to `package`, please update the manifest to the new key name -[CHECKING] bar [..] -[CHECKING] foo [..] -[FINISHED] [..] -") - .run(); -} - #[cargo_test] fn no_warning_ws() { // https://github.com/rust-lang/cargo/issues/7378, avoid warnings in a workspace. From dafacbb76b60991b9d6b98fc7197429fc69bdf72 Mon Sep 17 00:00:00 2001 From: Eric Huss <eric@huss.org> Date: Mon, 13 Jan 2020 13:36:20 -0800 Subject: [PATCH 2/2] Update tests for formatting changes due to `anyhow` changes. --- tests/testsuite/profile_config.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/testsuite/profile_config.rs b/tests/testsuite/profile_config.rs index 685cc3facda..8d389e56776 100644 --- a/tests/testsuite/profile_config.rs +++ b/tests/testsuite/profile_config.rs @@ -125,10 +125,10 @@ fn profile_config_error_paths() { .with_status(101) .with_stderr( "\ -[ERROR] error in [..]/foo/.cargo/config: \ - could not load config key `profile.dev`: \ - error in [..]/home/.cargo/config: \ - `profile.dev.rpath` expected true/false, but found a string +[ERROR] error in [..]/foo/.cargo/config: could not load config key `profile.dev` + +Caused by: + error in [..]/home/.cargo/config: `profile.dev.rpath` expected true/false, but found a string ", ) .run(); @@ -181,10 +181,10 @@ fn profile_config_syntax_errors() { .with_status(101) .with_stderr( "\ -[ERROR] error in [..]/foo/.cargo/config: \ - could not load config key `profile.dev`: \ - error in [..]/foo/.cargo/config: \ - `profile.dev.codegen-units` expected an integer, but found a string +[ERROR] error in [..]/.cargo/config: could not load config key `profile.dev` + +Caused by: + error in [..]/foo/.cargo/config: `profile.dev.codegen-units` expected an integer, but found a string ", ) .run();