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

Remove most of the logic in fake rustc once -Ztarget-applies-to-host=true lands #94556

Open
jyn514 opened this issue Mar 3, 2022 · 3 comments
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@jyn514
Copy link
Member

jyn514 commented Mar 3, 2022

rust-lang/cargo#10395 made it possible to apply RUSTFLAGS to build scripts and proc-macros. Bootstrap currently special-cases this in fake rustc:

// FIXME(rust-lang/cargo#5754) we shouldn't be using special env vars
// here, but rather Cargo should know what flags to pass rustc itself.
// Override linker if necessary.
if let Ok(host_linker) = env::var("RUSTC_HOST_LINKER") {
cmd.arg(format!("-Clinker={}", host_linker));
}
if env::var_os("RUSTC_HOST_FUSE_LD_LLD").is_some() {
cmd.arg("-Clink-args=-fuse-ld=lld");
}
if let Ok(s) = env::var("RUSTC_HOST_CRT_STATIC") {
if s == "true" {
cmd.arg("-C").arg("target-feature=+crt-static");
}
if s == "false" {
cmd.arg("-C").arg("target-feature=-crt-static");
}
}
if stage == "0" {
// Cargo doesn't pass RUSTFLAGS to proc_macros:
// https://github.com/rust-lang/cargo/issues/4423
// Set `--cfg=bootstrap` explicitly instead.
cmd.arg("--cfg=bootstrap");
}

We should switch to using -Ztarget-applies-to-host=true on the next cargo update.
cc @Mark-Simulacrum @ehuss @jonhoo (thank you again @jonhoo for fixing this! ❤️)

@rustbot label: +A-rustbuild +C-enhancement

@rustbot rustbot added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Mar 3, 2022
@jonhoo
Copy link
Contributor

jonhoo commented Mar 3, 2022

Ah, so, unfortunately, -Ztarget-applies-to-host=true doesn't quite do what it says on the box. I discuss that issue in rust-lang/cargo#10395 (comment). Basically, you'll want to move towards =false, not =true, since =false is the "more correct and consistent" behavior. And then you'd want something like -Zhost-config on top of that. Alternatively my proposal in rust-lang/cargo#10395 (comment) was to adopt something along the lines of rust-lang/cc-rs#9 for RUSTFLAGS, which I think could work pretty well and would also address this use-case.

Also cc @alexcrichton since this may be a useful real-world example of where the current behavior falls short.

@jyn514
Copy link
Member Author

jyn514 commented Mar 6, 2022

@jonhoo is the difference between the three modes documented somewhere? I don't have any clear idea what they do.

@jonhoo
Copy link
Contributor

jonhoo commented Mar 7, 2022

The unstable docs should explain exactly what the feature does (and doesn't) do. The challenge is that =true matches Cargo's current behavior exactly, and =false makes rustflags not apply more often, with the expectation that users will be using [host] instead.

See rust-lang/cargo#10462 for the new envvar proposal though

@jyn514 jyn514 added C-cleanup Category: PRs that clean code up or issues documenting cleanup. and removed C-enhancement Category: An issue proposing an enhancement or a PR with one. labels May 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. 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

3 participants