[PATCH] Kbuild: add RUSTC_BOOTSTRAP to rustc-option

[PATCH] Kbuild: add RUSTC_BOOTSTRAP to rustc-option

[Alice Ryhl]

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>

Re: [PATCH] Kbuild: add RUSTC_BOOTSTRAP to rustc-option

[Alice Ryhl]

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

Re: [PATCH] Kbuild: add RUSTC_BOOTSTRAP to rustc-option

[Miguel Ojeda]

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

Re: [PATCH] Kbuild: add RUSTC_BOOTSTRAP to rustc-option

[Masahiro Yamada]

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

Re: [PATCH] Kbuild: add RUSTC_BOOTSTRAP to rustc-option

[Alice Ryhl]

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/

Re: [PATCH] Kbuild: add RUSTC_BOOTSTRAP to rustc-option

[Miguel Ojeda]

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

Re: [PATCH] Kbuild: add RUSTC_BOOTSTRAP to rustc-option

[Masahiro Yamada]

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