Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add target-specific RUSTFLAGS variants #10462

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
145 changes: 126 additions & 19 deletions src/cargo/core/compiler/build_context/target_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -606,22 +606,35 @@ fn env_args(
kind: CompileKind,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we also update doc comment for fn env_args?

flags: Flags,
) -> CargoResult<Vec<String>> {
let targeted_rustflags = config.cli_unstable().targeted_rustflags;
let target_applies_to_host = config.target_applies_to_host()?;

// 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.
// set elsewhere when `--target` isn't passed. Or, phrased differently, with
// `target-applies-to-host = false`, no `--target` behaves the same as `--target <host>`, and
// host artifacts are always "special" (they don't pick up `target.*` or `RUSTFLAGS` for
// example).
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(v) = rustflags_from_env(
targeted_rustflags,
host_triple,
kind,
false,
target_applies_to_host,
flags,
) {
// An environment variable that specifically applies to host artifacts was given.
// Use that!
return Ok(v);
} else {
// In all other cases, host artifacts just get flags from [host], regardless of
// --target. Or, phrased differently, no `--target` behaves the same as `--target
// <host>`, and host artifacts are always "special" (they don't pick up `RUSTFLAGS` for
// example).
// Otherwise, check [host], which is the only other place that can specify rustflags
// that will apply to host artifacts.
return Ok(rustflags_from_host(config, flags, host_triple)?.unwrap_or_else(Vec::new));
}
}
Expand All @@ -630,7 +643,14 @@ 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(flags) {
if let Some(rustflags) = rustflags_from_env(
targeted_rustflags,
host_triple,
kind,
true,
target_applies_to_host,
flags,
) {
Ok(rustflags)
} else if let Some(rustflags) =
rustflags_from_target(config, host_triple, target_cfg, kind, flags)?
Expand All @@ -643,28 +663,115 @@ fn env_args(
}
}

