From 2f7b52259b93c6591aed73ab18dbaf8256c035fe Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Sun, 6 May 2018 15:14:46 -0700 Subject: [PATCH 1/2] Config Profiles (RFC 2282 Part 2) Notes: - `-Z config-profile` CLI option is required to use. - Config values no longer reject mixed base types (integer, string, boolean) in order to support the mixed types in profiles. --- src/bin/cargo/cli.rs | 1 + src/cargo/core/features.rs | 2 + src/cargo/core/profiles.rs | 246 +++++++++++++++----- src/cargo/util/config.rs | 11 +- src/cargo/util/toml/mod.rs | 107 ++++++--- src/doc/src/reference/unstable.md | 20 ++ tests/testsuite/bad_config.rs | 40 ---- tests/testsuite/main.rs | 1 + tests/testsuite/profile_config.rs | 369 ++++++++++++++++++++++++++++++ 9 files changed, 657 insertions(+), 140 deletions(-) create mode 100644 tests/testsuite/profile_config.rs diff --git a/src/bin/cargo/cli.rs b/src/bin/cargo/cli.rs index 9fbd12cbc8e..41d99292368 100644 --- a/src/bin/cargo/cli.rs +++ b/src/bin/cargo/cli.rs @@ -21,6 +21,7 @@ Available unstable (nightly-only) flags: -Z no-index-update -- Do not update the registry, avoids a network request for benchmarking -Z offline -- Offline mode that does not perform network requests -Z unstable-options -- Allow the usage of unstable options such as --registry + -Z config-profile -- Read profiles from .cargo/config files Run with 'cargo -Z [FLAG] [SUBCOMMAND]'" ); diff --git a/src/cargo/core/features.rs b/src/cargo/core/features.rs index 29e644be390..c08d4cdddbe 100644 --- a/src/cargo/core/features.rs +++ b/src/cargo/core/features.rs @@ -309,6 +309,7 @@ pub struct CliUnstable { pub minimal_versions: bool, pub package_features: bool, pub advanced_env: bool, + pub config_profile: bool, } impl CliUnstable { @@ -344,6 +345,7 @@ impl CliUnstable { "minimal-versions" => self.minimal_versions = true, "package-features" => self.package_features = true, "advanced-env" => self.advanced_env = true, + "config-profile" => self.config_profile = true, _ => bail!("unknown `-Z` flag specified: {}", k), } diff --git a/src/cargo/core/profiles.rs b/src/cargo/core/profiles.rs index a83ed755c99..c9feafaed7f 100644 --- a/src/cargo/core/profiles.rs +++ b/src/cargo/core/profiles.rs @@ -1,12 +1,13 @@ -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; +use std::sync::atomic; use std::{cmp, fmt, hash}; use core::compiler::CompileMode; use core::interning::InternedString; -use core::{PackageId, PackageIdSpec, PackageSet, Shell}; +use core::{Features, PackageId, PackageIdSpec, PackageSet, Shell}; use util::lev_distance::lev_distance; -use util::toml::{ProfilePackageSpec, StringOrBool, TomlProfile, U32OrBool}; -use util::CargoResult; +use util::toml::{ProfilePackageSpec, StringOrBool, TomlProfile, TomlProfiles, U32OrBool}; +use util::{CargoResult, Config, ConfigValue}; /// Collection of all user profiles. #[derive(Clone, Debug)] @@ -20,34 +21,43 @@ pub struct Profiles { impl Profiles { pub fn new( - dev: Option, - release: Option, - test: Option, - bench: Option, - doc: Option, - ) -> Profiles { - Profiles { + profiles: Option<&TomlProfiles>, + config: &Config, + features: &Features, + warnings: &mut Vec, + ) -> CargoResult { + if let Some(profiles) = profiles { + profiles.validate(features, warnings)?; + } + Profiles::validate_config(config, warnings)?; + + Ok(Profiles { dev: ProfileMaker { default: Profile::default_dev(), - toml: dev, + toml: profiles.and_then(|p| p.dev.clone()), + config: TomlProfile::from_config(config, "dev", warnings)?, }, release: ProfileMaker { default: Profile::default_release(), - toml: release, + toml: profiles.and_then(|p| p.release.clone()), + config: TomlProfile::from_config(config, "release", warnings)?, }, test: ProfileMaker { default: Profile::default_test(), - toml: test, + toml: profiles.and_then(|p| p.test.clone()), + config: None, }, bench: ProfileMaker { default: Profile::default_bench(), - toml: bench, + toml: profiles.and_then(|p| p.bench.clone()), + config: None, }, doc: ProfileMaker { default: Profile::default_doc(), - toml: doc, + toml: profiles.and_then(|p| p.doc.clone()), + config: None, }, - } + }) } /// Retrieve the profile for a target. @@ -86,7 +96,7 @@ impl Profiles { CompileMode::Bench => &self.bench, CompileMode::Doc { .. } => &self.doc, }; - let mut profile = maker.profile_for(Some(pkg_id), is_member, profile_for); + let mut profile = maker.get_profile(Some(pkg_id), is_member, profile_for); // `panic` should not be set for tests/benches, or any of their // dependencies. if profile_for == ProfileFor::TestDependency || mode.is_any_test() { @@ -112,9 +122,9 @@ impl Profiles { /// select for the package that was actually built. pub fn base_profile(&self, release: bool) -> Profile { if release { - self.release.profile_for(None, true, ProfileFor::Any) + self.release.get_profile(None, true, ProfileFor::Any) } else { - self.dev.profile_for(None, true, ProfileFor::Any) + self.dev.get_profile(None, true, ProfileFor::Any) } } @@ -127,11 +137,77 @@ impl Profiles { self.doc.validate_packages(shell, packages)?; Ok(()) } + + fn validate_config(config: &Config, warnings: &mut Vec) -> CargoResult<()> { + static VALIDATE_ONCE: atomic::AtomicBool = atomic::ATOMIC_BOOL_INIT; + + if VALIDATE_ONCE.swap(true, atomic::Ordering::SeqCst) { + return Ok(()); + } + + // cv: Value> + if let Some(cv) = config.get_table("profile")? { + // Warn if config profiles without CLI option. + if !config.cli_unstable().config_profile { + warnings.push(format!( + "profile in config `{}` requires `-Z config-profile` command-line option", + cv.definition + )); + // Ignore the rest. + return Ok(()); + } + // Warn about unsupported profile names. + for (key, profile_cv) in cv.val.iter() { + if key != "dev" && key != "release" { + warnings.push(format!( + "profile `{}` in config `{}` is not supported", + key, + profile_cv.definition_path().display() + )); + } + } + // Warn about incorrect key names. + for profile_cv in cv.val.values() { + if let ConfigValue::Table(ref profile, _) = *profile_cv { + validate_profile_keys(profile, warnings); + if let Some(&ConfigValue::Table(ref bo_profile, _)) = + profile.get("build-override") + { + validate_profile_keys(bo_profile, warnings); + } + if let Some(&ConfigValue::Table(ref os, _)) = profile.get("overrides") { + for o_profile_cv in os.values() { + if let ConfigValue::Table(ref o_profile, _) = *o_profile_cv { + validate_profile_keys(o_profile, warnings); + } + } + } + } + } + } + return Ok(()); + + fn validate_profile_keys( + profile: &HashMap, + warnings: &mut Vec, + ) { + for (key, value_cv) in profile.iter() { + if !TOML_PROFILE_KEYS.iter().any(|k| k == key) { + warnings.push(format!( + "unused profile key `{}` in config `{}`", + key, + value_cv.definition_path().display() + )); + } + } + } + } } /// An object used for handling the profile override hierarchy. /// /// The precedence of profiles are (first one wins): +/// - Profiles in .cargo/config files (using same order as below). /// - [profile.dev.overrides.name] - A named package. /// - [profile.dev.overrides."*"] - This cannot apply to workspace members. /// - [profile.dev.build-override] - This can only apply to `build.rs` scripts @@ -140,12 +216,16 @@ impl Profiles { /// - Default (hard-coded) values. #[derive(Debug, Clone)] struct ProfileMaker { + /// The starting, hard-coded defaults for the profile. default: Profile, + /// The profile from the `Cargo.toml` manifest. toml: Option, + /// Profile loaded from `.cargo/config` files. + config: Option, } impl ProfileMaker { - fn profile_for( + fn get_profile( &self, pkg_id: Option<&PackageId>, is_member: bool, @@ -153,47 +233,28 @@ impl ProfileMaker { ) -> Profile { let mut profile = self.default; if let Some(ref toml) = self.toml { - merge_profile(&mut profile, toml); - if profile_for == ProfileFor::CustomBuild { - if let Some(ref build_override) = toml.build_override { - merge_profile(&mut profile, build_override); - } - } - if let Some(ref overrides) = toml.overrides { - if !is_member { - if let Some(all) = overrides.get(&ProfilePackageSpec::All) { - merge_profile(&mut profile, all); - } - } - if let Some(pkg_id) = pkg_id { - let mut matches = overrides.iter().filter_map( - |(key, spec_profile)| match key { - &ProfilePackageSpec::All => None, - &ProfilePackageSpec::Spec(ref s) => if s.matches(pkg_id) { - Some(spec_profile) - } else { - None - }, - }, - ); - if let Some(spec_profile) = matches.next() { - merge_profile(&mut profile, spec_profile); - // `validate_packages` should ensure that there are - // no additional matches. - assert!( - matches.next().is_none(), - "package `{}` matched multiple profile overrides", - pkg_id - ); - } - } - } + merge_toml(pkg_id, is_member, profile_for, &mut profile, toml); + } + if let Some(ref toml) = self.config { + merge_toml(pkg_id, is_member, profile_for, &mut profile, toml); } profile } fn validate_packages(&self, shell: &mut Shell, packages: &PackageSet) -> CargoResult<()> { - let toml = match self.toml { + self.validate_packages_toml(shell, packages, &self.toml, true)?; + self.validate_packages_toml(shell, packages, &self.config, false)?; + Ok(()) + } + + fn validate_packages_toml( + &self, + shell: &mut Shell, + packages: &PackageSet, + toml: &Option, + warn_unmatched: bool, + ) -> CargoResult<()> { + let toml = match *toml { Some(ref toml) => toml, None => return Ok(()), }; @@ -206,9 +267,9 @@ impl ProfileMaker { for pkg_id in packages.package_ids() { let matches: Vec<&PackageIdSpec> = overrides .keys() - .filter_map(|key| match key { - &ProfilePackageSpec::All => None, - &ProfilePackageSpec::Spec(ref spec) => if spec.matches(pkg_id) { + .filter_map(|key| match *key { + ProfilePackageSpec::All => None, + ProfilePackageSpec::Spec(ref spec) => if spec.matches(pkg_id) { Some(spec) } else { None @@ -237,9 +298,12 @@ impl ProfileMaker { } } + 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 let ProfilePackageSpec::Spec(ref spec) = *key { if !found.contains(spec) { return Some(spec); } @@ -258,7 +322,7 @@ impl ProfileMaker { } }) .collect(); - if name_matches.len() == 0 { + if name_matches.is_empty() { let suggestion = packages .package_ids() .map(|p| (lev_distance(spec.name(), &p.name()), p.name())) @@ -289,6 +353,50 @@ impl ProfileMaker { } } +fn merge_toml( + pkg_id: Option<&PackageId>, + is_member: bool, + profile_for: ProfileFor, + profile: &mut Profile, + toml: &TomlProfile, +) { + merge_profile(profile, toml); + if profile_for == ProfileFor::CustomBuild { + if let Some(ref build_override) = toml.build_override { + merge_profile(profile, build_override); + } + } + if let Some(ref overrides) = toml.overrides { + if !is_member { + if let Some(all) = overrides.get(&ProfilePackageSpec::All) { + merge_profile(profile, all); + } + } + if let Some(pkg_id) = pkg_id { + let mut matches = overrides + .iter() + .filter_map(|(key, spec_profile)| match *key { + ProfilePackageSpec::All => None, + ProfilePackageSpec::Spec(ref s) => if s.matches(pkg_id) { + Some(spec_profile) + } else { + None + }, + }); + if let Some(spec_profile) = matches.next() { + merge_profile(profile, spec_profile); + // `validate_packages` should ensure that there are + // no additional matches. + assert!( + matches.next().is_none(), + "package `{}` matched multiple profile overrides", + pkg_id + ); + } + } + } +} + 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); @@ -341,6 +449,20 @@ pub struct Profile { pub panic: Option, } +const TOML_PROFILE_KEYS: [&str; 11] = [ + "opt-level", + "lto", + "codegen-units", + "debug", + "debug-assertions", + "rpath", + "panic", + "overflow-checks", + "incremental", + "overrides", + "build-override", +]; + impl Default for Profile { fn default() -> Profile { Profile { diff --git a/src/cargo/util/config.rs b/src/cargo/util/config.rs index a9ff2eeae9d..e4b93828f15 100644 --- a/src/cargo/util/config.rs +++ b/src/cargo/util/config.rs @@ -1380,7 +1380,7 @@ impl ConfigValue { } } - fn into_toml(self) -> toml::Value { + pub fn into_toml(self) -> toml::Value { match self { CV::Boolean(s, _) => toml::Value::Boolean(s), CV::String(s, _) => toml::Value::String(s), @@ -1396,9 +1396,6 @@ impl ConfigValue { fn merge(&mut self, from: ConfigValue) -> CargoResult<()> { match (self, from) { - (&mut CV::String(..), CV::String(..)) - | (&mut CV::Integer(..), CV::Integer(..)) - | (&mut CV::Boolean(..), CV::Boolean(..)) => {} (&mut CV::List(ref mut old, _), CV::List(ref mut new, _)) => { let new = mem::replace(new, Vec::new()); old.extend(new.into_iter()); @@ -1428,13 +1425,17 @@ impl ConfigValue { }; } } - (expected, found) => { + (expected @ &mut CV::List(_, _), found) + | (expected @ &mut CV::Table(_, _), found) + | (expected, found @ CV::List(_, _)) + | (expected, found @ CV::Table(_, _)) => { return Err(internal(format!( "expected {}, but found {}", expected.desc(), found.desc() ))) } + _ => {} } Ok(()) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index a987eb31fa6..ac2a452c194 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -5,6 +5,7 @@ use std::path::{Path, PathBuf}; use std::rc::Rc; use std::str; +use failure::Error; use semver::{self, VersionReq}; use serde::de::{self, Deserialize}; use serde::ser; @@ -21,7 +22,7 @@ use core::{GitReference, PackageIdSpec, SourceId, WorkspaceConfig, WorkspaceRoot use sources::CRATES_IO; use util::errors::{CargoError, CargoResult, CargoResultExt}; use util::paths; -use util::{self, Config, ToUrl}; +use util::{self, Config, ConfigValue, ToUrl}; mod targets; use self::targets::targets; @@ -237,29 +238,29 @@ pub struct TomlManifest { #[derive(Deserialize, Serialize, Clone, Debug, Default)] pub struct TomlProfiles { - test: Option, - doc: Option, - bench: Option, - dev: Option, - release: Option, + pub test: Option, + pub doc: Option, + pub bench: Option, + pub dev: Option, + pub release: Option, } impl TomlProfiles { - fn validate(&self, features: &Features, warnings: &mut Vec) -> CargoResult<()> { + pub fn validate(&self, features: &Features, warnings: &mut Vec) -> CargoResult<()> { if let Some(ref test) = self.test { - test.validate("test", features, warnings)?; + test.validate("test", Some(features), warnings)?; } if let Some(ref doc) = self.doc { - doc.validate("doc", features, warnings)?; + doc.validate("doc", Some(features), warnings)?; } if let Some(ref bench) = self.bench { - bench.validate("bench", features, warnings)?; + bench.validate("bench", Some(features), warnings)?; } if let Some(ref dev) = self.dev { - dev.validate("dev", features, warnings)?; + dev.validate("dev", Some(features), warnings)?; } if let Some(ref release) = self.release { - release.validate("release", features, warnings)?; + release.validate("release", Some(features), warnings)?; } Ok(()) } @@ -419,18 +420,71 @@ impl<'de> de::Deserialize<'de> for ProfilePackageSpec { } impl TomlProfile { - fn validate( + pub fn from_config( + config: &Config, + name: &str, + warnings: &mut Vec, + ) -> CargoResult> { + if !config.cli_unstable().config_profile { + return Ok(None); + } + if let Some(util::config::Value { val, .. }) = + config.get_table(&format!("profile.{}", name))? + { + let cv = ConfigValue::Table(val.clone(), PathBuf::new()); + let toml = cv.into_toml(); + let profile: TomlProfile = + Deserialize::deserialize(toml).chain_err(|| error_path(&val))?; + profile + .validate(name, None, warnings) + .chain_err(|| error_path(&val))?; + return Ok(Some(profile)); + } + return Ok(None); + + fn error_path(table: &HashMap) -> Error { + let mut paths = HashSet::new(); + error_path_rec(table, &mut paths); + if paths.len() == 1 { + format_err!( + "error in config profile `{}`", + paths.into_iter().next().unwrap() + ) + } else { + let mut ps = paths.into_iter().collect::>(); + ps.sort(); // to help with testing + format_err!( + "error in config profile, possible locations: {}", + ps.join(", ") + ) + } + } + fn error_path_rec(table: &HashMap, paths: &mut HashSet) { + for cv in table.values() { + paths.insert(cv.definition_path().display().to_string()); + if let &ConfigValue::Table(ref t, _) = cv { + error_path_rec(t, paths); + } + } + } + } + + pub fn validate( &self, name: &str, - features: &Features, + features: Option<&Features>, warnings: &mut Vec, ) -> CargoResult<()> { if let Some(ref profile) = self.build_override { - features.require(Feature::profile_overrides())?; + if let Some(features) = features { + features.require(Feature::profile_overrides())?; + } profile.validate_override()?; } if let Some(ref override_map) = self.overrides { - features.require(Feature::profile_overrides())?; + if let Some(features) = features { + features.require(Feature::profile_overrides())?; + } for profile in override_map.values() { profile.validate_override()?; } @@ -760,7 +814,8 @@ impl TomlManifest { features .require(Feature::edition()) .chain_err(|| "editions are unstable")?; - edition.parse() + edition + .parse() .chain_err(|| "failed to parse the `edition` key")? } else { Edition::Edition2015 @@ -914,10 +969,7 @@ impl TomlManifest { `[workspace]`, only one can be specified" ), }; - if let Some(ref profiles) = me.profile { - profiles.validate(&features, &mut warnings)?; - } - let profiles = build_profiles(&me.profile); + let profiles = Profiles::new(me.profile.as_ref(), config, &features, &mut warnings)?; let publish = match project.publish { Some(VecStringOrBool::VecString(ref vecstring)) => { features @@ -1027,7 +1079,7 @@ impl TomlManifest { }; (me.replace(&mut cx)?, me.patch(&mut cx)?) }; - let profiles = build_profiles(&me.profile); + let profiles = Profiles::new(me.profile.as_ref(), config, &features, &mut warnings)?; let workspace_config = match me.workspace { Some(ref config) => WorkspaceConfig::Root(WorkspaceRootConfig::new( &root, @@ -1403,14 +1455,3 @@ impl fmt::Debug for PathValue { self.0.fmt(f) } } - -fn build_profiles(profiles: &Option) -> Profiles { - let profiles = profiles.as_ref(); - Profiles::new( - profiles.and_then(|p| p.dev.clone()), - profiles.and_then(|p| p.release.clone()), - profiles.and_then(|p| p.test.clone()), - profiles.and_then(|p| p.bench.clone()), - profiles.and_then(|p| p.doc.clone()), - ) -} diff --git a/src/doc/src/reference/unstable.md b/src/doc/src/reference/unstable.md index 91a85c99ba2..3e402cb0ac9 100644 --- a/src/doc/src/reference/unstable.md +++ b/src/doc/src/reference/unstable.md @@ -229,6 +229,26 @@ opt-level = 3 Overrides can only be specified for dev and release profiles. +### Config Profiles +* Tracking Issue: [rust-lang/rust#48683](https://github.com/rust-lang/rust/issues/48683) +* RFC: [#2282](https://github.com/rust-lang/rfcs/blob/master/text/2282-profile-dependencies.md) + +Profiles can be specified in `.cargo/config` files. The `-Z config-profile` +command-line flag is required to use this feature. The format is the same as +in a `Cargo.toml` manifest. If found in multiple config files, settings will +be merged using the regular [config hierarchy](reference/config.html#hierarchical-structure). +Config settings take precedence over manifest settings. + +```toml +[profile.dev] +opt-level = 3 +``` + +``` +cargo +nightly build -Z config-profile +``` + + ### Namespaced features * Original issue: [#1286](https://github.com/rust-lang/cargo/issues/1286) diff --git a/tests/testsuite/bad_config.rs b/tests/testsuite/bad_config.rs index 985447530c8..4b2671057e3 100644 --- a/tests/testsuite/bad_config.rs +++ b/tests/testsuite/bad_config.rs @@ -139,46 +139,6 @@ Caused by: ); } -#[test] -fn bad5() { - let p = project("foo") - .file( - ".cargo/config", - r#" - foo = "" - "#, - ) - .file( - "foo/.cargo/config", - r#" - foo = 2 - "#, - ) - .build(); - assert_that( - p.cargo("new") - .arg("-v") - .arg("foo") - .cwd(&p.root().join("foo")), - execs().with_status(101).with_stderr( - "\ -[ERROR] could not load Cargo configuration - -Caused by: - failed to merge configuration at `[..]` - -Caused by: - failed to merge key `foo` between files: - file 1: [..]foo[..]foo[..]config - file 2: [..]foo[..]config - -Caused by: - expected integer, but found string -", - ), - ); -} - #[test] fn bad6() { let p = project("foo") diff --git a/tests/testsuite/main.rs b/tests/testsuite/main.rs index b1b7496abb5..cb1ca949df0 100644 --- a/tests/testsuite/main.rs +++ b/tests/testsuite/main.rs @@ -71,6 +71,7 @@ mod path; mod plugins; mod proc_macro; mod profiles; +mod profile_config; mod profile_overrides; mod profile_targets; mod publish; diff --git a/tests/testsuite/profile_config.rs b/tests/testsuite/profile_config.rs new file mode 100644 index 00000000000..2644d85d712 --- /dev/null +++ b/tests/testsuite/profile_config.rs @@ -0,0 +1,369 @@ +use cargotest::support::{basic_lib_manifest, execs, paths, project}; +use cargotest::ChannelChanger; +use hamcrest::assert_that; + +#[test] +fn profile_config_gated() { + let p = project("foo") + .file("Cargo.toml", &basic_lib_manifest("foo")) + .file("src/lib.rs", "") + .file( + ".cargo/config", + r#" + [profile.dev] + debug = 1 + "#, + ) + .build(); + + assert_that( + p.cargo("build -v"), + execs().with_status(0).with_stderr_contains( + "\ +[WARNING] profile in config `[..]foo[/].cargo[/]config` requires `-Z config-profile` command-line option +", + ).with_stderr_contains("[..]-C debuginfo=2[..]"), + ); +} + +#[test] +fn profile_config_validate_warnings() { + let p = project("foo") + .file("Cargo.toml", &basic_lib_manifest("foo")) + .file("src/lib.rs", "") + .file( + ".cargo/config", + r#" + [profile.test] + opt-level = 3 + + [profile.asdf] + opt-level = 3 + + [profile.dev] + bad-key = true + + [profile.dev.build-override] + bad-key-bo = true + + [profile.dev.overrides.bar] + bad-key-bar = true + "#, + ) + .build(); + + assert_that( + p.cargo("build -v -Z config-profile") + .masquerade_as_nightly_cargo(), + execs() + .with_status(0) + .with_stderr_contains( + "\ +[WARNING] profile `test` in config `[..]foo[/].cargo[/]config` is not supported +", + ) + .with_stderr_contains( + "\ +[WARNING] profile `asdf` in config `[..]foo[/].cargo[/]config` is not supported +", + ) + .with_stderr_contains( + "\ +[WARNING] unused profile key `bad-key` in config `[..]foo[/].cargo[/]config` +[WARNING] unused profile key `bad-key-bo` in config `[..]foo[/].cargo[/]config` +[WARNING] unused profile key `bad-key-bar` in config `[..]foo[/].cargo[/]config` +", + ), + ); +} + +#[test] +fn profile_config_error_paths() { + // Due to how it's implemented, we are uncertain where a merged error + // comes from. + let p = project("foo") + .file("Cargo.toml", &basic_lib_manifest("foo")) + .file("src/lib.rs", "") + .file( + ".cargo/config", + r#" + [profile.dev] + opt-level = 3 + "#, + ) + .file( + paths::home().join(".cargo/config"), + r#" + [profile.dev] + rpath = "foo" + "#, + ) + .build(); + + assert_that( + p.cargo("build -Z config-profile") + .masquerade_as_nightly_cargo(), + execs().with_status(101).with_stderr( + "\ +[ERROR] failed to parse manifest at `[..]foo[/]Cargo.toml` + +Caused by: + error in config profile, possible locations: [..]foo[/].cargo[/]config, [..]home[/].cargo[/]config + +Caused by: + invalid type: string \"foo\", expected a boolean for key `rpath` +", + ), + ); +} + +#[test] +fn profile_config_validate_errors() { + let p = project("foo") + .file("Cargo.toml", &basic_lib_manifest("foo")) + .file("src/lib.rs", "") + .file( + ".cargo/config", + r#" + [profile.dev.overrides.foo] + panic = "abort" + "#, + ) + .build(); + + assert_that( + p.cargo("build -Z config-profile") + .masquerade_as_nightly_cargo(), + execs().with_status(101).with_stderr( + "\ +[ERROR] failed to parse manifest at `[..]foo[/]Cargo.toml` + +Caused by: + error in config profile `[..]foo[/].cargo[/]config` + +Caused by: + `panic` may not be specified in a profile override. +", + ), + ); +} + +#[test] +fn profile_config_syntax_errors() { + let p = project("foo") + .file("Cargo.toml", &basic_lib_manifest("foo")) + .file("src/lib.rs", "") + .file( + ".cargo/config", + r#" + [profile.dev] + codegen-units = "foo" + "#, + ) + .build(); + + assert_that( + p.cargo("build -Z config-profile") + .masquerade_as_nightly_cargo(), + execs().with_status(101).with_stderr( + "\ +[ERROR] failed to parse manifest at [..] + +Caused by: + error in config profile `[..]foo[/].cargo[/]config` + +Caused by: + invalid type: string \"foo\", expected u32 for key `codegen-units` +", + ), + ); +} + +#[test] +fn profile_config_override_spec_multiple() { + let p = project("foo") + .file( + "Cargo.toml", + r#" + cargo-features = ["profile-overrides"] + + [package] + name = "foo" + version = "0.0.1" + + [dependencies] + bar = { path = "bar" } + "#, + ) + .file( + ".cargo/config", + r#" + [profile.dev.overrides.bar] + opt-level = 3 + + [profile.dev.overrides."bar:0.5.0"] + opt-level = 3 + "#, + ) + .file("src/lib.rs", "") + .file("bar/Cargo.toml", &basic_lib_manifest("bar")) + .file("bar/src/lib.rs", "") + .build(); + + // Unfortunately this doesn't tell you which file, hopefully it's not too + // much of a problem. + assert_that( + p.cargo("build -v -Z config-profile") + .masquerade_as_nightly_cargo(), + execs().with_status(101).with_stderr( + "\ +[ERROR] multiple profile overrides in profile `dev` match package `bar v0.5.0 ([..])` +found profile override specs: bar, bar:0.5.0", + ), + ); +} + +#[test] +fn profile_config_all_options() { + // Ensure all profile options are supported. + let p = project("foo") + .file("Cargo.toml", &basic_lib_manifest("foo")) + .file("src/lib.rs", "") + .file( + ".cargo/config", + r#" + [profile.release] + opt-level = 1 + debug = true + debug-assertions = true + overflow-checks = false + rpath = true + lto = true + codegen-units = 2 + panic = "abort" + incremental = true + "#, + ) + .build(); + + assert_that( + p.cargo("build --release -v -Z config-profile") + .masquerade_as_nightly_cargo(), + execs().with_status(0).with_stderr( + "\ +[COMPILING] foo [..] +[RUNNING] `rustc --crate-name foo [..] \ + -C opt-level=1 \ + -C panic=abort \ + -C codegen-units=2 \ + -C debuginfo=2 \ + -C debug-assertions=on \ + -C overflow-checks=off [..]\ + -C rpath [..] +[FINISHED] release [optimized + debuginfo] [..] +", + ), + ); +} + +#[test] +fn profile_config_override_precedence() { + // Config values take precedence over manifest values. + let p = project("foo") + .file( + "Cargo.toml", + r#" + cargo-features = ["profile-overrides"] + + [package] + name = "foo" + version = "0.0.1" + + [dependencies] + bar = {path = "bar"} + + [profile.dev] + codegen-units = 2 + + [profile.dev.overrides.bar] + opt-level = 3 + "#, + ) + .file("src/lib.rs", "") + .file("bar/Cargo.toml", &basic_lib_manifest("bar")) + .file("bar/src/lib.rs", "") + .file( + ".cargo/config", + r#" + [profile.dev.overrides.bar] + opt-level = 2 + "#, + ) + .build(); + + assert_that( + p.cargo("build -v -Z config-profile") + .masquerade_as_nightly_cargo(), + execs().with_status(0).with_stderr( + "\ +[COMPILING] bar [..] +[RUNNING] `rustc --crate-name bar [..] -C opt-level=2 -C codegen-units=2 [..] +[COMPILING] foo [..] +[RUNNING] `rustc --crate-name foo [..]-C codegen-units=2 [..] +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]", + ), + ); +} + +#[test] +fn profile_config_no_warn_unknown_override() { + let p = project("foo") + .file("Cargo.toml", &basic_lib_manifest("foo")) + .file("src/lib.rs", "") + .file( + ".cargo/config", + r#" + [profile.dev.overrides.bar] + codegen-units = 4 + "#, + ) + .build(); + + assert_that( + p.cargo("build -Z config-profile") + .masquerade_as_nightly_cargo(), + execs() + .with_status(0) + .with_stderr_does_not_contain("[..]warning[..]"), + ); +} + +#[test] +fn profile_config_mixed_types() { + let p = project("foo") + .file("Cargo.toml", &basic_lib_manifest("foo")) + .file("src/lib.rs", "") + .file( + ".cargo/config", + r#" + [profile.dev] + opt-level = 3 + "#, + ) + .file( + paths::home().join(".cargo/config"), + r#" + [profile.dev] + opt-level = 's' + "#, + ) + .build(); + + assert_that( + p.cargo("build -v -Z config-profile") + .masquerade_as_nightly_cargo(), + execs() + .with_status(0) + .with_stderr_contains("[..]-C opt-level=3 [..]"), + ); +} From 84a80d8f1a0e3c7fce400b60f35c84e862e1dcc2 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Thu, 31 May 2018 09:54:37 -0700 Subject: [PATCH 2/2] Use new config API. Also, require "override" feature if used in a config. --- src/cargo/core/profiles.rs | 116 ++++++++---------------------- src/cargo/util/config.rs | 26 ++++++- src/cargo/util/toml/mod.rs | 72 +++---------------- tests/testsuite/profile_config.rs | 110 ++++++++++++++++++---------- 4 files changed, 136 insertions(+), 188 deletions(-) diff --git a/src/cargo/core/profiles.rs b/src/cargo/core/profiles.rs index c9feafaed7f..24fa37d3b6a 100644 --- a/src/cargo/core/profiles.rs +++ b/src/cargo/core/profiles.rs @@ -1,13 +1,13 @@ -use std::collections::{HashMap, HashSet}; -use std::sync::atomic; +use std::collections::HashSet; use std::{cmp, fmt, hash}; use core::compiler::CompileMode; use core::interning::InternedString; use core::{Features, PackageId, PackageIdSpec, PackageSet, Shell}; +use util::errors::CargoResultExt; use util::lev_distance::lev_distance; use util::toml::{ProfilePackageSpec, StringOrBool, TomlProfile, TomlProfiles, U32OrBool}; -use util::{CargoResult, Config, ConfigValue}; +use util::{CargoResult, Config}; /// Collection of all user profiles. #[derive(Clone, Debug)] @@ -29,18 +29,20 @@ impl Profiles { if let Some(profiles) = profiles { profiles.validate(features, warnings)?; } - Profiles::validate_config(config, warnings)?; + + let config_profiles = config.profiles()?; + config_profiles.validate(features, warnings)?; Ok(Profiles { dev: ProfileMaker { default: Profile::default_dev(), toml: profiles.and_then(|p| p.dev.clone()), - config: TomlProfile::from_config(config, "dev", warnings)?, + config: config_profiles.dev.clone(), }, release: ProfileMaker { default: Profile::default_release(), toml: profiles.and_then(|p| p.release.clone()), - config: TomlProfile::from_config(config, "release", warnings)?, + config: config_profiles.release.clone(), }, test: ProfileMaker { default: Profile::default_test(), @@ -137,71 +139,6 @@ impl Profiles { self.doc.validate_packages(shell, packages)?; Ok(()) } - - fn validate_config(config: &Config, warnings: &mut Vec) -> CargoResult<()> { - static VALIDATE_ONCE: atomic::AtomicBool = atomic::ATOMIC_BOOL_INIT; - - if VALIDATE_ONCE.swap(true, atomic::Ordering::SeqCst) { - return Ok(()); - } - - // cv: Value> - if let Some(cv) = config.get_table("profile")? { - // Warn if config profiles without CLI option. - if !config.cli_unstable().config_profile { - warnings.push(format!( - "profile in config `{}` requires `-Z config-profile` command-line option", - cv.definition - )); - // Ignore the rest. - return Ok(()); - } - // Warn about unsupported profile names. - for (key, profile_cv) in cv.val.iter() { - if key != "dev" && key != "release" { - warnings.push(format!( - "profile `{}` in config `{}` is not supported", - key, - profile_cv.definition_path().display() - )); - } - } - // Warn about incorrect key names. - for profile_cv in cv.val.values() { - if let ConfigValue::Table(ref profile, _) = *profile_cv { - validate_profile_keys(profile, warnings); - if let Some(&ConfigValue::Table(ref bo_profile, _)) = - profile.get("build-override") - { - validate_profile_keys(bo_profile, warnings); - } - if let Some(&ConfigValue::Table(ref os, _)) = profile.get("overrides") { - for o_profile_cv in os.values() { - if let ConfigValue::Table(ref o_profile, _) = *o_profile_cv { - validate_profile_keys(o_profile, warnings); - } - } - } - } - } - } - return Ok(()); - - fn validate_profile_keys( - profile: &HashMap, - warnings: &mut Vec, - ) { - for (key, value_cv) in profile.iter() { - if !TOML_PROFILE_KEYS.iter().any(|k| k == key) { - warnings.push(format!( - "unused profile key `{}` in config `{}`", - key, - value_cv.definition_path().display() - )); - } - } - } - } } /// An object used for handling the profile override hierarchy. @@ -449,20 +386,6 @@ pub struct Profile { pub panic: Option, } -const TOML_PROFILE_KEYS: [&str; 11] = [ - "opt-level", - "lto", - "codegen-units", - "debug", - "debug-assertions", - "rpath", - "panic", - "overflow-checks", - "incremental", - "overrides", - "build-override", -]; - impl Default for Profile { fn default() -> Profile { Profile { @@ -605,3 +528,26 @@ impl ProfileFor { &ALL } } + +/// Profiles loaded from .cargo/config files. +#[derive(Clone, Debug, Deserialize, Default)] +pub struct ConfigProfiles { + dev: Option, + release: Option, +} + +impl ConfigProfiles { + pub fn validate(&self, features: &Features, warnings: &mut Vec) -> CargoResult<()> { + if let Some(ref profile) = self.dev { + profile + .validate("dev", features, warnings) + .chain_err(|| format_err!("config profile `profile.dev` is not valid"))?; + } + if let Some(ref profile) = self.release { + profile + .validate("release", features, warnings) + .chain_err(|| format_err!("config profile `profile.release` is not valid"))?; + } + Ok(()) + } +} diff --git a/src/cargo/util/config.rs b/src/cargo/util/config.rs index e4b93828f15..6af6dc91919 100644 --- a/src/cargo/util/config.rs +++ b/src/cargo/util/config.rs @@ -22,6 +22,7 @@ use lazycell::LazyCell; use serde::{de, de::IntoDeserializer, Serialize, Serializer}; use toml; +use core::profiles::ConfigProfiles; use core::shell::Verbosity; use core::{CliUnstable, Shell, SourceId, Workspace}; use ops; @@ -77,6 +78,8 @@ pub struct Config { target_dir: Option, /// Environment variables, separated to assist testing. env: HashMap, + /// Profiles loaded from config. + profiles: LazyCell, } impl Config { @@ -132,6 +135,7 @@ impl Config { creation_time: Instant::now(), target_dir: None, env, + profiles: LazyCell::new(), } } @@ -246,6 +250,25 @@ impl Config { .map(AsRef::as_ref) } + pub fn profiles(&self) -> CargoResult<&ConfigProfiles> { + self.profiles.try_borrow_with(|| { + let ocp = self.get::>("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()) + } + }) + } + pub fn values(&self) -> CargoResult<&HashMap> { self.values.try_borrow_with(|| self.load_values()) } @@ -1380,7 +1403,7 @@ impl ConfigValue { } } - pub fn into_toml(self) -> toml::Value { + fn into_toml(self) -> toml::Value { match self { CV::Boolean(s, _) => toml::Value::Boolean(s), CV::String(s, _) => toml::Value::String(s), @@ -1425,6 +1448,7 @@ impl ConfigValue { }; } } + // Allow switching types except for tables or arrays. (expected @ &mut CV::List(_, _), found) | (expected @ &mut CV::Table(_, _), found) | (expected, found @ CV::List(_, _)) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index ac2a452c194..0d4cd89d96a 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -5,7 +5,6 @@ use std::path::{Path, PathBuf}; use std::rc::Rc; use std::str; -use failure::Error; use semver::{self, VersionReq}; use serde::de::{self, Deserialize}; use serde::ser; @@ -22,7 +21,7 @@ use core::{GitReference, PackageIdSpec, SourceId, WorkspaceConfig, WorkspaceRoot use sources::CRATES_IO; use util::errors::{CargoError, CargoResult, CargoResultExt}; use util::paths; -use util::{self, Config, ConfigValue, ToUrl}; +use util::{self, Config, ToUrl}; mod targets; use self::targets::targets; @@ -248,19 +247,19 @@ pub struct TomlProfiles { impl TomlProfiles { pub fn validate(&self, features: &Features, warnings: &mut Vec) -> CargoResult<()> { if let Some(ref test) = self.test { - test.validate("test", Some(features), warnings)?; + test.validate("test", features, warnings)?; } if let Some(ref doc) = self.doc { - doc.validate("doc", Some(features), warnings)?; + doc.validate("doc", features, warnings)?; } if let Some(ref bench) = self.bench { - bench.validate("bench", Some(features), warnings)?; + bench.validate("bench", features, warnings)?; } if let Some(ref dev) = self.dev { - dev.validate("dev", Some(features), warnings)?; + dev.validate("dev", features, warnings)?; } if let Some(ref release) = self.release { - release.validate("release", Some(features), warnings)?; + release.validate("release", features, warnings)?; } Ok(()) } @@ -420,71 +419,18 @@ impl<'de> de::Deserialize<'de> for ProfilePackageSpec { } impl TomlProfile { - pub fn from_config( - config: &Config, - name: &str, - warnings: &mut Vec, - ) -> CargoResult> { - if !config.cli_unstable().config_profile { - return Ok(None); - } - if let Some(util::config::Value { val, .. }) = - config.get_table(&format!("profile.{}", name))? - { - let cv = ConfigValue::Table(val.clone(), PathBuf::new()); - let toml = cv.into_toml(); - let profile: TomlProfile = - Deserialize::deserialize(toml).chain_err(|| error_path(&val))?; - profile - .validate(name, None, warnings) - .chain_err(|| error_path(&val))?; - return Ok(Some(profile)); - } - return Ok(None); - - fn error_path(table: &HashMap) -> Error { - let mut paths = HashSet::new(); - error_path_rec(table, &mut paths); - if paths.len() == 1 { - format_err!( - "error in config profile `{}`", - paths.into_iter().next().unwrap() - ) - } else { - let mut ps = paths.into_iter().collect::>(); - ps.sort(); // to help with testing - format_err!( - "error in config profile, possible locations: {}", - ps.join(", ") - ) - } - } - fn error_path_rec(table: &HashMap, paths: &mut HashSet) { - for cv in table.values() { - paths.insert(cv.definition_path().display().to_string()); - if let &ConfigValue::Table(ref t, _) = cv { - error_path_rec(t, paths); - } - } - } - } - pub fn validate( &self, name: &str, - features: Option<&Features>, + features: &Features, warnings: &mut Vec, ) -> CargoResult<()> { if let Some(ref profile) = self.build_override { - if let Some(features) = features { - features.require(Feature::profile_overrides())?; - } + features.require(Feature::profile_overrides())?; profile.validate_override()?; } if let Some(ref override_map) = self.overrides { - if let Some(features) = features { - features.require(Feature::profile_overrides())?; - } + features.require(Feature::profile_overrides())?; for profile in override_map.values() { profile.validate_override()?; } diff --git a/tests/testsuite/profile_config.rs b/tests/testsuite/profile_config.rs index 2644d85d712..73613be9e57 100644 --- a/tests/testsuite/profile_config.rs +++ b/tests/testsuite/profile_config.rs @@ -18,18 +18,30 @@ fn profile_config_gated() { assert_that( p.cargo("build -v"), - execs().with_status(0).with_stderr_contains( - "\ -[WARNING] profile in config `[..]foo[/].cargo[/]config` requires `-Z config-profile` command-line option + execs() + .with_status(0) + .with_stderr_contains( + "\ +[WARNING] profiles in config files require `-Z config-profile` command-line option ", - ).with_stderr_contains("[..]-C debuginfo=2[..]"), + ) + .with_stderr_contains("[..]-C debuginfo=2[..]"), ); } #[test] fn profile_config_validate_warnings() { let p = project("foo") - .file("Cargo.toml", &basic_lib_manifest("foo")) + .file( + "Cargo.toml", + r#" + cargo-features = ["profile-overrides"] + + [package] + name = "foo" + version = "0.0.1" + "#, + ) .file("src/lib.rs", "") .file( ".cargo/config", @@ -53,34 +65,24 @@ fn profile_config_validate_warnings() { .build(); assert_that( - p.cargo("build -v -Z config-profile") + p.cargo("build -Z config-profile") .masquerade_as_nightly_cargo(), - execs() - .with_status(0) - .with_stderr_contains( - "\ -[WARNING] profile `test` in config `[..]foo[/].cargo[/]config` is not supported -", - ) - .with_stderr_contains( - "\ -[WARNING] profile `asdf` in config `[..]foo[/].cargo[/]config` is not supported -", - ) - .with_stderr_contains( - "\ -[WARNING] unused profile key `bad-key` in config `[..]foo[/].cargo[/]config` -[WARNING] unused profile key `bad-key-bo` in config `[..]foo[/].cargo[/]config` -[WARNING] unused profile key `bad-key-bar` in config `[..]foo[/].cargo[/]config` + execs().with_status(0).with_stderr_unordered( + "\ +[WARNING] unused key `profile.asdf` in config file `[..].cargo[/]config` +[WARNING] unused key `profile.test` in config file `[..].cargo[/]config` +[WARNING] unused key `profile.dev.bad-key` in config file `[..].cargo[/]config` +[WARNING] unused key `profile.dev.overrides.bar.bad-key-bar` in config file `[..].cargo[/]config` +[WARNING] unused key `profile.dev.build-override.bad-key-bo` in config file `[..].cargo[/]config` +[COMPILING] foo [..] +[FINISHED] [..] ", - ), + ), ); } #[test] fn profile_config_error_paths() { - // Due to how it's implemented, we are uncertain where a merged error - // comes from. let p = project("foo") .file("Cargo.toml", &basic_lib_manifest("foo")) .file("src/lib.rs", "") @@ -108,10 +110,7 @@ fn profile_config_error_paths() { [ERROR] failed to parse manifest at `[..]foo[/]Cargo.toml` Caused by: - error in config profile, possible locations: [..]foo[/].cargo[/]config, [..]home[/].cargo[/]config - -Caused by: - invalid type: string \"foo\", expected a boolean for key `rpath` + error in [..].cargo[/]config: `profile.dev.rpath` expected true/false, but found a string ", ), ); @@ -120,7 +119,16 @@ Caused by: #[test] fn profile_config_validate_errors() { let p = project("foo") - .file("Cargo.toml", &basic_lib_manifest("foo")) + .file( + "Cargo.toml", + r#" + cargo-features = ["profile-overrides"] + + [package] + name = "foo" + version = "0.0.1" + "#, + ) .file("src/lib.rs", "") .file( ".cargo/config", @@ -139,7 +147,7 @@ fn profile_config_validate_errors() { [ERROR] failed to parse manifest at `[..]foo[/]Cargo.toml` Caused by: - error in config profile `[..]foo[/].cargo[/]config` + config profile `profile.dev` is not valid Caused by: `panic` may not be specified in a profile override. @@ -170,10 +178,7 @@ fn profile_config_syntax_errors() { [ERROR] failed to parse manifest at [..] Caused by: - error in config profile `[..]foo[/].cargo[/]config` - -Caused by: - invalid type: string \"foo\", expected u32 for key `codegen-units` + error in [..].cargo[/]config: `profile.dev.codegen-units` expected an integer, but found a string ", ), ); @@ -206,7 +211,16 @@ fn profile_config_override_spec_multiple() { "#, ) .file("src/lib.rs", "") - .file("bar/Cargo.toml", &basic_lib_manifest("bar")) + .file( + "bar/Cargo.toml", + r#" + cargo-features = ["profile-overrides"] + + [package] + name = "bar" + version = "0.5.0" + "#, + ) .file("bar/src/lib.rs", "") .build(); @@ -290,7 +304,16 @@ fn profile_config_override_precedence() { "#, ) .file("src/lib.rs", "") - .file("bar/Cargo.toml", &basic_lib_manifest("bar")) + .file( + "bar/Cargo.toml", + r#" + cargo-features = ["profile-overrides"] + + [package] + name = "bar" + version = "0.0.1" + "#, + ) .file("bar/src/lib.rs", "") .file( ".cargo/config", @@ -318,7 +341,16 @@ fn profile_config_override_precedence() { #[test] fn profile_config_no_warn_unknown_override() { let p = project("foo") - .file("Cargo.toml", &basic_lib_manifest("foo")) + .file( + "Cargo.toml", + r#" + cargo-features = ["profile-overrides"] + + [package] + name = "foo" + version = "0.0.1" + "#, + ) .file("src/lib.rs", "") .file( ".cargo/config",