cargo icon indicating copy to clipboard operation
cargo copied to clipboard

Add target-specific RUSTFLAGS variants

Open jonhoo opened this issue 2 years ago • 6 comments

What does this PR try to resolve?

This is a continuation from the discussion in https://github.com/rust-lang/cargo/pull/10395#issuecomment-1055691480, and is aimed at being able to solve problems like https://github.com/rust-lang/cargo/issues/4423 by extending the current RUSTFLAGS environment variable to also have per-target variants as in https://github.com/alexcrichton/cc-rs/pull/9.

To quote the unstable docs I wrote for the feature:

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
  2. <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 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.

How should we test and review this PR?

The PR includes two new tests that check for the expected behavior of these flags, and how they behave when more than one is provided.

jonhoo avatar Mar 05 '22 00:03 jonhoo

r? @alexcrichton

(rust-highfive has picked a reviewer for you, use r? to override)

rust-highfive avatar Mar 05 '22 00:03 rust-highfive

I did, as part of this, discover a problem with target-applies-to-host and rustflags as currently implemented (even without this PR). Currently, kind.is_host() returns true for all artifacts when --target is not specified. This means that the function that tries to determine the appropriate rust flags (env_args) has no way to discern host artifacts from target artifacts in that setting. Which in turn means that it cannot correctly implement target-applies-to-host = false when --target isn't specified. The code in Cargo today ends up not taking the fall-through path here for any compilation artifacts

https://github.com/rust-lang/cargo/blob/0a3f2b4a3093cbdf1cd3c00a90cbea57cbba0196/src/cargo/core/compiler/build_context/target_info.rs#L616-L619

and instead takes the path intended for host artifacts here

https://github.com/rust-lang/cargo/blob/0a3f2b4a3093cbdf1cd3c00a90cbea57cbba0196/src/cargo/core/compiler/build_context/target_info.rs#L621-L625

The net result of this is that if --target isn't specified, and target-applies-to-host = false, neither RUSTFLAGS, target.*.rustflags, nor build.rustflags is used for any artifact, and [host] (if set) applies to all artifacts.

This limitation continues to apply with this PR — if --target is not passed and target-applies-to-host = false, RUSTFLAGS_$TARGET and TARGET_RUSTFLAGS do not apply at all, and HOST_RUSTFLAGS applies to all artifacts (relevant code).

To fix this, I think we need a way to tell env_args whether the compilation unit under consideration is a host artifact like a plugin or build script separately from just kind.is_host(). Unfortunately, I'm not familiar enough with that part of the code-base to figure out how feasible that is. I suspect this is what this now-removed comment was alluding to:

https://github.com/rust-lang/cargo/blob/fb47df72e16b674736ac74a3b57ca7389fe6ab99/src/cargo/core/compiler/build_context/target_info.rs#L595-L602

jonhoo avatar Mar 05 '22 00:03 jonhoo

/cc @messense given https://github.com/rust-lang/cargo/issues/4423#issuecomment-1058794825

jonhoo avatar Mar 05 '22 00:03 jonhoo

Thanks for the PR, but I'm going to be stepping down from the Cargo team so I'm going to un-assign myself from this. The Cargo team will help review this when they get a chance.

alexcrichton avatar Mar 08 '22 17:03 alexcrichton

:umbrella: The latest upstream changes (presumably #10486) made this pull request unmergeable. Please resolve the merge conflicts.

bors avatar Apr 12 '22 15:04 bors

:umbrella: The latest upstream changes (presumably #10566) made this pull request unmergeable. Please resolve the merge conflicts.

bors avatar May 06 '22 23:05 bors