Make rustflags logic more linear

src/cargo/core/compiler/build_context/target_info.rs

@@ -596,74 +596,92 @@ fn env_args(
     kind: CompileKind,
     name: &str,
 ) -> CargoResult<Vec<String>> {
-    // The `target-applies-to-host` setting is somewhat misleading in name.
+    // The `target-applies-to-host` setting is somewhat misleading in name. What it really does it
-    //
+    // opt into a _particular_ legacy Cargo behavior for how rustflags are picked up.
-    // What it really does it opt into a _particular_ past Cargo behavior for `[target.<host>]` for
-    // host artifacts, namely that `linker` and `runner` from `[target.<host>]` are respected, but
-    // `rustflags` is _not_.
-    //
-    // The code (and comments) below are written to first apply `target-applies-to-host` _as if it
-    // was consistent_, and then adjusts the settings to match the legacy behavior that
-    // `target-applies-to-host = true` _really_ enables. This was done to (hopefully) make the
-    // logic flow clearer, and to make future modifications to the rules for when rustflags get
-    // applied less likely to break the legacy behavior.
     let target_applies_to_host = config.target_applies_to_host()?;
+    if target_applies_to_host == true {
+        // We're operating in "legacy" Cargo mode, where the presence or absence of --target makes
+        // a difference for which configuration options are picked up. Specifically:
+        //
+        // If --target is not passed, all configuration sources are consulted whether we're
+        // compiling a host artifact or not.
+        if requested_kinds == [CompileKind::Host] {
+            // RUSTFLAGS takes priority
+            if let Some(rustflags) = rustflags_from_env(name) {
+                Ok(rustflags)
 
-    // Include untargeted configuration sources (like `RUSTFLAGS`) if
+            // [host] is a new feature, so we have it take priority over [target.<host>]
-    //
+            } else if let Some(rustflags) = rustflags_from_host(config, host_triple)? {
-    //  - we're compiling artifacts for the target platform; or
+                Ok(rustflags)
-    //  - 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.
-    //
-    // 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 mut include_generic = !kind.is_host()
-        || requested_kinds == [CompileKind::Host]
-        || (target_applies_to_host
-            && requested_kinds == [CompileKind::Target(CompileTarget::new(host_triple)?)]);
 
-    // Include targeted configuration sources (like `target.*.rustflags`) if
+            // but [target.<host>] _does_ apply to host artifacts
-    //
+            } else if let Some(rustflags) =
-    //  - we're compiling artifacts for the target platform; or
+                rustflags_from_target(config, host_triple, target_cfg, kind, name)?
-    //  - we're not cross-compiling; or
+            {
-    //  - we're compiling host artifacts and the user has requested that the host pick up target
+                Ok(rustflags)
-    //    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 mut include_for_target =
-        !kind.is_host() || requested_kinds == [CompileKind::Host] || target_applies_to_host;
 
-    // Include host-based configuration sources (like `host.*.rustflags`) if
+            // and otherwise we fall back to [build]
-    //
+            } else if let Some(rustflags) = rustflags_from_build(config, name)? {
-    //  - we're compiling host artifacts; or
+                Ok(rustflags)
-    //  - we're not cross-compiling
+            } else {
-    //
+                Ok(Vec::new())
-    // 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];
-
-    // Apply the legacy behavior of target_applies_to_host.
-    if target_applies_to_host && kind.is_host() && requested_kinds != [CompileKind::Host] {
-        include_generic = false;
-        include_for_target = false;
             }
 
-    if include_generic {
+        // If --target _is_ passed, then host artifacts traditionally got _no_ rustflags applies.
+        // Since [host] is a new feature, we can dictate that it is respect even in this mode
+        // though.
+        } else if kind.is_host() {
+            if let Some(rustflags) = rustflags_from_host(config, host_triple)? {
+                Ok(rustflags)
+            } else {
+                Ok(Vec::new())
+            }
+
+        // All other artifacts pick up the RUSTFLAGS, [target.*], and [build], in that order.
+        } else {
+            if let Some(rustflags) = rustflags_from_env(name) {
+                Ok(rustflags)
+            } else if let Some(rustflags) =
+                rustflags_from_target(config, host_triple, target_cfg, kind, name)?
+            {
+                Ok(rustflags)
+            } else if let Some(rustflags) = rustflags_from_build(config, name)? {
+                Ok(rustflags)
+            } else {
+                Ok(Vec::new())
+            }
+        }
+
+    // If we're _not_ in legacy mode, then host artifacts just get flags from [host], regardless of
+    // --target. Or, phrased differently, no `--target` behaves the same as `--target <host>`, and
+    // host artifacts are always "special" (they don't pick up `RUSTFLAGS` for example).
+    } else if kind.is_host() {
+        Ok(rustflags_from_host(config, host_triple)?.unwrap_or_else(Vec::new))
+
+    // All other artifacts pick up the RUSTFLAGS, [target.*], and [build], in that order.
+    } else {
+        if let Some(rustflags) = rustflags_from_env(name) {
+            Ok(rustflags)
+        } else if let Some(rustflags) =
+            rustflags_from_target(config, host_triple, target_cfg, kind, name)?
+        {
+            Ok(rustflags)
+        } else if let Some(rustflags) = rustflags_from_build(config, name)? {
+            Ok(rustflags)
+        } else {
+            Ok(Vec::new())
+        }
+    }
+}
+
+fn rustflags_from_env(name: &str) -> Option<Vec<String>> {
     // 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 Some(Vec::new());
         }
-            return Ok(a.split('\x1f').map(str::to_string).collect());
+        return Some(a.split('\x1f').map(str::to_string).collect());
     }
 
     // Then try RUSTFLAGS from the environment
@@ -673,17 +691,27 @@ fn env_args(
             .map(str::trim)
             .filter(|s| !s.is_empty())
             .map(str::to_string);
-            return Ok(args.collect());
+        return Some(args.collect());
-        }
     }
 
