From 20d96f3d94b4d39ac49b158695bec76dc6823327 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Sat, 12 Mar 2022 01:23:18 +0800 Subject: [PATCH 1/8] Tests for setting multitarget in config --- tests/testsuite/multitarget.rs | 141 ++++++++++++++++++++++++++++++++- 1 file changed, 139 insertions(+), 2 deletions(-) diff --git a/tests/testsuite/multitarget.rs b/tests/testsuite/multitarget.rs index afa8ea3c9bc..5b4e3cff1fe 100644 --- a/tests/testsuite/multitarget.rs +++ b/tests/testsuite/multitarget.rs @@ -10,7 +10,27 @@ fn double_target_rejected() { .build(); p.cargo("build --target a --target b") - .with_stderr("error: specifying multiple `--target` flags requires `-Zmultitarget`") + .with_stderr("[ERROR] specifying multiple `--target` flags requires `-Zmultitarget`") + .with_status(101) + .run(); +} + +#[cargo_test] +fn double_target_rejected_with_config() { + let p = project() + .file("Cargo.toml", &basic_manifest("foo", "1.0.0")) + .file("src/main.rs", "fn main() {}") + .file( + ".cargo/config.toml", + r#" + [build] + target = ["a", "b"] + "#, + ) + .build(); + + p.cargo("build") + .with_stderr("[ERROR] specifying multiple `--target` flags requires `-Zmultitarget`") .with_status(101) .run(); } @@ -39,6 +59,35 @@ fn simple_build() { assert!(p.target_bin(t2, "foo").is_file()); } +#[cargo_test] +fn simple_build_with_config() { + if cross_compile::disabled() { + return; + } + let t1 = cross_compile::alternate(); + let t2 = rustc_host(); + let p = project() + .file("Cargo.toml", &basic_manifest("foo", "1.0.0")) + .file("src/main.rs", "fn main() {}") + .file( + ".cargo/config.toml", + &format!( + r#" + [unstable] + multitarget = true + [build] + target = ["{t1}", "{t2}"] + "# + ), + ) + .build(); + + p.cargo("build").masquerade_as_nightly_cargo().run(); + + assert!(p.target_bin(t1, "foo").is_file()); + assert!(p.target_bin(t2, "foo").is_file()); +} + #[cargo_test] fn simple_test() { if !cross_compile::can_run_on_host() { @@ -70,7 +119,7 @@ fn simple_run() { .build(); p.cargo("run -Z multitarget --target a --target b") - .with_stderr("error: only one `--target` argument is supported") + .with_stderr("[ERROR] only one `--target` argument is supported") .with_status(101) .masquerade_as_nightly_cargo() .run(); @@ -142,3 +191,91 @@ fn same_value_twice() { assert!(p.target_bin(t, "foo").is_file()); } + +#[cargo_test] +fn same_value_twice_with_config() { + if cross_compile::disabled() { + return; + } + let t = rustc_host(); + let p = project() + .file("Cargo.toml", &basic_manifest("foo", "1.0.0")) + .file("src/main.rs", "fn main() {}") + .file( + ".cargo/config.toml", + &format!( + r#" + [unstable] + multitarget = true + [build] + target = ["{t}", "{t}"] + "# + ), + ) + .build(); + + p.cargo("build").masquerade_as_nightly_cargo().run(); + + assert!(p.target_bin(t, "foo").is_file()); +} + +#[cargo_test] +fn works_with_config_in_both_string_or_list() { + if cross_compile::disabled() { + return; + } + let t = rustc_host(); + let p = project() + .file("Cargo.toml", &basic_manifest("foo", "1.0.0")) + .file("src/main.rs", "fn main() {}") + .file( + ".cargo/config.toml", + &format!( + r#" + [unstable] + multitarget = true + [build] + target = "{t}" + "# + ), + ) + .build(); + + p.cargo("build").masquerade_as_nightly_cargo().run(); + + assert!(p.target_bin(t, "foo").is_file()); + + p.cargo("clean").run(); + + p.change_file( + ".cargo/config.toml", + &format!( + r#" + [unstable] + multitarget = true + [build] + target = ["{t}"] + "# + ), + ); + + p.cargo("build").masquerade_as_nightly_cargo().run(); + + assert!(p.target_bin(t, "foo").is_file()); +} + +#[cargo_test] +fn works_with_env() { + let t = rustc_host(); + let p = project() + .file("Cargo.toml", &basic_manifest("foo", "1.0.0")) + .file("src/main.rs", "fn main() {}") + .build(); + + p.cargo("build") + .env("CARGO_BUILD_TARGET", t) + .masquerade_as_nightly_cargo() + .run(); + + assert!(p.target_bin(t, "foo").is_file()); +} From 76055d2b63b8ef55c3d142d95f5656b5834ea707 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Sat, 12 Mar 2022 01:54:34 +0800 Subject: [PATCH 2/8] Support multitarget in .cargo/config.toml For backward compatible reason, `build.target` starts accepting both array of string and a single string. --- src/cargo/core/compiler/compile_kind.rs | 38 ++++++++++++------ src/cargo/util/config/mod.rs | 52 ++++++++++++++++++++++++- 2 files changed, 78 insertions(+), 12 deletions(-) diff --git a/src/cargo/core/compiler/compile_kind.rs b/src/cargo/core/compiler/compile_kind.rs index 605bb27f2ca..b108a7c9bc0 100644 --- a/src/cargo/core/compiler/compile_kind.rs +++ b/src/cargo/core/compiler/compile_kind.rs @@ -1,4 +1,5 @@ use crate::core::Target; +use crate::util::config::BuildTargetConfigValue; use crate::util::errors::CargoResult; use crate::util::interning::InternedString; use crate::util::{Config, StableHasher}; @@ -66,19 +67,34 @@ impl CompileKind { .into_iter() .collect()); } - let kind = match &config.build_config()?.target { - Some(val) => { - let value = if val.raw_value().ends_with(".json") { - let path = val.clone().resolve_path(config); - path.to_str().expect("must be utf-8 in toml").to_string() - } else { - val.raw_value().to_string() - }; - CompileKind::Target(CompileTarget::new(&value)?) + + let kinds = match &config.build_config()?.target { + None => vec![CompileKind::Host], + Some(build_target_config) => { + let values = build_target_config.values(config); + if values.len() > 1 && !config.cli_unstable().multitarget { + bail!("specifying multiple `--target` flags requires `-Zmultitarget`") + } + values + .into_iter() + .map(|k| { + use BuildTargetConfigValue::*; + let value = match &k { + Path(p) => p.to_str().expect("must be utf-8 in toml"), + Simple(s) => s, + }; + CompileTarget::new(value).map(CompileKind::Target) + }) + // First collect into a set to deduplicate any `--target` passed + // more than once... + .collect::>>()? + // ... then generate a flat list for everything else to use. + .into_iter() + .collect() } - None => CompileKind::Host, }; - Ok(vec![kind]) + + Ok(kinds) } /// Hash used for fingerprinting. diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index 12d7301f58e..afabcc3ea1c 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -2169,7 +2169,7 @@ pub struct CargoBuildConfig { pub dep_info_basedir: Option, pub target_dir: Option, pub incremental: Option, - pub target: Option, + pub target: Option, pub jobs: Option, pub rustflags: Option, pub rustdocflags: Option, @@ -2180,6 +2180,56 @@ pub struct CargoBuildConfig { pub out_dir: Option, } +/// Configuration for `build.target`. +/// +/// Accepts in the following forms: +/// +/// ```toml +/// target = "a" +/// target = ["a"] +/// target = ["a", "b"] +/// ``` +#[derive(Debug, Deserialize)] +#[serde(transparent)] +pub struct BuildTargetConfig { + inner: Value, +} + +#[derive(Debug, Deserialize)] +#[serde(untagged)] +enum BuildTargetConfigInner { + One(String), + Many(Vec), +} + +impl BuildTargetConfig { + /// Gets values of `build.target` as a list of [`BuildTargetConfigValue`]. + pub fn values(&self, config: &Config) -> Vec> { + let def_root = self.inner.definition.root(config); + fn map<'a>(s: &'a str, root: &Path) -> BuildTargetConfigValue<'a> { + if s.ends_with(".json") { + // To absolute path. + BuildTargetConfigValue::Path(root.join(s)) + } else { + BuildTargetConfigValue::Simple(s) + } + } + match &self.inner.val { + BuildTargetConfigInner::One(s) => vec![map(s, def_root)], + BuildTargetConfigInner::Many(v) => v.iter().map(|s| map(s, def_root)).collect(), + } + } +} + +/// Represents a value of `build.target`. +pub enum BuildTargetConfigValue<'a> { + /// Path to a target specification file (in JSON). + /// + Path(PathBuf), + /// A string. Probably a target triple. + Simple(&'a str), +} + #[derive(Deserialize, Default)] struct TermConfig { verbose: Option, From 069b884f4c4e297484e1812e6bef9395078cd380 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Sat, 12 Mar 2022 01:59:03 +0800 Subject: [PATCH 3/8] Doc for support multitarget in config --- src/doc/src/reference/unstable.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/doc/src/reference/unstable.md b/src/doc/src/reference/unstable.md index 1c1db0ba713..3ad2bb94eef 100644 --- a/src/doc/src/reference/unstable.md +++ b/src/doc/src/reference/unstable.md @@ -235,6 +235,12 @@ or running tests for both targets: cargo test --target x86_64-unknown-linux-gnu --target i686-unknown-linux-gnu ``` +This can also be specified in `.cargo/config.toml` files. + +```toml +[build] +target = ["x86_64-unknown-linux-gnu", "i686-unknown-linux-gnu"] +``` #### New `dir-name` attribute From fcc88163662c79899d877566eba7dd90032f1b0c Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Sat, 12 Mar 2022 02:26:22 +0800 Subject: [PATCH 4/8] Derive debug from `BuildTargetConfigValue` --- src/cargo/util/config/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index afabcc3ea1c..764cd7be91f 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -2222,6 +2222,7 @@ impl BuildTargetConfig { } /// Represents a value of `build.target`. +#[derive(Debug)] pub enum BuildTargetConfigValue<'a> { /// Path to a target specification file (in JSON). /// From 40e13609e710cfdce2d972d3e1d93fbe9f324d92 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Tue, 29 Mar 2022 13:15:11 +0800 Subject: [PATCH 5/8] Remove unnecessary nightly masquerade --- tests/testsuite/multitarget.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/testsuite/multitarget.rs b/tests/testsuite/multitarget.rs index 5b4e3cff1fe..a598df5d319 100644 --- a/tests/testsuite/multitarget.rs +++ b/tests/testsuite/multitarget.rs @@ -272,10 +272,7 @@ fn works_with_env() { .file("src/main.rs", "fn main() {}") .build(); - p.cargo("build") - .env("CARGO_BUILD_TARGET", t) - .masquerade_as_nightly_cargo() - .run(); + p.cargo("build").env("CARGO_BUILD_TARGET", t).run(); assert!(p.target_bin(t, "foo").is_file()); } From 086145e593c1bfd0e25d69409f14967be9b1f9d9 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Tue, 29 Mar 2022 14:08:33 +0800 Subject: [PATCH 6/8] Remove unnecessary enum to reduce code complexity The relevant is not in the hot path for a cargo build, so it acceptable to allocation String in order to reduce code coomplexity --- src/cargo/core/compiler/compile_kind.rs | 44 +++++++---------------- src/cargo/util/config/mod.rs | 48 +++++++++++++------------ 2 files changed, 39 insertions(+), 53 deletions(-) diff --git a/src/cargo/core/compiler/compile_kind.rs b/src/cargo/core/compiler/compile_kind.rs index b108a7c9bc0..c3c921d4146 100644 --- a/src/cargo/core/compiler/compile_kind.rs +++ b/src/cargo/core/compiler/compile_kind.rs @@ -1,5 +1,4 @@ use crate::core::Target; -use crate::util::config::BuildTargetConfigValue; use crate::util::errors::CargoResult; use crate::util::interning::InternedString; use crate::util::{Config, StableHasher}; @@ -53,11 +52,8 @@ impl CompileKind { config: &Config, targets: &[String], ) -> CargoResult> { - if targets.len() > 1 && !config.cli_unstable().multitarget { - bail!("specifying multiple `--target` flags requires `-Zmultitarget`") - } - if !targets.is_empty() { - return Ok(targets + let dedup = |targets: &[String]| { + Ok(targets .iter() .map(|value| Ok(CompileKind::Target(CompileTarget::new(value)?))) // First collect into a set to deduplicate any `--target` passed @@ -65,36 +61,22 @@ impl CompileKind { .collect::>>()? // ... then generate a flat list for everything else to use. .into_iter() - .collect()); + .collect()) + }; + + if !targets.is_empty() { + if targets.len() > 1 && !config.cli_unstable().multitarget { + bail!("specifying multiple `--target` flags requires `-Zmultitarget`") + } + return dedup(targets); } let kinds = match &config.build_config()?.target { - None => vec![CompileKind::Host], - Some(build_target_config) => { - let values = build_target_config.values(config); - if values.len() > 1 && !config.cli_unstable().multitarget { - bail!("specifying multiple `--target` flags requires `-Zmultitarget`") - } - values - .into_iter() - .map(|k| { - use BuildTargetConfigValue::*; - let value = match &k { - Path(p) => p.to_str().expect("must be utf-8 in toml"), - Simple(s) => s, - }; - CompileTarget::new(value).map(CompileKind::Target) - }) - // First collect into a set to deduplicate any `--target` passed - // more than once... - .collect::>>()? - // ... then generate a flat list for everything else to use. - .into_iter() - .collect() - } + None => Ok(vec![CompileKind::Host]), + Some(build_target_config) => dedup(&build_target_config.values(config)?), }; - Ok(kinds) + kinds } /// Hash used for fingerprinting. diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index 764cd7be91f..1886be972eb 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -2203,34 +2203,38 @@ enum BuildTargetConfigInner { } impl BuildTargetConfig { - /// Gets values of `build.target` as a list of [`BuildTargetConfigValue`]. - pub fn values(&self, config: &Config) -> Vec> { - let def_root = self.inner.definition.root(config); - fn map<'a>(s: &'a str, root: &Path) -> BuildTargetConfigValue<'a> { + /// Gets values of `build.target` as a list of strings. + pub fn values(&self, config: &Config) -> CargoResult> { + let map = |s: &String| { if s.ends_with(".json") { - // To absolute path. - BuildTargetConfigValue::Path(root.join(s)) + // Path to a target specification file (in JSON). + // + self.inner + .definition + .root(config) + .join(s) + .to_str() + .expect("must be utf-8 in toml") + .to_string() } else { - BuildTargetConfigValue::Simple(s) + // A string. Probably a target triple. + s.to_string() } - } - match &self.inner.val { - BuildTargetConfigInner::One(s) => vec![map(s, def_root)], - BuildTargetConfigInner::Many(v) => v.iter().map(|s| map(s, def_root)).collect(), - } + }; + let values = match &self.inner.val { + BuildTargetConfigInner::One(s) => vec![map(s)], + BuildTargetConfigInner::Many(v) => { + if v.len() > 1 && !config.cli_unstable().multitarget { + bail!("specifying multiple `target` in `build.target` config value requires `-Zmultitarget`") + } else { + v.iter().map(map).collect() + } + } + }; + Ok(values) } } -/// Represents a value of `build.target`. -#[derive(Debug)] -pub enum BuildTargetConfigValue<'a> { - /// Path to a target specification file (in JSON). - /// - Path(PathBuf), - /// A string. Probably a target triple. - Simple(&'a str), -} - #[derive(Deserialize, Default)] struct TermConfig { verbose: Option, From 3c8ba9a9e01c4f7798a3dbb1eb89028753056c66 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Tue, 29 Mar 2022 14:10:50 +0800 Subject: [PATCH 7/8] test: fix to error message for multitarget --- tests/testsuite/multitarget.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testsuite/multitarget.rs b/tests/testsuite/multitarget.rs index a598df5d319..cecfb073fb1 100644 --- a/tests/testsuite/multitarget.rs +++ b/tests/testsuite/multitarget.rs @@ -30,7 +30,7 @@ fn double_target_rejected_with_config() { .build(); p.cargo("build") - .with_stderr("[ERROR] specifying multiple `--target` flags requires `-Zmultitarget`") + .with_stderr("[ERROR] specifying multiple `target` in `build.target` config value requires `-Zmultitarget`") .with_status(101) .run(); } From c18275b1aad92a5aa4dbafd4cb010c171b0eb460 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Thu, 31 Mar 2022 07:23:52 +0800 Subject: [PATCH 8/8] Guard that array value of `build.target` should be in nightly --- src/cargo/util/config/mod.rs | 4 ++-- tests/testsuite/multitarget.rs | 21 +++++++++++++++++++-- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index 1886be972eb..15414ece9af 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -2224,8 +2224,8 @@ impl BuildTargetConfig { let values = match &self.inner.val { BuildTargetConfigInner::One(s) => vec![map(s)], BuildTargetConfigInner::Many(v) => { - if v.len() > 1 && !config.cli_unstable().multitarget { - bail!("specifying multiple `target` in `build.target` config value requires `-Zmultitarget`") + if !config.cli_unstable().multitarget { + bail!("specifying an array in `build.target` config value requires `-Zmultitarget`") } else { v.iter().map(map).collect() } diff --git a/tests/testsuite/multitarget.rs b/tests/testsuite/multitarget.rs index cecfb073fb1..d4aced728f5 100644 --- a/tests/testsuite/multitarget.rs +++ b/tests/testsuite/multitarget.rs @@ -16,7 +16,7 @@ fn double_target_rejected() { } #[cargo_test] -fn double_target_rejected_with_config() { +fn array_of_target_rejected_with_config() { let p = project() .file("Cargo.toml", &basic_manifest("foo", "1.0.0")) .file("src/main.rs", "fn main() {}") @@ -30,7 +30,24 @@ fn double_target_rejected_with_config() { .build(); p.cargo("build") - .with_stderr("[ERROR] specifying multiple `target` in `build.target` config value requires `-Zmultitarget`") + .with_stderr( + "[ERROR] specifying an array in `build.target` config value requires `-Zmultitarget`", + ) + .with_status(101) + .run(); + + p.change_file( + ".cargo/config.toml", + r#" + [build] + target = ["a"] + "#, + ); + + p.cargo("build") + .with_stderr( + "[ERROR] specifying an array in `build.target` config value requires `-Zmultitarget`", + ) .with_status(101) .run(); }