[PATCH 1/2] rust: suppress error messages from CONFIG_{RUSTC,BINDGEN}_VERSION_TEXT

[PATCH 1/2] rust: suppress error messages from CONFIG_{RUSTC,BINDGEN}_VERSION_TEXT

[Masahiro Yamada]

While this is a somewhat unusual case, I encountered odd error messages
when I ran Kconfig in a foreign architecture chroot.

  $ make allmodconfig
  sh: 1: rustc: not found
  sh: 1: bindgen: not found
  #
  # configuration written to .config
  #

The successful execution of 'command -v rustc' does not necessarily mean
that 'rustc --version' will succeed.

  $ sh -c 'command -v rustc'
  /home/masahiro/.cargo/bin/rustc
  $ sh -c 'rustc --version'
  sh: 1: rustc: not found

Here, 'rustc' is built for x86, and I ran it in an arm64 system.

The current code:

  command -v $(RUSTC) >/dev/null 2>&1 && $(RUSTC) --version || echo n

can be turned into:

  command -v $(RUSTC) >/dev/null 2>&1 && $(RUSTC) --version 2>/dev/null || echo n

However, I did not understand the necessity of 'command -v $(RUSTC)'.

I simplified it to:

  $(RUSTC) --version 2>/dev/null || echo n

Fixes: 2f7ab1267dc9 ("Kbuild: add Rust support")
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 init/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/init/Kconfig b/init/Kconfig
index c41260ffe99a..466849f9f1b9 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1919,12 +1919,12 @@ config RUST
 config RUSTC_VERSION_TEXT
 	string
 	depends on RUST
-	default $(shell,command -v $(RUSTC) >/dev/null 2>&1 && $(RUSTC) --version || echo n)
+	default $(shell,$(RUSTC) --version 2>/dev/null || echo n)
 
 config BINDGEN_VERSION_TEXT
 	string
 	depends on RUST
-	default $(shell,command -v $(BINDGEN) >/dev/null 2>&1 && $(BINDGEN) --version || echo n)
+	default $(shell,$(BINDGEN) --version 2>/dev/null || echo n)
 
 #
 # Place an empty function call at each tracepoint site. Can be
-- 
2.43.0

[PATCH 2/2] rust: fix the default format for CONFIG_{RUSTC,BINDGEN}_VERSION_TEXT

[Masahiro Yamada]

Another oddity in these config entries is their default value can fall
back to 'n', which is a value for bool or tristate symbols.

The '|| echo n' is an incorrect workaround to avoid the syntax error.
This is not a big deal, as the entry is hidden by 'depends on RUST' in
situations where '$(RUSTC) --version' or '$(BINDGEN) --version' fails.
Anyway, it looks odd.

The default of a string type symbol should be a double-quoted string
literal. Turn it into an empty string when the version command fails.

Fixes: 2f7ab1267dc9 ("Kbuild: add Rust support")
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 init/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/init/Kconfig b/init/Kconfig
index 466849f9f1b9..47e074a93d94 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1919,12 +1919,12 @@ config RUST
 config RUSTC_VERSION_TEXT
 	string
 	depends on RUST
-	default $(shell,$(RUSTC) --version 2>/dev/null || echo n)
+	default "$(shell,$(RUSTC) --version 2>/dev/null)"
 
 config BINDGEN_VERSION_TEXT
 	string
 	depends on RUST
-	default $(shell,$(BINDGEN) --version 2>/dev/null || echo n)
+	default "$(shell,$(BINDGEN) --version 2>/dev/null)"
 
 #
 # Place an empty function call at each tracepoint site. Can be
-- 
2.43.0

Re: [PATCH 2/2] rust: fix the default format for CONFIG_{RUSTC,BINDGEN}_VERSION_TEXT

[Miguel Ojeda]

On Sat, Jul 27, 2024 at 4:03 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> The default of a string type symbol should be a double-quoted string
> literal. Turn it into an empty string when the version command fails.

Makes sense, thanks Masahiro!

If you want to take it through your tree:

Acked-by: Miguel Ojeda <ojeda@kernel.org>
Tested-by: Miguel Ojeda <ojeda@kernel.org>

Otherwise I can take it too.

Cheers,
Miguel