+    // No rustflags to be collected from the environment
+    None
+}
+
+fn rustflags_from_target(
+    config: &Config,
+    host_triple: &str,
+    target_cfg: Option<&[Cfg]>,
+    kind: CompileKind,
+    name: &str,
+) -> CargoResult<Option<Vec<String>>> {
     let mut rustflags = Vec::new();
 
     let name = name
         .chars()
         .flat_map(|c| c.to_lowercase())
         .collect::<String>();
-    if include_for_target {
+
     // Then the target.*.rustflags value...
     let target = match &kind {
         CompileKind::Host => host_triple,
@@ -708,33 +736,32 @@ fn env_args(
                 rustflags.extend(cfg_rustflags.as_slice().iter().cloned());
             });
     }
-    }
 
-    if include_for_host {
+    if rustflags.is_empty() {
+        Ok(None)
+    } else {
+        Ok(Some(rustflags))
+    }
+}
+
+fn rustflags_from_host(config: &Config, host_triple: &str) -> CargoResult<Option<Vec<String>>> {
     let target_cfg = config.host_cfg_triple(host_triple)?;
     if let Some(rf) = target_cfg.rustflags {
-            rustflags.extend(rf.val.as_slice().iter().cloned());
+        Ok(Some(rf.val.as_slice().iter().cloned().collect()))
-        }
+    } else {
+        Ok(None)
     }
+}
 
