Auto merge of #5959 - japaric:cfg-runner, r=alexcrichton

add support for `target.'cfg(..)'.runner`

`cfg` can be used to reduce the number of `runner`s one needs to type in
`.cargo/config`. So instead of writing this:

``` toml
[target.thumbv6m-none-eabi]
runner = "arm-none-eabi-gdb"

[target.thumbv7m-none-eabi]
runner = "arm-none-eabi-gdb"

[target.thumbv7em-none-eabi]
runner = "arm-none-eabi-gdb"

[target.thumbv7em-none-eabihf]
runner = "arm-none-eabi-gdb"
```

one can write:

``` toml
[target.'cfg(all(target_arch = "arm", target_os = "none"))']
runner = "arm-none-eabi-gdb"
```

Handling of edge cases:

- When `target.$triple.runner` matches it will be chosen regardless of the
  existence of others `target.'cfg(..)'.runner`s.

- If more than one `target.'cfg(..)'.runner` matches the target the command will
  fail.

closes #5946

---

Does this sound like a reasonable feature / implementation?

Author: bors

src/cargo/core/compiler/build_context/mod.rs

@@ -1,7 +1,7 @@
 use std::collections::HashMap;
 use std::env;
 use std::path::{Path, PathBuf};
-use std::str::{self, FromStr};
+use std::str;
 
 use core::profiles::Profiles;
 use core::{Dependency, Workspace};
