From 90a61d183f85f081a2b689773e424eb328c9e5b4 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Thu, 17 Feb 2022 10:10:53 -0800 Subject: [PATCH 01/15] Run target-applies-to-host tests on stable --- tests/testsuite/build_script.rs | 66 +++++++++++++-------------------- 1 file changed, 25 insertions(+), 41 deletions(-) diff --git a/tests/testsuite/build_script.rs b/tests/testsuite/build_script.rs index 40d8067d548..4e7f1cf2569 100644 --- a/tests/testsuite/build_script.rs +++ b/tests/testsuite/build_script.rs @@ -411,12 +411,10 @@ fn custom_build_env_var_rustc_linker_host_target() { // 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 --target") - .arg(&target) - .masquerade_as_nightly_cargo() - .run(); - } + p.cargo("build -Z target-applies-to-host --target") + .arg(&target) + .masquerade_as_nightly_cargo() + .run(); } #[cargo_test] @@ -448,13 +446,11 @@ fn custom_build_env_var_rustc_linker_host_target_env() { // 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 --target") - .env("CARGO_TARGET_APPLIES_TO_HOST", "false") - .arg(&target) - .masquerade_as_nightly_cargo() - .run(); - } + p.cargo("build -Z target-applies-to-host --target") + .env("CARGO_TARGET_APPLIES_TO_HOST", "false") + .arg(&target) + .masquerade_as_nightly_cargo() + .run(); } #[cargo_test] @@ -485,18 +481,16 @@ fn custom_build_invalid_host_config_feature_flag() { .build(); // build.rs should fail due to -Zhost-config being set without -Ztarget-applies-to-host - if cargo_test_support::is_nightly() { - p.cargo("build -Z host-config --target") - .arg(&target) - .masquerade_as_nightly_cargo() - .with_status(101) - .with_stderr_contains( - "\ + p.cargo("build -Z host-config --target") + .arg(&target) + .masquerade_as_nightly_cargo() + .with_status(101) + .with_stderr_contains( + "\ error: the -Zhost-config flag requires the -Ztarget-applies-to-host flag to be set ", - ) - .run(); - } + ) + .run(); } #[cargo_test] @@ -530,8 +524,7 @@ fn custom_build_env_var_rustc_linker_host_target_with_bad_host_config() { .build(); // build.rs should fail due to bad target linker being set - if cargo_test_support::is_nightly() { - p.cargo("build -Z target-applies-to-host -Z host-config --verbose --target") + p.cargo("build -Z target-applies-to-host -Z host-config --verbose --target") .arg(&target) .masquerade_as_nightly_cargo() .with_status(101) @@ -543,7 +536,6 @@ fn custom_build_env_var_rustc_linker_host_target_with_bad_host_config() { " ) .run(); - } } #[cargo_test] @@ -576,8 +568,7 @@ fn custom_build_env_var_rustc_linker_bad_host() { .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") + p.cargo("build -Z target-applies-to-host -Z host-config --verbose --target") .arg(&target) .masquerade_as_nightly_cargo() .with_status(101) @@ -589,7 +580,6 @@ fn custom_build_env_var_rustc_linker_bad_host() { " ) .run(); - } } #[cargo_test] @@ -624,8 +614,7 @@ fn custom_build_env_var_rustc_linker_bad_host_with_arch() { .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") + p.cargo("build -Z target-applies-to-host -Z host-config --verbose --target") .arg(&target) .masquerade_as_nightly_cargo() .with_status(101) @@ -637,7 +626,6 @@ fn custom_build_env_var_rustc_linker_bad_host_with_arch() { " ) .run(); - } } #[cargo_test] @@ -671,12 +659,10 @@ fn custom_build_env_var_rustc_linker_cross_arch_host() { .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") - .arg(&target) - .masquerade_as_nightly_cargo() - .run(); - } + p.cargo("build -Z target-applies-to-host -Z host-config --verbose --target") + .arg(&target) + .masquerade_as_nightly_cargo() + .run(); } #[cargo_test] @@ -712,8 +698,7 @@ fn custom_build_env_var_rustc_linker_bad_cross_arch_host() { .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") + p.cargo("build -Z target-applies-to-host -Z host-config --verbose --target") .arg(&target) .masquerade_as_nightly_cargo() .with_status(101) @@ -725,7 +710,6 @@ fn custom_build_env_var_rustc_linker_bad_cross_arch_host() { " ) .run(); - } } #[cargo_test] From 040f0d10a6317dc53a46c797190048cceacce189 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Thu, 17 Feb 2022 10:22:03 -0800 Subject: [PATCH 02/15] Simplify unused build scripts in tests The tests in question all tail to even build the build script, so the contents of `main` don't matter, and make the tests seem more complex than they really are. --- tests/testsuite/build_script.rs | 69 ++++----------------------------- 1 file changed, 8 insertions(+), 61 deletions(-) diff --git a/tests/testsuite/build_script.rs b/tests/testsuite/build_script.rs index 4e7f1cf2569..95449e3aa98 100644 --- a/tests/testsuite/build_script.rs +++ b/tests/testsuite/build_script.rs @@ -355,16 +355,7 @@ fn custom_build_env_var_rustc_linker_bad_host_target() { target ), ) - .file( - "build.rs", - r#" - use std::env; - - fn main() { - assert!(env::var("RUSTC_LINKER").unwrap().ends_with("/path/to/linker")); - } - "#, - ) + .file("build.rs", "fn main() {}") .file("src/lib.rs", "") .build(); @@ -467,16 +458,7 @@ fn custom_build_invalid_host_config_feature_flag() { target ), ) - .file( - "build.rs", - r#" - use std::env; - - fn main() { - assert!(env::var("RUSTC_LINKER").unwrap().ends_with("/path/to/linker")); - } - "#, - ) + .file("build.rs", "fn main() {}") .file("src/lib.rs", "") .build(); @@ -510,16 +492,7 @@ fn custom_build_env_var_rustc_linker_host_target_with_bad_host_config() { target ), ) - .file( - "build.rs", - r#" - use std::env; - - fn main() { - assert!(env::var("RUSTC_LINKER").unwrap().ends_with("/path/to/target/linker")); - } - "#, - ) + .file("build.rs", "fn main() {}") .file("src/lib.rs", "") .build(); @@ -554,16 +527,7 @@ fn custom_build_env_var_rustc_linker_bad_host() { target ), ) - .file( - "build.rs", - r#" - use std::env; - - fn main() { - assert!(env::var("RUSTC_LINKER").unwrap().ends_with("/path/to/target/linker")); - } - "#, - ) + .file("build.rs", "fn main() {}") .file("src/lib.rs", "") .build(); @@ -600,16 +564,7 @@ fn custom_build_env_var_rustc_linker_bad_host_with_arch() { target, target ), ) - .file( - "build.rs", - r#" - use std::env; - - fn main() { - assert!(env::var("RUSTC_LINKER").unwrap().ends_with("/path/to/target/linker")); - } - "#, - ) + .file("build.rs", "fn main() {}") .file("src/lib.rs", "") .build(); @@ -658,7 +613,8 @@ fn custom_build_env_var_rustc_linker_cross_arch_host() { .file("src/lib.rs", "") .build(); - // build.rs should fail due to bad host linker being set + // build.rs should be built fine since cross target != host target. + // assertion should succeed since it's still passed the target linker p.cargo("build -Z target-applies-to-host -Z host-config --verbose --target") .arg(&target) .masquerade_as_nightly_cargo() @@ -684,16 +640,7 @@ fn custom_build_env_var_rustc_linker_bad_cross_arch_host() { cross_target, target ), ) - .file( - "build.rs", - r#" - use std::env; - - fn main() { - assert!(env::var("RUSTC_LINKER").unwrap().ends_with("/path/to/target/linker")); - } - "#, - ) + .file("build.rs", "fn main() {}") .file("src/lib.rs", "") .build(); From fb47df72e16b674736ac74a3b57ca7389fe6ab99 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Wed, 16 Feb 2022 15:59:26 -0800 Subject: [PATCH 03/15] Add regression test for #10206 --- tests/testsuite/rustflags.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/testsuite/rustflags.rs b/tests/testsuite/rustflags.rs index 4f825b4f31d..163f410a1dd 100644 --- a/tests/testsuite/rustflags.rs +++ b/tests/testsuite/rustflags.rs @@ -1435,3 +1435,21 @@ fn remap_path_prefix_works() { .with_stdout("/foo/home/.cargo/registry/src/[..]/bar-0.1.0/src/lib.rs") .run(); } + +#[cargo_test] +fn host_config_rustflags_with_target() { + // regression test for https://github.com/rust-lang/cargo/issues/10206 + let p = project() + .file("src/lib.rs", "") + .file("build.rs.rs", "fn main() { assert!(cfg!(foo)); }") + .build(); + + p.cargo("build") + .masquerade_as_nightly_cargo() + .arg("-Zhost-config") + .arg("-Ztarget-applies-to-host=no") + .arg("-Zunstable-options") + .arg("--config") + .arg("host.rustflags=[\"--cfg=foo\"]") + .run(); +} From 4f41727106933b9d0e7427da5af24db98725ef01 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Wed, 16 Feb 2022 15:35:53 -0800 Subject: [PATCH 04/15] 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.]` and from `RUSTFLAGS`/`build.rustflags` if `--target ` is given. --- .../compiler/build_context/target_info.rs | 183 +++++++++++------- src/cargo/util/config/mod.rs | 2 +- src/cargo/util/config/target.rs | 29 ++- src/doc/src/reference/unstable.md | 43 ++-- tests/testsuite/build_script.rs | 9 +- tests/testsuite/rustflags.rs | 148 +++++++++++++- 6 files changed, 310 insertions(+), 104 deletions(-) diff --git a/src/cargo/core/compiler/build_context/target_info.rs b/src/cargo/core/compiler/build_context/target_info.rs index 505910d5fa2..c7c4535a921 100644 --- a/src/cargo/core/compiler/build_context/target_info.rs +++ b/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.`, and if +/// ` == ` `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> { - // 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 ` 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..rustflags` and run `cargo + // build` (with no `--target`). So, we respect `target.`.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::(); - // 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::>(&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::>(&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 --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index 0f183cdf563..83604e896c1 100644 --- a/src/cargo/util/config/mod.rs +++ b/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 { + pub fn target_applies_to_host(&self) -> CargoResult> { target::get_target_applies_to_host(self) } diff --git a/src/cargo/util/config/target.rs b/src/cargo/util/config/target.rs index 21089ce491b..407f120e5d1 100644 --- a/src/cargo/util/config/target.rs +++ b/src/cargo/util/config/target.rs @@ -66,20 +66,29 @@ pub(super) fn load_target_cfgs(config: &Config) -> CargoResult CargoResult { +pub(super) fn get_target_applies_to_host(config: &Config) -> CargoResult> { if config.cli_unstable().target_applies_to_host { - if let Ok(target_applies_to_host) = config.get::("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::>("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 --git a/src/doc/src/reference/unstable.md b/src/doc/src/reference/unstable.md index 7ca9d7b9f6b..9a19bc698f9 100644 --- a/src/doc/src/reference/unstable.md +++ b/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.]` 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.]` are respected for host artifacts. When it is +set to `false`, _no_ options from `[target.]` are respected +for host artifacts. + +In the specific case of `rustflags`, this settings also affects whether +`--target ` 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,8 +551,9 @@ 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 @@ -544,6 +561,7 @@ options to be set. 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 --git a/tests/testsuite/build_script.rs b/tests/testsuite/build_script.rs index 95449e3aa98..f57c120588e 100644 --- a/tests/testsuite/build_script.rs +++ b/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,7 +495,7 @@ 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() @@ -504,8 +503,8 @@ fn custom_build_env_var_rustc_linker_host_target_with_bad_host_config() { .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(); diff --git a/tests/testsuite/rustflags.rs b/tests/testsuite/rustflags.rs index 163f410a1dd..d0ee0ac931d 100644 --- a/tests/testsuite/rustflags.rs +++ b/tests/testsuite/rustflags.rs @@ -272,6 +272,47 @@ fn env_rustflags_build_script_with_target() { .run(); } +#[cargo_test] +fn env_rustflags_build_script_with_target_and_applies_to_host_kind() { + // RUSTFLAGS *should* be passed to rustc for build scripts when --target is specified as the + // host triple and target-applies-to-host-kind is enabled. + // In this test if --cfg foo is not passed the build will fail. + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + build = "build.rs" + "#, + ) + .file("src/lib.rs", "") + .file( + "build.rs", + r#" + fn main() { } + #[cfg(not(foo))] + fn main() { } + "#, + ) + .file( + ".cargo/config.toml", + r#" + target-applies-to-host = true + "#, + ) + .build(); + + let host = rustc_host(); + p.cargo("build --target") + .masquerade_as_nightly_cargo() + .arg(host) + .arg("-Ztarget-applies-to-host") + .env("RUSTFLAGS", "--cfg foo") + .run(); +} + #[cargo_test] fn env_rustflags_build_script_dep_with_target() { // RUSTFLAGS should not be passed to rustc for build scripts @@ -664,7 +705,7 @@ fn build_rustflags_normal_source_with_target() { let host = &rustc_host(); - // Use RUSTFLAGS to pass an argument that will generate an error + // Use build.rustflags to pass an argument that will generate an error p.cargo("build --lib --target") .arg(host) .with_status(101) @@ -690,6 +731,25 @@ fn build_rustflags_normal_source_with_target() { .with_status(101) .with_stderr_contains("[..]bogus[..]") .run(); + + // With -Ztarget-applies-to-host build.rustflags should apply to build.rs since target == host + p.change_file("build.rs", "fn main() {}"); + p.change_file( + ".cargo/config", + r#" + target-applies-to-host = true + + [build] + rustflags = ["-Clink-arg=this is a bogus argument"] + "#, + ); + p.cargo("build --lib --target") + .arg(host) + .masquerade_as_nightly_cargo() + .arg("-Ztarget-applies-to-host") + .with_status(101) + .with_stderr_contains("[..]build_script_build[..]") + .run(); } #[cargo_test] @@ -993,6 +1053,89 @@ fn target_rustflags_normal_source() { .run(); } +#[cargo_test] +fn target_rustflags_also_for_build_scripts() { + let p = project() + .file("src/lib.rs", "") + .file( + "build.rs", + r#" + fn main() { } + #[cfg(not(foo))] + fn main() { } + "#, + ) + .file( + ".cargo/config", + &format!( + " + [target.{}] + rustflags = [\"--cfg=foo\"] + ", + rustc_host() + ), + ) + .build(); + + p.cargo("build").run(); +} + +#[cargo_test] +fn target_rustflags_not_for_build_scripts_with_target() { + let host = rustc_host(); + let p = project() + .file("src/lib.rs", "") + .file( + "build.rs", + r#" + fn main() { } + #[cfg(foo)] + fn main() { } + "#, + ) + .file( + ".cargo/config", + &format!( + " + [target.{}] + rustflags = [\"--cfg=foo\"] + ", + host + ), + ) + .build(); + + p.cargo("build --target").arg(host).run(); + + // Enabling -Ztarget-applies-to-host should not make a difference without the config setting + p.cargo("build --target") + .arg(host) + .masquerade_as_nightly_cargo() + .arg("-Ztarget-applies-to-host") + .run(); + + // But if we also set the setting, then the rustflags from `target.` should apply + p.change_file( + ".cargo/config", + &format!( + " + target-applies-to-host = true + + [target.{}] + rustflags = [\"--cfg=foo\"] + ", + host + ), + ); + p.cargo("build --target") + .arg(host) + .masquerade_as_nightly_cargo() + .arg("-Ztarget-applies-to-host") + .with_status(101) + .with_stderr_contains("[..]the name `main` is defined multiple times[..]") + .run(); +} + // target.{}.rustflags takes precedence over build.rustflags #[cargo_test] fn target_rustflags_precedence() { @@ -1442,12 +1585,13 @@ fn host_config_rustflags_with_target() { let p = project() .file("src/lib.rs", "") .file("build.rs.rs", "fn main() { assert!(cfg!(foo)); }") + .file(".cargo/config.toml", "target-applies-to-host = false") .build(); p.cargo("build") .masquerade_as_nightly_cargo() .arg("-Zhost-config") - .arg("-Ztarget-applies-to-host=no") + .arg("-Ztarget-applies-to-host") .arg("-Zunstable-options") .arg("--config") .arg("host.rustflags=[\"--cfg=foo\"]") From 49f90f5055f3d8aef3f30f58222d58508a48c488 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Mon, 21 Feb 2022 10:20:32 -0800 Subject: [PATCH 05/15] Retain legacy behavior with tath != false --- .../compiler/build_context/target_info.rs | 38 +++++++++---- src/cargo/util/config/mod.rs | 2 +- src/cargo/util/config/target.rs | 9 +-- tests/testsuite/rustflags.rs | 56 +++++++++++++++---- 4 files changed, 78 insertions(+), 27 deletions(-) diff --git a/src/cargo/core/compiler/build_context/target_info.rs b/src/cargo/core/compiler/build_context/target_info.rs index c7c4535a921..b50507671a6 100644 --- a/src/cargo/core/compiler/build_context/target_info.rs +++ b/src/cargo/core/compiler/build_context/target_info.rs @@ -585,11 +585,9 @@ fn output_err_info(cmd: &ProcessBuilder, stdout: &str, stderr: &str) -> String { /// /// 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.`, and if -/// ` == ` `RUSTFLAGS` and `CARGO_ENCODED_RUSTFLAGS`. +/// For those artifacts, _only_ `host.*.rustflags` is respected, and no other configuration +/// sources, _regardless of the value of `target-applies-to-host`_. This is counterintuitive, but +/// necessary to retain bacwkards compatibility with older versions of Cargo. fn env_args( config: &Config, requested_kinds: &[CompileKind], @@ -598,6 +596,17 @@ fn env_args( kind: CompileKind, name: &str, ) -> CargoResult> { + // The `target-applies-to-host` setting is somewhat misleading in name. + // + // What it really does it opt into a _particular_ past Cargo behavior for `[target.]` for + // host artifacts, namely that `linker` and `runner` from `[target.]` 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()?; // Include untargeted configuration sources (like `RUSTFLAGS`) if @@ -611,10 +620,11 @@ fn env_args( // for the target, and `target-applies-to-host` makes it so that the host is affected by its // target's config, so if `--target ` then `RUSTFLAGS` should also apply to the // host. - let include_generic = !kind.is_host() + let mut include_generic = !kind.is_host() || requested_kinds == [CompileKind::Host] - || (target_applies_to_host == Some(true) + || (target_applies_to_host && requested_kinds == [CompileKind::Target(CompileTarget::new(host_triple)?)]); + // Include targeted configuration sources (like `target.*.rustflags`) if // // - we're compiling artifacts for the target platform; or @@ -628,9 +638,9 @@ fn env_args( // build` (with no `--target`). So, we respect `target.`.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); + 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 // // - we're compiling host artifacts; or @@ -640,6 +650,12 @@ fn env_args( // 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 { // First try CARGO_ENCODED_RUSTFLAGS from the environment. // Prefer this over RUSTFLAGS since it's less prone to encoding errors. @@ -753,7 +769,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 != Some(false) { + let host_config = if target_applies_to_host { config.target_cfg_triple(&rustc.host)? } else { config.host_cfg_triple(&rustc.host)? diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index 83604e896c1..0f183cdf563 100644 --- a/src/cargo/util/config/mod.rs +++ b/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> { + pub fn target_applies_to_host(&self) -> CargoResult { target::get_target_applies_to_host(self) } diff --git a/src/cargo/util/config/target.rs b/src/cargo/util/config/target.rs index 407f120e5d1..1174a12cc52 100644 --- a/src/cargo/util/config/target.rs +++ b/src/cargo/util/config/target.rs @@ -66,7 +66,8 @@ pub(super) fn load_target_cfgs(config: &Config) -> CargoResult CargoResult> { +pub(super) fn get_target_applies_to_host(config: &Config) -> CargoResult { + let default = true; if config.cli_unstable().target_applies_to_host { if let Ok(target_applies_to_host) = config.get::>("target-applies-to-host") { if config.cli_unstable().host_config { @@ -79,16 +80,16 @@ pub(super) fn get_target_applies_to_host(config: &Config) -> CargoResult {} None => { // -Zhost-config automatically disables target-applies-to-host - return Ok(Some(false)); + return Ok(false); } } } - return Ok(target_applies_to_host); + 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"); } - Ok(None) + Ok(default) } /// Loads a single `[host]` table for the given triple. diff --git a/tests/testsuite/rustflags.rs b/tests/testsuite/rustflags.rs index d0ee0ac931d..a271a94d35c 100644 --- a/tests/testsuite/rustflags.rs +++ b/tests/testsuite/rustflags.rs @@ -273,10 +273,10 @@ fn env_rustflags_build_script_with_target() { } #[cargo_test] -fn env_rustflags_build_script_with_target_and_applies_to_host_kind() { - // RUSTFLAGS *should* be passed to rustc for build scripts when --target is specified as the - // host triple and target-applies-to-host-kind is enabled. - // In this test if --cfg foo is not passed the build will fail. +fn env_rustflags_build_script_with_target_doesnt_apply_to_host_kind() { + // RUSTFLAGS should *not* be passed to rustc for build scripts when --target is specified as the + // host triple even if target-applies-to-host-kind is enabled, to match legacy Cargo behavior. + // In this test if --cfg foo is passed the build will fail. let p = project() .file( "Cargo.toml", @@ -292,7 +292,7 @@ fn env_rustflags_build_script_with_target_and_applies_to_host_kind() { "build.rs", r#" fn main() { } - #[cfg(not(foo))] + #[cfg(foo)] fn main() { } "#, ) @@ -732,7 +732,8 @@ fn build_rustflags_normal_source_with_target() { .with_stderr_contains("[..]bogus[..]") .run(); - // With -Ztarget-applies-to-host build.rustflags should apply to build.rs since target == host + // With -Ztarget-applies-to-host build.rustflags should still not apply to build.rs, even when + // target == host, to match legacy Cargo behavior. p.change_file("build.rs", "fn main() {}"); p.change_file( ".cargo/config", @@ -740,7 +741,7 @@ fn build_rustflags_normal_source_with_target() { target-applies-to-host = true [build] - rustflags = ["-Clink-arg=this is a bogus argument"] + rustflags = ["-Z", "bogus"] "#, ); p.cargo("build --lib --target") @@ -748,7 +749,7 @@ fn build_rustflags_normal_source_with_target() { .masquerade_as_nightly_cargo() .arg("-Ztarget-applies-to-host") .with_status(101) - .with_stderr_contains("[..]build_script_build[..]") + .with_stderr_does_not_contain("[..]build_script_build[..]") .run(); } @@ -1114,7 +1115,8 @@ fn target_rustflags_not_for_build_scripts_with_target() { .arg("-Ztarget-applies-to-host") .run(); - // But if we also set the setting, then the rustflags from `target.` should apply + // Even with the setting, the rustflags from `target.` should not apply, to match the legacy + // Cargo behavior. p.change_file( ".cargo/config", &format!( @@ -1131,8 +1133,40 @@ fn target_rustflags_not_for_build_scripts_with_target() { .arg(host) .masquerade_as_nightly_cargo() .arg("-Ztarget-applies-to-host") - .with_status(101) - .with_stderr_contains("[..]the name `main` is defined multiple times[..]") + .run(); +} + +#[cargo_test] +fn host_rustflags_for_build_scripts() { + let host = rustc_host(); + let p = project() + .file("src/lib.rs", "") + .file( + "build.rs", + r#" + // Ensure that --cfg=foo is passed. + fn main() { assert!(cfg!(foo)); } + "#, + ) + .file( + ".cargo/config", + &format!( + " + target-applies-to-host = false + + [host.{}] + rustflags = [\"--cfg=foo\"] + ", + host + ), + ) + .build(); + + p.cargo("build --target") + .arg(host) + .masquerade_as_nightly_cargo() + .arg("-Ztarget-applies-to-host") + .arg("-Zhost-config") .run(); } From 216eeeac338f0396433071c7f74df9bdeaf2eb50 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Mon, 21 Feb 2022 10:28:50 -0800 Subject: [PATCH 06/15] Fix up doc to reflect latest semantics change --- src/doc/src/reference/unstable.md | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/doc/src/reference/unstable.md b/src/doc/src/reference/unstable.md index 9a19bc698f9..0bdd4453bd9 100644 --- a/src/doc/src/reference/unstable.md +++ b/src/doc/src/reference/unstable.md @@ -517,16 +517,11 @@ that are _always_ built for the host platform. 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.]` are respected for host artifacts. When it is -set to `false`, _no_ options from `[target.]` are respected -for host artifacts. - -In the specific case of `rustflags`, this settings also affects whether -`--target ` picks up the same configuration as if -`--target` was not supplied in the first place. +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.]` are respected for host +artifacts. In the future, `target-applies-to-host` may end up defaulting to `false` to provide more sane and consistent default behavior. When set to From 745329c3b12ab3929248d3a7bf59b65c2602c0c5 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Wed, 23 Feb 2022 16:35:41 -0800 Subject: [PATCH 07/15] Remove env_var from test names that don't use env --- tests/testsuite/build_script.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/testsuite/build_script.rs b/tests/testsuite/build_script.rs index f57c120588e..b1e00b7b7be 100644 --- a/tests/testsuite/build_script.rs +++ b/tests/testsuite/build_script.rs @@ -476,7 +476,7 @@ error: the -Zhost-config flag requires -Ztarget-applies-to-host } #[cargo_test] -fn custom_build_env_var_rustc_linker_host_target_with_bad_host_config() { +fn custom_build_linker_host_target_with_bad_host_config() { let target = rustc_host(); let p = project() .file( @@ -511,7 +511,7 @@ fn custom_build_env_var_rustc_linker_host_target_with_bad_host_config() { } #[cargo_test] -fn custom_build_env_var_rustc_linker_bad_host() { +fn custom_build_linker_bad_host() { let target = rustc_host(); let p = project() .file( @@ -546,7 +546,7 @@ fn custom_build_env_var_rustc_linker_bad_host() { } #[cargo_test] -fn custom_build_env_var_rustc_linker_bad_host_with_arch() { +fn custom_build_linker_bad_host_with_arch() { let target = rustc_host(); let p = project() .file( @@ -621,7 +621,7 @@ fn custom_build_env_var_rustc_linker_cross_arch_host() { } #[cargo_test] -fn custom_build_env_var_rustc_linker_bad_cross_arch_host() { +fn custom_build_linker_bad_cross_arch_host() { let target = rustc_host(); let cross_target = cross_compile::alternate(); let p = project() From f23894d93fefaf191e8d665994a5a4b40a340168 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Wed, 23 Feb 2022 16:36:14 -0800 Subject: [PATCH 08/15] Make rustflags logic more linear --- .../compiler/build_context/target_info.rs | 261 ++++++++++-------- src/cargo/util/config/target.rs | 28 +- src/doc/src/reference/unstable.md | 9 +- tests/testsuite/build_script.rs | 2 +- tests/testsuite/rustflags.rs | 66 +++++ 5 files changed, 225 insertions(+), 141 deletions(-) diff --git a/src/cargo/core/compiler/build_context/target_info.rs b/src/cargo/core/compiler/build_context/target_info.rs index b50507671a6..7f5a0248cb8 100644 --- a/src/cargo/core/compiler/build_context/target_info.rs +++ b/src/cargo/core/compiler/build_context/target_info.rs @@ -596,145 +596,172 @@ fn env_args( kind: CompileKind, name: &str, ) -> CargoResult> { - // The `target-applies-to-host` setting is somewhat misleading in name. - // - // What it really does it opt into a _particular_ past Cargo behavior for `[target.]` for - // host artifacts, namely that `linker` and `runner` from `[target.]` 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. + // 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. 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) + + // [host] is a new feature, so we have it take priority over [target.] + } else if let Some(rustflags) = rustflags_from_host(config, host_triple)? { + Ok(rustflags) + + // but [target.] _does_ apply to host artifacts + } else if let Some(rustflags) = + rustflags_from_target(config, host_triple, target_cfg, kind, name)? + { + Ok(rustflags) + + // and otherwise we fall back to [build] + } else if let Some(rustflags) = rustflags_from_build(config, name)? { + Ok(rustflags) + } else { + Ok(Vec::new()) + } - // Include untargeted configuration sources (like `RUSTFLAGS`) if - // - // - 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. - // - // 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 ` 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 - // - // - 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..rustflags` and run `cargo - // build` (with no `--target`). So, we respect `target.`.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 - // - // - 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]; - - // 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 --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()) + } - 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()); + // 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()) } - 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()); + // 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 `, 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> { + // 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 Some(Vec::new()); } + return Some(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 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>> { let mut rustflags = Vec::new(); let name = name .chars() .flat_map(|c| c.to_lowercase()) .collect::(); - 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::>(&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()); - }); - } - } - 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()); - } + // 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::>(&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()); + }); } - if !rustflags.is_empty() { - return Ok(rustflags); + if rustflags.is_empty() { + Ok(None) + } else { + Ok(Some(rustflags)) } +} - 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()); - } +fn rustflags_from_host(config: &Config, host_triple: &str) -> CargoResult>> { + let target_cfg = config.host_cfg_triple(host_triple)?; + if let Some(rf) = target_cfg.rustflags { + Ok(Some(rf.val.as_slice().iter().cloned().collect())) + } else { + Ok(None) } +} - Ok(Vec::new()) +fn rustflags_from_build(config: &Config, name: &str) -> CargoResult>> { + // Then the `build.rustflags` value. + let build = config.build_config()?; + let list = if name.eq_ignore_ascii_case("rustflags") { + &build.rustflags + } else { + &build.rustdocflags + }; + Ok(list.as_ref().map(|l| l.as_slice().to_vec())) } /// Collection of information about `rustc` and the host and target. diff --git a/src/cargo/util/config/target.rs b/src/cargo/util/config/target.rs index 1174a12cc52..21089ce491b 100644 --- a/src/cargo/util/config/target.rs +++ b/src/cargo/util/config/target.rs @@ -67,29 +67,19 @@ pub(super) fn load_target_cfgs(config: &Config) -> CargoResult CargoResult { - let default = true; if config.cli_unstable().target_applies_to_host { - if let Ok(target_applies_to_host) = config.get::>("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(false); - } - } - } - return Ok(target_applies_to_host.unwrap_or(default)); + if let Ok(target_applies_to_host) = config.get::("target-applies-to-host") { + Ok(target_applies_to_host) + } else { + Ok(!config.cli_unstable().host_config) } } 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. diff --git a/src/doc/src/reference/unstable.md b/src/doc/src/reference/unstable.md index 0bdd4453bd9..f79ed6bc22d 100644 --- a/src/doc/src/reference/unstable.md +++ b/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.]` 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 -`target-applies-to-host` to `false`, and will error if -`target-applies-to-host` is set to `true`. +Setting `-Zhost-config` changes the default for `target-applies-to-host` to +`false` from `true`. ```console cargo +nightly -Ztarget-applies-to-host -Zhost-config build --target x86_64-unknown-linux-gnu diff --git a/tests/testsuite/build_script.rs b/tests/testsuite/build_script.rs index b1e00b7b7be..d0ef3a2e727 100644 --- a/tests/testsuite/build_script.rs +++ b/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(); diff --git a/tests/testsuite/rustflags.rs b/tests/testsuite/rustflags.rs index a271a94d35c..9b5441ef02c 100644 --- a/tests/testsuite/rustflags.rs +++ b/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(); From 84eb8205de4063796dfd4e1fdbe11cc8b72ef4c7 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Wed, 23 Feb 2022 16:36:53 -0800 Subject: [PATCH 09/15] Don't == true --- src/cargo/core/compiler/build_context/target_info.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cargo/core/compiler/build_context/target_info.rs b/src/cargo/core/compiler/build_context/target_info.rs index 7f5a0248cb8..dad05401006 100644 --- a/src/cargo/core/compiler/build_context/target_info.rs +++ b/src/cargo/core/compiler/build_context/target_info.rs @@ -599,7 +599,7 @@ fn env_args( // 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. let target_applies_to_host = config.target_applies_to_host()?; - if target_applies_to_host == true { + if target_applies_to_host { // 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: // From df90291ee5757134dc5df35194a0903b44acc174 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Wed, 23 Feb 2022 16:45:09 -0800 Subject: [PATCH 10/15] Handle host.rustdocflags --- .../compiler/build_context/target_info.rs | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/cargo/core/compiler/build_context/target_info.rs b/src/cargo/core/compiler/build_context/target_info.rs index dad05401006..7b69fcbd20f 100644 --- a/src/cargo/core/compiler/build_context/target_info.rs +++ b/src/cargo/core/compiler/build_context/target_info.rs @@ -611,7 +611,7 @@ fn env_args( Ok(rustflags) // [host] is a new feature, so we have it take priority over [target.] - } else if let Some(rustflags) = rustflags_from_host(config, host_triple)? { + } else if let Some(rustflags) = rustflags_from_host(config, name, host_triple)? { Ok(rustflags) // but [target.] _does_ apply to host artifacts @@ -631,7 +631,7 @@ fn env_args( // 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)? { + if let Some(rustflags) = rustflags_from_host(config, name, host_triple)? { Ok(rustflags) } else { Ok(Vec::new()) @@ -656,7 +656,7 @@ fn env_args( // --target. Or, phrased differently, no `--target` behaves the same as `--target `, 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)) + Ok(rustflags_from_host(config, name, host_triple)?.unwrap_or_else(Vec::new)) // All other artifacts pick up the RUSTFLAGS, [target.*], and [build], in that order. } else { @@ -711,7 +711,6 @@ fn rustflags_from_target( .chars() .flat_map(|c| c.to_lowercase()) .collect::(); - // Then the target.*.rustflags value... let target = match &kind { CompileKind::Host => host_triple, @@ -744,13 +743,19 @@ fn rustflags_from_target( } } -fn rustflags_from_host(config: &Config, host_triple: &str) -> CargoResult>> { +fn rustflags_from_host( + config: &Config, + name: &str, + host_triple: &str, +) -> CargoResult>> { let target_cfg = config.host_cfg_triple(host_triple)?; - if let Some(rf) = target_cfg.rustflags { - Ok(Some(rf.val.as_slice().iter().cloned().collect())) + let list = if name.eq_ignore_ascii_case("rustflags") { + &target_cfg.rustflags } else { - Ok(None) - } + // host.rustdocflags is not a thing, since it does not make sense + return Ok(None); + }; + Ok(list.as_ref().map(|l| l.val.as_slice().to_vec())) } fn rustflags_from_build(config: &Config, name: &str) -> CargoResult>> { From 153bca2bf8f5d728e7b35a0d63bedd1e6dab5e63 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Thu, 24 Feb 2022 09:33:38 -0800 Subject: [PATCH 11/15] Unify logic of legacy and non-legacy code paths --- .../compiler/build_context/target_info.rs | 95 ++++++------------- 1 file changed, 27 insertions(+), 68 deletions(-) diff --git a/src/cargo/core/compiler/build_context/target_info.rs b/src/cargo/core/compiler/build_context/target_info.rs index 7b69fcbd20f..fbdd97d9546 100644 --- a/src/cargo/core/compiler/build_context/target_info.rs +++ b/src/cargo/core/compiler/build_context/target_info.rs @@ -596,81 +596,40 @@ fn env_args( kind: CompileKind, name: &str, ) -> CargoResult> { - // 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. let target_applies_to_host = config.target_applies_to_host()?; - if target_applies_to_host { - // 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) - - // [host] is a new feature, so we have it take priority over [target.] - } else if let Some(rustflags) = rustflags_from_host(config, name, host_triple)? { - Ok(rustflags) - - // but [target.] _does_ apply to host artifacts - } else if let Some(rustflags) = - rustflags_from_target(config, host_triple, target_cfg, kind, name)? - { - Ok(rustflags) - - // and otherwise we fall back to [build] - } else if let Some(rustflags) = rustflags_from_build(config, name)? { - Ok(rustflags) - } else { - Ok(Vec::new()) - } - - // 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, name, host_triple)? { - Ok(rustflags) - } else { - Ok(Vec::new()) - } - // All other artifacts pick up the RUSTFLAGS, [target.*], and [build], in that order. + // Host artifacts should not generally pick up rustflags from anywhere except [host]. + // + // The one exception to this is if `target-applies-to-host = true`, which opts into a + // particular (inconsistent) past Cargo behavior where host artifacts _do_ pick up rustflags + // set elsewhere when `--target` isn't passed. + if kind.is_host() { + if target_applies_to_host && requested_kinds == [CompileKind::Host] { + // This is the past Cargo behavior where we fall back to the same logic as for other + // artifacts without --target. } 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()) - } + // In all other cases, host artifacts just get flags from [host], regardless of + // --target. Or, phrased differently, no `--target` behaves the same as `--target + // `, and host artifacts are always "special" (they don't pick up `RUSTFLAGS` for + // example). + return Ok(rustflags_from_host(config, name, host_triple)?.unwrap_or_else(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 `, and - // host artifacts are always "special" (they don't pick up `RUSTFLAGS` for example). - } else if kind.is_host() { - Ok(rustflags_from_host(config, name, host_triple)?.unwrap_or_else(Vec::new)) + } // All other artifacts pick up the RUSTFLAGS, [target.*], and [build], in that order. + // NOTE: It is impossible to have a [host] section and reach this logic with kind.is_host(), + // since [host] implies `target-applies-to-host = false`, which always early-returns above. + + 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 { - 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()) - } + Ok(Vec::new()) } } From 4ae6651d4b1e400eba73c231924d6f20728d6a88 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Thu, 24 Feb 2022 14:56:09 -0800 Subject: [PATCH 12/15] Make rustflags/rustdocflags an enum --- .../compiler/build_context/target_info.rs | 74 ++++++++++++------- 1 file changed, 46 insertions(+), 28 deletions(-) diff --git a/src/cargo/core/compiler/build_context/target_info.rs b/src/cargo/core/compiler/build_context/target_info.rs index fbdd97d9546..acf8ea60350 100644 --- a/src/cargo/core/compiler/build_context/target_info.rs +++ b/src/cargo/core/compiler/build_context/target_info.rs @@ -146,7 +146,7 @@ impl TargetInfo { &rustc.host, None, kind, - "RUSTFLAGS", + Flags::Rust, )?; let extra_fingerprint = kind.fingerprint_hash(); let mut process = rustc.workspace_process(); @@ -252,7 +252,7 @@ impl TargetInfo { &rustc.host, Some(&cfg), kind, - "RUSTFLAGS", + Flags::Rust, )?, rustdocflags: env_args( config, @@ -260,7 +260,7 @@ impl TargetInfo { &rustc.host, Some(&cfg), kind, - "RUSTDOCFLAGS", + Flags::Rustdoc, )?, cfg, supports_split_debuginfo, @@ -566,6 +566,28 @@ fn output_err_info(cmd: &ProcessBuilder, stdout: &str, stderr: &str) -> String { result } +#[derive(Debug, Copy, Clone)] +enum Flags { + Rust, + Rustdoc, +} + +impl Flags { + fn as_key(self) -> &'static str { + match self { + Flags::Rust => "rustflags", + Flags::Rustdoc => "rustdocflags", + } + } + + fn as_env(self) -> &'static str { + match self { + Flags::Rust => "RUSTFLAGS", + Flags::Rustdoc => "RUSTDOCFLAGS", + } + } +} + /// Acquire extra flags to pass to the compiler from various locations. /// /// The locations are: @@ -594,7 +616,7 @@ fn env_args( host_triple: &str, target_cfg: Option<&[Cfg]>, kind: CompileKind, - name: &str, + flags: Flags, ) -> CargoResult> { let target_applies_to_host = config.target_applies_to_host()?; @@ -612,7 +634,7 @@ fn env_args( // --target. Or, phrased differently, no `--target` behaves the same as `--target // `, and host artifacts are always "special" (they don't pick up `RUSTFLAGS` for // example). - return Ok(rustflags_from_host(config, name, host_triple)?.unwrap_or_else(Vec::new)); + return Ok(rustflags_from_host(config, flags, host_triple)?.unwrap_or_else(Vec::new)); } } @@ -620,23 +642,23 @@ fn env_args( // NOTE: It is impossible to have a [host] section and reach this logic with kind.is_host(), // since [host] implies `target-applies-to-host = false`, which always early-returns above. - if let Some(rustflags) = rustflags_from_env(name) { + if let Some(rustflags) = rustflags_from_env(flags) { Ok(rustflags) } else if let Some(rustflags) = - rustflags_from_target(config, host_triple, target_cfg, kind, name)? + rustflags_from_target(config, host_triple, target_cfg, kind, flags)? { Ok(rustflags) - } else if let Some(rustflags) = rustflags_from_build(config, name)? { + } else if let Some(rustflags) = rustflags_from_build(config, flags)? { Ok(rustflags) } else { Ok(Vec::new()) } } -fn rustflags_from_env(name: &str) -> Option> { +fn rustflags_from_env(flags: Flags) -> Option> { // 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 let Ok(a) = env::var(format!("CARGO_ENCODED_{}", flags.as_env())) { if a.is_empty() { return Some(Vec::new()); } @@ -644,7 +666,7 @@ fn rustflags_from_env(name: &str) -> Option> { } // Then try RUSTFLAGS from the environment - if let Ok(a) = env::var(name) { + if let Ok(a) = env::var(flags.as_env()) { let args = a .split(' ') .map(str::trim) @@ -662,20 +684,16 @@ fn rustflags_from_target( host_triple: &str, target_cfg: Option<&[Cfg]>, kind: CompileKind, - name: &str, + flag: Flags, ) -> CargoResult>> { let mut rustflags = Vec::new(); - let name = name - .chars() - .flat_map(|c| c.to_lowercase()) - .collect::(); // 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); + let key = format!("target.{}.{}", target, flag.as_key()); if let Some(args) = config.get::>(&key)? { rustflags.extend(args.as_slice().iter().cloned()); } @@ -704,26 +722,26 @@ fn rustflags_from_target( fn rustflags_from_host( config: &Config, - name: &str, + flag: Flags, host_triple: &str, ) -> CargoResult>> { let target_cfg = config.host_cfg_triple(host_triple)?; - let list = if name.eq_ignore_ascii_case("rustflags") { - &target_cfg.rustflags - } else { - // host.rustdocflags is not a thing, since it does not make sense - return Ok(None); + let list = match flag { + Flags::Rust => &target_cfg.rustflags, + Flags::Rustdoc => { + // host.rustdocflags is not a thing, since it does not make sense + return Ok(None); + } }; Ok(list.as_ref().map(|l| l.val.as_slice().to_vec())) } -fn rustflags_from_build(config: &Config, name: &str) -> CargoResult>> { +fn rustflags_from_build(config: &Config, flag: Flags) -> CargoResult>> { // Then the `build.rustflags` value. let build = config.build_config()?; - let list = if name.eq_ignore_ascii_case("rustflags") { - &build.rustflags - } else { - &build.rustdocflags + let list = match flag { + Flags::Rust => &build.rustflags, + Flags::Rustdoc => &build.rustdocflags, }; Ok(list.as_ref().map(|l| l.as_slice().to_vec())) } From b4ce8a8ed2fab8b09486ae867ef901c4bdba7d83 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Thu, 24 Feb 2022 14:56:23 -0800 Subject: [PATCH 13/15] Make docs more accurate --- src/doc/src/reference/unstable.md | 37 ++++++++++++++++++------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/src/doc/src/reference/unstable.md b/src/doc/src/reference/unstable.md index f79ed6bc22d..5c61f99813b 100644 --- a/src/doc/src/reference/unstable.md +++ b/src/doc/src/reference/unstable.md @@ -508,27 +508,32 @@ 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) -Historically, Cargo has respected the `linker` and `runner` options (but -_not_ `rustflags`) of `[target.]` configuration sections -when building and running build scripts, plugins, and other artifacts -that are _always_ built for the host platform. +Historically, Cargo's behavior for whether the `linker` and `rustflags` +configuration options from environment variables and `[target]` are +respected for build scripts, plugins, and other artifacts that are +_always_ built for the host platform has been somewhat inconsistent. +When `--target` is _not_ passed, Cargo respects the same `linker` and +`rustflags` for build scripts as for all other compile artifacts. When +`--target` _is_ passed, however, Cargo respects `linker` from +`[target.]`, and does not pick up any `rustflags` +configuration. This dual behavior is confusing, but also makes it +difficult to correctly configure builds where the host triple and the +target triple happen to be the same, but artifacts intended to run on +the build host should still be configured differently. + `-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, 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.]` are respected for host -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. +these properties. 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.]`, +`RUSTFLAGS`, or `[build]` are respected for host artifacts regardless of +whether `--target` is passed to Cargo. To customize artifacts intended +to be run on the host, use `[host]` ([`host-config`](#host-config)). 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. +to provide more sane and consistent default behavior. ```toml # config.toml From 54674d365bae68a78e51f8f1337328159e8fe990 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Thu, 24 Feb 2022 14:56:46 -0800 Subject: [PATCH 14/15] Improve precision of host build.rustflags tests --- tests/testsuite/rustflags.rs | 24 ++---------------------- 1 file changed, 2 insertions(+), 22 deletions(-) diff --git a/tests/testsuite/rustflags.rs b/tests/testsuite/rustflags.rs index 9b5441ef02c..f630da106ac 100644 --- a/tests/testsuite/rustflags.rs +++ b/tests/testsuite/rustflags.rs @@ -731,26 +731,6 @@ fn build_rustflags_normal_source_with_target() { .with_status(101) .with_stderr_contains("[..]bogus[..]") .run(); - - // With -Ztarget-applies-to-host build.rustflags should still not apply to build.rs, even when - // target == host, to match legacy Cargo behavior. - p.change_file("build.rs", "fn main() {}"); - p.change_file( - ".cargo/config", - r#" - target-applies-to-host = true - - [build] - rustflags = ["-Z", "bogus"] - "#, - ); - p.cargo("build --lib --target") - .arg(host) - .masquerade_as_nightly_cargo() - .arg("-Ztarget-applies-to-host") - .with_status(101) - .with_stderr_does_not_contain("[..]build_script_build[..]") - .run(); } #[cargo_test] @@ -1161,7 +1141,7 @@ fn build_rustflags_for_build_scripts() { // 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[..]") + .with_stderr_contains("[..]previous definition of the value `main` here[..]") .run(); // But should _not_ apply _with_ --target @@ -1172,7 +1152,7 @@ fn build_rustflags_for_build_scripts() { .masquerade_as_nightly_cargo() .arg("-Ztarget-applies-to-host") .with_status(101) - .with_stderr_does_not_contain("[..]build_script_build[..]") + .with_stderr_contains("[..]previous definition of the value `main` here[..]") .run(); p.cargo("build --target") .arg(host) From 56db82957fa356b63a58176e591593ee1d5c5ee3 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Thu, 24 Feb 2022 15:17:31 -0800 Subject: [PATCH 15/15] Avoid #[cfg] duplicated main in tests It makes the logic really hard to follow. --- tests/testsuite/rustflags.rs | 56 +++++++++++++----------------------- 1 file changed, 20 insertions(+), 36 deletions(-) diff --git a/tests/testsuite/rustflags.rs b/tests/testsuite/rustflags.rs index f630da106ac..ba7473938f4 100644 --- a/tests/testsuite/rustflags.rs +++ b/tests/testsuite/rustflags.rs @@ -55,7 +55,6 @@ fn env_rustflags_normal_source() { fn env_rustflags_build_script() { // RUSTFLAGS should be passed to rustc for build scripts // when --target is not specified. - // In this test if --cfg foo is passed the build will fail. let p = project() .file( "Cargo.toml", @@ -70,9 +69,7 @@ fn env_rustflags_build_script() { .file( "build.rs", r#" - fn main() { } - #[cfg(not(foo))] - fn main() { } + fn main() { assert!(cfg!(foo)); } "#, ) .build(); @@ -243,7 +240,6 @@ fn env_rustflags_normal_source_with_target() { fn env_rustflags_build_script_with_target() { // RUSTFLAGS should not be passed to rustc for build scripts // when --target is specified. - // In this test if --cfg foo is passed the build will fail. let p = project() .file( "Cargo.toml", @@ -258,9 +254,7 @@ fn env_rustflags_build_script_with_target() { .file( "build.rs", r#" - fn main() { } - #[cfg(foo)] - fn main() { } + fn main() { assert!(!cfg!(foo)); } "#, ) .build(); @@ -276,7 +270,6 @@ fn env_rustflags_build_script_with_target() { fn env_rustflags_build_script_with_target_doesnt_apply_to_host_kind() { // RUSTFLAGS should *not* be passed to rustc for build scripts when --target is specified as the // host triple even if target-applies-to-host-kind is enabled, to match legacy Cargo behavior. - // In this test if --cfg foo is passed the build will fail. let p = project() .file( "Cargo.toml", @@ -291,9 +284,7 @@ fn env_rustflags_build_script_with_target_doesnt_apply_to_host_kind() { .file( "build.rs", r#" - fn main() { } - #[cfg(foo)] - fn main() { } + fn main() { assert!(!cfg!(foo)); } "#, ) .file( @@ -519,7 +510,6 @@ fn build_rustflags_normal_source() { fn build_rustflags_build_script() { // RUSTFLAGS should be passed to rustc for build scripts // when --target is not specified. - // In this test if --cfg foo is passed the build will fail. let p = project() .file( "Cargo.toml", @@ -534,9 +524,7 @@ fn build_rustflags_build_script() { .file( "build.rs", r#" - fn main() { } - #[cfg(not(foo))] - fn main() { } + fn main() { assert!(cfg!(foo)); } "#, ) .file( @@ -737,7 +725,6 @@ fn build_rustflags_normal_source_with_target() { fn build_rustflags_build_script_with_target() { // RUSTFLAGS should not be passed to rustc for build scripts // when --target is specified. - // In this test if --cfg foo is passed the build will fail. let p = project() .file( "Cargo.toml", @@ -752,9 +739,7 @@ fn build_rustflags_build_script_with_target() { .file( "build.rs", r#" - fn main() { } - #[cfg(foo)] - fn main() { } + fn main() { assert!(!cfg!(foo)); } "#, ) .file( @@ -1041,9 +1026,7 @@ fn target_rustflags_also_for_build_scripts() { .file( "build.rs", r#" - fn main() { } - #[cfg(not(foo))] - fn main() { } + fn main() { assert!(cfg!(foo)); } "#, ) .file( @@ -1069,9 +1052,7 @@ fn target_rustflags_not_for_build_scripts_with_target() { .file( "build.rs", r#" - fn main() { } - #[cfg(foo)] - fn main() { } + fn main() { assert!(!cfg!(foo)); } "#, ) .file( @@ -1124,9 +1105,7 @@ fn build_rustflags_for_build_scripts() { .file( "build.rs", r#" - fn main() { } - #[cfg(foo)] - fn main() { } + fn main() { assert!(cfg!(foo)); } "#, ) .file( @@ -1139,25 +1118,26 @@ fn build_rustflags_for_build_scripts() { .build(); // With "legacy" behavior, build.rustflags should apply to build scripts without --target - p.cargo("build") - .with_status(101) - .with_stderr_contains("[..]previous definition of the value `main` here[..]") - .run(); + p.cargo("build").run(); // But should _not_ apply _with_ --target - p.cargo("build --target").arg(host).run(); + p.cargo("build --target") + .arg(host) + .with_status(101) + .with_stderr_contains("[..]assertion failed[..]") + .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_contains("[..]previous definition of the value `main` here[..]") .run(); p.cargo("build --target") .arg(host) .masquerade_as_nightly_cargo() .arg("-Ztarget-applies-to-host") + .with_status(101) + .with_stderr_contains("[..]assertion failed[..]") .run(); // When set to false though, the "proper" behavior where host artifacts _only_ pick up on @@ -1174,11 +1154,15 @@ fn build_rustflags_for_build_scripts() { p.cargo("build") .masquerade_as_nightly_cargo() .arg("-Ztarget-applies-to-host") + .with_status(101) + .with_stderr_contains("[..]assertion failed[..]") .run(); p.cargo("build --target") .arg(host) .masquerade_as_nightly_cargo() .arg("-Ztarget-applies-to-host") + .with_status(101) + .with_stderr_contains("[..]assertion failed[..]") .run(); }