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
Fix rustc-args passing issue in bootstrap #113948
Conversation
@bors r+ rollup |
…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#112508 (Tweak spans for self arg, fix borrow suggestion for signature mismatch) - rust-lang#113901 (Get rid of subst-relate incompleteness in new solver) - rust-lang#113948 (Fix rustc-args passing issue in bootstrap) - rust-lang#113950 (Remove Scope::Elision from bound-vars resolution.) - rust-lang#113957 (Add regression test for issue rust-lang#113941 - naive layout isn't refined) - rust-lang#113959 (Migrate GUI colors test to original CSS color format) r? `@ghost` `@rustbot` modify labels: rollup
self.config.cmd.rustc_args().iter().map(|s| s.to_string()).collect::<Vec<_>>(); | ||
if !rustc_args.is_empty() { | ||
cargo.env("RUSTFLAGS", &rustc_args.join(" ")); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand this patch, it doesn't make sense to me. This is setting the RUSTFLAGS env var, but when this Cargo
instance is later turned into a Command
, the RUSTFLAGS env var is anyway overwritten again:
rust/src/bootstrap/src/core/builder.rs
Lines 2414 to 2418 in c350d3c
fn from(mut cargo: Cargo) -> Command { | |
let rustflags = &cargo.rustflags.0; | |
if !rustflags.is_empty() { | |
cargo.command.env("RUSTFLAGS", rustflags); | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be added to rustflags
instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also the --help text says this should only be passed when running tests; I don't see that reflected in the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this isn't correct and fixed in #123489.
Fixes #113178, r? @jyn514