Fix bug with PathAndArg config values

Author: Jake-Shadle

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::<Option<config::PathAndArgs>>(&key)? {
         let path = v.path.resolve_program(bcx.config);
         return Ok(Some((path, v.args)));

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<String> = 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<String> = vals.into_iter().map(|vd| vd.0).collect();
+        visitor.visit_newtype_struct(vals.into_deserializer())
     }
 
     fn deserialize_enum<V>(

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<Vec<(String, Definition)>> {
+    fn get_list_or_string(
+        &self,
+        key: &ConfigKey,
+        merge: bool,
+    ) -> CargoResult<Vec<(String, Definition)>> {
         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<String>);
+
 #[macro_export]
 macro_rules! __shell_print {
     ($config:expr, $which:ident, $newline:literal, $($arg:tt)*) => ({

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::<StringList>::deserialize(deserializer)?;
+        let vsl = Value::<UnmergedStringList>::deserialize(deserializer)?;
         let mut strings = vsl.val.0;
         if strings.is_empty() {
             return Err(D::Error::invalid_length(0, &"at least one element"));

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() {