support generic target tables and env variables

jameshilliard · jameshilliard · commit 3b4f9d8178b2 · 2022-12-27T17:24:14.000-07:00
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.
diff --git 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<ConfigMapAccess<'config>, ConfigError> {
         let mut fields = Vec::new();
-        if let Some(mut v) = de.config.get_table(&de.key)? {
-            // `v: Value<HashMap<String, CV>>`
-            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<HashMap<String, CV>>`
+                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
@@ -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<Option<bool>> {
+        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<HashMap<String, CV>>`.
     ///
     /// 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<T> {
+        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<bool> {
+        match self {
+            CV::String(_, _def) => Ok(true),
+            _ => Ok(false),
+        }
+    }
+
     pub fn table(&self, key: &str) -> CargoResult<(&HashMap<String, ConfigValue>, &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
@@ -85,13 +85,7 @@ pub(super) fn get_target_applies_to_host(config: &Config) -> CargoResult<bool> {
 /// Loads a single `[host]` table for the given triple.
 pub(super) fn load_host_triple(config: &Config, triple: &str) -> CargoResult<TargetConfig> {
     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<Tar
 
 /// Loads a single `[target]` table for the given triple.
 pub(super) fn load_target_triple(config: &Config, triple: &str) -> CargoResult<TargetConfig> {
-    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<T> {
+    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<TargetConfig> {
+fn load_config_table(config: &Config, prefix: &str, triple: &str) -> CargoResult<TargetConfig> {
     // 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<PathAndArgs> = config.get(&format!("{}.runner", prefix))?;
-    let rustflags: OptValue<StringList> = config.get(&format!("{}.rustflags", prefix))?;
-    let linker: OptValue<ConfigRelativePath> = config.get(&format!("{}.linker", prefix))?;
+    let runner: OptValue<PathAndArgs> = load_config_val(config, "runner", prefix, triple)?;
+    let rustflags: OptValue<StringList> = load_config_val(config, "rustflags", prefix, triple)?;
+    let linker: OptValue<ConfigRelativePath> = 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/bad_config.rs b/tests/testsuite/bad_config.rs
@@ -19,8 +19,9 @@ fn bad1() {
         .with_status(101)
         .with_stderr(
             "\
-[ERROR] expected table for configuration key `target.nonexistent-target`, \
-but found string in [..]config
+[ERROR] invalid configuration for key `target.nonexistent-target`
+expected a table, but found a string for `target.nonexistent-target` \
+in [..]config
 ",
         )
         .run();
diff --git 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(&["target-applies-to-host", "host-config"])
+            .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(&["target-applies-to-host", "host-config"])
+            .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(&["target-applies-to-host", "host-config"])
+            .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(&["target-applies-to-host", "host-config"])
+            .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();
