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

bootstrap: CARGO_TARGET_$TARGET_RUSTFLAGS is appended to RUSTFLAGS #94874

Open
jonhoo opened this issue Mar 12, 2022 · 6 comments
Open

bootstrap: CARGO_TARGET_$TARGET_RUSTFLAGS is appended to RUSTFLAGS #94874

jonhoo opened this issue Mar 12, 2022 · 6 comments
Assignees
Labels
C-bug Category: This is a bug. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@jonhoo
Copy link
Contributor

jonhoo commented Mar 12, 2022

In the bootstrap build, we respect CARGO_TARGET_$TARGET_RUSTFLAGS so that users can set target-specific rustflags if they wish. Unfortunately, the semantics of CARGO_TARGET_$TARGET_RUSTFLAGS in bootstrap do not match those of Cargo. In Cargo, CARGO_TARGET_$TARGET_RUSTFLAGS are equivalent to setting target.$target.rustflags, and it is overridden by the RUSTFLAGS environment variable. Whereas what bootstrap currently does is concatenate them:

rust/src/bootstrap/builder.rs

Lines 1802 to 1807 in 2c6a29a

// Inherit `RUSTFLAGS` by default ...
self.env(prefix);
// ... and also handle target-specific env RUSTFLAGS if they're configured.
let target_specific = format!("CARGO_TARGET_{}_{}", crate::envify(&self.1.triple), prefix);
self.env(&target_specific);

Now, bootstrap is in a slightly weird position where it always sets RUSTFLAGS, so the "usual" rules don't really make sense. Instead, the intention is probably to allow users to set a "general" set of flags for rustc (through RUSTFLAGS) more akin to build.rustflags and then override them for specific targets if desired. But unfortunately that doesn't match today's semantics either since the two environment variables are simply merged.

The big downside of having them always merge is that it makes it much more difficult to deal with targets that need fewer flags than the "general" set. For example, I have to set -Clink-arg=--gcc-toolchain=... in RUSTFLAGS. But when trying to build for Android targets that particular GCC toolchain won't work. So, I need to "reset" rustflags to not include the general set from RUSTFLAGS for those targets. But today, I have no way of doing that.

Now, in theory, I could just not set RUSTFLAGS and all would be well. But unfortunately that won't work either because RUSTFLAGS is the only environment variable that can be set to pass flags to rustc for building rustbuild itself (see, for example, #94234). And since in my environment --gcc-toolchain= is a required argument to the linker, I'm stuck between a rock and a hard place — either I set RUSTFLAGS and my Android builds fail, or I don't set it and the rustbuild build fails.

Meta

rustc --version --verbose:

rustc 1.59.0 (9d1b2106e 2022-02-23)
binary: rustc
commit-hash: 9d1b2106e23b1abd32fce1f17267604a5102f57a
commit-date: 2022-02-23
host: x86_64-unknown-linux-gnu
release: 1.59.0
LLVM version: 13.0.0
@jonhoo jonhoo added the C-bug Category: This is a bug. label Mar 12, 2022
@jyn514
Copy link
Member

jyn514 commented Mar 13, 2022

RUSTFLAGS is the only environment variable that can be set to pass flags to rustc for building rustbuild itself

This whole problem could be avoided once #94829 is implemented.

@jyn514
Copy link
Member

jyn514 commented Mar 13, 2022

Or alternatively, it might make sense to add a RUSTFLAGS_BOOTSTRAP_ITSELF flag or similar.

@jyn514 jyn514 added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Apr 27, 2023
@onur-ozkan onur-ozkan self-assigned this Sep 27, 2023
@intelfx
Copy link
Contributor

intelfx commented Mar 10, 2024

@jonhoo, would this issue be solved by teaching bootstrap.py to append $CARGO_TARGET_<host-triple>_RUSTFLAGS to rustflags for the "bootstrap itself" invocation? Then you could maybe set -C link-arg=--gcc-toolchain=... for all targets that need it (including presumably the host), and not set $RUSTFLAGS at all?

@jonhoo
Copy link
Contributor Author

jonhoo commented Mar 10, 2024

@intelfx That feels like it probably just adds another corner case users need to be aware of 😅

@intelfx
Copy link
Contributor

intelfx commented Mar 10, 2024

@intelfx That feels like it probably just adds another corner case users need to be aware of 😅

The $RUSTFLAGS_BOOTSTRAP_ITSELF idea is about the same level of corner-casery, though :-) This way you only have one rule: when bootstrapping, rustflags from different sources are combined rather than override each other.

What I wanted to know is — is this idea semantically correct? i. e. if you just make the bootstrap-the-bootstrap invocation use the same flags as the bootstrap itself for the host triple, will it always work, or are there plausible cases where bootstrap-the-bootstrap would need different flags than bootstrap-the-compiler for target == host?

@jonhoo
Copy link
Contributor Author

jonhoo commented Mar 30, 2024

It's a good question. I think that would be fine, but it's also been a while since I was very deeply steeped in this, so take it with a grain of salt.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

No branches or pull requests

4 participants