Flags from *FLAGS* env vars now have precedence over bootstrap own flags and not the other way around. #148911
Conversation
rustbot
added
S-waiting-on-review
karolzwolak
force-pushed
the
bootstrap-rustflags-precedence
branch
2 times, most recently
from
ce939ab to
456ce9a
Compare
This comment has been minimized.
This comment has been minimized.
karolzwolak
force-pushed
the
bootstrap-rustflags-precedence
branch
from
456ce9a to
e0750a1
Compare
| // #71458. | ||
| let mut rustdocflags = rustflags.clone(); | ||
| rustdocflags.propagate_rustflag_envs(build_compiler_stage); | ||
| rustdocflags.propagate_cargo_env("RUSTDOCFLAGS"); |
There was a problem hiding this comment.
To uphold the rule that RUSTDOCFLAGS have preference, the env. vars. for rustdocflags should be only propagated after bootstrap sets its flags, right? Near line 1360.
Actually, now that I think about it, the changes from this PR are not enough. Because after being created, the Cargo struct has a rustflag method, which is used in other places in bootstrap to configure additional flags.
So if we really want the external env. var. to override everything, we should only do the actual propagation in impl From<Cargo> for BootstrapCommand, where the rustflags are "materialized".
There was a problem hiding this comment.
Yeah you're right on both counts. Kinda sloppy on my side. Will fix that.
There was a problem hiding this comment.
Please also add a comment at the place the env. vars. will be propagated to explain why we do it in this way. Thank you!
There was a problem hiding this comment.
Okay I think it should work now. I also added that comment and slightly changed the name of the PR.
I tested it and seems to be working.
I've run RUSTFLAGS="-Clinker=clang -Clink-arg=--ld-path=wild --verbose" x build and collected the RUSTFLAGS that are send to cargo. They now are at the end as expected.
diff --git a/before b/after
index 65643d6..8dfc789 100644
--- a/before
+++ b/after
@@ -1,8 +1,4 @@
RUSTFLAGS=
--Clinker=clang
--Clink-arg=--ld-path=wild
---verbose
---cfg=bootstrap
--cfg=windows_raw_dylib
-Csymbol-mangling-version=v0
-Zunstable-options
@@ -21,3 +17,7 @@ RUSTFLAGS=
-Zdefault-visibility=protected
-Clto=off
-Clink-args=-Wl,--icf=all
+-Clinker=clang
+-Clink-arg=--ld-path=wild
+--verbose
+--cfg=bootstrapEDIT: I've got the filenames mixed up in the diff (I used a for after and b for before 🤦)
…flags and not the other way around.
karolzwolak
force-pushed
the
bootstrap-rustflags-precedence
branch
from
e0750a1 to
a0015a3
Compare
karolzwolak
changed the title
RUSTFLAGS now have precedence over bootstrap own flags and not the other way around.4 participants
Before this PR extra flags from env variables like
RUSTFLAGShad lower precedence than bootstrap flags. This PR changes that, and tus makes overriding flags passed to the compiler easier and more reliable.This is technically a breaking change — but it only affects people using these env variables.
This change was discussed on zulip by members of bootstrap team.
r? @Kobzol