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

add support for target.'cfg(..)'.runner #5959

Merged
merged 2 commits into from
Sep 9, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 4 additions & 11 deletions src/cargo/core/compiler/build_context/mod.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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
}
Expand Down
52 changes: 42 additions & 10 deletions src/cargo/core/compiler/compilation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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> {
Expand Down Expand Up @@ -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(),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made this eager which means that in the case of cfg collision cargo run will immediately fail.

Alternatively, if we kept this lazy in that scenario cargo run will first build the crate and then emit an error message about the runners (I think).

I don't know if making this eager has any perceivable effects on existing uses of cargo run.

Which behavior do we want here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah I think eagerly loading it here should be fine!

target_runner: target_runner(&bcx)?,
})
}

Expand Down Expand Up @@ -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`.
Expand All @@ -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);
Expand Down Expand Up @@ -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)
}
11 changes: 11 additions & 0 deletions src/cargo/util/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
5 changes: 5 additions & 0 deletions src/doc/src/reference/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
80 changes: 80 additions & 0 deletions tests/testsuite/tool_paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}