Enable propagating rustflags to build scripts

This patch implements more complete logic for applying rustflags to
build scripts and other host artifacts. In the default configuration, it
only makes build scripts (and plugins and whatnot) pick up on
`rustflags` from `[host]`, which fixes #10206 but otherwise preserves
existing behavior in all its inconsistent glory. The same is the case if
`target-applies-to-host` is explicitly set to `false`.

When `target-applies-to-host` is explicitly set to `true`, rustflags
will start to be applied in the same way that `linker` and `runner` are
today -- namely they'll be applied from `[target.<host triple>]` and
from `RUSTFLAGS`/`build.rustflags` if `--target <host triple>` is given.

Author: Jon Gjengset

Diff for: src/cargo/core/compiler/build_context/target_info.rs

@@ -573,17 +573,23 @@ fn output_err_info(cmd: &ProcessBuilder, stdout: &str, stderr: &str) -> String {
 ///  - the `CARGO_ENCODED_RUSTFLAGS` environment variable
 ///  - the `RUSTFLAGS` environment variable
 ///
-/// then if this was not found
+/// then if none of those were found
 ///
 ///  - `target.*.rustflags` from the config (.cargo/config)
 ///  - `target.cfg(..).rustflags` from the config
+///  - `host.*.rustflags` from the config if compiling a host artifact or without `--target`
 ///
-/// then if neither of these were found
+/// then if none of those were found
 ///
 ///  - `build.rustflags` from the config
 ///
-/// Note that if a `target` is specified, no args will be passed to host code (plugins, build
-/// scripts, ...), even if it is the same as the target.
+/// The behavior differs slightly when cross-compiling (or, specifically, when `--target` is
+/// provided) for artifacts that are always built for the host (plugins, build scripts, ...).
+/// For those artifacts, the behavior depends on `target-applies-to-host`. In the default
+/// configuration (where `target-applies-to-host` is unset), and if `target-applies-to-host =
+/// false`, host artifacts _only_ respect `host.*.rustflags`, and no other configuration sources.
+/// If `target-applies-to-host = true`, host artifacts also respect `target.<host triple>`, and if
+/// `<target triple> == <host triple>` `RUSTFLAGS` and `CARGO_ENCODED_RUSTFLAGS`.
 fn env_args(
     config: &Config,
     requested_kinds: &[CompileKind],
@@ -592,49 +598,67 @@ fn env_args(
     kind: CompileKind,
     name: &str,
 ) -> CargoResult<Vec<String>> {
-    // We *want* to apply RUSTFLAGS only to builds for the
-    // requested target architecture, and not to things like build
-    // scripts and plugins, which may be for an entirely different
-    // architecture. Cargo's present architecture makes it quite
-    // hard to only apply flags to things that are not build
-    // scripts and plugins though, so we do something more hacky
-    // instead to avoid applying the same RUSTFLAGS to multiple targets
-    // arches:
+    let target_applies_to_host = config.target_applies_to_host()?;
+
+    // Include untargeted configuration sources (like `RUSTFLAGS`) if
     //
-    // 1) If --target is not specified we just apply RUSTFLAGS to
-    // all builds; they are all going to have the same target.
+    //  - we're compiling artifacts for the target platform; or
+    //  - we're not cross-compiling; or
+    //  - we're compiling host artifacts, the requested target matches the host, and the user has
+    //    requested that the host pick up target configurations.
     //
-    // 2) If --target *is* specified then we only apply RUSTFLAGS
-    // to compilation units with the Target kind, which indicates
-    // it was chosen by the --target flag.
+    // The rationale for the third condition is that `RUSTFLAGS` is intended to affect compilation
+    // for the target, and `target-applies-to-host` makes it so that the host is affected by its
+    // target's config, so if `--target <host triple>` then `RUSTFLAGS` should also apply to the
+    // host.
+    let include_generic = !kind.is_host()
+        || requested_kinds == [CompileKind::Host]
+        || (target_applies_to_host == Some(true)
+            && requested_kinds == [CompileKind::Target(CompileTarget::new(host_triple)?)]);
+    // Include targeted configuration sources (like `target.*.rustflags`) if
     //
-    // This means that, e.g., even if the specified --target is the
-    // same as the host, build scripts in plugins won't get
-    // RUSTFLAGS.
-    if requested_kinds != [CompileKind::Host] && kind.is_host() {
-        // This is probably a build script or plugin and we're
-        // compiling with --target. In this scenario there are
-        // no rustflags we can apply.
-        return Ok(Vec::new());
-    }
-
-    // First try CARGO_ENCODED_RUSTFLAGS from the environment.
-    // Prefer this over RUSTFLAGS since it's less prone to encoding errors.
-    if let Ok(a) = env::var(format!("CARGO_ENCODED_{}", name)) {
-        if a.is_empty() {
-            return Ok(Vec::new());
+    //  - we're compiling artifacts for the target platform; or
+    //  - we're not cross-compiling; or
+    //  - we're compiling host artifacts and the user has requested that the host pick up target
+    //    configurations.
+    //
+    // The middle condition here may seem counter-intuitive. If `target-applies-to-host-kind` is
+    // disabled, then `target.*.rustflags` should arguably not apply to host artifacts. However,
+    // this is likely surprising to users who set `target.<host triple>.rustflags` and run `cargo
+    // build` (with no `--target`). So, we respect `target.<host triple>`.rustflags` when
+    // `--target` _isn't_ supplied, which arguably matches the mental model established by
+    // respecting `RUSTFLAGS` when `--target` isn't supplied.
+    let include_for_target = !kind.is_host()
+        || requested_kinds == [CompileKind::Host]
+        || target_applies_to_host == Some(true);
+    // Include host-based configuration sources (like `host.*.rustflags`) if
+    //
+    //  - we're compiling host artifacts; or
+    //  - we're not cross-compiling
+    //
+    // Note that we do _not_ read `host.*.rustflags` just because the host's target is the same as
+    // the requested target, as that's the whole point of the `host` section in the first place.
+    let include_for_host = kind.is_host() || requested_kinds == [CompileKind::Host];
+
+    if include_generic {
+        // First try CARGO_ENCODED_RUSTFLAGS from the environment.
+        // Prefer this over RUSTFLAGS since it's less prone to encoding errors.
+        if let Ok(a) = env::var(format!("CARGO_ENCODED_{}", name)) {
+            if a.is_empty() {
+                return Ok(Vec::new());
+            }
+            return Ok(a.split('\x1f').map(str::to_string).collect());
         }
-        return Ok(a.split('\x1f').map(str::to_string).collect());
-    }
 
-    // Then try RUSTFLAGS from the environment
-    if let Ok(a) = env::var(name) {
-        let args = a
-            .split(' ')
-            .map(str::trim)
-            .filter(|s| !s.is_empty())
-            .map(str::to_string);
-        return Ok(args.collect());
+        // Then try RUSTFLAGS from the environment
+        if let Ok(a) = env::var(name) {
+            let args = a
+                .split(' ')
+                .map(str::trim)
+                .filter(|s| !s.is_empty())
+                .map(str::to_string);
+            return Ok(args.collect());
+        }
     }
 
     let mut rustflags = Vec::new();
@@ -643,44 +667,55 @@ fn env_args(
         .chars()
         .flat_map(|c| c.to_lowercase())
         .collect::<String>();
-    // Then the target.*.rustflags value...
-    let target = match &kind {
-        CompileKind::Host => host_triple,
-        CompileKind::Target(target) => target.short_name(),
-    };
-    let key = format!("target.{}.{}", target, name);
-    if let Some(args) = config.get::<Option<StringList>>(&key)? {
-        rustflags.extend(args.as_slice().iter().cloned());
+    if include_for_target {
+        // Then the target.*.rustflags value...
+        let target = match &kind {
+            CompileKind::Host => host_triple,
+            CompileKind::Target(target) => target.short_name(),
+        };
+        let key = format!("target.{}.{}", target, name);
+        if let Some(args) = config.get::<Option<StringList>>(&key)? {
+            rustflags.extend(args.as_slice().iter().cloned());
+        }
+        // ...including target.'cfg(...)'.rustflags
+        if let Some(target_cfg) = target_cfg {
+            config
+                .target_cfgs()?
+                .iter()
+                .filter_map(|(key, cfg)| {
+                    cfg.rustflags
+                        .as_ref()
+                        .map(|rustflags| (key, &rustflags.val))
+                })
+                .filter(|(key, _rustflags)| CfgExpr::matches_key(key, target_cfg))
+                .for_each(|(_key, cfg_rustflags)| {
+                    rustflags.extend(cfg_rustflags.as_slice().iter().cloned());
+                });
+        }
     }
-    // ...including target.'cfg(...)'.rustflags
-    if let Some(target_cfg) = target_cfg {
-        config
-            .target_cfgs()?
-            .iter()
-            .filter_map(|(key, cfg)| {
-                cfg.rustflags
-                    .as_ref()
-                    .map(|rustflags| (key, &rustflags.val))
-            })
-            .filter(|(key, _rustflags)| CfgExpr::matches_key(key, target_cfg))
-            .for_each(|(_key, cfg_rustflags)| {
-                rustflags.extend(cfg_rustflags.as_slice().iter().cloned());
-            });
+
+    if include_for_host {
+        let target_cfg = config.host_cfg_triple(host_triple)?;
+        if let Some(rf) = target_cfg.rustflags {
+            rustflags.extend(rf.val.as_slice().iter().cloned());
+        }
     }
 
     if !rustflags.is_empty() {
         return Ok(rustflags);
     }
 
-    // Then the `build.rustflags` value.
-    let build = config.build_config()?;
-    let list = if name == "rustflags" {
-        &build.rustflags
-    } else {
-        &build.rustdocflags
-    };
-    if let Some(list) = list {
-        return Ok(list.as_slice().to_vec());
+    if include_generic {
+        // Then the `build.rustflags` value.
+        let build = config.build_config()?;
+        let list = if name == "rustflags" {
+            &build.rustflags
+        } else {
+            &build.rustdocflags
+        };
+        if let Some(list) = list {
+            return Ok(list.as_slice().to_vec());
+        }
     }
 
     Ok(Vec::new())
@@ -718,7 +753,7 @@ impl<'cfg> RustcTargetData<'cfg> {
         let mut target_info = HashMap::new();
         let target_applies_to_host = config.target_applies_to_host()?;
         let host_info = TargetInfo::new(config, requested_kinds, &rustc, CompileKind::Host)?;
-        let host_config = if target_applies_to_host {
+        let host_config = if target_applies_to_host != Some(false) {
             config.target_cfg_triple(&rustc.host)?
         } else {
             config.host_cfg_triple(&rustc.host)?

Diff for: src/cargo/util/config/mod.rs

@@ -1551,7 +1551,7 @@ impl Config {
     }
 
     /// Returns true if the `[target]` table should be applied to host targets.
-    pub fn target_applies_to_host(&self) -> CargoResult<bool> {
+    pub fn target_applies_to_host(&self) -> CargoResult<Option<bool>> {
         target::get_target_applies_to_host(self)
     }
 

Diff for: src/cargo/util/config/target.rs

@@ -66,20 +66,29 @@ pub(super) fn load_target_cfgs(config: &Config) -> CargoResult<Vec<(String, Targ
 }
 
 /// Returns true if the `[target]` table should be applied to host targets.
-pub(super) fn get_target_applies_to_host(config: &Config) -> CargoResult<bool> {
+pub(super) fn get_target_applies_to_host(config: &Config) -> CargoResult<Option<bool>> {
     if config.cli_unstable().target_applies_to_host {
-        if let Ok(target_applies_to_host) = config.get::<bool>("target-applies-to-host") {
-            Ok(target_applies_to_host)
-        } else {
-            Ok(!config.cli_unstable().host_config)
+        if let Ok(target_applies_to_host) = config.get::<Option<bool>>("target-applies-to-host") {
+            if config.cli_unstable().host_config {
+                match target_applies_to_host {
+                    Some(true) => {
+                        anyhow::bail!(
+                            "the -Zhost-config flag requires target-applies-to-host = false"
+                        );
+                    }
+                    Some(false) => {}
+                    None => {
+                        // -Zhost-config automatically disables target-applies-to-host
+                        return Ok(Some(false));
+                    }
+                }
+            }
+            return Ok(target_applies_to_host);
         }
     } else if config.cli_unstable().host_config {
-        anyhow::bail!(
-            "the -Zhost-config flag requires the -Ztarget-applies-to-host flag to be set"
-        );
-    } else {
-        Ok(true)
+        anyhow::bail!("the -Zhost-config flag requires -Ztarget-applies-to-host");
     }
+    Ok(None)
 }
 
 /// Loads a single `[host]` table for the given triple.

Diff for: src/doc/src/reference/unstable.md

@@ -508,14 +508,30 @@ CLI paths are relative to the current working directory.
 * Original Pull Request: [#9322](https://github.com/rust-lang/cargo/pull/9322)
 * Tracking Issue: [#9453](https://github.com/rust-lang/cargo/issues/9453)
 
-The `target-applies-to-host` key in a config file can be used set the desired
-behavior for passing target config flags to build scripts.
-
-It requires the `-Ztarget-applies-to-host` command-line option.
-
-The current default for `target-applies-to-host` is `true`, which will be
-changed to `false` in the future, if `-Zhost-config` is used the new `false`
-default will be set for `target-applies-to-host`.
+Historically, Cargo has respected the `linker` and `runner` options (but
+_not_ `rustflags`) of `[target.<host triple>]` configuration sections
+when building and running build scripts, plugins, and other artifacts
+that are _always_ built for the host platform.
+`-Ztarget-applies-to-host` enables the top-level
+`target-applies-to-host` setting in Cargo configuration files which
+allows users to opt into different (and more consistent) behavior for
+these properties.
+
+When `target-applies-to-host` is unset in the configuration file, the
+existing Cargo behavior is preserved (though see `-Zhost-config`, which
+changes that default). When it is set to `true`, _all_ options from
+`[target.<host triple>]` are respected for host artifacts. When it is
+set to `false`, _no_ options from `[target.<host triple>]` are respected
+for host artifacts.
+
+In the specific case of `rustflags`, this settings also affects whether
+`--target <host-triple>` picks up the same configuration as if
+`--target` was not supplied in the first place.
+
+In the future, `target-applies-to-host` may end up defaulting to `false`
+to provide more sane and consistent default behavior. When set to
+`false`, `host-config` can be used to customize the behavior for host
+artifacts.
 
 ```toml
 # config.toml
@@ -535,15 +551,17 @@ such as build scripts that must run on the host system instead of the target
 system when cross compiling. It supports both generic and host arch specific
 tables. Matching host arch tables take precedence over generic host tables.
 
-It requires the `-Zhost-config` and `-Ztarget-applies-to-host` command-line
-options to be set.
+It requires the `-Zhost-config` and `-Ztarget-applies-to-host`
+command-line options to be set, and that `target-applies-to-host =
+false` is set in the Cargo configuration file.
 
 ```toml
 # config.toml
 [host]
 linker = "/path/to/host/linker"
 [host.x86_64-unknown-linux-gnu]
 linker = "/path/to/host/arch/linker"
+rustflags = ["-Clink-arg=--verbose"]
 [target.x86_64-unknown-linux-gnu]
 linker = "/path/to/target/linker"
 ```
@@ -552,8 +570,9 @@ The generic `host` table above will be entirely ignored when building on a
 `x86_64-unknown-linux-gnu` host as the `host.x86_64-unknown-linux-gnu` table
 takes precedence.
 
-Setting `-Zhost-config` changes the default for `target-applies-to-host` to
-`false` from `true`.
+Setting `-Zhost-config` changes the default value of
+`target-applies-to-host` to `false`, and will error if
+`target-applies-to-host` is set to `true`.
 
 ```console
 cargo +nightly -Ztarget-applies-to-host -Zhost-config build --target x86_64-unknown-linux-gnu

Diff for: tests/testsuite/build_script.rs

@@ -469,7 +469,7 @@ fn custom_build_invalid_host_config_feature_flag() {
         .with_status(101)
         .with_stderr_contains(
             "\
-error: the -Zhost-config flag requires the -Ztarget-applies-to-host flag to be set
+error: the -Zhost-config flag requires -Ztarget-applies-to-host
 ",
         )
         .run();
@@ -483,7 +483,6 @@ fn custom_build_env_var_rustc_linker_host_target_with_bad_host_config() {
             ".cargo/config",
             &format!(
                 r#"
-                target-applies-to-host = true
                 [host]
                 linker = "/path/to/host/linker"
                 [target.{}]
@@ -496,16 +495,16 @@ fn custom_build_env_var_rustc_linker_host_target_with_bad_host_config() {
         .file("src/lib.rs", "")
         .build();
 
-    // build.rs should fail due to bad target linker being set
+    // build.rs should fail due to bad host linker being set
     p.cargo("build -Z target-applies-to-host -Z host-config --verbose --target")
             .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/target/linker [..]`
-[ERROR] linker `[..]/path/to/target/linker` not found
+[RUNNING] `rustc --crate-name build_script_build build.rs [..]--crate-type bin [..]-C linker=[..]/path/to/host/linker [..]`
+[ERROR] linker `[..]/path/to/host/linker` not found
 "
             )
             .run();