| Message ID | 20241008-rustc-option-bootstrap-v1-1-9eb06261d4f7@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Kbuild: add RUSTC_BOOTSTRAP to rustc-option | expand |
On Tue, Oct 8, 2024 at 3:49 PM Alice Ryhl <aliceryhl@google.com> wrote: > > When using unstable features with the Rust compiler, you must either use > a nightly compiler release or set the RUSTC_BOOTSTRAP environment > variable. Otherwise, the compiler will emit a compiler error. This > environment variable is missing when rustc-option is executed, so add > the environment variable. > > This change is necessary to avoid two kinds of problems: > > 1. When using rustc-option to test whether a -Z flag is available, the > check will always fail, even if the flag is available. > 2. If KBUILD_RUSTFLAGS happens to contain any -Z flags from the > environment, even if unrelated to the flag being tested, then all > invocations of rustc-option everywhere will fail. > > I was not actually able to trigger the second kind of problem with the > makefiles that exist today, but it seems like it could easily start > being a problem due to complicated interactions between changes. It is > better to fix this now so it doesn't surprise us later. > > I added the flag under try-run as this seemed like the easiest way to > make sure that the fix applies to all variations of rustc-option, > including new ones added in the future. > > Signed-off-by: Alice Ryhl <aliceryhl@google.com> It should be "export". Also, this doesn't seem to be sufficient at all. Now I'm running into this error: make[1]: Entering directory '/home/aliceryhl/lout' warning: ignoring --out-dir flag due to -o flag error[E0463]: can't find crate for `std` | = note: the `aarch64-unknown-none` target may not be installed = help: consider downloading the target with `rustup target add aarch64-unknown-none` = help: consider building the standard library from source with `cargo build -Zbuild-std` error: aborting due to 1 previous error; 1 warning emitted For more information about this error, try `rustc --explain E0463`. I think this patch is going to need more work. Sorry for sending it prematurely. Alice
On Tue, Oct 8, 2024 at 3:49 PM Alice Ryhl <aliceryhl@google.com> wrote: > > When using unstable features with the Rust compiler, you must either use > a nightly compiler release or set the RUSTC_BOOTSTRAP environment > variable. Otherwise, the compiler will emit a compiler error. This > environment variable is missing when rustc-option is executed, so add > the environment variable. Yeah, `$(shell ...` does not pass the environment, so we need it. > This change is necessary to avoid two kinds of problems: > > 1. When using rustc-option to test whether a -Z flag is available, the > check will always fail, even if the flag is available. > 2. If KBUILD_RUSTFLAGS happens to contain any -Z flags from the > environment, even if unrelated to the flag being tested, then all > invocations of rustc-option everywhere will fail. > > I was not actually able to trigger the second kind of problem with the > makefiles that exist today, but it seems like it could easily start > being a problem due to complicated interactions between changes. It is > better to fix this now so it doesn't surprise us later. > > I added the flag under try-run as this seemed like the easiest way to > make sure that the fix applies to all variations of rustc-option, > including new ones added in the future. > > Signed-off-by: Alice Ryhl <aliceryhl@google.com> I think we need an `export` there. I am also rechecking this, and I didn't get a reply from: https://lore.kernel.org/rust-for-linux/CANiq72mv5E0PvZRW5eAEvqvqj74PH01hcRhLWTouB4z32jTeSA@mail.gmail.com/ And I forgot about it, which is my mistake too -- I still think we need it (and we should not use `-o` either together with it, I think). I can take a look... Cheers, Miguel
On Tue, Oct 8, 2024 at 11:25 PM Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > > On Tue, Oct 8, 2024 at 3:49 PM Alice Ryhl <aliceryhl@google.com> wrote: > > > > When using unstable features with the Rust compiler, you must either use > > a nightly compiler release or set the RUSTC_BOOTSTRAP environment > > variable. Otherwise, the compiler will emit a compiler error. This > > environment variable is missing when rustc-option is executed, so add > > the environment variable. > > Yeah, `$(shell ...` does not pass the environment, so we need it. Really? $(shell ...) inherits env variables in my understanding. > > This change is necessary to avoid two kinds of problems: > > > > 1. When using rustc-option to test whether a -Z flag is available, the > > check will always fail, even if the flag is available. > > 2. If KBUILD_RUSTFLAGS happens to contain any -Z flags from the > > environment, even if unrelated to the flag being tested, then all > > invocations of rustc-option everywhere will fail. > > > > I was not actually able to trigger the second kind of problem with the > > makefiles that exist today, but it seems like it could easily start > > being a problem due to complicated interactions between changes. It is > > better to fix this now so it doesn't surprise us later. > > > > I added the flag under try-run as this seemed like the easiest way to > > make sure that the fix applies to all variations of rustc-option, > > including new ones added in the future. > > > > Signed-off-by: Alice Ryhl <aliceryhl@google.com> > > I think we need an `export` there. > > I am also rechecking this, and I didn't get a reply from: > > https://lore.kernel.org/rust-for-linux/CANiq72mv5E0PvZRW5eAEvqvqj74PH01hcRhLWTouB4z32jTeSA@mail.gmail.com/ > > And I forgot about it, which is my mistake too -- I still think we > need it (and we should not use `-o` either together with it, I think). > > I can take a look... No. You need to understand who expands it. TMPOUT=$(TMPOUT); was needed because you used -out-dir="$$TMPOUT" It should be -out-dir=$(TMPOUT) Such pointless code is unneeded. -- Best Regards Masahiro Yamada
On 10/8/24 4:24 PM, Alice Ryhl wrote: > On Tue, Oct 8, 2024 at 3:49 PM Alice Ryhl <aliceryhl@google.com> wrote: >> >> When using unstable features with the Rust compiler, you must either use >> a nightly compiler release or set the RUSTC_BOOTSTRAP environment >> variable. Otherwise, the compiler will emit a compiler error. This >> environment variable is missing when rustc-option is executed, so add >> the environment variable. >> >> This change is necessary to avoid two kinds of problems: >> >> 1. When using rustc-option to test whether a -Z flag is available, the >> check will always fail, even if the flag is available. >> 2. If KBUILD_RUSTFLAGS happens to contain any -Z flags from the >> environment, even if unrelated to the flag being tested, then all >> invocations of rustc-option everywhere will fail. >> >> I was not actually able to trigger the second kind of problem with the >> makefiles that exist today, but it seems like it could easily start >> being a problem due to complicated interactions between changes. It is >> better to fix this now so it doesn't surprise us later. >> >> I added the flag under try-run as this seemed like the easiest way to >> make sure that the fix applies to all variations of rustc-option, >> including new ones added in the future. >> >> Signed-off-by: Alice Ryhl <aliceryhl@google.com> > > It should be "export". Also, this doesn't seem to be sufficient at > all. Now I'm running into this error: > > make[1]: Entering directory '/home/aliceryhl/lout' > warning: ignoring --out-dir flag due to -o flag > > error[E0463]: can't find crate for `std` > | > = note: the `aarch64-unknown-none` target may not be installed > = help: consider downloading the target with `rustup target add > aarch64-unknown-none` > = help: consider building the standard library from source with > `cargo build -Zbuild-std` > > error: aborting due to 1 previous error; 1 warning emitted > > For more information about this error, try `rustc --explain E0463`. > > I think this patch is going to need more work. Sorry for sending it prematurely. v2 is here: https://lore.kernel.org/all/20241008-rustc-option-bootstrap-v2-1-e6e155b8f9f3@google.com/
On Tue, Oct 8, 2024 at 8:45 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > Really? > > $(shell ...) inherits env variables in my understanding. I mean the Make-exported variables (not the external environment), i.e. `RUSTC_BOOTSTRAP=1` that we export in the main `Makefile`. Those are not exported into the `shell` function. However, it turns out this changes in GNU Make 4.4 in commit 98da874c4303 ("[SV 10593] Export variables to $(shell ...) commands"): * WARNING: Backward-incompatibility! Previously makefile variables marked as export were not exported to commands started by the $(shell ...) function. Now, all exported variables are exported to $(shell ...). If this leads to recursion during expansion, then for backward-compatibility the value from the original environment is used. To detect this change search for 'shell-export' in the .FEATURES variable. And indeed: export A := .PHONY: a $(shell echo $$A) a: ; @echo exported Gives: $ make-4.3 make: 'a' is up to date. $ make-4.4.1 exported Cheers, Miguel
On Wed, Oct 9, 2024 at 5:06 AM Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > > On Tue, Oct 8, 2024 at 8:45 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > Really? > > > > $(shell ...) inherits env variables in my understanding. > > I mean the Make-exported variables (not the external environment), > i.e. `RUSTC_BOOTSTRAP=1` that we export in the main `Makefile`. Those > are not exported into the `shell` function. > > However, it turns out this changes in GNU Make 4.4 in commit > 98da874c4303 ("[SV 10593] Export variables to $(shell ...) commands"): > > * WARNING: Backward-incompatibility! > Previously makefile variables marked as export were not exported > to commands > started by the $(shell ...) function. Now, all exported variables are > exported to $(shell ...). If this leads to recursion during > expansion, then > for backward-compatibility the value from the original > environment is used. > To detect this change search for 'shell-export' in the .FEATURES variable. > > And indeed: > > export A := .PHONY: a > $(shell echo $$A) > a: ; @echo exported > > Gives: > > $ make-4.3 > make: 'a' is up to date. > > $ make-4.4.1 > exported OK, I reached the same understanding now. -- Best Regards Masahiro Yamada
diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler index 057305eae85c..50eada69aed9 100644 --- a/scripts/Makefile.compiler +++ b/scripts/Makefile.compiler @@ -21,6 +21,7 @@ TMPOUT = $(if $(KBUILD_EXTMOD),$(firstword $(KBUILD_EXTMOD))/).tmp_$$$$ # automatically cleaned up. try-run = $(shell set -e; \ TMP=$(TMPOUT)/tmp; \ + RUSTC_BOOTSTRAP=1; \ trap "rm -rf $(TMPOUT)" EXIT; \ mkdir -p $(TMPOUT); \ if ($(1)) >/dev/null 2>&1; \
When using unstable features with the Rust compiler, you must either use a nightly compiler release or set the RUSTC_BOOTSTRAP environment variable. Otherwise, the compiler will emit a compiler error. This environment variable is missing when rustc-option is executed, so add the environment variable. This change is necessary to avoid two kinds of problems: 1. When using rustc-option to test whether a -Z flag is available, the check will always fail, even if the flag is available. 2. If KBUILD_RUSTFLAGS happens to contain any -Z flags from the environment, even if unrelated to the flag being tested, then all invocations of rustc-option everywhere will fail. I was not actually able to trigger the second kind of problem with the makefiles that exist today, but it seems like it could easily start being a problem due to complicated interactions between changes. It is better to fix this now so it doesn't surprise us later. I added the flag under try-run as this seemed like the easiest way to make sure that the fix applies to all variations of rustc-option, including new ones added in the future. Signed-off-by: Alice Ryhl <aliceryhl@google.com> --- scripts/Makefile.compiler | 1 + 1 file changed, 1 insertion(+) --- base-commit: 8cf0b93919e13d1e8d4466eb4080a4c4d9d66d7b change-id: 20241008-rustc-option-bootstrap-607e5bf3114c Best regards,