Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: make subcommands pick up [env] #10780

Closed
wants to merge 9 commits into from
11 changes: 10 additions & 1 deletion src/bin/cargo/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,16 @@ fn execute_external_subcommand(config: &Config, cmd: &str, args: &[&str]) -> Cli
};

let cargo_exe = config.cargo_exe()?;
let err = match ProcessBuilder::new(&command)
let mut cmd = ProcessBuilder::new(&command);

// Apply [env] section to subcommands
for (key, value) in config.env_config()?.iter() {
if util::config::env_config_valid(key, value)? && value.applies_to_subcommands() {
cmd.env(key, value.resolve(config));
}
}

let err = match cmd
.env(cargo::CARGO_ENV, cargo_exe)
.args(args)
.exec_replace()
Expand Down
9 changes: 5 additions & 4 deletions src/cargo/core/compiler/compilation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -355,14 +355,15 @@ impl<'cfg> Compilation<'cfg> {

// Apply any environment variables from the config
for (key, value) in self.config.env_config()?.iter() {
// fail on invalid env options, skip vars that exist in the env
if !config::env_config_valid(key, value)? {
continue;
}
// never override a value that has already been set by cargo
if cmd.get_envs().contains_key(key) {
continue;
}

if value.is_force() || env::var_os(key).is_none() {
cmd.env(key, value.resolve(self.config));
}
cmd.env(key, value.resolve(self.config));
}

Ok(cmd)
Expand Down
24 changes: 24 additions & 0 deletions src/cargo/util/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2357,6 +2357,8 @@ enum EnvConfigValueInner {
force: bool,
#[serde(default)]
relative: bool,
#[serde(default)]
apply_to_subcommands: bool,
},
}

Expand All @@ -2374,6 +2376,16 @@ impl EnvConfigValue {
}
}

pub fn applies_to_subcommands(&self) -> bool {
match self.inner.val {
EnvConfigValueInner::Simple(_) => true,
EnvConfigValueInner::WithOptions {
apply_to_subcommands,
..
} => apply_to_subcommands,
}
}

pub fn resolve<'a>(&'a self, config: &Config) -> Cow<'a, OsStr> {
match self.inner.val {
EnvConfigValueInner::Simple(ref s) => Cow::Borrowed(OsStr::new(s.as_str())),
Expand All @@ -2395,6 +2407,18 @@ impl EnvConfigValue {

pub type EnvConfig = HashMap<String, EnvConfigValue>;

/// Whether a given pair (k, v) repredenting an environment variable coming from the [env] section
/// of Cargo's config is valid and should be used.
pub fn env_config_valid(k: &str, v: &EnvConfigValue) -> CargoResult<bool> {
if k.starts_with("CARGO_") {
bail!("setting CARGO_ variables from [env] is not allowed.");
}
if !v.is_force() && env::var_os(k).is_some() {
return Ok(false);
}
Ok(true)
}

/// A type to deserialize a list of strings from a toml file.
///
/// Supports deserializing either a whitespace-separated list of arguments in a
Expand Down
7 changes: 6 additions & 1 deletion src/doc/src/reference/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ repository. Can be overridden with the `--vcs` CLI option.
### `[env]`

The `[env]` section allows you to set additional environment variables for
build scripts, rustc invocations, `cargo run` and `cargo build`.
build scripts, rustc invocations, subcommands, `cargo run` and `cargo build`.

```toml
[env]
Expand All @@ -499,10 +499,15 @@ is relative to the parent directory of the `.cargo` directory that contains the
`config.toml` file. The value of the environment variable will be the full
absolute path.

Normally, variables will also apply to third-party subcommands, such as when
invoking `cargo flamegraph`. This can be opted out of by setting the
`apply_to_subcommands` flag.

```toml
[env]
TMPDIR = { value = "/home/tmp", force = true }
OPENSSL_DIR = { value = "vendor/openssl", relative = true }
PATH = { value = "/bin/", force = true, apply_to_subcommands = false }
```

### `[future-incompat-report]`
Expand Down
79 changes: 71 additions & 8 deletions tests/testsuite/cargo_env_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,29 +128,92 @@ fn env_relative() {
}

#[cargo_test]
fn env_no_override() {
fn env_external_subcommand() {
let p = project()
.file("Cargo.toml", &basic_bin_manifest("unchanged"))
.file("Cargo.toml", &basic_bin_manifest("cargo-fake-subcommand"))
.file(
"src/main.rs",
r#"
use std::env;
fn main() {
// ENV_TEST_SUB should be available to the build AND to the subcommand.
assert_eq!(env!("ENV_TEST_SUB"), "TEST_VALUE");
assert_eq!(&env::var("ENV_TEST_SUB").unwrap(), "TEST_VALUE");

// ENV_TEST_NOSUB should be available to the build, but not to the subcommand.
assert!(env::var_os("ENV_TEST_NOSUB").is_none());
assert_eq!(env!("ENV_TEST_NOSUB"), "TEST_VALUE");
}
"#,
)
.file(
".cargo/config",
r#"
[env]
ENV_TEST_SUB = "TEST_VALUE"
ENV_TEST_NOSUB = { value = "TEST_VALUE", apply_to_subcommands = false }
"#,
)
.build();
p.cargo("install --path .").run();
p.cargo("fake-subcommand").run();
}

#[cargo_test]
fn env_no_cargo_vars() {
let p = project()
.file("Cargo.toml", &basic_bin_manifest("foo"))
.file(
"src/main.rs",
r#"
use std::env;
fn main() {
println!( "CARGO_PKG_NAME:{}", env!("CARGO_PKG_NAME") );
}
"#,
)
.file(
".cargo/config",
r#"
[env]
CARGO_PKG_NAME = { value = "from-config", force = true }
CARGO_HOME = { value = "/dev/null", force = true }
"#,
)
.build();

p.cargo("run -Zconfigurable-env")
.masquerade_as_nightly_cargo()
.with_stdout_contains("CARGO_PKG_NAME:unchanged")
p.cargo("build")
.with_status(101)
.with_stderr("[..]setting CARGO_ variables from [env] is not allowed.")
.run();
}

#[cargo_test]
fn env_build_script() {
let p = project()
.file("Cargo.toml", &basic_bin_manifest("foo"))
.file(
"src/main.rs",
"fn main() {}",
)
.file(
"build.rs",
r#"
use std::env;

fn main() {
// env should be set during the build script's build and execution.
assert_eq!(env!("ENV_TEST_VAR"), "TEST_VAR_VALUE");
assert_eq!(env::var("ENV_TEST_VAR").unwrap(), "TEST_VAR_VALUE");
}
"#
)
.file(
".cargo/config",
r#"
[env]
ENV_TEST_VAR = "TEST_VAR_VALUE"
"#,
)
.build();

p.cargo("build")
.run();
}