From 66511052c59b51014493e2c884b3391a2fadf943 Mon Sep 17 00:00:00 2001 From: James Hilliard Date: Sat, 19 Jun 2021 02:29:27 -0600 Subject: [PATCH] support generic target tables and env variables When building for targets from a meta build system like buildroot it is preferable to be able to unconditionally set target config/env variables without having to care about the target triple as we use target specific toolchains that will only support a single target architecture typically. --- src/cargo/util/config/de.rs | 11 ++- src/cargo/util/config/mod.rs | 34 +++++++ src/cargo/util/config/target.rs | 67 +++++++++---- tests/testsuite/build_script.rs | 162 ++++++++++++++++++++++++++++++++ 4 files changed, 254 insertions(+), 20 deletions(-) diff --git a/src/cargo/util/config/de.rs b/src/cargo/util/config/de.rs index 6fddc7e71f0f..d50f3e454746 100644 --- a/src/cargo/util/config/de.rs +++ b/src/cargo/util/config/de.rs @@ -206,10 +206,13 @@ enum KeyKind { impl<'config> ConfigMapAccess<'config> { fn new_map(de: Deserializer<'config>) -> Result, ConfigError> { let mut fields = Vec::new(); - if let Some(mut v) = de.config.get_table(&de.key)? { - // `v: Value>` - for (key, _value) in v.val.drain() { - fields.push(KeyKind::CaseSensitive(key)); + let key_is_table = de.config.is_table(&de.key)?; + if key_is_table.is_some() && key_is_table.unwrap() { + if let Some(mut v) = de.config.get_table(&de.key)? { + // `v: Value>` + for (key, _value) in v.val.drain() { + fields.push(KeyKind::CaseSensitive(key)); + } } } if de.config.cli_unstable().advanced_env { diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index 5743f9baf3f3..73afc8d767f0 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -759,6 +759,16 @@ impl Config { Ok(false) } + fn has_env_key(&self, key: &ConfigKey) -> bool { + if self.env.contains_key(key.as_env_key()) { + return true; + } + + self.check_environment_key_case_mismatch(key); + + false + } + fn check_environment_key_case_mismatch(&self, key: &ConfigKey) { if let Some(env_key) = self.upper_case_env.get(key.as_env_key()) { let _ = self.shell().warn(format!( @@ -901,6 +911,14 @@ impl Config { Ok(()) } + fn is_table(&self, key: &ConfigKey) -> CargoResult> { + match self.get_cv(key)? { + Some(CV::Table(_, _def)) => Ok(Some(true)), + Some(_) => Ok(Some(false)), + None => Ok(None), + } + } + /// Low-level method for getting a config value as an `OptValue>`. /// /// NOTE: This does not read from env. The caller is responsible for that. @@ -1719,6 +1737,15 @@ impl Config { T::deserialize(d).map_err(|e| e.into()) } + pub fn get_env_only<'de, T: serde::de::Deserialize<'de>>(&self, key: &str) -> CargoResult { + let d = Deserializer { + config: self, + key: ConfigKey::from_str(key), + env_prefix_ok: false, + }; + T::deserialize(d).map_err(|e| e.into()) + } + pub fn assert_package_cache_locked<'a>(&self, f: &'a Filesystem) -> &'a Path { let ret = f.as_path_unlocked(); assert!( @@ -2023,6 +2050,13 @@ impl ConfigValue { } } + pub fn is_string(&self) -> CargoResult { + match self { + CV::String(_, _def) => Ok(true), + _ => Ok(false), + } + } + pub fn table(&self, key: &str) -> CargoResult<(&HashMap, &Definition)> { match self { CV::Table(table, def) => Ok((table, def)), diff --git a/src/cargo/util/config/target.rs b/src/cargo/util/config/target.rs index 8190529a6860..72d982c988a9 100644 --- a/src/cargo/util/config/target.rs +++ b/src/cargo/util/config/target.rs @@ -85,13 +85,7 @@ pub(super) fn get_target_applies_to_host(config: &Config) -> CargoResult { /// Loads a single `[host]` table for the given triple. pub(super) fn load_host_triple(config: &Config, triple: &str) -> CargoResult { if config.cli_unstable().host_config { - let host_triple_prefix = format!("host.{}", triple); - let host_triple_key = ConfigKey::from_str(&host_triple_prefix); - let host_prefix = match config.get_cv(&host_triple_key)? { - Some(_) => host_triple_prefix, - None => "host".to_string(), - }; - load_config_table(config, &host_prefix) + load_config_table(config, "host", triple) } else { Ok(TargetConfig { runner: None, @@ -104,21 +98,56 @@ pub(super) fn load_host_triple(config: &Config, triple: &str) -> CargoResult CargoResult { - load_config_table(config, &format!("target.{}", triple)) + load_config_table(config, "target", triple) +} + +fn load_config_val<'de, T: serde::de::Deserialize<'de>>( + config: &Config, + key: &str, + prefix: &str, + triple: &str, +) -> CargoResult { + let triple_str = &format!("{}.{}.{}", prefix, triple, key); + let triple_key = &ConfigKey::from_str(triple_str); + if config.has_env_key(triple_key) { + return config.get_env_only(triple_str); + } + + let generic_str = &format!("{}.{}", prefix, key); + let generic_key = &ConfigKey::from_str(generic_str); + if config.has_env_key(generic_key) { + return config.get_env_only(generic_str); + } + + let generic_table = config.is_table(&ConfigKey::from_str(&format!("{}", prefix)))?; + let triple_table = config.is_table(&ConfigKey::from_str(&format!("{}.{}", prefix, triple)))?; + if !triple_table.is_some() && generic_table.is_some() && generic_table.unwrap() { + return config.get(generic_str); + } + config.get(triple_str) } /// Loads a single table for the given prefix. -fn load_config_table(config: &Config, prefix: &str) -> CargoResult { +fn load_config_table(config: &Config, prefix: &str, triple: &str) -> CargoResult { // This needs to get each field individually because it cannot fetch the // struct all at once due to `links_overrides`. Can't use `serde(flatten)` // because it causes serde to use `deserialize_map` which means the config // deserializer does not know which keys to deserialize, which means // environment variables would not work. - let runner: OptValue = config.get(&format!("{}.runner", prefix))?; - let rustflags: OptValue = config.get(&format!("{}.rustflags", prefix))?; - let linker: OptValue = config.get(&format!("{}.linker", prefix))?; + let runner: OptValue = load_config_val(config, "runner", prefix, triple)?; + let rustflags: OptValue = load_config_val(config, "rustflags", prefix, triple)?; + let linker: OptValue = load_config_val(config, "linker", prefix, triple)?; // Links do not support environment variables. - let target_key = ConfigKey::from_str(prefix); + let generic_key = ConfigKey::from_str(&format!("{}", prefix)); + let triple_key = ConfigKey::from_str(&format!("{}.{}", prefix, triple)); + let generic_table = config.is_table(&generic_key)?; + let triple_table = config.is_table(&triple_key)?; + let target_key = if !triple_table.is_some() && generic_table.is_some() && generic_table.unwrap() + { + generic_key + } else { + triple_key + }; let links_overrides = match config.get_table(&target_key)? { Some(links) => parse_links_overrides(&target_key, links.val, config)?, None => BTreeMap::new(), @@ -150,7 +179,11 @@ fn parse_links_overrides( _ => {} } let mut output = BuildOutput::default(); - let table = value.table(&format!("{}.{}", target_key, lib_name))?.0; + let table_key = &format!("{}.{}", target_key, lib_name); + let table = value + .table(table_key) + .map_err(|e| anyhow::anyhow!("invalid configuration for key `{}`\n{}", table_key, e))? + .0; // We require deterministic order of evaluation, so we must sort the pairs by key first. let mut pairs = Vec::new(); for (k, value) in table { @@ -227,8 +260,10 @@ fn parse_links_overrides( anyhow::bail!("`{}` is not supported in build script overrides", key); } _ => { - let val = value.string(key)?.0; - output.metadata.push((key.clone(), val.to_string())); + if value.is_string()? { + let val = value.string(key)?.0; + output.metadata.push((key.clone(), val.to_string())); + } } } } diff --git a/tests/testsuite/build_script.rs b/tests/testsuite/build_script.rs index e81f6ef1ece7..2128be3eb813 100644 --- a/tests/testsuite/build_script.rs +++ b/tests/testsuite/build_script.rs @@ -360,6 +360,119 @@ fn custom_build_env_var_rustc_linker() { p.cargo("build --target").arg(&target).run(); } +#[cargo_test] +fn custom_build_env_var_rustc_generic_linker() { + let target = rustc_host(); + let p = project() + .file( + ".cargo/config", + r#" + [target] + linker = "/path/to/linker" + "#, + ) + .file( + "build.rs", + r#" + use std::env; + + fn main() { + assert!(env::var("RUSTC_LINKER").unwrap().ends_with("/path/to/linker")); + } + "#, + ) + .file("src/lib.rs", "") + .build(); + + // no crate type set => linker never called => build succeeds if and + // only if build.rs succeeds, despite linker binary not existing. + if cargo_test_support::is_nightly() { + p.cargo("build -Z target-applies-to-host -Z host-config --target") + .arg(&target) + .masquerade_as_nightly_cargo() + .run(); + } +} + +#[cargo_test] +fn custom_build_env_var_rustc_triple_overrides_generic_linker() { + let target = rustc_host(); + let p = project() + .file( + ".cargo/config", + &format!( + r#" + [target] + linker = "/path/to/generic/linker" + [target.{}] + linker = "/path/to/linker" + "#, + target + ), + ) + .file( + "build.rs", + r#" + use std::env; + + fn main() { + assert!(env::var("RUSTC_LINKER").unwrap().ends_with("/path/to/linker")); + } + "#, + ) + .file("src/lib.rs", "") + .build(); + + // no crate type set => linker never called => build succeeds if and + // only if build.rs succeeds, despite linker binary not existing. + if cargo_test_support::is_nightly() { + p.cargo("build -Z target-applies-to-host -Z host-config --target") + .arg(&target) + .masquerade_as_nightly_cargo() + .run(); + } +} + +#[cargo_test] +fn custom_build_env_var_rustc_generic_env_overrides_tables() { + let target = rustc_host(); + let p = project() + .file( + ".cargo/config", + &format!( + r#" + [target] + linker = "/path/to/generic/table/linker" + [target.{}] + linker = "/path/to/triple/table/linker" + "#, + target + ), + ) + .file( + "build.rs", + r#" + use std::env; + + fn main() { + assert!(env::var("RUSTC_LINKER").unwrap().ends_with("/path/to/linker")); + } + "#, + ) + .file("src/lib.rs", "") + .build(); + + // no crate type set => linker never called => build succeeds if and + // only if build.rs succeeds, despite linker binary not existing. + if cargo_test_support::is_nightly() { + p.cargo("build -Z target-applies-to-host -Z host-config --target") + .env("CARGO_TARGET_LINKER", "/path/to/linker") + .arg(&target) + .masquerade_as_nightly_cargo() + .run(); + } +} + #[cargo_test] fn custom_build_env_var_rustc_linker_bad_host_target() { let target = rustc_host(); @@ -601,6 +714,55 @@ fn custom_build_linker_bad_host_with_arch() { .run(); } +#[cargo_test] +fn custom_build_env_var_rustc_linker_bad_host_with_arch_env_override() { + let target = rustc_host(); + let p = project() + .file( + ".cargo/config", + &format!( + r#" + [host] + linker = "/path/to/host/linker" + [host.{}] + linker = "/path/to/host/arch/linker" + [target.{}] + linker = "/path/to/target/linker" + "#, + target, target + ), + ) + .file( + "build.rs", + r#" + use std::env; + + fn main() { + assert!(env::var("RUSTC_LINKER").unwrap().ends_with("/path/to/target/linker")); + } + "#, + ) + .file("src/lib.rs", "") + .build(); + + // build.rs should fail due to bad host linker being set + if cargo_test_support::is_nightly() { + p.cargo("build -Z target-applies-to-host -Z host-config --verbose --target") + .env("CARGO_HOST_LINKER", "/path/to/env/host/linker") + .arg(&target) + .masquerade_as_nightly_cargo() + .with_status(101) + .with_stderr_contains( + "\ +[COMPILING] foo v0.0.1 ([CWD]) +[RUNNING] `rustc --crate-name build_script_build build.rs [..]--crate-type bin [..]-C linker=[..]/path/to/env/host/linker [..]` +[ERROR] linker `[..]/path/to/env/host/linker` not found +" + ) + .run(); + } +} + #[cargo_test] fn custom_build_env_var_rustc_linker_cross_arch_host() { let target = rustc_host();