@@ -393,16 +393,9 @@ fn env_args(
     // ...including target.'cfg(...)'.rustflags
     if let Some(target_cfg) = target_cfg {
         if let Some(table) = config.get_table("target")? {
-            let cfgs = table.val.keys().filter_map(|t| {
-                if t.starts_with("cfg(") && t.ends_with(')') {
-                    let cfg = &t[4..t.len() - 1];
-                    CfgExpr::from_str(cfg).ok().and_then(|c| {
-                        if c.matches(target_cfg) {
-                            Some(t)
-                        } else {
-                            None
-                        }
-                    })
+            let cfgs = table.val.keys().filter_map(|key| {
+                if CfgExpr::matches_key(key, target_cfg) {
+                    Some(key)
                 } else {
                     None
                 }

src/cargo/core/compiler/compilation.rs

@@ -4,10 +4,9 @@ use std::ffi::OsStr;
 use std::path::PathBuf;
 
 use semver::Version;
-use lazycell::LazyCell;
 
 use core::{Edition, Package, PackageId, Target, TargetKind};
-use util::{self, join_paths, process, CargoResult, Config, ProcessBuilder};
+use util::{self, join_paths, process, CargoResult, CfgExpr, Config, ProcessBuilder};
 use super::BuildContext;
 
 pub struct Doctest {
@@ -77,7 +76,7 @@ pub struct Compilation<'cfg> {
     config: &'cfg Config,
     rustc_process: ProcessBuilder,
 
-    target_runner: LazyCell<Option<(PathBuf, Vec<String>)>>,
+    target_runner: Option<(PathBuf, Vec<String>)>,
 }
 
 impl<'cfg> Compilation<'cfg> {
@@ -124,7 +123,7 @@ impl<'cfg> Compilation<'cfg> {
             rustc_process: rustc,
             host: bcx.host_triple().to_string(),
             target: bcx.target_triple().to_string(),
-            target_runner: LazyCell::new(),
+            target_runner: target_runner(&bcx)?,
         })
     }
 
@@ -156,11 +155,8 @@ impl<'cfg> Compilation<'cfg> {
         self.fill_env(process(cmd), pkg, true)
     }
 
-    fn target_runner(&self) -> CargoResult<&Option<(PathBuf, Vec<String>)>> {
-        self.target_runner.try_borrow_with(|| {
-            let key = format!("target.{}.runner", self.target);
-            Ok(self.config.get_path_and_args(&key)?.map(|v| v.val))
-        })
+    fn target_runner(&self) -> &Option<(PathBuf, Vec<String>)> {
+        &self.target_runner
     }
 
     /// See `process`.
@@ -169,7 +165,7 @@ impl<'cfg> Compilation<'cfg> {
         cmd: T,
         pkg: &Package,
     ) -> CargoResult<ProcessBuilder> {
-        let builder = if let Some((ref runner, ref args)) = *self.target_runner()? {
+        let builder = if let Some((ref runner, ref args)) = *self.target_runner() {
             let mut builder = process(runner);
             builder.args(args);
             builder.arg(cmd);
@@ -263,3 +259,39 @@ fn pre_version_component(v: &Version) -> String {
 
     ret
 }
+
+fn target_runner(bcx: &BuildContext) -> CargoResult<Option<(PathBuf, Vec<String>)>> {
+    let target = bcx.target_triple();
+
+    // try target.{}.runner
+    let key = format!("target.{}.runner", target);
+    if let Some(v) = bcx.config.get_path_and_args(&key)? {
+        return Ok(Some(v.val));
+    }
+
+    // try target.'cfg(...)'.runner
+    if let Some(target_cfg) = bcx.target_info.cfg() {
+        if let Some(table) = bcx.config.get_table("target")? {
+            let mut matching_runner = None;
+
+            for key in table.val.keys() {
+                if CfgExpr::matches_key(key, target_cfg) {
+                    let key = format!("target.{}.runner", key);
+                    if let Some(runner) = bcx.config.get_path_and_args(&key)? {
+                        // more than one match, error out
+                        if matching_runner.is_some() {
+                            bail!("several matching instances of `target.'cfg(..)'.runner` \
+                                   in `.cargo/config`")
+                        }
+
+                        matching_runner = Some(runner.val);
+                    }
+                }
+            }
+
+            return Ok(matching_runner);
+        }
+    }
+
+    Ok(None)
+}

src/cargo/util/cfg.rs

@@ -60,6 +60,17 @@ impl fmt::Display for Cfg {
 }
 
 impl CfgExpr {
+    /// Utility function to check if the key, "cfg(..)" matches the `target_cfg`
+    pub fn matches_key(key: &str, target_cfg: &[Cfg]) -> bool {
+        if key.starts_with("cfg(") && key.ends_with(')') {
+            let cfg = &key[4..key.len() - 1 ];
+
+            CfgExpr::from_str(cfg).ok().map(|ce| ce.matches(target_cfg)).unwrap_or(false)
+        } else {
+            false
+        }
+    }
+
     pub fn matches(&self, cfg: &[Cfg]) -> bool {
         match *self {
             CfgExpr::Not(ref e) => !e.matches(cfg),

src/doc/src/reference/config.md

@@ -83,6 +83,11 @@ rustflags = ["..", ".."]
 # are concatenated. The `cfg` syntax only applies to rustflags, and not to
 # linker.
 rustflags = ["..", ".."]
+# Similar for the $triple configuration, but using the `cfg` syntax.
+# If one or more `cfg`s, and a $triple target are candidates, then the $triple
+# will be used
+# If several `cfg` are candidates, then the build will error
+runner = ".."
 
 # Configuration keys related to the registry
 [registry]

tests/testsuite/tool_paths.rs

@@ -167,3 +167,83 @@ fn custom_runner() {
 ",
         ).run();
 }
+
+// can set a custom runner via `target.'cfg(..)'.runner`
+#[test]
+fn custom_runner_cfg() {
+    let p = project()
+        .file("src/main.rs", "fn main() {}")
+        .file(
+            ".cargo/config",
+            r#"
+            [target.'cfg(not(target_os = "none"))']
+            runner = "nonexistent-runner -r"
+            "#,
+        ).build();
+
+    p.cargo("run -- --param")
+        .with_status(101)
+        .with_stderr_contains(&format!(
+            "\
+[COMPILING] foo v0.0.1 (CWD)
+[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
+[RUNNING] `nonexistent-runner -r target/debug/foo[EXE] --param`
+",
+        )).run();
+}
+
+// custom runner set via `target.$triple.runner` have precende over `target.'cfg(..)'.runner`
+#[test]
+fn custom_runner_cfg_precedence() {
+    let target = rustc_host();
+
+    let p = project()
+        .file("src/main.rs", "fn main() {}")
+        .file(
+            ".cargo/config",
+            &format!(
+                r#"
+            [target.'cfg(not(target_os = "none"))']
+            runner = "ignored-runner"
+
+            [target.{}]
+            runner = "nonexistent-runner -r"
+        "#,
+                target
+            ),
+        ).build();
+
+    p.cargo("run -- --param")
+        .with_status(101)
+        .with_stderr_contains(&format!(
+            "\
+            [COMPILING] foo v0.0.1 (CWD)
+[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
+[RUNNING] `nonexistent-runner -r target/debug/foo[EXE] --param`
+",
+        )).run();
+}
+
+#[test]
+fn custom_runner_cfg_collision() {
+    let p = project()
+        .file("src/main.rs", "fn main() {}")
+        .file(
+            ".cargo/config",
+            r#"
+            [target.'cfg(not(target_arch = "avr"))']
+            runner = "true"
+
+            [target.'cfg(not(target_os = "none"))']
+            runner = "false"
+            "#,
+        ).build();
+
+    p.cargo("run -- --param")
+        .with_status(101)
+        .with_stderr_contains(&format!(
+            "\
+[ERROR] several matching instances of `target.'cfg(..)'.runner` in `.cargo/config`
+",
+        )).run();
+}