Skip to content

Commit

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

When `target-applies-to-host` is explicitly set to `true`, rustflags
will start to be applied in the same way that `linker` and `runner` are
today -- namely they'll be applied from `[target.<host triple>]` and
from `RUSTFLAGS`/`build.rustflags` if `--target <host triple>` is given.
  • Loading branch information
Jon Gjengset committed Feb 18, 2022
1 parent fb47df7 commit 4f41727
Show file tree
Hide file tree
Showing 6 changed files with 310 additions and 104 deletions.
183 changes: 109 additions & 74 deletions src/cargo/core/compiler/build_context/target_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -573,17 +573,23 @@ fn output_err_info(cmd: &ProcessBuilder, stdout: &str, stderr: &str) -> String {
/// - the `CARGO_ENCODED_RUSTFLAGS` environment variable
/// - the `RUSTFLAGS` environment variable
///
/// then if this was not found
/// then if none of those were found
///
/// - `target.*.rustflags` from the config (.cargo/config)
/// - `target.cfg(..).rustflags` from the config
/// - `host.*.rustflags` from the config if compiling a host artifact or without `--target`
///
/// then if neither of these were found
/// then if none of those were found
///
/// - `build.rustflags` from the config
///
/// Note that if a `target` is specified, no args will be passed to host code (plugins, build
/// scripts, ...), even if it is the same as the target.
/// The behavior differs slightly when cross-compiling (or, specifically, when `--target` is
/// provided) for artifacts that are always built for the host (plugins, build scripts, ...).
/// For those artifacts, the behavior depends on `target-applies-to-host`. In the default
/// configuration (where `target-applies-to-host` is unset), and if `target-applies-to-host =
/// false`, host artifacts _only_ respect `host.*.rustflags`, and no other configuration sources.
/// If `target-applies-to-host = true`, host artifacts also respect `target.<host triple>`, and if
/// `<target triple> == <host triple>` `RUSTFLAGS` and `CARGO_ENCODED_RUSTFLAGS`.
fn env_args(
config: &Config,
requested_kinds: &[CompileKind],
Expand All @@ -592,49 +598,67 @@ fn env_args(
kind: CompileKind,
name: &str,
) -> CargoResult<Vec<String>> {
// We *want* to apply RUSTFLAGS only to builds for the
// requested target architecture, and not to things like build
// scripts and plugins, which may be for an entirely different
// architecture. Cargo's present architecture makes it quite
// hard to only apply flags to things that are not build
// scripts and plugins though, so we do something more hacky
// instead to avoid applying the same RUSTFLAGS to multiple targets
// arches:
let target_applies_to_host = config.target_applies_to_host()?;

// Include untargeted configuration sources (like `RUSTFLAGS`) if
//
// 1) If --target is not specified we just apply RUSTFLAGS to
// all builds; they are all going to have the same target.
// - we're compiling artifacts for the target platform; or
// - we're not cross-compiling; or
// - we're compiling host artifacts, the requested target matches the host, and the user has
// requested that the host pick up target configurations.
//
// 2) If --target *is* specified then we only apply RUSTFLAGS
// to compilation units with the Target kind, which indicates
// it was chosen by the --target flag.
// The rationale for the third condition is that `RUSTFLAGS` is intended to affect compilation
// for the target, and `target-applies-to-host` makes it so that the host is affected by its
// target's config, so if `--target <host triple>` then `RUSTFLAGS` should also apply to the
// host.
let include_generic = !kind.is_host()
|| requested_kinds == [CompileKind::Host]
|| (target_applies_to_host == Some(true)
&& requested_kinds == [CompileKind::Target(CompileTarget::new(host_triple)?)]);
// Include targeted configuration sources (like `target.*.rustflags`) if
//
// This means that, e.g., even if the specified --target is the
// same as the host, build scripts in plugins won't get
// RUSTFLAGS.
if requested_kinds != [CompileKind::Host] && kind.is_host() {
// This is probably a build script or plugin and we're
// compiling with --target. In this scenario there are
// no rustflags we can apply.
return Ok(Vec::new());
}

// First try CARGO_ENCODED_RUSTFLAGS from the environment.
// Prefer this over RUSTFLAGS since it's less prone to encoding errors.
if let Ok(a) = env::var(format!("CARGO_ENCODED_{}", name)) {
if a.is_empty() {
return Ok(Vec::new());
// - we're compiling artifacts for the target platform; or
// - we're not cross-compiling; or
// - we're compiling host artifacts and the user has requested that the host pick up target
// configurations.
//
// The middle condition here may seem counter-intuitive. If `target-applies-to-host-kind` is
// disabled, then `target.*.rustflags` should arguably not apply to host artifacts. However,
// this is likely surprising to users who set `target.<host triple>.rustflags` and run `cargo
// build` (with no `--target`). So, we respect `target.<host triple>`.rustflags` when
// `--target` _isn't_ supplied, which arguably matches the mental model established by
// respecting `RUSTFLAGS` when `--target` isn't supplied.
let include_for_target = !kind.is_host()
|| requested_kinds == [CompileKind::Host]
|| target_applies_to_host == Some(true);
// Include host-based configuration sources (like `host.*.rustflags`) if
//
// - we're compiling host artifacts; or
// - we're not cross-compiling
//
// Note that we do _not_ read `host.*.rustflags` just because the host's target is the same as
// the requested target, as that's the whole point of the `host` section in the first place.
let include_for_host = kind.is_host() || requested_kinds == [CompileKind::Host];

if include_generic {
// First try CARGO_ENCODED_RUSTFLAGS from the environment.
// Prefer this over RUSTFLAGS since it's less prone to encoding errors.
if let Ok(a) = env::var(format!("CARGO_ENCODED_{}", name)) {
if a.is_empty() {
return Ok(Vec::new());
}
return Ok(a.split('\x1f').map(str::to_string).collect());
}
return Ok(a.split('\x1f').map(str::to_string).collect());
}

// Then try RUSTFLAGS from the environment
if let Ok(a) = env::var(name) {
let args = a
.split(' ')
.map(str::trim)
.filter(|s| !s.is_empty())
.map(str::to_string);
return Ok(args.collect());
// Then try RUSTFLAGS from the environment
if let Ok(a) = env::var(name) {
let args = a
.split(' ')
.map(str::trim)
.filter(|s| !s.is_empty())
.map(str::to_string);
return Ok(args.collect());
}
}

let mut rustflags = Vec::new();
Expand All @@ -643,44 +667,55 @@ fn env_args(
.chars()
.flat_map(|c| c.to_lowercase())
.collect::<String>();
// Then the target.*.rustflags value...
let target = match &kind {
CompileKind::Host => host_triple,
CompileKind::Target(target) => target.short_name(),
};
let key = format!("target.{}.{}", target, name);
if let Some(args) = config.get::<Option<StringList>>(&key)? {
rustflags.extend(args.as_slice().iter().cloned());
if include_for_target {
// Then the target.*.rustflags value...
let target = match &kind {
CompileKind::Host => host_triple,
CompileKind::Target(target) => target.short_name(),
};
let key = format!("target.{}.{}", target, name);
if let Some(args) = config.get::<Option<StringList>>(&key)? {
rustflags.extend(args.as_slice().iter().cloned());
}
// ...including target.'cfg(...)'.rustflags
if let Some(target_cfg) = target_cfg {
config
.target_cfgs()?
.iter()
.filter_map(|(key, cfg)| {
cfg.rustflags
.as_ref()
.map(|rustflags| (key, &rustflags.val))
})
.filter(|(key, _rustflags)| CfgExpr::matches_key(key, target_cfg))
.for_each(|(_key, cfg_rustflags)| {
rustflags.extend(cfg_rustflags.as_slice().iter().cloned());
});
}
}
// ...including target.'cfg(...)'.rustflags
if let Some(target_cfg) = target_cfg {
config
.target_cfgs()?
.iter()
.filter_map(|(key, cfg)| {
cfg.rustflags
.as_ref()
.map(|rustflags| (key, &rustflags.val))
})
.filter(|(key, _rustflags)| CfgExpr::matches_key(key, target_cfg))
.for_each(|(_key, cfg_rustflags)| {
rustflags.extend(cfg_rustflags.as_slice().iter().cloned());
});

if include_for_host {
let target_cfg = config.host_cfg_triple(host_triple)?;
if let Some(rf) = target_cfg.rustflags {
rustflags.extend(rf.val.as_slice().iter().cloned());
}
}

if !rustflags.is_empty() {
return Ok(rustflags);
}

// Then the `build.rustflags` value.
let build = config.build_config()?;
let list = if name == "rustflags" {
&build.rustflags
} else {
&build.rustdocflags
};
if let Some(list) = list {
return Ok(list.as_slice().to_vec());
if include_generic {
// Then the `build.rustflags` value.
let build = config.build_config()?;
let list = if name == "rustflags" {
&build.rustflags
} else {
&build.rustdocflags
};
if let Some(list) = list {
return Ok(list.as_slice().to_vec());
}
}

Ok(Vec::new())
Expand Down Expand Up @@ -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)?
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/util/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1551,7 +1551,7 @@ impl Config {
}

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

Expand Down
29 changes: 19 additions & 10 deletions src/cargo/util/config/target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,20 +66,29 @@ pub(super) fn load_target_cfgs(config: &Config) -> CargoResult<Vec<(String, Targ
}

/// Returns true if the `[target]` table should be applied to host targets.
pub(super) fn get_target_applies_to_host(config: &Config) -> CargoResult<bool> {
pub(super) fn get_target_applies_to_host(config: &Config) -> CargoResult<Option<bool>> {
if config.cli_unstable().target_applies_to_host {
if let Ok(target_applies_to_host) = config.get::<bool>("target-applies-to-host") {
Ok(target_applies_to_host)
} else {
Ok(!config.cli_unstable().host_config)
if let Ok(target_applies_to_host) = config.get::<Option<bool>>("target-applies-to-host") {
if config.cli_unstable().host_config {
match target_applies_to_host {
Some(true) => {
anyhow::bail!(
"the -Zhost-config flag requires target-applies-to-host = false"
);
}
Some(false) => {}
None => {
// -Zhost-config automatically disables target-applies-to-host
return Ok(Some(false));
}
}
}
return Ok(target_applies_to_host);
}
} else if config.cli_unstable().host_config {
anyhow::bail!(
"the -Zhost-config flag requires the -Ztarget-applies-to-host flag to be set"
);
} else {
Ok(true)
anyhow::bail!("the -Zhost-config flag requires -Ztarget-applies-to-host");
}
Ok(None)
}

/// Loads a single `[host]` table for the given triple.
Expand Down
43 changes: 31 additions & 12 deletions src/doc/src/reference/unstable.md
Original file line number Diff line number Diff line change
Expand Up @@ -508,14 +508,30 @@ CLI paths are relative to the current working directory.
* Original Pull Request: [#9322](https://github.com/rust-lang/cargo/pull/9322)
* Tracking Issue: [#9453](https://github.com/rust-lang/cargo/issues/9453)

The `target-applies-to-host` key in a config file can be used set the desired
behavior for passing target config flags to build scripts.

It requires the `-Ztarget-applies-to-host` command-line option.

The current default for `target-applies-to-host` is `true`, which will be
changed to `false` in the future, if `-Zhost-config` is used the new `false`
default will be set for `target-applies-to-host`.
Historically, Cargo has respected the `linker` and `runner` options (but
_not_ `rustflags`) of `[target.<host triple>]` configuration sections
when building and running build scripts, plugins, and other artifacts
that are _always_ built for the host platform.
`-Ztarget-applies-to-host` enables the top-level
`target-applies-to-host` setting in Cargo configuration files which
allows users to opt into different (and more consistent) behavior for
these properties.

When `target-applies-to-host` is unset in the configuration file, the
existing Cargo behavior is preserved (though see `-Zhost-config`, which
changes that default). When it is set to `true`, _all_ options from
`[target.<host triple>]` are respected for host artifacts. When it is
set to `false`, _no_ options from `[target.<host triple>]` are respected
for host artifacts.

In the specific case of `rustflags`, this settings also affects whether
`--target <host-triple>` picks up the same configuration as if
`--target` was not supplied in the first place.

In the future, `target-applies-to-host` may end up defaulting to `false`
to provide more sane and consistent default behavior. When set to
`false`, `host-config` can be used to customize the behavior for host
artifacts.

```toml
# config.toml
Expand All @@ -535,15 +551,17 @@ such as build scripts that must run on the host system instead of the target
system when cross compiling. It supports both generic and host arch specific
tables. Matching host arch tables take precedence over generic host tables.

It requires the `-Zhost-config` and `-Ztarget-applies-to-host` command-line
options to be set.
It requires the `-Zhost-config` and `-Ztarget-applies-to-host`
command-line options to be set, and that `target-applies-to-host =
false` is set in the Cargo configuration file.

```toml
# config.toml
[host]
linker = "/path/to/host/linker"
[host.x86_64-unknown-linux-gnu]
linker = "/path/to/host/arch/linker"
rustflags = ["-Clink-arg=--verbose"]
[target.x86_64-unknown-linux-gnu]
linker = "/path/to/target/linker"
```
Expand All @@ -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
Expand Down
9 changes: 4 additions & 5 deletions tests/testsuite/build_script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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.{}]
Expand All @@ -496,16 +495,16 @@ fn custom_build_env_var_rustc_linker_host_target_with_bad_host_config() {
.file("src/lib.rs", "")
.build();

// build.rs should fail due to bad target linker being set
// build.rs should fail due to bad host linker being set
p.cargo("build -Z target-applies-to-host -Z host-config --verbose --target")
.arg(&target)
.masquerade_as_nightly_cargo()
.with_status(101)
.with_stderr_contains(
"\
[COMPILING] foo v0.0.1 ([CWD])
[RUNNING] `rustc --crate-name build_script_build build.rs [..]--crate-type bin [..]-C linker=[..]/path/to/target/linker [..]`
[ERROR] linker `[..]/path/to/target/linker` not found
[RUNNING] `rustc --crate-name build_script_build build.rs [..]--crate-type bin [..]-C linker=[..]/path/to/host/linker [..]`
[ERROR] linker `[..]/path/to/host/linker` not found
"
)
.run();
Expand Down
Loading

0 comments on commit 4f41727

Please sign in to comment.