-    if !rustflags.is_empty() {
+fn rustflags_from_build(config: &Config, name: &str) -> CargoResult<Option<Vec<String>>> {
-        return Ok(rustflags);
-    }
-
-    if include_generic {
     // Then the `build.rustflags` value.
     let build = config.build_config()?;
-        let list = if name == "rustflags" {
+    let list = if name.eq_ignore_ascii_case("rustflags") {
         &build.rustflags
     } else {
         &build.rustdocflags
     };
-        if let Some(list) = list {
+    Ok(list.as_ref().map(|l| l.as_slice().to_vec()))
-            return Ok(list.as_slice().to_vec());
-        }
-    }
-
-    Ok(Vec::new())
 }
 
 /// Collection of information about `rustc` and the host and target.

src/cargo/util/config/target.rs

@@ -67,29 +67,19 @@ 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> {
-    let default = true;
     if config.cli_unstable().target_applies_to_host {
-        if let Ok(target_applies_to_host) = config.get::<Option<bool>>("target-applies-to-host") {
+        if let Ok(target_applies_to_host) = config.get::<bool>("target-applies-to-host") {
-            if config.cli_unstable().host_config {
+            Ok(target_applies_to_host)
-                match target_applies_to_host {
+        } else {
-                    Some(true) => {
+            Ok(!config.cli_unstable().host_config)
-                        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(false);
-                    }
-                }
-            }
-            return Ok(target_applies_to_host.unwrap_or(default));
         }
     } else if config.cli_unstable().host_config {
-        anyhow::bail!("the -Zhost-config flag requires -Ztarget-applies-to-host");
+        anyhow::bail!(
+            "the -Zhost-config flag requires the -Ztarget-applies-to-host flag to be set"
+        );
+    } else {
+        Ok(true)
     }
-    Ok(default)
 }
 
 /// Loads a single `[host]` table for the given triple.

src/doc/src/reference/unstable.md

@@ -521,7 +521,9 @@ When `target-applies-to-host` is unset, or set to `true`, in the
 configuration file, the existing Cargo behavior is preserved (though see
 `-Zhost-config`, which changes that default). When it is set to `false`,
 no options from `[target.<host triple>]` are respected for host
-artifacts.
+artifacts. Furthermore, when set to `false`, host artifacts do not pick
+up flags from `RUSTFLAGS` or `[build]`, even if `--target` is _not_
+passed to Cargo.
 
 In the future, `target-applies-to-host` may end up defaulting to `false`
 to provide more sane and consistent default behavior. When set to
@@ -565,9 +567,8 @@ 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 value of
+Setting `-Zhost-config` changes the default for `target-applies-to-host` to
-`target-applies-to-host` to `false`, and will error if
+`false` from `true`.
-`target-applies-to-host` is set to `true`.
 
 ```console
 cargo +nightly -Ztarget-applies-to-host -Zhost-config build --target x86_64-unknown-linux-gnu

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 -Ztarget-applies-to-host
+error: the -Zhost-config flag requires the -Ztarget-applies-to-host flag to be set
 ",
         )
         .run();

tests/testsuite/rustflags.rs

@@ -1136,6 +1136,72 @@ fn target_rustflags_not_for_build_scripts_with_target() {
         .run();
 }
 
+#[cargo_test]
+fn build_rustflags_for_build_scripts() {
+    let host = rustc_host();
+    let p = project()
+        .file("src/lib.rs", "")
+        .file(
+            "build.rs",
+            r#"
+                fn main() { }
+                #[cfg(foo)]
+                fn main() { }
+            "#,
+        )
+        .file(
+            ".cargo/config",
+            "
+            [build]
+            rustflags = [\"--cfg=foo\"]
+            ",
+        )
+        .build();
+
+    // With "legacy" behavior, build.rustflags should apply to build scripts without --target
+    p.cargo("build")
+        .with_status(101)
+        .with_stderr_does_not_contain("[..]build_script_build[..]")
+        .run();
+
+    // But should _not_ apply _with_ --target
+    p.cargo("build --target").arg(host).run();
+
+    // Enabling -Ztarget-applies-to-host should not make a difference without the config setting
+    p.cargo("build")
+        .masquerade_as_nightly_cargo()
+        .arg("-Ztarget-applies-to-host")
+        .with_status(101)
+        .with_stderr_does_not_contain("[..]build_script_build[..]")
+        .run();
+    p.cargo("build --target")
+        .arg(host)
+        .masquerade_as_nightly_cargo()
+        .arg("-Ztarget-applies-to-host")
+        .run();
+
+    // When set to false though, the "proper" behavior where host artifacts _only_ pick up on
+    // [host] should be applied.
+    p.change_file(
+        ".cargo/config",
+        "
+        target-applies-to-host = false
+
+        [build]
+        rustflags = [\"--cfg=foo\"]
+        ",
+    );
+    p.cargo("build")
+        .masquerade_as_nightly_cargo()
+        .arg("-Ztarget-applies-to-host")
+        .run();
+    p.cargo("build --target")
+        .arg(host)
+        .masquerade_as_nightly_cargo()
+        .arg("-Ztarget-applies-to-host")
+        .run();
+}
+
 #[cargo_test]
 fn host_rustflags_for_build_scripts() {
     let host = rustc_host();