fn rustflags_from_env(flags: Flags) -> Option<Vec<String>> {
// First try CARGO_ENCODED_RUSTFLAGS from the environment.
// Prefer this over RUSTFLAGS since it's less prone to encoding errors.
if let Ok(a) = env::var(format!("CARGO_ENCODED_{}", flags.as_env())) {
if a.is_empty() {
return Some(Vec::new());
#[derive(Debug, PartialEq, Eq, PartialOrd, Ord)]
enum Specificity {
Generic = 0,
Kind = 1,
Target = 2,
Comment on lines +668 to +670
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why setting discriminant manually, supposed rustc can derive Ord with default discriminants?

}

fn get_var_variants(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These new functions and enum are not immediately clear to me. Should we add some docs for them?

targeted_rustflags: bool,
var_base: &str,
host_triple: &str,
kind: CompileKind,
allow_generic: bool,
target_applies_to_host: bool,
) -> Option<(Specificity, String)> {
if !targeted_rustflags {
return if allow_generic {
env::var(var_base).ok().map(|v| (Specificity::Generic, v))
} else {
None
};
}

let compiling_for = match &kind {
CompileKind::Host => host_triple,
CompileKind::Target(target) => target.short_name(),
};
let target_in_env: String = compiling_for
.chars()
.flat_map(|c| c.to_uppercase())
.map(|c| if c == '-' { '_' } else { c })
.collect();
if !kind.is_host() || target_applies_to_host {
if let Ok(v) = env::var(format!("{}_{}", var_base, target_in_env)) {
return Some((Specificity::Target, v));
}
return Some(a.split('\x1f').map(str::to_string).collect());
}
let kind = if kind.is_host() { "HOST" } else { "TARGET" };
if let Ok(v) = env::var(format!("{}_{}", kind, var_base)) {
return Some((Specificity::Kind, v));
}
// NOTE: TARGET_RUSTFLAGS never applies to host artifacts, even when
// target-applies-to-host = true, and even if --target isn't passed.
if allow_generic {
env::var(var_base).ok().map(|v| (Specificity::Generic, v))
} else {
None
}
Comment on lines +709 to +713
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(nit) I believe here we can use allow_generic.then(…) 🎉

}

// Then try RUSTFLAGS from the environment
if let Ok(a) = env::var(flags.as_env()) {
fn rustflags_from_env(
targeted_rustflags: bool,
host_triple: &str,
kind: CompileKind,
allow_generic: bool,
target_applies_to_host: bool,
flag: Flags,
) -> Option<Vec<String>> {
let encoded = get_var_variants(
targeted_rustflags,
&format!("CARGO_ENCODED_{}", flag.as_env()),
host_triple,
kind,
allow_generic,
target_applies_to_host,
)
.map(|(s, a)| {
if a.is_empty() {
(s, Vec::new())
} else {
(s, a.split('\x1f').map(str::to_string).collect())
}
Comment on lines +733 to +737
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If str::split is a no-op for empty string, could we reduce these to one line?

});
let spacesep = get_var_variants(
targeted_rustflags,
flag.as_env(),
host_triple,
kind,
allow_generic,
target_applies_to_host,
)
.map(|(s, a)| {
let args = a
.split(' ')
.map(str::trim)
.filter(|s| !s.is_empty())
.map(str::to_string);
return Some(args.collect());
}
(s, args.collect())
});

// No rustflags to be collected from the environment
None
match (encoded, spacesep) {
(Some((_, v)), None) | (None, Some((_, v))) => {
// If just one is set, use that.
Some(v)
}
(None, None) => {
// If neither is set, well, there's nothing to do here.
None
}
(Some((enc_s, enc_v)), Some((spc_s, spc_v))) => {
// If _both_ are set, prefer the more specific one.
// Ties go to CARGO_ENCODED_RUSTFLAGS since it's less prone to encoding errors.
if enc_s >= spc_s {
Some(enc_v)
} else {
Some(spc_v)
}
}
}
}

fn rustflags_from_target(
Expand Down
2 changes: 2 additions & 0 deletions src/cargo/core/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,7 @@ unstable_cli_options!(
host_config: bool = ("Enable the [host] section in the .cargo/config.toml file"),
http_registry: bool = ("Support HTTP-based crate registries"),
target_applies_to_host: bool = ("Enable the `target-applies-to-host` key in the .cargo/config.toml file"),
targeted_rustflags: bool = ("Make Cargo respect target-specific variants for RUSTFLAGS and CARGO_ENCODED_RUSTFLAGS"),
rustdoc_map: bool = ("Allow passing external documentation mappings to rustdoc"),
separate_nightlies: bool = (HIDDEN),
terminal_width: Option<Option<usize>> = ("Provide a terminal width to rustc for error truncation"),
Expand Down Expand Up @@ -861,6 +862,7 @@ impl CliUnstable {
"jobserver-per-rustc" => self.jobserver_per_rustc = parse_empty(k, v)?,
"host-config" => self.host_config = parse_empty(k, v)?,
"target-applies-to-host" => self.target_applies_to_host = parse_empty(k, v)?,
"targeted-rustflags" => self.targeted_rustflags = parse_empty(k, v)?,
"features" => {
// For now this is still allowed (there are still some
// unstable options like "compare"). This should be removed at
Expand Down
27 changes: 27 additions & 0 deletions src/doc/src/reference/unstable.md
Original file line number Diff line number Diff line change
Expand Up @@ -1202,6 +1202,33 @@ For instance:
cargo check -Z unstable-options -Z check-cfg-features
```

### targeted-rustflags

The `-Z targeted-rustflags` argument tells Cargo to also look for
target-specific variants of the `RUSTFLAGS` and
`CARGO_ENCODED_RUSTFLAGS` variables (and the same for `RUSTDOCFLAGS`).
The following variants are supported:

1. `<var>_<target>` - for example, `RUSTFLAGS_X86_64_UNKNOWN_LINUX_GNU`
1. `<build-kind>_<var>` - for example, `HOST_RUSTFLAGS` or `TARGET_RUSTFLAGS`

`HOST_RUSTFLAGS` allows setting flags to be passed to rustc for
host-artifacts (like build scripts) when cross-compiling, or _all_
Comment on lines +1215 to +1216
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this interact with artifact dependencies (RFC 3028)?
If an artifact dependency specified with a { …, target = "<triple>" }, even without --target specified, Cargo will still try to build that dependency on that target platform.

  • Should it receive HOST_RUSTFLAGS or TARGET_RUSTFLAGS?
  • If it is a build-deps, which one it should get?

artifacts when not cross-compiling. See also the `host-config` feature.
Note that Cargo considers _any_ `--target` as cross-compiling, even if
the specified target is equal to the host's target triple.
`TARGET_RUSTFLAGS` specifically applies to all artifacts being built for
the target triple specified with `--target`.

This feature interacts with the `target-applies-to-host` setting. When
set to `false`, `<var>_<host target>` will _not_ apply to artifacts
built for the host, like build scripts.

More specific flags always take precedence over less specific ones, with
ties broken in favor of `CARGO_ENCODED_RUSTFLAGS`. So, for example, if
`RUSTFLAGS_$TARGET` and `TARGET_CARGO_ENCODED_RUSTFLAGS` are both
specified, `RUSTFLAGS_$TARGET` would be used.
Comment on lines +1227 to +1230
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to make an example of this precedence chain? Such as

The precedence for which value is used is done in the following order (first
match wins):
1. `[profile.dev.package.name]` — A named package.
2. `[profile.dev.package."*"]` — For any non-workspace member.
3. `[profile.dev.build-override]` — Only for build scripts, proc macros, and
their dependencies.
4. `[profile.dev]` — Settings in `Cargo.toml`.
5. Default values built-in to Cargo.


### check-cfg-well-known-names

* RFC: [#3013](https://github.com/rust-lang/rfcs/pull/3013)
Expand Down
Loading