From 95de3e9feab088f1dcc69f609e70a144b4c0f597 Mon Sep 17 00:00:00 2001 From: Martin Donlon Date: Mon, 15 Feb 2021 15:17:00 -0800 Subject: [PATCH 1/4] Add support for [env] section in .cargo/config.toml This adds support for an `[env]` section in the config.toml files. Environment variables set in this section will be applied to the environment of any processes executed by cargo. ``` [env] FOOBAR = "Apple" PATH_TO_SOME_TOOL = { value = "bin/tool", relative = true } USERNAME = { value = "test_user", force = true } ``` --- src/cargo/core/compiler/compilation.rs | 9 ++ src/cargo/util/config/mod.rs | 73 ++++++++++++++++ tests/testsuite/cargo_env_config.rs | 110 +++++++++++++++++++++++++ tests/testsuite/main.rs | 1 + 4 files changed, 193 insertions(+) create mode 100644 tests/testsuite/cargo_env_config.rs diff --git a/src/cargo/core/compiler/compilation.rs b/src/cargo/core/compiler/compilation.rs index ba4f0f8f576..8e0483907bd 100644 --- a/src/cargo/core/compiler/compilation.rs +++ b/src/cargo/core/compiler/compilation.rs @@ -339,6 +339,15 @@ impl<'cfg> Compilation<'cfg> { ) .env("CARGO_PKG_AUTHORS", &pkg.authors().join(":")) .cwd(pkg.root()); + + + // Apply any environment variables from the config + for (key, value) in self.config.env_config()?.iter() { + if value.is_force() || cmd.get_env(&key).is_none() { + cmd.env(&key, value.resolve(&self.config)); + } + } + Ok(cmd) } } diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index 7cea1bcecc3..1ee30aa6569 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -62,6 +62,8 @@ use std::path::{Path, PathBuf}; use std::str::FromStr; use std::sync::Once; use std::time::Instant; +use std::borrow::Cow; +use std::ffi::OsStr; use anyhow::{anyhow, bail, format_err}; use curl::easy::Easy; @@ -181,6 +183,7 @@ pub struct Config { target_cfgs: LazyCell>, doc_extern_map: LazyCell, progress_config: ProgressConfig, + env_config: LazyCell, } impl Config { @@ -264,6 +267,7 @@ impl Config { target_cfgs: LazyCell::new(), doc_extern_map: LazyCell::new(), progress_config: ProgressConfig::default(), + env_config: LazyCell::new(), } } @@ -1244,6 +1248,41 @@ impl Config { &self.progress_config } + /// Create an EnvConfigValue hashmap from the "env" table + fn get_env_config(&self) -> CargoResult { + // We cannot use pure serde handling for this. The Value<_> type does not + // work when parsing the env table to a hashmap. So iterator through each + // entry in the "env" table, determine it's type and then use `get` method + // to deserialize it. + let env_table = &self.get_table(&ConfigKey::from_str("env"))?; + let mut vars = EnvConfig::new(); + + if env_table.is_none() { + return Ok(vars); + } + + let env_table = &env_table.as_ref().unwrap().val; + + for (key, value) in env_table.iter() { + let full_key = format!("env.{}", key); + let e = match value { + ConfigValue::Table(..) => self.get::(&full_key)?, + _ => { + let v = self.get::>(&full_key)?; + EnvConfigValue::from_value(v) + } + }; + vars.insert(key.clone(), e); + } + Ok(vars) + } + + pub fn env_config(&self) -> CargoResult<&EnvConfig> { + self.env_config.try_borrow_with(|| { + self.get_env_config() + }) + } + /// This is used to validate the `term` table has valid syntax. /// /// This is necessary because loading the term settings happens very @@ -1953,6 +1992,40 @@ where deserializer.deserialize_option(ProgressVisitor) } +#[derive(Debug, Deserialize)] +pub struct EnvConfigValue { + value: Value, + #[serde(default)] + force: bool, + #[serde(default)] + relative: bool, +} + +impl EnvConfigValue { + fn from_value(value: Value) -> EnvConfigValue { + EnvConfigValue { + value, + force: false, + relative: false + } + } + + pub fn is_force(&self) -> bool { + self.force + } + + pub fn resolve<'a>(&'a self, config: &Config) -> Cow<'a, OsStr> { + if self.relative { + let p = self.value.definition.root(config).join(&self.value.val); + Cow::Owned(p.into_os_string()) + } else { + Cow::Borrowed(OsStr::new(&self.value.val)) + } + } +} + +pub type EnvConfig = HashMap; + /// A type to deserialize a list of strings from a toml file. /// /// Supports deserializing either a whitespace-separated list of arguments in a diff --git a/tests/testsuite/cargo_env_config.rs b/tests/testsuite/cargo_env_config.rs new file mode 100644 index 00000000000..742f668950c --- /dev/null +++ b/tests/testsuite/cargo_env_config.rs @@ -0,0 +1,110 @@ +//! Tests for `[env]` config. + +use cargo_test_support::{basic_bin_manifest, project}; + +#[cargo_test] +fn env_basic() { + let p = project() + .file("Cargo.toml", &basic_bin_manifest("foo")) + .file("src/main.rs", r#" + use std::env; + fn main() { + println!( "compile-time:{}", env!("ENV_TEST_1233") ); + println!( "run-time:{}", env::var("ENV_TEST_1233").unwrap()); + } + "#) + .file( + ".cargo/config", + r#" + [env] + ENV_TEST_1233 = "Hello" + "#, + ) + .build(); + + p.cargo("run") + .with_stdout_contains("compile-time:Hello") + .with_stdout_contains("run-time:Hello") + .run(); +} + +#[cargo_test] +fn env_invalid() { + let p = project() + .file("Cargo.toml", &basic_bin_manifest("foo")) + .file("src/main.rs", r#" + fn main() { + } + "#) + .file( + ".cargo/config", + r#" + [env] + ENV_TEST_BOOL = false + "#, + ) + .build(); + + p.cargo("build") + .with_status(101) + .with_stderr_contains("[..]`env.ENV_TEST_BOOL` expected a string, but found a boolean") + .run(); +} + +#[cargo_test] +fn env_force() { + let p = project() + .file("Cargo.toml", &basic_bin_manifest("foo")) + .file("src/main.rs", r#" + use std::env; + fn main() { + println!( "ENV_TEST_FORCED:{}", env!("ENV_TEST_FORCED") ); + println!( "ENV_TEST_UNFORCED:{}", env!("ENV_TEST_UNFORCED") ); + } + "#) + .file( + ".cargo/config", + r#" + [env] + ENV_TEST_UNFORCED = "from-config" + ENV_TEST_FORCED = { value = "from-config", force = true } + "#, + ) + .build(); + + p.cargo("run") + .env("ENV_TEST_FORCED", "from-env") + .env("ENV_TEST_UNFORCED", "from-env") + .with_stdout_contains("ENV_TEST_FORCED:from-config") + .with_stdout_contains("ENV_TEST_UNFORCED:from-env") + .run(); +} + +#[cargo_test] +fn env_relative() { + let p = project() + .file("Cargo.toml", &basic_bin_manifest("foo2")) + .file("src/main.rs", r#" + use std::env; + use std::path::Path; + fn main() { + println!( "ENV_TEST_RELATIVE:{}", env!("ENV_TEST_RELATIVE") ); + println!( "ENV_TEST_ABSOLUTE:{}", env!("ENV_TEST_ABSOLUTE") ); + + assert!( Path::new(env!("ENV_TEST_ABSOLUTE")).is_absolute() ); + assert!( !Path::new(env!("ENV_TEST_RELATIVE")).is_absolute() ); + } + "#) + .file( + ".cargo/config", + r#" + [env] + ENV_TEST_RELATIVE = "Cargo.toml" + ENV_TEST_ABSOLUTE = { value = "Cargo.toml", relative = true } + "#, + ) + .build(); + + p.cargo("run") + .run(); +} diff --git a/tests/testsuite/main.rs b/tests/testsuite/main.rs index 4df5679189b..3f4c9653eb7 100644 --- a/tests/testsuite/main.rs +++ b/tests/testsuite/main.rs @@ -24,6 +24,7 @@ mod build_script_extra_link_arg; mod cache_messages; mod cargo_alias_config; mod cargo_command; +mod cargo_env_config; mod cargo_features; mod cargo_targets; mod cfg; From 3aa99422ca7808f1bc5621fda3a8f32f27273d9b Mon Sep 17 00:00:00 2001 From: Martin Donlon Date: Mon, 15 Feb 2021 15:29:08 -0800 Subject: [PATCH 2/4] Run `cargo fmt --all` Fix formatting errors --- src/cargo/core/compiler/compilation.rs | 1 - src/cargo/util/config/mod.rs | 10 +++--- tests/testsuite/cargo_env_config.rs | 47 ++++++++++++++++---------- 3 files changed, 33 insertions(+), 25 deletions(-) diff --git a/src/cargo/core/compiler/compilation.rs b/src/cargo/core/compiler/compilation.rs index 8e0483907bd..50f5945a804 100644 --- a/src/cargo/core/compiler/compilation.rs +++ b/src/cargo/core/compiler/compilation.rs @@ -339,7 +339,6 @@ impl<'cfg> Compilation<'cfg> { ) .env("CARGO_PKG_AUTHORS", &pkg.authors().join(":")) .cwd(pkg.root()); - // Apply any environment variables from the config for (key, value) in self.config.env_config()?.iter() { diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index 1ee30aa6569..c5d658eda5d 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -49,10 +49,12 @@ //! translate from `ConfigValue` and environment variables to the caller's //! desired type. +use std::borrow::Cow; use std::cell::{RefCell, RefMut}; use std::collections::hash_map::Entry::{Occupied, Vacant}; use std::collections::{HashMap, HashSet}; use std::env; +use std::ffi::OsStr; use std::fmt; use std::fs::{self, File}; use std::io::prelude::*; @@ -62,8 +64,6 @@ use std::path::{Path, PathBuf}; use std::str::FromStr; use std::sync::Once; use std::time::Instant; -use std::borrow::Cow; -use std::ffi::OsStr; use anyhow::{anyhow, bail, format_err}; use curl::easy::Easy; @@ -1278,9 +1278,7 @@ impl Config { } pub fn env_config(&self) -> CargoResult<&EnvConfig> { - self.env_config.try_borrow_with(|| { - self.get_env_config() - }) + self.env_config.try_borrow_with(|| self.get_env_config()) } /// This is used to validate the `term` table has valid syntax. @@ -2006,7 +2004,7 @@ impl EnvConfigValue { EnvConfigValue { value, force: false, - relative: false + relative: false, } } diff --git a/tests/testsuite/cargo_env_config.rs b/tests/testsuite/cargo_env_config.rs index 742f668950c..874976b38ed 100644 --- a/tests/testsuite/cargo_env_config.rs +++ b/tests/testsuite/cargo_env_config.rs @@ -6,16 +6,19 @@ use cargo_test_support::{basic_bin_manifest, project}; fn env_basic() { let p = project() .file("Cargo.toml", &basic_bin_manifest("foo")) - .file("src/main.rs", r#" + .file( + "src/main.rs", + r#" use std::env; fn main() { println!( "compile-time:{}", env!("ENV_TEST_1233") ); println!( "run-time:{}", env::var("ENV_TEST_1233").unwrap()); } - "#) + "#, + ) .file( - ".cargo/config", - r#" + ".cargo/config", + r#" [env] ENV_TEST_1233 = "Hello" "#, @@ -32,13 +35,16 @@ fn env_basic() { fn env_invalid() { let p = project() .file("Cargo.toml", &basic_bin_manifest("foo")) - .file("src/main.rs", r#" + .file( + "src/main.rs", + r#" fn main() { } - "#) + "#, + ) .file( - ".cargo/config", - r#" + ".cargo/config", + r#" [env] ENV_TEST_BOOL = false "#, @@ -55,16 +61,19 @@ fn env_invalid() { fn env_force() { let p = project() .file("Cargo.toml", &basic_bin_manifest("foo")) - .file("src/main.rs", r#" + .file( + "src/main.rs", + r#" use std::env; fn main() { println!( "ENV_TEST_FORCED:{}", env!("ENV_TEST_FORCED") ); println!( "ENV_TEST_UNFORCED:{}", env!("ENV_TEST_UNFORCED") ); } - "#) + "#, + ) .file( - ".cargo/config", - r#" + ".cargo/config", + r#" [env] ENV_TEST_UNFORCED = "from-config" ENV_TEST_FORCED = { value = "from-config", force = true } @@ -84,7 +93,9 @@ fn env_force() { fn env_relative() { let p = project() .file("Cargo.toml", &basic_bin_manifest("foo2")) - .file("src/main.rs", r#" + .file( + "src/main.rs", + r#" use std::env; use std::path::Path; fn main() { @@ -94,10 +105,11 @@ fn env_relative() { assert!( Path::new(env!("ENV_TEST_ABSOLUTE")).is_absolute() ); assert!( !Path::new(env!("ENV_TEST_RELATIVE")).is_absolute() ); } - "#) + "#, + ) .file( - ".cargo/config", - r#" + ".cargo/config", + r#" [env] ENV_TEST_RELATIVE = "Cargo.toml" ENV_TEST_ABSOLUTE = { value = "Cargo.toml", relative = true } @@ -105,6 +117,5 @@ fn env_relative() { ) .build(); - p.cargo("run") - .run(); + p.cargo("run").run(); } From 7990ab53f5483ab58da61d1a54836dbee46e9231 Mon Sep 17 00:00:00 2001 From: Martin Donlon Date: Mon, 15 Feb 2021 20:21:22 -0800 Subject: [PATCH 3/4] Add `-Z configurable-env` to gate [env] usage Added the `-Z configurable-env` option which controls whether the data from the [env] section is used. Updated the tests to pass the flag and to masquerade as the nightly cargo. --- src/cargo/core/compiler/compilation.rs | 10 ++++++---- src/cargo/core/features.rs | 2 ++ tests/testsuite/cargo_env_config.rs | 13 +++++++++---- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/cargo/core/compiler/compilation.rs b/src/cargo/core/compiler/compilation.rs index 50f5945a804..40389e7ae7b 100644 --- a/src/cargo/core/compiler/compilation.rs +++ b/src/cargo/core/compiler/compilation.rs @@ -340,10 +340,12 @@ impl<'cfg> Compilation<'cfg> { .env("CARGO_PKG_AUTHORS", &pkg.authors().join(":")) .cwd(pkg.root()); - // Apply any environment variables from the config - for (key, value) in self.config.env_config()?.iter() { - if value.is_force() || cmd.get_env(&key).is_none() { - cmd.env(&key, value.resolve(&self.config)); + if self.config.cli_unstable().configurable_env { + // Apply any environment variables from the config + for (key, value) in self.config.env_config()?.iter() { + if value.is_force() || cmd.get_env(&key).is_none() { + cmd.env(&key, value.resolve(&self.config)); + } } } diff --git a/src/cargo/core/features.rs b/src/cargo/core/features.rs index 494e3399300..3b6c4e9d7d3 100644 --- a/src/cargo/core/features.rs +++ b/src/cargo/core/features.rs @@ -444,6 +444,7 @@ pub struct CliUnstable { pub weak_dep_features: bool, pub extra_link_arg: bool, pub credential_process: bool, + pub configurable_env: bool, } const STABILIZED_COMPILE_PROGRESS: &str = "The progress bar is now always \ @@ -598,6 +599,7 @@ impl CliUnstable { "doctest-xcompile" => self.doctest_xcompile = parse_empty(k, v)?, "panic-abort-tests" => self.panic_abort_tests = parse_empty(k, v)?, "jobserver-per-rustc" => self.jobserver_per_rustc = parse_empty(k, v)?, + "configurable-env" => self.configurable_env = parse_empty(k, v)?, "features" => { // For now this is still allowed (there are still some // unstable options like "compare"). This should be removed at diff --git a/tests/testsuite/cargo_env_config.rs b/tests/testsuite/cargo_env_config.rs index 874976b38ed..963455382bc 100644 --- a/tests/testsuite/cargo_env_config.rs +++ b/tests/testsuite/cargo_env_config.rs @@ -25,7 +25,8 @@ fn env_basic() { ) .build(); - p.cargo("run") + p.cargo("run -Zconfigurable-env") + .masquerade_as_nightly_cargo() .with_stdout_contains("compile-time:Hello") .with_stdout_contains("run-time:Hello") .run(); @@ -51,7 +52,8 @@ fn env_invalid() { ) .build(); - p.cargo("build") + p.cargo("build -Zconfigurable-env") + .masquerade_as_nightly_cargo() .with_status(101) .with_stderr_contains("[..]`env.ENV_TEST_BOOL` expected a string, but found a boolean") .run(); @@ -81,7 +83,8 @@ fn env_force() { ) .build(); - p.cargo("run") + p.cargo("run -Zconfigurable-env") + .masquerade_as_nightly_cargo() .env("ENV_TEST_FORCED", "from-env") .env("ENV_TEST_UNFORCED", "from-env") .with_stdout_contains("ENV_TEST_FORCED:from-config") @@ -117,5 +120,7 @@ fn env_relative() { ) .build(); - p.cargo("run").run(); + p.cargo("run -Zconfigurable-env") + .masquerade_as_nightly_cargo() + .run(); } From 05acf73604a2ac0e16e0bc5f58fff59166ddb84c Mon Sep 17 00:00:00 2001 From: Martin Donlon Date: Sat, 20 Feb 2021 13:49:08 -0800 Subject: [PATCH 4/4] Change env section parsing `untagged` enum variants are not recognised by serde if the variant contains a `Value` type. This change uses an transparent "inner" enum `Value` member, which is handled correctly by serde. --- src/cargo/util/config/mod.rs | 84 ++++++++++++----------------- tests/testsuite/cargo_env_config.rs | 21 +++++--- 2 files changed, 49 insertions(+), 56 deletions(-) diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index c5d658eda5d..a0dc0963c17 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -1248,37 +1248,9 @@ impl Config { &self.progress_config } - /// Create an EnvConfigValue hashmap from the "env" table - fn get_env_config(&self) -> CargoResult { - // We cannot use pure serde handling for this. The Value<_> type does not - // work when parsing the env table to a hashmap. So iterator through each - // entry in the "env" table, determine it's type and then use `get` method - // to deserialize it. - let env_table = &self.get_table(&ConfigKey::from_str("env"))?; - let mut vars = EnvConfig::new(); - - if env_table.is_none() { - return Ok(vars); - } - - let env_table = &env_table.as_ref().unwrap().val; - - for (key, value) in env_table.iter() { - let full_key = format!("env.{}", key); - let e = match value { - ConfigValue::Table(..) => self.get::(&full_key)?, - _ => { - let v = self.get::>(&full_key)?; - EnvConfigValue::from_value(v) - } - }; - vars.insert(key.clone(), e); - } - Ok(vars) - } - pub fn env_config(&self) -> CargoResult<&EnvConfig> { - self.env_config.try_borrow_with(|| self.get_env_config()) + self.env_config + .try_borrow_with(|| self.get::("env")) } /// This is used to validate the `term` table has valid syntax. @@ -1991,33 +1963,47 @@ where } #[derive(Debug, Deserialize)] +#[serde(untagged)] +enum EnvConfigValueInner { + Simple(String), + WithOptions { + value: String, + #[serde(default)] + force: bool, + #[serde(default)] + relative: bool, + }, +} + +#[derive(Debug, Deserialize)] +#[serde(transparent)] pub struct EnvConfigValue { - value: Value, - #[serde(default)] - force: bool, - #[serde(default)] - relative: bool, + inner: Value, } impl EnvConfigValue { - fn from_value(value: Value) -> EnvConfigValue { - EnvConfigValue { - value, - force: false, - relative: false, - } - } - pub fn is_force(&self) -> bool { - self.force + match self.inner.val { + EnvConfigValueInner::Simple(_) => false, + EnvConfigValueInner::WithOptions { force, .. } => force, + } } pub fn resolve<'a>(&'a self, config: &Config) -> Cow<'a, OsStr> { - if self.relative { - let p = self.value.definition.root(config).join(&self.value.val); - Cow::Owned(p.into_os_string()) - } else { - Cow::Borrowed(OsStr::new(&self.value.val)) + match self.inner.val { + EnvConfigValueInner::Simple(ref s) => Cow::Borrowed(OsStr::new(s.as_str())), + EnvConfigValueInner::WithOptions { + ref value, + relative, + .. + } => { + if relative { + let p = self.inner.definition.root(config).join(&value); + Cow::Owned(p.into_os_string()) + } else { + Cow::Borrowed(OsStr::new(value.as_str())) + } + } } } } diff --git a/tests/testsuite/cargo_env_config.rs b/tests/testsuite/cargo_env_config.rs index 963455382bc..83ebdbb18f6 100644 --- a/tests/testsuite/cargo_env_config.rs +++ b/tests/testsuite/cargo_env_config.rs @@ -55,7 +55,7 @@ fn env_invalid() { p.cargo("build -Zconfigurable-env") .masquerade_as_nightly_cargo() .with_status(101) - .with_stderr_contains("[..]`env.ENV_TEST_BOOL` expected a string, but found a boolean") + .with_stderr_contains("[..]could not load config key `env.ENV_TEST_BOOL`") .run(); } @@ -70,6 +70,7 @@ fn env_force() { fn main() { println!( "ENV_TEST_FORCED:{}", env!("ENV_TEST_FORCED") ); println!( "ENV_TEST_UNFORCED:{}", env!("ENV_TEST_UNFORCED") ); + println!( "ENV_TEST_UNFORCED_DEFAULT:{}", env!("ENV_TEST_UNFORCED_DEFAULT") ); } "#, ) @@ -77,7 +78,8 @@ fn env_force() { ".cargo/config", r#" [env] - ENV_TEST_UNFORCED = "from-config" + ENV_TEST_UNFORCED_DEFAULT = "from-config" + ENV_TEST_UNFORCED = { value = "from-config", force = false } ENV_TEST_FORCED = { value = "from-config", force = true } "#, ) @@ -87,8 +89,10 @@ fn env_force() { .masquerade_as_nightly_cargo() .env("ENV_TEST_FORCED", "from-env") .env("ENV_TEST_UNFORCED", "from-env") + .env("ENV_TEST_UNFORCED_DEFAULT", "from-env") .with_stdout_contains("ENV_TEST_FORCED:from-config") .with_stdout_contains("ENV_TEST_UNFORCED:from-env") + .with_stdout_contains("ENV_TEST_UNFORCED_DEFAULT:from-env") .run(); } @@ -102,11 +106,13 @@ fn env_relative() { use std::env; use std::path::Path; fn main() { + println!( "ENV_TEST_REGULAR:{}", env!("ENV_TEST_REGULAR") ); + println!( "ENV_TEST_REGULAR_DEFAULT:{}", env!("ENV_TEST_REGULAR_DEFAULT") ); println!( "ENV_TEST_RELATIVE:{}", env!("ENV_TEST_RELATIVE") ); - println!( "ENV_TEST_ABSOLUTE:{}", env!("ENV_TEST_ABSOLUTE") ); - assert!( Path::new(env!("ENV_TEST_ABSOLUTE")).is_absolute() ); - assert!( !Path::new(env!("ENV_TEST_RELATIVE")).is_absolute() ); + assert!( Path::new(env!("ENV_TEST_RELATIVE")).is_absolute() ); + assert!( !Path::new(env!("ENV_TEST_REGULAR")).is_absolute() ); + assert!( !Path::new(env!("ENV_TEST_REGULAR_DEFAULT")).is_absolute() ); } "#, ) @@ -114,8 +120,9 @@ fn env_relative() { ".cargo/config", r#" [env] - ENV_TEST_RELATIVE = "Cargo.toml" - ENV_TEST_ABSOLUTE = { value = "Cargo.toml", relative = true } + ENV_TEST_REGULAR = { value = "Cargo.toml", relative = false } + ENV_TEST_REGULAR_DEFAULT = "Cargo.toml" + ENV_TEST_RELATIVE = { value = "Cargo.toml", relative = true } "#, ) .build();