Re: [PATCH 1/2] rust: suppress error messages from CONFIG_{RUSTC,BINDGEN}_VERSION_TEXT

[Miguel Ojeda]

On Sat, Jul 27, 2024 at 4:03 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> The successful execution of 'command -v rustc' does not necessarily mean
> that 'rustc --version' will succeed.

+1, it could also be e.g. non-executable.

By the way, I was checking recently `pahole-version.sh` for a series I
will send about `RUSTC_VERSION` and I saw that one has a `[ ! -x`
check after the `command -v` for the non-executable case. But taking
into account what you say here, I wonder if something needs to be done
there too, e.g.

    $ echo 'bad' > bad-pahole
    $ chmod u+x bad-pahole
    $ make PAHOLE=./bad-pahole defconfig
    ...
    ./bad-pahole: 1: bad: not found
    init/Kconfig:112: syntax error
    init/Kconfig:112: invalid statement

So perhaps we can put in pahole-version.sh something like:

    if output=$("$@" --version 2>/dev/null); then
        echo $output | sed -E 's/v([0-9]+)\.([0-9]+)/\1\2/'
    else
        echo 0
        exit 1
    fi

i.e. similar to what you do here for `rustc`/`bindgen`.

> However, I did not understand the necessity of 'command -v $(RUSTC)'.

I agree, I don't think it is needed.

If you want to take it through your tree:

Acked-by: Miguel Ojeda <ojeda@kernel.org>
Tested-by: Miguel Ojeda <ojeda@kernel.org>

Otherwise, I can take it too.

Cheers,
Miguel

Re: [PATCH 1/2] rust: suppress error messages from CONFIG_{RUSTC,BINDGEN}_VERSION_TEXT

[Masahiro Yamada]

On Sun, Jul 28, 2024 at 2:05 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Sat, Jul 27, 2024 at 4:03 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > The successful execution of 'command -v rustc' does not necessarily mean
> > that 'rustc --version' will succeed.
>
> +1, it could also be e.g. non-executable.
>
> By the way, I was checking recently `pahole-version.sh` for a series I
> will send about `RUSTC_VERSION` and I saw that one has a `[ ! -x`
> check after the `command -v` for the non-executable case. But taking
> into account what you say here, I wonder if something needs to be done
> there too, e.g.
>
>     $ echo 'bad' > bad-pahole
>     $ chmod u+x bad-pahole
>     $ make PAHOLE=./bad-pahole defconfig
>     ...
>     ./bad-pahole: 1: bad: not found
>     init/Kconfig:112: syntax error
>     init/Kconfig:112: invalid statement
>
> So perhaps we can put in pahole-version.sh something like:
>
>     if output=$("$@" --version 2>/dev/null); then
>         echo $output | sed -E 's/v([0-9]+)\.([0-9]+)/\1\2/'
>     else
>         echo 0
>         exit 1
>     fi
>
> i.e. similar to what you do here for `rustc`/`bindgen`.



Agree.
pahole-version.sh should be fixed too.




>
> > However, I did not understand the necessity of 'command -v $(RUSTC)'.
>
> I agree, I don't think it is needed.
>
> If you want to take it through your tree:
>
> Acked-by: Miguel Ojeda <ojeda@kernel.org>
> Tested-by: Miguel Ojeda <ojeda@kernel.org>
>
> Otherwise, I can take it too.


Yes, please take this to your tree. Thanks.






-- 
Best Regards
Masahiro Yamada

Re: [PATCH 1/2] rust: suppress error messages from CONFIG_{RUSTC,BINDGEN}_VERSION_TEXT

[Miguel Ojeda]

On Sun, Jul 28, 2024 at 10:03 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Agree.
> pahole-version.sh should be fixed too.

https://lore.kernel.org/all/20240728125527.690726-1-ojeda@kernel.org/

> Yes, please take this to your tree. Thanks.

Sounds good -- applied to `rust-fixes`.

Rebased on top of v6.11-rc1 since there was a conflict with a change last cycle:

    https://github.com/Rust-for-Linux/linux/commit/5ce86c6c861352c9346ebb5c96ed70cb67414aa3
    https://github.com/Rust-for-Linux/linux/commit/aacf93e87f0d808ef46e621aa56caea336b4433c

Thanks!

Cheers,
Miguel