From a9a154f783fde8dad9f4ed437fc3e42f7cfe1b63 Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Mon, 17 Aug 2020 19:23:49 +0200 Subject: [PATCH 1/2] Fix bug with PathAndArg config values --- src/cargo/core/compiler/compilation.rs | 1 + src/cargo/util/config/de.rs | 17 +++++++++------ src/cargo/util/config/mod.rs | 24 ++++++++++++++++++++- src/cargo/util/config/path.rs | 4 ++-- tests/testsuite/tool_paths.rs | 30 ++++++++++++++++++++++++++ 5 files changed, 67 insertions(+), 9 deletions(-) diff --git a/src/cargo/core/compiler/compilation.rs b/src/cargo/core/compiler/compilation.rs index e2b71ed02..f45997aa4 100644 --- a/src/cargo/core/compiler/compilation.rs +++ b/src/cargo/core/compiler/compilation.rs @@ -346,6 +346,7 @@ fn target_runner( // try target.{}.runner let key = format!("target.{}.runner", target); + if let Some(v) = bcx.config.get::>(&key)? { let path = v.path.resolve_program(bcx.config); return Ok(Some((path, v.args))); diff --git a/src/cargo/util/config/de.rs b/src/cargo/util/config/de.rs index 311ba4116..b822732eb 100644 --- a/src/cargo/util/config/de.rs +++ b/src/cargo/util/config/de.rs @@ -57,6 +57,7 @@ impl<'de, 'config> de::Deserializer<'de> for Deserializer<'config> { (None, Some(_)) => true, _ => false, }; + if use_env { // Future note: If you ever need to deserialize a non-self describing // map type, this should implement a starts_with check (similar to how @@ -187,13 +188,17 @@ impl<'de, 'config> de::Deserializer<'de> for Deserializer<'config> { where V: de::Visitor<'de>, { - if name == "StringList" { - let vals = self.config.get_list_or_string(&self.key)?; - let vals: Vec = vals.into_iter().map(|vd| vd.0).collect(); - visitor.visit_newtype_struct(vals.into_deserializer()) + let merge = if name == "StringList" { + true + } else if name == "UnmergedStringList" { + false } else { - visitor.visit_newtype_struct(self) - } + return visitor.visit_newtype_struct(self); + }; + + let vals = self.config.get_list_or_string(&self.key, merge)?; + let vals: Vec = vals.into_iter().map(|vd| vd.0).collect(); + visitor.visit_newtype_struct(vals.into_deserializer()) } fn deserialize_enum( diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index af356ce27..99f74ebd4 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -587,8 +587,21 @@ impl Config { } /// Helper for StringList type to get something that is a string or list. - fn get_list_or_string(&self, key: &ConfigKey) -> CargoResult> { + fn get_list_or_string( + &self, + key: &ConfigKey, + merge: bool, + ) -> CargoResult> { let mut res = Vec::new(); + + if !merge { + self.get_env_list(key, &mut res)?; + + if !res.is_empty() { + return Ok(res); + } + } + match self.get_cv(key)? { Some(CV::List(val, _def)) => res.extend(val), Some(CV::String(val, def)) => { @@ -602,6 +615,7 @@ impl Config { } self.get_env_list(key, &mut res)?; + Ok(res) } @@ -1766,6 +1780,14 @@ impl StringList { } } +/// StringList automatically merges config values with environment values, +/// this instead follows the precedence rules, so that eg. a string list found +/// in the environment will be used instead of one in a config file. +/// +/// This is currently only used by `PathAndArgs` +#[derive(Debug, Deserialize)] +pub struct UnmergedStringList(Vec); + #[macro_export] macro_rules! __shell_print { ($config:expr, $which:ident, $newline:literal, $($arg:tt)*) => ({ diff --git a/src/cargo/util/config/path.rs b/src/cargo/util/config/path.rs index 030ff085c..f859b5563 100644 --- a/src/cargo/util/config/path.rs +++ b/src/cargo/util/config/path.rs @@ -1,4 +1,4 @@ -use super::{Config, StringList, Value}; +use super::{Config, UnmergedStringList, Value}; use serde::{de::Error, Deserialize}; use std::path::PathBuf; @@ -55,7 +55,7 @@ impl<'de> serde::Deserialize<'de> for PathAndArgs { where D: serde::Deserializer<'de>, { - let vsl = Value::::deserialize(deserializer)?; + let vsl = Value::::deserialize(deserializer)?; let mut strings = vsl.val.0; if strings.is_empty() { return Err(D::Error::invalid_length(0, &"at least one element")); diff --git a/tests/testsuite/tool_paths.rs b/tests/testsuite/tool_paths.rs index ad7f2bc95..3eefec780 100644 --- a/tests/testsuite/tool_paths.rs +++ b/tests/testsuite/tool_paths.rs @@ -278,6 +278,36 @@ fn custom_runner_env() { .run(); } +#[cargo_test] +fn custom_runner_env_overrides_config() { + let target = rustc_host(); + let p = project() + .file("src/main.rs", "fn main() {}") + .file( + ".cargo/config.toml", + &format!( + r#" + [target.{}] + runner = "should-not-run -r" + "#, + target + ), + ) + .build(); + + let key = format!( + "CARGO_TARGET_{}_RUNNER", + target.to_uppercase().replace('-', "_") + ); + + p.cargo("run") + .env(&key, "should-run --foo") + .stream() + .with_status(101) + .with_stderr_contains("[RUNNING] `should-run --foo target/debug/foo[EXE]`") + .run(); +} + #[cargo_test] #[cfg(unix)] // Assumes `true` is in PATH. fn custom_runner_env_true() { From ddc7090f495afaff48b39ccc74036a1e75c6368c Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Mon, 17 Aug 2020 19:53:16 +0200 Subject: [PATCH 2/2] Remove debug only code --- tests/testsuite/tool_paths.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/testsuite/tool_paths.rs b/tests/testsuite/tool_paths.rs index 3eefec780..6781f7399 100644 --- a/tests/testsuite/tool_paths.rs +++ b/tests/testsuite/tool_paths.rs @@ -302,7 +302,6 @@ fn custom_runner_env_overrides_config() { p.cargo("run") .env(&key, "should-run --foo") - .stream() .with_status(101) .with_stderr_contains("[RUNNING] `should-run --foo target/debug/foo[EXE]`") .run();