scripts/Makefile.compiler | 1 + 1 file changed, 1 insertion(+)
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(+)
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; \
---
base-commit: 8cf0b93919e13d1e8d4466eb4080a4c4d9d66d7b
change-id: 20241008-rustc-option-bootstrap-607e5bf3114c
Best regards,
--
Alice Ryhl <aliceryhl@google.com>
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 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 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 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
© 2016 - 2024 Red Hat, Inc.