[PATCH v2 00/11] `scripts/rust_is_available.sh` improvements

[PATCH v2 00/11] `scripts/rust_is_available.sh` improvements

[Miguel Ojeda]

This is the patch series to improve `scripts/rust_is_available.sh`.

The major addition in v2 is the test suite in the last commit. I added
it because I wanted to have a proper way to test any further changes to
it (such as the suggested `set --` idea to avoid forking by Masahiro),
and so that adding new checks was easier to justify too (i.e. vs. the
added complexity).

In addition, there are also a few new checks in the script, to cover for
even some more cases, which hopefully make problematic setups easier to
identify and solve by users building the kernel. For instance, running
the script externally gives:

    $ scripts/rust_is_available.sh
    ***
    *** Environment variable 'RUSTC' is not set.
    ***
    *** This script is intended to be called from Kbuild.
    *** Please use the 'rustavailable' target to call it instead.
    *** Otherwise, the results may not be meaningful.
    ***
    *** Please see Documentation/rust/quick-start.rst for details
    *** on how to set up the Rust support.
    ***

I also changed it to avoid setting `-e` as Masahiro suggested.
Similarly, I now check for `$RUSTC`, `$BINDGEN` and `$CC`, instead of
`$MAKEFLAGS`, as he also suggested (but I gave it their own error
message rather than use the `${CC?: is not set}` approach. This goes in
line with the reasons outlined above, i.e. trying to give users a clear
error of what step exactly failed).

In the test suite I included previously problematic compiler version
strings we got reports for. The test suite covers all current branches
in the script, and we should keep it that way in the future.

The patch series also include Masahiro's patch to remove the `-v`
option, as well as Russell's patch for supporting multiple arguments
in `$CC`.

All in all, this should solve all the issues we got so far (unless I
have missed something) and improve things further with the new checks
plus the test suite to hopefully have an easier time in the future.

Testers for this one are appreciated, especially if you have uncommon or
custom setups for building the kernel.

This could go through either the Kbuild or the Rust tree.

Masahiro Yamada (1):
  kbuild: rust_is_available: remove -v option

Miguel Ojeda (9):
  docs: rust: add paragraph about finding a suitable `libclang`
  kbuild: rust_is_available: print docs reference
  kbuild: rust_is_available: add check for `bindgen` invocation
  kbuild: rust_is_available: check that environment variables are set
  kbuild: rust_is_available: fix confusion when a version appears in the
    path
  kbuild: rust_is_available: normalize version matching
  kbuild: rust_is_available: handle failures calling `$RUSTC`/`$BINDGEN`
  kbuild: rust_is_available: check that output looks as expected
  kbuild: rust_is_available: add test suite

Russell Currey (1):
  kbuild: rust_is_available: fix version check when CC has multiple
    arguments

 Documentation/rust/quick-start.rst |  17 ++
 Makefile                           |   4 +-
 scripts/rust_is_available.sh       | 233 +++++++++++++------
 scripts/rust_is_available_test.py  | 346 +++++++++++++++++++++++++++++
 4 files changed, 532 insertions(+), 68 deletions(-)
 create mode 100755 scripts/rust_is_available_test.py


base-commit: 858fd168a95c5b9669aac8db6c14a9aeab446375
-- 
2.41.0

[PATCH v2 01/11] kbuild: rust_is_available: remove -v option

[Miguel Ojeda]

From: Masahiro Yamada <masahiroy@kernel.org>

The -v option is passed when this script is invoked from Makefile,
but not when invoked from Kconfig.

As you can see in scripts/Kconfig.include, the 'success' macro suppresses
stdout and stderr anyway, so this script does not need to be quiet.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Miguel Ojeda <ojeda@kernel.org>
Tested-by: Miguel Ojeda <ojeda@kernel.org>
Reviewed-by: Nathan Chancellor <nathan@kernel.org>
Link: https://lore.kernel.org/r/20230109061436.3146442-1-masahiroy@kernel.org
[ Reworded prefix to match the others in the patch series. ]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 Makefile                     |  4 +-
 scripts/rust_is_available.sh | 96 +++++++++++++++---------------------
 2 files changed, 42 insertions(+), 58 deletions(-)

diff --git a/Makefile b/Makefile
index 0d3a9d3e73c1..f0c50c7bd6d2 100644
--- a/Makefile
+++ b/Makefile
@@ -1289,7 +1289,7 @@ prepare0: archprepare
 # All the preparing..
 prepare: prepare0
 ifdef CONFIG_RUST
-	$(Q)$(CONFIG_SHELL) $(srctree)/scripts/rust_is_available.sh -v
+	$(Q)$(CONFIG_SHELL) $(srctree)/scripts/rust_is_available.sh
 	$(Q)$(MAKE) $(build)=rust
 endif
 
@@ -1823,7 +1823,7 @@ $(DOC_TARGETS):
 # "Is Rust available?" target
 PHONY += rustavailable
 rustavailable:
-	$(Q)$(CONFIG_SHELL) $(srctree)/scripts/rust_is_available.sh -v && echo "Rust is available!"
+	$(Q)$(CONFIG_SHELL) $(srctree)/scripts/rust_is_available.sh && echo "Rust is available!"
 
 # Documentation target
 #
diff --git a/scripts/rust_is_available.sh b/scripts/rust_is_available.sh
index aebbf1913970..f43a010eaf30 100755
--- a/scripts/rust_is_available.sh
+++ b/scripts/rust_is_available.sh
@@ -2,8 +2,6 @@
 # SPDX-License-Identifier: GPL-2.0
 #
 # Tests whether a suitable Rust toolchain is available.
-#
-# Pass `-v` for human output and more checks (as warnings).
 
 set -e
 
@@ -23,21 +21,17 @@ get_canonical_version()
 
 # Check that the Rust compiler exists.
 if ! command -v "$RUSTC" >/dev/null; then
-	if [ "$1" = -v ]; then
-		echo >&2 "***"
-		echo >&2 "*** Rust compiler '$RUSTC' could not be found."
-		echo >&2 "***"
-	fi
+	echo >&2 "***"
+	echo >&2 "*** Rust compiler '$RUSTC' could not be found."
+	echo >&2 "***"
 	exit 1
 fi
 
 # Check that the Rust bindings generator exists.
 if ! command -v "$BINDGEN" >/dev/null; then
-	if [ "$1" = -v ]; then
-		echo >&2 "***"
-		echo >&2 "*** Rust bindings generator '$BINDGEN' could not be found."
-		echo >&2 "***"
-	fi
+	echo >&2 "***"
+	echo >&2 "*** Rust bindings generator '$BINDGEN' could not be found."
+	echo >&2 "***"
 	exit 1
 fi
 
@@ -53,16 +47,14 @@ rust_compiler_min_version=$($min_tool_version rustc)
 rust_compiler_cversion=$(get_canonical_version $rust_compiler_version)
 rust_compiler_min_cversion=$(get_canonical_version $rust_compiler_min_version)
 if [ "$rust_compiler_cversion" -lt "$rust_compiler_min_cversion" ]; then
-	if [ "$1" = -v ]; then
-		echo >&2 "***"
-		echo >&2 "*** Rust compiler '$RUSTC' is too old."
-		echo >&2 "***   Your version:    $rust_compiler_version"
-		echo >&2 "***   Minimum version: $rust_compiler_min_version"
-		echo >&2 "***"
-	fi
+	echo >&2 "***"
+	echo >&2 "*** Rust compiler '$RUSTC' is too old."
+	echo >&2 "***   Your version:    $rust_compiler_version"
+	echo >&2 "***   Minimum version: $rust_compiler_min_version"
+	echo >&2 "***"
 	exit 1
 fi
-if [ "$1" = -v ] && [ "$rust_compiler_cversion" -gt "$rust_compiler_min_cversion" ]; then
+if [ "$rust_compiler_cversion" -gt "$rust_compiler_min_cversion" ]; then
 	echo >&2 "***"
 	echo >&2 "*** Rust compiler '$RUSTC' is too new. This may or may not work."
 	echo >&2 "***   Your version:     $rust_compiler_version"
@@ -82,16 +74,14 @@ rust_bindings_generator_min_version=$($min_tool_version bindgen)
 rust_bindings_generator_cversion=$(get_canonical_version $rust_bindings_generator_version)
 rust_bindings_generator_min_cversion=$(get_canonical_version $rust_bindings_generator_min_version)
 if [ "$rust_bindings_generator_cversion" -lt "$rust_bindings_generator_min_cversion" ]; then
-	if [ "$1" = -v ]; then
-		echo >&2 "***"
-		echo >&2 "*** Rust bindings generator '$BINDGEN' is too old."
-		echo >&2 "***   Your version:    $rust_bindings_generator_version"
-		echo >&2 "***   Minimum version: $rust_bindings_generator_min_version"
-		echo >&2 "***"
-	fi
+	echo >&2 "***"
+	echo >&2 "*** Rust bindings generator '$BINDGEN' is too old."
+	echo >&2 "***   Your version:    $rust_bindings_generator_version"
+	echo >&2 "***   Minimum version: $rust_bindings_generator_min_version"
+	echo >&2 "***"
 	exit 1
 fi
-if [ "$1" = -v ] && [ "$rust_bindings_generator_cversion" -gt "$rust_bindings_generator_min_cversion" ]; then
+if [ "$rust_bindings_generator_cversion" -gt "$rust_bindings_generator_min_cversion" ]; then
 	echo >&2 "***"
 	echo >&2 "*** Rust bindings generator '$BINDGEN' is too new. This may or may not work."
 	echo >&2 "***   Your version:     $rust_bindings_generator_version"
@@ -110,13 +100,11 @@ bindgen_libclang_min_version=$($min_tool_version llvm)
 bindgen_libclang_cversion=$(get_canonical_version $bindgen_libclang_version)
 bindgen_libclang_min_cversion=$(get_canonical_version $bindgen_libclang_min_version)
 if [ "$bindgen_libclang_cversion" -lt "$bindgen_libclang_min_cversion" ]; then
-	if [ "$1" = -v ]; then
-		echo >&2 "***"
-		echo >&2 "*** libclang (used by the Rust bindings generator '$BINDGEN') is too old."
-		echo >&2 "***   Your version:    $bindgen_libclang_version"
-		echo >&2 "***   Minimum version: $bindgen_libclang_min_version"
-		echo >&2 "***"
-	fi
+	echo >&2 "***"
+	echo >&2 "*** libclang (used by the Rust bindings generator '$BINDGEN') is too old."
+	echo >&2 "***   Your version:    $bindgen_libclang_version"
+	echo >&2 "***   Minimum version: $bindgen_libclang_min_version"
+	echo >&2 "***"
 	exit 1
 fi
 
@@ -125,21 +113,19 @@ fi
 #
 # In the future, we might be able to perform a full version check, see
 # https://github.com/rust-lang/rust-bindgen/issues/2138.
-if [ "$1" = -v ]; then
-	cc_name=$($(dirname $0)/cc-version.sh "$CC" | cut -f1 -d' ')
-	if [ "$cc_name" = Clang ]; then
-		clang_version=$( \
-			LC_ALL=C "$CC" --version 2>/dev/null \
-				| sed -nE '1s:.*version ([0-9]+\.[0-9]+\.[0-9]+).*:\1:p'
-		)
-		if [ "$clang_version" != "$bindgen_libclang_version" ]; then
-			echo >&2 "***"
-			echo >&2 "*** libclang (used by the Rust bindings generator '$BINDGEN')"
-			echo >&2 "*** version does not match Clang's. This may be a problem."
-			echo >&2 "***   libclang version: $bindgen_libclang_version"
-			echo >&2 "***   Clang version:    $clang_version"
-			echo >&2 "***"
-		fi
+cc_name=$($(dirname $0)/cc-version.sh "$CC" | cut -f1 -d' ')
+if [ "$cc_name" = Clang ]; then
+	clang_version=$( \
+		LC_ALL=C "$CC" --version 2>/dev/null \
+			| sed -nE '1s:.*version ([0-9]+\.[0-9]+\.[0-9]+).*:\1:p'
+	)
+	if [ "$clang_version" != "$bindgen_libclang_version" ]; then
+		echo >&2 "***"
+		echo >&2 "*** libclang (used by the Rust bindings generator '$BINDGEN')"
+		echo >&2 "*** version does not match Clang's. This may be a problem."
+		echo >&2 "***   libclang version: $bindgen_libclang_version"
+		echo >&2 "***   Clang version:    $clang_version"
+		echo >&2 "***"
 	fi
 fi
 
@@ -150,11 +136,9 @@ rustc_sysroot=$("$RUSTC" $KRUSTFLAGS --print sysroot)
 rustc_src=${RUST_LIB_SRC:-"$rustc_sysroot/lib/rustlib/src/rust/library"}
 rustc_src_core="$rustc_src/core/src/lib.rs"
 if [ ! -e "$rustc_src_core" ]; then
-	if [ "$1" = -v ]; then
-		echo >&2 "***"
-		echo >&2 "*** Source code for the 'core' standard library could not be found"
-		echo >&2 "*** at '$rustc_src_core'."
-		echo >&2 "***"
-	fi
+	echo >&2 "***"
+	echo >&2 "*** Source code for the 'core' standard library could not be found"
+	echo >&2 "*** at '$rustc_src_core'."
+	echo >&2 "***"
 	exit 1
 fi
-- 
2.41.0

[PATCH v2 02/11] kbuild: rust_is_available: fix version check when CC has multiple arguments

[Miguel Ojeda]

From: Russell Currey <ruscur@russell.cc>

rust_is_available.sh uses cc-version.sh to identify which C compiler is
in use, as scripts/Kconfig.include does.  cc-version.sh isn't designed to
be able to handle multiple arguments in one variable, i.e. "ccache clang".
Its invocation in rust_is_available.sh quotes "$CC", which makes
$1 == "ccache clang" instead of the intended $1 == ccache & $2 == clang.

cc-version.sh could also be changed to handle having "ccache clang" as one
argument, but it only has the one consumer upstream, making it simpler to
fix the caller here.

Signed-off-by: Russell Currey <ruscur@russell.cc>
Fixes: 78521f3399ab ("scripts: add `rust_is_available.sh`")
Link: https://github.com/Rust-for-Linux/linux/pull/873
[ Reworded title prefix and reflow line to 75 columns. ]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 scripts/rust_is_available.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/rust_is_available.sh b/scripts/rust_is_available.sh
index f43a010eaf30..0c9be438e4cd 100755
--- a/scripts/rust_is_available.sh
+++ b/scripts/rust_is_available.sh
@@ -113,10 +113,10 @@ fi
 #
 # In the future, we might be able to perform a full version check, see
 # https://github.com/rust-lang/rust-bindgen/issues/2138.
-cc_name=$($(dirname $0)/cc-version.sh "$CC" | cut -f1 -d' ')
+cc_name=$($(dirname $0)/cc-version.sh $CC | cut -f1 -d' ')
 if [ "$cc_name" = Clang ]; then
 	clang_version=$( \
-		LC_ALL=C "$CC" --version 2>/dev/null \
+		LC_ALL=C $CC --version 2>/dev/null \
 			| sed -nE '1s:.*version ([0-9]+\.[0-9]+\.[0-9]+).*:\1:p'
 	)
 	if [ "$clang_version" != "$bindgen_libclang_version" ]; then
-- 
2.41.0

[PATCH v2 03/11] docs: rust: add paragraph about finding a suitable `libclang`

[Miguel Ojeda]

Sometimes users need to tweak the finding process of `libclang`
for `bindgen` via the `clang-sys`-provided environment variables.

Thus add a paragraph to the setting up guide, including a reference
to `clang-sys`'s relevant documentation.

Link: https://lore.kernel.org/rust-for-linux/CAKwvOdm5JT4wbdQQYuW+RT07rCi6whGBM2iUAyg8A1CmLXG6Nw@mail.gmail.com/
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 Documentation/rust/quick-start.rst | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/Documentation/rust/quick-start.rst b/Documentation/rust/quick-start.rst
index 13b7744b1e27..a635be69e062 100644
--- a/Documentation/rust/quick-start.rst
+++ b/Documentation/rust/quick-start.rst
@@ -100,6 +100,23 @@ Install it via (note that this will download and build the tool from source)::
 
 	cargo install --locked --version $(scripts/min-tool-version.sh bindgen) bindgen
 
+``bindgen`` needs to find a suitable ``libclang`` in order to work. If it is
+not found (or a different ``libclang`` than the one found should be used),
+the process can be tweaked using the environment variables understood by
+``clang-sys`` (the Rust bindings crate that ``bindgen`` uses to access
+``libclang``):
+
+* ``LLVM_CONFIG_PATH`` can be pointed to an ``llvm-config`` executable.
+
+* Or ``LIBCLANG_PATH`` can be pointed to a ``libclang`` shared library
+  or to the directory containing it.
+
+* Or ``CLANG_PATH`` can be pointed to a ``clang`` executable.
+
+For details, please see ``clang-sys``'s documentation at:
+
+	https://github.com/KyleMayes/clang-sys#environment-variables
+
 
 Requirements: Developing
 ------------------------
-- 
2.41.0

[PATCH v2 04/11] kbuild: rust_is_available: print docs reference

[Miguel Ojeda]

People trying out the Rust support in the kernel may get
warnings and errors from `scripts/rust_is_available.sh`
from the `rustavailable` target or the build step.

Some of those users may be following the Quick Start guide,
but others may not (likely those getting warnings from
the build step instead of the target).

While the messages are fairly clear on what the problem is,
it may not be clear how to solve the particular issue,
especially for those not aware of the documentation.

We could add all sorts of details on the script for each one,
but it is better to point users to the documentation instead,
where it is easily readable in different formats. It also
avoids duplication.

Thus add a reference to the documentation whenever the script
fails or there is at least a warning.

Reviewed-by: Finn Behrens <fin@nyantec.com>
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 scripts/rust_is_available.sh | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/scripts/rust_is_available.sh b/scripts/rust_is_available.sh
index 0c9be438e4cd..6b8131d5b547 100755
--- a/scripts/rust_is_available.sh
+++ b/scripts/rust_is_available.sh
@@ -19,6 +19,20 @@ get_canonical_version()
 	echo $((100000 * $1 + 100 * $2 + $3))
 }
 
+# Print a reference to the Quick Start guide in the documentation.
+print_docs_reference()
+{
+	echo >&2 "***"
+	echo >&2 "*** Please see Documentation/rust/quick-start.rst for details"
+	echo >&2 "*** on how to set up the Rust support."
+	echo >&2 "***"
+}
+
+# If the script fails for any reason, or if there was any warning, then
+# print a reference to the documentation on exit.
+warning=0
+trap 'if [ $? -ne 0 ] || [ $warning -ne 0 ]; then print_docs_reference; fi' EXIT
+
 # Check that the Rust compiler exists.
 if ! command -v "$RUSTC" >/dev/null; then
 	echo >&2 "***"
@@ -60,6 +74,7 @@ if [ "$rust_compiler_cversion" -gt "$rust_compiler_min_cversion" ]; then
 	echo >&2 "***   Your version:     $rust_compiler_version"
 	echo >&2 "***   Expected version: $rust_compiler_min_version"
 	echo >&2 "***"
+	warning=1
 fi
 
 # Check that the Rust bindings generator is suitable.
@@ -87,6 +102,7 @@ if [ "$rust_bindings_generator_cversion" -gt "$rust_bindings_generator_min_cvers
 	echo >&2 "***   Your version:     $rust_bindings_generator_version"
 	echo >&2 "***   Expected version: $rust_bindings_generator_min_version"
 	echo >&2 "***"
+	warning=1
 fi
 
 # Check that the `libclang` used by the Rust bindings generator is suitable.
@@ -126,6 +142,7 @@ if [ "$cc_name" = Clang ]; then
 		echo >&2 "***   libclang version: $bindgen_libclang_version"
 		echo >&2 "***   Clang version:    $clang_version"
 		echo >&2 "***"
+		warning=1
 	fi
 fi
 
-- 
2.41.0

[PATCH v2 05/11] kbuild: rust_is_available: add check for `bindgen` invocation

[Miguel Ojeda]

`scripts/rust_is_available.sh` calls `bindgen` with a special
header in order to check whether the `libclang` version in use
is suitable.

However, the invocation itself may fail if, for instance, `bindgen`
cannot locate `libclang`. This is fine for Kconfig (since the
script will still fail and therefore disable Rust as it should),
but it is pretty confusing for users of the `rustavailable` target
given the error will be unrelated:

    ./scripts/rust_is_available.sh: 21: arithmetic expression: expecting primary: "100000 *  + 100 *  + "
    make: *** [Makefile:1816: rustavailable] Error 2

Instead, run the `bindgen` invocation independently in a previous
step, saving its output and return code. If it fails, then show
the user a proper error message. Otherwise, continue as usual
with the saved output.

Since the previous patch we show a reference to the docs, and
the docs now explain how `bindgen` looks for `libclang`,
thus the error message can leverage the documentation, avoiding
duplication here (and making users aware of the setup guide in
the documentation).

Reported-by: Nick Desaulniers <ndesaulniers@google.com>
Link: https://lore.kernel.org/rust-for-linux/CAKwvOdm5JT4wbdQQYuW+RT07rCi6whGBM2iUAyg8A1CmLXG6Nw@mail.gmail.com/
Reported-by: François Valenduc <francoisvalenduc@gmail.com>
Closes: https://github.com/Rust-for-Linux/linux/issues/934
Reported-by: Alexandru Radovici <msg4alex@gmail.com>
Closes: https://github.com/Rust-for-Linux/linux/pull/921
Reported-by: Matthew Leach <dev@mattleach.net>
Closes: https://lore.kernel.org/rust-for-linux/20230507084116.1099067-1-dev@mattleach.net/
Fixes: 78521f3399ab ("scripts: add `rust_is_available.sh`")
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 scripts/rust_is_available.sh | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/scripts/rust_is_available.sh b/scripts/rust_is_available.sh
index 6b8131d5b547..1bdff4472cbe 100755
--- a/scripts/rust_is_available.sh
+++ b/scripts/rust_is_available.sh
@@ -106,8 +106,28 @@ if [ "$rust_bindings_generator_cversion" -gt "$rust_bindings_generator_min_cvers
 fi
 
 # Check that the `libclang` used by the Rust bindings generator is suitable.
+#
+# In order to do that, first invoke `bindgen` to get the `libclang` version
+# found by `bindgen`. This step may already fail if, for instance, `libclang`
+# is not found, thus inform the user in such a case.
+bindgen_libclang_output=$( \
+	LC_ALL=C "$BINDGEN" $(dirname $0)/rust_is_available_bindgen_libclang.h 2>&1 >/dev/null
+) || bindgen_libclang_code=$?
+if [ -n "$bindgen_libclang_code" ]; then
+	echo >&2 "***"
+	echo >&2 "*** Running '$BINDGEN' to check the libclang version (used by the Rust"
+	echo >&2 "*** bindings generator) failed with code $bindgen_libclang_code. This may be caused by"
+	echo >&2 "*** a failure to locate libclang. See output and docs below for details:"
+	echo >&2 "***"
+	echo >&2 "$bindgen_libclang_output"
+	echo >&2 "***"
+	exit 1
+fi
+
+# `bindgen` returned successfully, thus use the output to check that the version
+# of the `libclang` found by the Rust bindings generator is suitable.
 bindgen_libclang_version=$( \
-	LC_ALL=C "$BINDGEN" $(dirname $0)/rust_is_available_bindgen_libclang.h 2>&1 >/dev/null \
+	echo "$bindgen_libclang_output" \
 		| grep -F 'clang version ' \
 		| grep -oE '[0-9]+\.[0-9]+\.[0-9]+' \
 		| head -n 1 \
-- 
2.41.0

[PATCH v2 06/11] kbuild: rust_is_available: check that environment variables are set

[Miguel Ojeda]

Sometimes [1] users may attempt to setup the Rust support by
checking what Kbuild does and they end up finding out about
`scripts/rust_is_available.sh`. Inevitably, they run the script
directly, but unless they setup the required variables,
the result of the script is not meaningful.

We could add some defaults to the variables, but that could be
confusing for those that may override the defaults (compared
to their kernel builds), and `$CC` would not be a simple default
in any case.

Therefore, instead, explicitly check whether the expected variables
are set (`$RUSTC`, `$BINDGEN` and `$CC`). If not, print an explanation
about the fact that the script is meant to be called from Kbuild,
since that is the most likely cause for the variables not being set.

Link: https://lore.kernel.org/oe-kbuild-all/Y6r4mXz5NS0+HVXo@zn.tnic/ [1]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 scripts/rust_is_available.sh | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/scripts/rust_is_available.sh b/scripts/rust_is_available.sh
index 1bdff4472cbe..7e0368babe64 100755
--- a/scripts/rust_is_available.sh
+++ b/scripts/rust_is_available.sh
@@ -28,11 +28,40 @@ print_docs_reference()
 	echo >&2 "***"
 }
 
+# Print an explanation about the fact that the script is meant to be called from Kbuild.
+print_kbuild_explanation()
+{
+	echo >&2 "***"
+	echo >&2 "*** This script is intended to be called from Kbuild."
+	echo >&2 "*** Please use the 'rustavailable' target to call it instead."
+	echo >&2 "*** Otherwise, the results may not be meaningful."
+	exit 1
+}
+
 # If the script fails for any reason, or if there was any warning, then
 # print a reference to the documentation on exit.
 warning=0
 trap 'if [ $? -ne 0 ] || [ $warning -ne 0 ]; then print_docs_reference; fi' EXIT
 
+# Check that the expected environment variables are set.
+if [ -z "${RUSTC+x}" ]; then
+	echo >&2 "***"
+	echo >&2 "*** Environment variable 'RUSTC' is not set."
+	print_kbuild_explanation
+fi
+
+if [ -z "${BINDGEN+x}" ]; then
+	echo >&2 "***"
+	echo >&2 "*** Environment variable 'BINDGEN' is not set."
+	print_kbuild_explanation
+fi
+
+if [ -z "${CC+x}" ]; then
+	echo >&2 "***"
+	echo >&2 "*** Environment variable 'CC' is not set."
+	print_kbuild_explanation
+fi
+
 # Check that the Rust compiler exists.
 if ! command -v "$RUSTC" >/dev/null; then
 	echo >&2 "***"
-- 
2.41.0

[PATCH v2 07/11] kbuild: rust_is_available: fix confusion when a version appears in the path

[Miguel Ojeda]

`bindgen`'s output for `libclang`'s version check contains paths, which
in turn may contain strings that look like version numbers [1][2]:

    .../6.1.0-dev/.../rust_is_available_bindgen_libclang.h:2:9: warning: clang version 11.1.0  [-W#pragma-messages], err: false

which the script will pick up as the version instead of the latter.

It is also the case that versions may appear after the actual version
(e.g. distribution's version text), which was the reason behind `head` [3]:

    .../rust-is-available-bindgen-libclang.h:2:9: warning: clang version 13.0.0 (Fedora 13.0.0-3.fc35) [-W#pragma-messages], err: false

Thus instead ask for a match after the `clang version` string.

Reported-by: Jordan Isaacs <mail@jdisaacs.com>
Closes: https://github.com/Rust-for-Linux/linux/issues/942 [1]
Reported-by: Ethan D. Twardy <ethan.twardy@gmail.com>
Closes: https://lore.kernel.org/rust-for-linux/20230528131802.6390-2-ethan.twardy@gmail.com/ [2]
Reported-by: Tiago Lam <tiagolam@gmail.com>
Closes: https://github.com/Rust-for-Linux/linux/pull/789 [3]
Fixes: 78521f3399ab ("scripts: add `rust_is_available.sh`")
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 scripts/rust_is_available.sh | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/scripts/rust_is_available.sh b/scripts/rust_is_available.sh
index 7e0368babe64..810691af66eb 100755
--- a/scripts/rust_is_available.sh
+++ b/scripts/rust_is_available.sh
@@ -157,9 +157,7 @@ fi
 # of the `libclang` found by the Rust bindings generator is suitable.
 bindgen_libclang_version=$( \
 	echo "$bindgen_libclang_output" \
-		| grep -F 'clang version ' \
-		| grep -oE '[0-9]+\.[0-9]+\.[0-9]+' \
-		| head -n 1 \
+		| sed -nE 's:.*clang version ([0-9]+\.[0-9]+\.[0-9]+).*:\1:p'
 )
 bindgen_libclang_min_version=$($min_tool_version llvm)
 bindgen_libclang_cversion=$(get_canonical_version $bindgen_libclang_version)
-- 
2.41.0

[PATCH v2 08/11] kbuild: rust_is_available: normalize version matching

[Miguel Ojeda]

In order to match the version string, `sed` is used in a couple
cases, and `grep` and `head` in a couple others.

Make the script more consistent and easier to understand by
using the same method, `sed`, for all of them.

This makes the version matching also a bit more strict for
the changed cases, since the strings `rustc ` and `bindgen `
will now be required, which should be fine since `rustc`
complains if one attempts to call it with another program
name, and `bindgen` uses a hardcoded string.

In addition, clarify why one of the existing `sed` commands
does not provide an address like the others.

Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 scripts/rust_is_available.sh | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/scripts/rust_is_available.sh b/scripts/rust_is_available.sh
index 810691af66eb..b7e0781fdea9 100755
--- a/scripts/rust_is_available.sh
+++ b/scripts/rust_is_available.sh
@@ -83,8 +83,7 @@ fi
 # Non-stable and distributions' versions may have a version suffix, e.g. `-dev`.
 rust_compiler_version=$( \
 	LC_ALL=C "$RUSTC" --version 2>/dev/null \
-		| head -n 1 \
-		| grep -oE '[0-9]+\.[0-9]+\.[0-9]+' \
+		| sed -nE '1s:.*rustc ([0-9]+\.[0-9]+\.[0-9]+).*:\1:p'
 )
 rust_compiler_min_version=$($min_tool_version rustc)
 rust_compiler_cversion=$(get_canonical_version $rust_compiler_version)
@@ -111,8 +110,7 @@ fi
 # Non-stable and distributions' versions may have a version suffix, e.g. `-dev`.
 rust_bindings_generator_version=$( \
 	LC_ALL=C "$BINDGEN" --version 2>/dev/null \
-		| head -n 1 \
-		| grep -oE '[0-9]+\.[0-9]+\.[0-9]+' \
+		| sed -nE '1s:.*bindgen ([0-9]+\.[0-9]+\.[0-9]+).*:\1:p'
 )
 rust_bindings_generator_min_version=$($min_tool_version bindgen)
 rust_bindings_generator_cversion=$(get_canonical_version $rust_bindings_generator_version)
@@ -155,6 +153,9 @@ fi
 
 # `bindgen` returned successfully, thus use the output to check that the version
 # of the `libclang` found by the Rust bindings generator is suitable.
+#
+# Unlike other version checks, note that this one does not necessarily appear
+# in the first line of the output, thus no `sed` address is provided.
 bindgen_libclang_version=$( \
 	echo "$bindgen_libclang_output" \
 		| sed -nE 's:.*clang version ([0-9]+\.[0-9]+\.[0-9]+).*:\1:p'
-- 
2.41.0

[PATCH v2 09/11] kbuild: rust_is_available: handle failures calling `$RUSTC`/`$BINDGEN`

[Miguel Ojeda]

The script already checks if `$RUSTC` and `$BINDGEN` exists via
`command`, but the environment variables may point to a
non-executable file, or the programs may fail for some other reason.
While the script successfully exits with a failure as it should,
the error given can be quite confusing depending on the shell and
the behavior of its `command`. For instance, with `dash`:

    $ RUSTC=./mm BINDGEN=bindgen CC=clang scripts/rust_is_available.sh
    scripts/rust_is_available.sh: 19: arithmetic expression: expecting primary: "100000 *  + 100 *  + "

Thus detect failure exit codes when calling `$RUSTC` and `$BINDGEN` and
print a better message, in a similar way to what we do when extracting
the `libclang` version found by `bindgen`.

Link: https://lore.kernel.org/rust-for-linux/CAK7LNAQYk6s11MASRHW6oxtkqF00EJVqhHOP=5rynWt-QDUsXw@mail.gmail.com/
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 scripts/rust_is_available.sh | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/scripts/rust_is_available.sh b/scripts/rust_is_available.sh
index b7e0781fdea9..da8296cd9b8d 100755
--- a/scripts/rust_is_available.sh
+++ b/scripts/rust_is_available.sh
@@ -81,8 +81,20 @@ fi
 # Check that the Rust compiler version is suitable.
 #
 # Non-stable and distributions' versions may have a version suffix, e.g. `-dev`.
+rust_compiler_output=$( \
+	LC_ALL=C "$RUSTC" --version 2>/dev/null
+) || rust_compiler_code=$?
+if [ -n "$rust_compiler_code" ]; then
+	echo >&2 "***"
+	echo >&2 "*** Running '$RUSTC' to check the Rust compiler version failed with"
+	echo >&2 "*** code $rust_compiler_code. See output and docs below for details:"
+	echo >&2 "***"
+	echo >&2 "$rust_compiler_output"
+	echo >&2 "***"
+	exit 1
+fi
 rust_compiler_version=$( \
-	LC_ALL=C "$RUSTC" --version 2>/dev/null \
+	echo "$rust_compiler_output" \
 		| sed -nE '1s:.*rustc ([0-9]+\.[0-9]+\.[0-9]+).*:\1:p'
 )
 rust_compiler_min_version=$($min_tool_version rustc)
@@ -108,8 +120,20 @@ fi
 # Check that the Rust bindings generator is suitable.
 #
 # Non-stable and distributions' versions may have a version suffix, e.g. `-dev`.
+rust_bindings_generator_output=$( \
+	LC_ALL=C "$BINDGEN" --version 2>/dev/null
+) || rust_bindings_generator_code=$?
+if [ -n "$rust_bindings_generator_code" ]; then
+	echo >&2 "***"
+	echo >&2 "*** Running '$BINDGEN' to check the Rust bindings generator version failed with"
+	echo >&2 "*** code $rust_bindings_generator_code. See output and docs below for details:"
+	echo >&2 "***"
+	echo >&2 "$rust_bindings_generator_output"
+	echo >&2 "***"
+	exit 1
+fi
 rust_bindings_generator_version=$( \
-	LC_ALL=C "$BINDGEN" --version 2>/dev/null \
+	echo "$rust_bindings_generator_output" \
 		| sed -nE '1s:.*bindgen ([0-9]+\.[0-9]+\.[0-9]+).*:\1:p'
 )
 rust_bindings_generator_min_version=$($min_tool_version bindgen)
-- 
2.41.0

[PATCH v2 10/11] kbuild: rust_is_available: check that output looks as expected

[Miguel Ojeda]

The script already checks for `$RUSTC` and `$BINDGEN` existing
and exiting without failure. However, one may still pass an
unexpected binary that does not output what the later parsing
expects. The script still successfully reports a failure as
expected, but the error is confusing. For instance:

    $ RUSTC=true BINDGEN=bindgen CC=clang scripts/rust_is_available.sh
    scripts/rust_is_available.sh: 19: arithmetic expression: expecting primary: "100000 *  + 100 *  + "
    ***
    *** Please see Documentation/rust/quick-start.rst for details
    *** on how to set up the Rust support.
    ***

Thus add an explicit check and a proper message for unexpected
output from the called command.

Similarly, do so for the `libclang` version parsing, too.

Link: https://lore.kernel.org/rust-for-linux/CAK7LNAQYk6s11MASRHW6oxtkqF00EJVqhHOP=5rynWt-QDUsXw@mail.gmail.com/
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 scripts/rust_is_available.sh | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/scripts/rust_is_available.sh b/scripts/rust_is_available.sh
index da8296cd9b8d..117018946b57 100755
--- a/scripts/rust_is_available.sh
+++ b/scripts/rust_is_available.sh
@@ -97,6 +97,15 @@ rust_compiler_version=$( \
 	echo "$rust_compiler_output" \
 		| sed -nE '1s:.*rustc ([0-9]+\.[0-9]+\.[0-9]+).*:\1:p'
 )
+if [ -z "$rust_compiler_version" ]; then
+	echo >&2 "***"
+	echo >&2 "*** Running '$RUSTC' to check the Rust compiler version did not return"
+	echo >&2 "*** an expected output. See output and docs below for details:"
+	echo >&2 "***"
+	echo >&2 "$rust_compiler_output"
+	echo >&2 "***"
+	exit 1
+fi
 rust_compiler_min_version=$($min_tool_version rustc)
 rust_compiler_cversion=$(get_canonical_version $rust_compiler_version)
 rust_compiler_min_cversion=$(get_canonical_version $rust_compiler_min_version)
@@ -136,6 +145,15 @@ rust_bindings_generator_version=$( \
 	echo "$rust_bindings_generator_output" \
 		| sed -nE '1s:.*bindgen ([0-9]+\.[0-9]+\.[0-9]+).*:\1:p'
 )
+if [ -z "$rust_bindings_generator_version" ]; then
+	echo >&2 "***"
+	echo >&2 "*** Running '$BINDGEN' to check the bindings generator version did not return"
+	echo >&2 "*** an expected output. See output and docs below for details:"
+	echo >&2 "***"
+	echo >&2 "$rust_bindings_generator_output"
+	echo >&2 "***"
+	exit 1
+fi
 rust_bindings_generator_min_version=$($min_tool_version bindgen)
 rust_bindings_generator_cversion=$(get_canonical_version $rust_bindings_generator_version)
 rust_bindings_generator_min_cversion=$(get_canonical_version $rust_bindings_generator_min_version)
@@ -184,6 +202,16 @@ bindgen_libclang_version=$( \
 	echo "$bindgen_libclang_output" \
 		| sed -nE 's:.*clang version ([0-9]+\.[0-9]+\.[0-9]+).*:\1:p'
 )
+if [ -z "$bindgen_libclang_version" ]; then
+	echo >&2 "***"
+	echo >&2 "*** Running '$BINDGEN' to check the libclang version (used by the Rust"
+	echo >&2 "*** bindings generator) did not return an expected output. See output"
+	echo >&2 "*** and docs below for details:"
+	echo >&2 "***"
+	echo >&2 "$bindgen_libclang_output"
+	echo >&2 "***"
+	exit 1
+fi
 bindgen_libclang_min_version=$($min_tool_version llvm)
 bindgen_libclang_cversion=$(get_canonical_version $bindgen_libclang_version)
 bindgen_libclang_min_cversion=$(get_canonical_version $bindgen_libclang_min_version)
-- 
2.41.0

[PATCH v2 11/11] kbuild: rust_is_available: add test suite

[Miguel Ojeda]

The `rust_is_available.sh` script runs for everybody compiling the
kernel, even if not using Rust. Therefore, it is important to ensure
that the script is correct to avoid breaking people's compilation.

In addition, the script needs to be able to handle a set of subtle
cases, including parsing version strings of different tools.

Therefore, maintenance of this script can be greatly eased with
a set of tests.

Thus add a test suite to cover hopefully most of the setups that
the script may encounter in the wild. Extra setups can be easily
added later on if missing.

The script currently covers all the branches of the shell script,
including several ways in which they may be entered.

Python is used for this script, since the script under test
does not depend on Rust, thus hopefully making it easier for others
to use if the need arises.

Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 scripts/rust_is_available_test.py | 346 ++++++++++++++++++++++++++++++
 1 file changed, 346 insertions(+)
 create mode 100755 scripts/rust_is_available_test.py

diff --git a/scripts/rust_is_available_test.py b/scripts/rust_is_available_test.py
new file mode 100755
index 000000000000..57613fe5ed75
--- /dev/null
+++ b/scripts/rust_is_available_test.py
@@ -0,0 +1,346 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+
+"""Tests the `rust_is_available.sh` script.
+
+Some of the tests require the real programs to be available in `$PATH`
+under their canonical name (and with the expected versions).
+"""
+
+import enum
+import os
+import pathlib
+import stat
+import subprocess
+import tempfile
+import unittest
+
+class TestRustIsAvailable(unittest.TestCase):
+    @enum.unique
+    class Expected(enum.Enum):
+        SUCCESS = enum.auto()
+        SUCCESS_WITH_WARNINGS = enum.auto()
+        SUCCESS_WITH_EXTRA_OUTPUT = enum.auto()
+        FAILURE = enum.auto()
+
+    @classmethod
+    def generate_executable(cls, content):
+        path = pathlib.Path(cls.tempdir.name)
+        name = str(len(tuple(path.iterdir())))
+        path = path / name
+        with open(path, "w") as file_:
+            file_.write(content)
+        os.chmod(path, os.stat(path).st_mode | stat.S_IXUSR)
+        return path
+
+    @classmethod
+    def generate_clang(cls, stdout):
+        return cls.generate_executable(f"""#!/usr/bin/env python3
+import sys
+if "-E" in " ".join(sys.argv):
+    print({repr("Clang " + " ".join(cls.llvm_default_version.split(" ")))})
+else:
+    print({repr(stdout)})
+""")
+
+    @classmethod
+    def generate_rustc(cls, stdout):
+        return cls.generate_executable(f"""#!/usr/bin/env python3
+import sys
+if "--print sysroot" in " ".join(sys.argv):
+    print({repr(cls.rust_default_sysroot)})
+else:
+    print({repr(stdout)})
+""")
+
+    @classmethod
+    def generate_bindgen(cls, version_stdout, libclang_stderr):
+        return cls.generate_executable(f"""#!/usr/bin/env python3
+import sys
+if "rust_is_available_bindgen_libclang.h" in " ".join(sys.argv):
+    print({repr(libclang_stderr)}, file=sys.stderr)
+else:
+    print({repr(version_stdout)})
+""")
+
+    @classmethod
+    def generate_bindgen_version(cls, stdout):
+        return cls.generate_bindgen(stdout, cls.bindgen_default_bindgen_libclang_stderr)
+
+    @classmethod
+    def generate_bindgen_libclang(cls, stderr):
+        return cls.generate_bindgen(cls.bindgen_default_bindgen_version_stdout, stderr)
+
+    @classmethod
+    def setUpClass(cls):
+        cls.tempdir = tempfile.TemporaryDirectory()
+
+        cls.missing = pathlib.Path(cls.tempdir.name) / "missing"
+
+        cls.nonexecutable = pathlib.Path(cls.tempdir.name) / "nonexecutable"
+        with open(cls.nonexecutable, "w") as file_:
+            file_.write("nonexecutable")
+
+        cls.unexpected_binary = "true"
+
+        cls.rustc_default_version = subprocess.check_output(("scripts/min-tool-version.sh", "rustc")).decode().strip()
+        cls.bindgen_default_version = subprocess.check_output(("scripts/min-tool-version.sh", "bindgen")).decode().strip()
+        cls.llvm_default_version = subprocess.check_output(("scripts/min-tool-version.sh", "llvm")).decode().strip()
+        cls.rust_default_sysroot = subprocess.check_output(("rustc", "--print", "sysroot")).decode().strip()
+
+        cls.bindgen_default_bindgen_version_stdout = f"bindgen {cls.bindgen_default_version}"
+        cls.bindgen_default_bindgen_libclang_stderr = f"scripts/rust_is_available_bindgen_libclang.h:2:9: warning: clang version {cls.llvm_default_version} [-W#pragma-messages], err: false"
+
+        cls.default_rustc = cls.generate_rustc(f"rustc {cls.rustc_default_version}")
+        cls.default_bindgen =  cls.generate_bindgen(cls.bindgen_default_bindgen_version_stdout, cls.bindgen_default_bindgen_libclang_stderr)
+        cls.default_cc = cls.generate_clang(f"clang version {cls.llvm_default_version}")
+
+    def run_script(self, expected, override_env):
+        env = {
+            "RUSTC": self.default_rustc,
+            "BINDGEN": self.default_bindgen,
+            "CC": self.default_cc,
+        }
+
+        for key, value in override_env.items():
+            if value is None:
+                del env[key]
+                continue
+            env[key] = value
+
+        result = subprocess.run("scripts/rust_is_available.sh", env=env, capture_output=True)
+
+        # The script should never output anything to `stdout`.
+        self.assertEqual(result.stdout, b"")
+
+        if expected == self.Expected.SUCCESS:
+            # When expecting a success, the script should return 0
+            # and it should not output anything to `stderr`.
+            self.assertEqual(result.returncode, 0)
+            self.assertEqual(result.stderr, b"")
+        elif expected == self.Expected.SUCCESS_WITH_EXTRA_OUTPUT:
+            # When expecting a success with extra output (that is not warnings,
+            # which is the common case), the script should return 0 and it
+            # should output at least something to `stderr` (the output should
+            # be checked further by the test).
+            self.assertEqual(result.returncode, 0)
+            self.assertNotEqual(result.stderr, b"")
+        elif expected == self.Expected.SUCCESS_WITH_WARNINGS:
+            # When expecting a success with warnings, the script should return 0
+            # and it should output at least the instructions to `stderr`.
+            self.assertEqual(result.returncode, 0)
+            self.assertIn(b"Please see Documentation/rust/quick-start.rst for details", result.stderr)
+        else:
+            # When expecting a failure, the script should return non-0
+            # and it should output at least the instructions to `stderr`.
+            self.assertNotEqual(result.returncode, 0)
+            self.assertIn(b"Please see Documentation/rust/quick-start.rst for details", result.stderr)
+
+        # The output will generally be UTF-8 (i.e. unless the user has
+        # put strange values in the environment).
+        result.stderr = result.stderr.decode()
+
+        return result
+
+    def test_rustc_unset(self):
+        result = self.run_script(self.Expected.FAILURE, { "RUSTC": None })
+        self.assertIn("Environment variable 'RUSTC' is not set.", result.stderr)
+        self.assertIn("This script is intended to be called from Kbuild.", result.stderr)
+
+    def test_bindgen_unset(self):
+        result = self.run_script(self.Expected.FAILURE, { "BINDGEN": None })
+        self.assertIn("Environment variable 'BINDGEN' is not set.", result.stderr)
+        self.assertIn("This script is intended to be called from Kbuild.", result.stderr)
+
+    def test_cc_unset(self):
+        result = self.run_script(self.Expected.FAILURE, { "CC": None })
+        self.assertIn("Environment variable 'CC' is not set.", result.stderr)
+        self.assertIn("This script is intended to be called from Kbuild.", result.stderr)
+
+    def test_rustc_missing(self):
+        result = self.run_script(self.Expected.FAILURE, { "RUSTC": self.missing })
+        self.assertIn(f"Rust compiler '{self.missing}' could not be found.", result.stderr)
+
+    def test_bindgen_missing(self):
+        result = self.run_script(self.Expected.FAILURE, { "BINDGEN": self.missing })
+        self.assertIn(f"Rust bindings generator '{self.missing}' could not be found.", result.stderr)
+
+    def test_rustc_nonexecutable(self):
+        result = self.run_script(self.Expected.FAILURE, { "RUSTC": self.nonexecutable })
+        self.assertIn(f"Running '{self.nonexecutable}' to check the Rust compiler version failed with", result.stderr)
+
+    def test_rustc_unexpected_binary(self):
+        result = self.run_script(self.Expected.FAILURE, { "RUSTC": self.unexpected_binary })
+        self.assertIn(f"Running '{self.unexpected_binary}' to check the Rust compiler version did not return", result.stderr)
+
+    def test_rustc_unexpected_name(self):
+        rustc = self.generate_rustc(f"unexpected {self.rustc_default_version} (a8314ef7d 2022-06-27)")
+        result = self.run_script(self.Expected.FAILURE, { "RUSTC": rustc })
+        self.assertIn(f"Running '{rustc}' to check the Rust compiler version did not return", result.stderr)
+
+    def test_rustc_unexpected_version(self):
+        rustc = self.generate_rustc("rustc unexpected (a8314ef7d 2022-06-27)")
+        result = self.run_script(self.Expected.FAILURE, { "RUSTC": rustc })
+        self.assertIn(f"Running '{rustc}' to check the Rust compiler version did not return", result.stderr)
+
+    def test_rustc_no_minor(self):
+        rustc = self.generate_rustc(f"rustc {'.'.join(self.rustc_default_version.split('.')[:2])} (a8314ef7d 2022-06-27)")
+        result = self.run_script(self.Expected.FAILURE, { "RUSTC": rustc })
+        self.assertIn(f"Running '{rustc}' to check the Rust compiler version did not return", result.stderr)
+
+    def test_rustc_old_version(self):
+        rustc = self.generate_rustc("rustc 1.60.0 (a8314ef7d 2022-06-27)")
+        result = self.run_script(self.Expected.FAILURE, { "RUSTC": rustc })
+        self.assertIn(f"Rust compiler '{rustc}' is too old.", result.stderr)
+
+    def test_rustc_new_version(self):
+        rustc = self.generate_rustc("rustc 1.999.0 (a8314ef7d 2099-06-27)")
+        result = self.run_script(self.Expected.SUCCESS_WITH_WARNINGS, { "RUSTC": rustc })
+        self.assertIn(f"Rust compiler '{rustc}' is too new. This may or may not work.", result.stderr)
+
+    def test_bindgen_nonexecutable(self):
+        result = self.run_script(self.Expected.FAILURE, { "BINDGEN": self.nonexecutable })
+        self.assertIn(f"Running '{self.nonexecutable}' to check the Rust bindings generator version failed with", result.stderr)
+
+    def test_bindgen_unexpected_binary(self):
+        result = self.run_script(self.Expected.FAILURE, { "BINDGEN": self.unexpected_binary })
+        self.assertIn(f"Running '{self.unexpected_binary}' to check the bindings generator version did not return", result.stderr)
+
+    def test_bindgen_unexpected_name(self):
+        bindgen = self.generate_bindgen_version(f"unexpected {self.bindgen_default_version}")
+        result = self.run_script(self.Expected.FAILURE, { "BINDGEN": bindgen })
+        self.assertIn(f"Running '{bindgen}' to check the bindings generator version did not return", result.stderr)
+
+    def test_bindgen_unexpected_version(self):
+        bindgen = self.generate_bindgen_version("bindgen unexpected")
+        result = self.run_script(self.Expected.FAILURE, { "BINDGEN": bindgen })
+        self.assertIn(f"Running '{bindgen}' to check the bindings generator version did not return", result.stderr)
+
+    def test_bindgen_no_minor(self):
+        bindgen = self.generate_bindgen_version(f"bindgen {'.'.join(self.bindgen_default_version.split('.')[:2])}")
+        result = self.run_script(self.Expected.FAILURE, { "BINDGEN": bindgen })
+        self.assertIn(f"Running '{bindgen}' to check the bindings generator version did not return", result.stderr)
+
+    def test_bindgen_old_version(self):
+        bindgen = self.generate_bindgen_version("bindgen 0.50.0")
+        result = self.run_script(self.Expected.FAILURE, { "BINDGEN": bindgen })
+        self.assertIn(f"Rust bindings generator '{bindgen}' is too old.", result.stderr)
+
+    def test_bindgen_new_version(self):
+        bindgen = self.generate_bindgen_version("bindgen 0.999.0")
+        result = self.run_script(self.Expected.SUCCESS_WITH_WARNINGS, { "BINDGEN": bindgen })
+        self.assertIn(f"Rust bindings generator '{bindgen}' is too new. This may or may not work.", result.stderr)
+
+    def test_bindgen_libclang_failure(self):
+        for env in (
+            { "LLVM_CONFIG_PATH": self.missing },
+            { "LIBCLANG_PATH": self.missing },
+            { "CLANG_PATH": self.missing },
+        ):
+            with self.subTest(env=env):
+                result = self.run_script(self.Expected.FAILURE, env | { "PATH": os.environ["PATH"], "BINDGEN": "bindgen" })
+                self.assertIn("Running 'bindgen' to check the libclang version (used by the Rust", result.stderr)
+                self.assertIn("bindings generator) failed with code ", result.stderr)
+
+    def test_bindgen_libclang_unexpected_version(self):
+        bindgen = self.generate_bindgen_libclang("scripts/rust_is_available_bindgen_libclang.h:2:9: warning: clang version unexpected [-W#pragma-messages], err: false")
+        result = self.run_script(self.Expected.FAILURE, { "BINDGEN": bindgen })
+        self.assertIn(f"Running '{bindgen}' to check the libclang version (used by the Rust", result.stderr)
+        self.assertIn("bindings generator) did not return an expected output. See output", result.stderr)
+
+    def test_bindgen_libclang_old_version(self):
+        bindgen = self.generate_bindgen_libclang("scripts/rust_is_available_bindgen_libclang.h:2:9: warning: clang version 10.0.0 [-W#pragma-messages], err: false")
+        result = self.run_script(self.Expected.FAILURE, { "BINDGEN": bindgen })
+        self.assertIn(f"libclang (used by the Rust bindings generator '{bindgen}') is too old.", result.stderr)
+
+    def test_clang_matches_bindgen_libclang_different_bindgen(self):
+        bindgen = self.generate_bindgen_libclang("scripts/rust_is_available_bindgen_libclang.h:2:9: warning: clang version 999.0.0 [-W#pragma-messages], err: false")
+        result = self.run_script(self.Expected.SUCCESS_WITH_WARNINGS, { "BINDGEN": bindgen })
+        self.assertIn("version does not match Clang's. This may be a problem.", result.stderr)
+
+    def test_clang_matches_bindgen_libclang_different_clang(self):
+        cc = self.generate_clang("clang version 999.0.0")
+        result = self.run_script(self.Expected.SUCCESS_WITH_WARNINGS, { "CC": cc })
+        self.assertIn("version does not match Clang's. This may be a problem.", result.stderr)
+
+    def test_rustc_src_core_krustflags(self):
+        result = self.run_script(self.Expected.FAILURE, { "PATH": os.environ["PATH"], "RUSTC": "rustc", "KRUSTFLAGS": f"--sysroot={self.missing}" })
+        self.assertIn("Source code for the 'core' standard library could not be found", result.stderr)
+
+    def test_rustc_src_core_rustlibsrc(self):
+        result = self.run_script(self.Expected.FAILURE, { "RUST_LIB_SRC": self.missing })
+        self.assertIn("Source code for the 'core' standard library could not be found", result.stderr)
+
+    def test_success_cc_unknown(self):
+        result = self.run_script(self.Expected.SUCCESS_WITH_EXTRA_OUTPUT, { "CC": self.missing })
+        self.assertIn("unknown C compiler", result.stderr)
+
+    def test_success_cc_multiple_arguments_ccache(self):
+        clang = self.generate_clang(f"""Ubuntu clang version {self.llvm_default_version}-1ubuntu1
+Target: x86_64-pc-linux-gnu
+Thread model: posix
+InstalledDir: /usr/bin
+""")
+        result = self.run_script(self.Expected.SUCCESS, { "CC": f"{clang} clang" })
+
+    def test_success_rustc_version(self):
+        for rustc_stdout in (
+            f"rustc {self.rustc_default_version} (a8314ef7d 2022-06-27)",
+            f"rustc {self.rustc_default_version}-dev (a8314ef7d 2022-06-27)",
+            f"rustc {self.rustc_default_version}-1.60.0 (a8314ef7d 2022-06-27)",
+        ):
+            with self.subTest(rustc_stdout=rustc_stdout):
+                rustc = self.generate_rustc(rustc_stdout)
+                result = self.run_script(self.Expected.SUCCESS, { "RUSTC": rustc })
+
+    def test_success_bindgen_version(self):
+        for bindgen_stdout in (
+            f"bindgen {self.bindgen_default_version}",
+            f"bindgen {self.bindgen_default_version}-dev",
+            f"bindgen {self.bindgen_default_version}-0.999.0",
+        ):
+            with self.subTest(bindgen_stdout=bindgen_stdout):
+                bindgen = self.generate_bindgen_version(bindgen_stdout)
+                result = self.run_script(self.Expected.SUCCESS, { "BINDGEN": bindgen })
+
+    def test_success_bindgen_libclang(self):
+        for stderr in (
+            f"scripts/rust_is_available_bindgen_libclang.h:2:9: warning: clang version {self.llvm_default_version} (https://github.com/llvm/llvm-project.git 4a2c05b05ed07f1f620e94f6524a8b4b2760a0b1) [-W#pragma-messages], err: false",
+            f"/home/jd/Documents/dev/kernel-module-flake/linux-6.1/outputs/dev/lib/modules/6.1.0-development/source/scripts/rust_is_available_bindgen_libclang.h:2:9: warning: clang version {self.llvm_default_version}  [-W#pragma-messages], err: false",
+            f"scripts/rust_is_available_bindgen_libclang.h:2:9: warning: clang version {self.llvm_default_version} (Fedora 13.0.0-3.fc35) [-W#pragma-messages], err: false",
+            f"""
+/nix/store/dsd5gz46hdbdk2rfdimqddhq6m8m8fqs-bash-5.1-p16/bin/bash: warning: setlocale: LC_ALL: cannot change locale (c)
+scripts/rust_is_available_bindgen_libclang.h:2:9: warning: clang version {self.llvm_default_version}  [-W#pragma-messages], err: false
+""",
+            f"""
+/nix/store/dsd5gz46hdbdk2rfdimqddhq6m8m8fqs-bash-5.1.0-p16/bin/bash: warning: setlocale: LC_ALL: cannot change locale (c)
+/home/jd/Documents/dev/kernel-module-flake/linux-6.1/outputs/dev/lib/modules/6.1.0-development/source/scripts/rust_is_available_bindgen_libclang.h:2:9: warning: clang version {self.llvm_default_version} (Fedora 13.0.0-3.fc35) [-W#pragma-messages], err: false
+"""
+        ):
+            with self.subTest(stderr=stderr):
+                bindgen = self.generate_bindgen_libclang(stderr)
+                result = self.run_script(self.Expected.SUCCESS, { "BINDGEN": bindgen })
+
+    def test_success_clang_version(self):
+        for clang_stdout in (
+            f"clang version {self.llvm_default_version} (https://github.com/llvm/llvm-project.git 4a2c05b05ed07f1f620e94f6524a8b4b2760a0b1)",
+            f"clang version {self.llvm_default_version}-dev",
+            f"clang version {self.llvm_default_version}-2~ubuntu20.04.1",
+            f"Ubuntu clang version {self.llvm_default_version}-2~ubuntu20.04.1",
+        ):
+            with self.subTest(clang_stdout=clang_stdout):
+                clang = self.generate_clang(clang_stdout)
+                result = self.run_script(self.Expected.SUCCESS, { "CC": clang })
+
+    def test_success_real_programs(self):
+        for cc in ["gcc", "clang"]:
+            with self.subTest(cc=cc):
+                result = self.run_script(self.Expected.SUCCESS, {
+                    "PATH": os.environ["PATH"],
+                    "RUSTC": "rustc",
+                    "BINDGEN": "bindgen",
+                    "CC": cc,
+                })
+
+if __name__ == "__main__":
+    unittest.main()
-- 
2.41.0

Re: [PATCH v2 00/11] `scripts/rust_is_available.sh` improvements

[Heghedus Razvan]

On Fri Jun 16, 2023 at 3:16 AM EEST, Miguel Ojeda wrote:
> This is the patch series to improve `scripts/rust_is_available.sh`.
>
> The major addition in v2 is the test suite in the last commit. I added
> it because I wanted to have a proper way to test any further changes to
> it (such as the suggested `set --` idea to avoid forking by Masahiro),
> and so that adding new checks was easier to justify too (i.e. vs. the
> added complexity).
>
> In addition, there are also a few new checks in the script, to cover for
> even some more cases, which hopefully make problematic setups easier to
> identify and solve by users building the kernel. For instance, running
> the script externally gives:
>
>     $ scripts/rust_is_available.sh
>     ***
>     *** Environment variable 'RUSTC' is not set.
>     ***
>     *** This script is intended to be called from Kbuild.
>     *** Please use the 'rustavailable' target to call it instead.
>     *** Otherwise, the results may not be meaningful.
>     ***
>     *** Please see Documentation/rust/quick-start.rst for details
>     *** on how to set up the Rust support.
>     ***
>
> I also changed it to avoid setting `-e` as Masahiro suggested.
> Similarly, I now check for `$RUSTC`, `$BINDGEN` and `$CC`, instead of
> `$MAKEFLAGS`, as he also suggested (but I gave it their own error
> message rather than use the `${CC?: is not set}` approach. This goes in
> line with the reasons outlined above, i.e. trying to give users a clear
> error of what step exactly failed).
>
> In the test suite I included previously problematic compiler version
> strings we got reports for. The test suite covers all current branches
> in the script, and we should keep it that way in the future.
>
> The patch series also include Masahiro's patch to remove the `-v`
> option, as well as Russell's patch for supporting multiple arguments
> in `$CC`.
>
> All in all, this should solve all the issues we got so far (unless I
> have missed something) and improve things further with the new checks
> plus the test suite to hopefully have an easier time in the future.
>
> Testers for this one are appreciated, especially if you have uncommon or
> custom setups for building the kernel.

I gave this patch series a spin and is a nice improvement when trying to
use a broken setup.

When previously I had:
./scripts/rust_is_available.sh: line 21: 100000 *  + 100 *  + : syntax error: operand expected (error token is "+ ")
make: *** [Makefile:1883: rustavailable] Error 1

Now I have:

***
*** Running 'bindgen' to check the libclang version (used by the Rust
*** bindings generator) failed with code 101. This may be caused by
*** a failure to locate libclang. See output and docs below for details:
***
thread 'main' panicked at 'Unable to find libclang: "the `libclang` shared library at /opt/sdk/sysroots/x86_64-pokysdk-linux/usr/lib/libclang.so.14.0.3 could not be opened: /lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.34' not found (required by /opt/idc23/sysroots/x86_64-pokysdk-linux/usr/lib/libclang.so.14.0.3)"', /home/heghedusrazvan/.cargo/registry/src/github.com-1ecc6299db9ec823/bindgen-0.56.0/src/lib.rs:1922:31
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
***
***
*** Please see Documentation/rust/quick-start.rst for details
*** on how to set up the Rust support.
***
make: *** [Makefile:1883: rustavailable] Error 1

or:

***
*** Rust compiler 'rustc' is too new. This may or may not work.
***   Your version:     1.70.0
***   Expected version: 1.68.2
***
***
*** Running 'bindgen' to check the libclang version (used by the Rust
*** bindings generator) failed with code 101. This may be caused by
*** a failure to locate libclang. See output and docs below for details:
***
thread 'main' panicked at 'Unable to find libclang: "the `libclang` shared library at /opt/sdk/sysroots/x86_64-pokysdk-linux/usr/lib/libclang.so.14.0.3 could not be opened: libncurses.so.5: cannot open shared object file: No such file or directory"', /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bindgen-0.56.0/src/lib.rs:1922:31
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
***
***
*** Please see Documentation/rust/quick-start.rst for details
*** on how to set up the Rust support.
***
make: *** [Makefile:1883: rustavailable] Error 1

-- Razvan
>
> This could go through either the Kbuild or the Rust tree.
>
> Masahiro Yamada (1):
>   kbuild: rust_is_available: remove -v option
>
> Miguel Ojeda (9):
>   docs: rust: add paragraph about finding a suitable `libclang`
>   kbuild: rust_is_available: print docs reference
>   kbuild: rust_is_available: add check for `bindgen` invocation
>   kbuild: rust_is_available: check that environment variables are set
>   kbuild: rust_is_available: fix confusion when a version appears in the
>     path
>   kbuild: rust_is_available: normalize version matching
>   kbuild: rust_is_available: handle failures calling `$RUSTC`/`$BINDGEN`
>   kbuild: rust_is_available: check that output looks as expected
>   kbuild: rust_is_available: add test suite
>
> Russell Currey (1):
>   kbuild: rust_is_available: fix version check when CC has multiple
>     arguments
>
>  Documentation/rust/quick-start.rst |  17 ++
>  Makefile                           |   4 +-
>  scripts/rust_is_available.sh       | 233 +++++++++++++------
>  scripts/rust_is_available_test.py  | 346 +++++++++++++++++++++++++++++
>  4 files changed, 532 insertions(+), 68 deletions(-)
>  create mode 100755 scripts/rust_is_available_test.py
>
>
> base-commit: 858fd168a95c5b9669aac8db6c14a9aeab446375
> --
> 2.41.0

Re: [PATCH v2 01/11] kbuild: rust_is_available: remove -v option

[Martin Rodriguez Reboredo]

On 6/15/23 21:16, Miguel Ojeda wrote:
> From: Masahiro Yamada <masahiroy@kernel.org>
> 
> The -v option is passed when this script is invoked from Makefile,
> but not when invoked from Kconfig.
> 
> As you can see in scripts/Kconfig.include, the 'success' macro suppresses
> stdout and stderr anyway, so this script does not need to be quiet.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> Reviewed-by: Miguel Ojeda <ojeda@kernel.org>
> Tested-by: Miguel Ojeda <ojeda@kernel.org>
> Reviewed-by: Nathan Chancellor <nathan@kernel.org>
> Link: https://lore.kernel.org/r/20230109061436.3146442-1-masahiroy@kernel.org
> [ Reworded prefix to match the others in the patch series. ]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---
> 

Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>

Re: [PATCH v2 02/11] kbuild: rust_is_available: fix version check when CC has multiple arguments

[Martin Rodriguez Reboredo]

On 6/15/23 21:16, Miguel Ojeda wrote:
> From: Russell Currey <ruscur@russell.cc>
> 
> rust_is_available.sh uses cc-version.sh to identify which C compiler is
> in use, as scripts/Kconfig.include does.  cc-version.sh isn't designed to
> be able to handle multiple arguments in one variable, i.e. "ccache clang".
> Its invocation in rust_is_available.sh quotes "$CC", which makes
> $1 == "ccache clang" instead of the intended $1 == ccache & $2 == clang.
> 
> cc-version.sh could also be changed to handle having "ccache clang" as one
> argument, but it only has the one consumer upstream, making it simpler to
> fix the caller here.
> 
> Signed-off-by: Russell Currey <ruscur@russell.cc>
> Fixes: 78521f3399ab ("scripts: add `rust_is_available.sh`")
> Link: https://github.com/Rust-for-Linux/linux/pull/873
> [ Reworded title prefix and reflow line to 75 columns. ]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---
> [...]

Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>

Re: [PATCH v2 03/11] docs: rust: add paragraph about finding a suitable `libclang`

[Martin Rodriguez Reboredo]

On 6/15/23 21:16, Miguel Ojeda wrote:
> Sometimes users need to tweak the finding process of `libclang`
> for `bindgen` via the `clang-sys`-provided environment variables.
> 
> Thus add a paragraph to the setting up guide, including a reference
> to `clang-sys`'s relevant documentation.
> 
> Link: https://lore.kernel.org/rust-for-linux/CAKwvOdm5JT4wbdQQYuW+RT07rCi6whGBM2iUAyg8A1CmLXG6Nw@mail.gmail.com/
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---
> [...]

Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>

Re: [PATCH v2 04/11] kbuild: rust_is_available: print docs reference

[Martin Rodriguez Reboredo]

On 6/15/23 21:16, Miguel Ojeda wrote:
> People trying out the Rust support in the kernel may get
> warnings and errors from `scripts/rust_is_available.sh`
> from the `rustavailable` target or the build step.
> 
> Some of those users may be following the Quick Start guide,
> but others may not (likely those getting warnings from
> the build step instead of the target).
> 
> While the messages are fairly clear on what the problem is,
> it may not be clear how to solve the particular issue,
> especially for those not aware of the documentation.
> 
> We could add all sorts of details on the script for each one,
> but it is better to point users to the documentation instead,
> where it is easily readable in different formats. It also
> avoids duplication.
> 
> Thus add a reference to the documentation whenever the script
> fails or there is at least a warning.
> 
> Reviewed-by: Finn Behrens <fin@nyantec.com>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---
> [...]

Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>

Re: [PATCH v2 05/11] kbuild: rust_is_available: add check for `bindgen` invocation

[Martin Rodriguez Reboredo]

On 6/15/23 21:16, Miguel Ojeda wrote:
> `scripts/rust_is_available.sh` calls `bindgen` with a special
> header in order to check whether the `libclang` version in use
> is suitable.
> 
> However, the invocation itself may fail if, for instance, `bindgen`
> cannot locate `libclang`. This is fine for Kconfig (since the
> script will still fail and therefore disable Rust as it should),
> but it is pretty confusing for users of the `rustavailable` target
> given the error will be unrelated:
> 
>      ./scripts/rust_is_available.sh: 21: arithmetic expression: expecting primary: "100000 *  + 100 *  + "
>      make: *** [Makefile:1816: rustavailable] Error 2
> 
> Instead, run the `bindgen` invocation independently in a previous
> step, saving its output and return code. If it fails, then show
> the user a proper error message. Otherwise, continue as usual
> with the saved output.
> 
> Since the previous patch we show a reference to the docs, and
> the docs now explain how `bindgen` looks for `libclang`,
> thus the error message can leverage the documentation, avoiding
> duplication here (and making users aware of the setup guide in
> the documentation).
> 
> Reported-by: Nick Desaulniers <ndesaulniers@google.com>
> Link: https://lore.kernel.org/rust-for-linux/CAKwvOdm5JT4wbdQQYuW+RT07rCi6whGBM2iUAyg8A1CmLXG6Nw@mail.gmail.com/
> Reported-by: François Valenduc <francoisvalenduc@gmail.com>
> Closes: https://github.com/Rust-for-Linux/linux/issues/934
> Reported-by: Alexandru Radovici <msg4alex@gmail.com>
> Closes: https://github.com/Rust-for-Linux/linux/pull/921
> Reported-by: Matthew Leach <dev@mattleach.net>
> Closes: https://lore.kernel.org/rust-for-linux/20230507084116.1099067-1-dev@mattleach.net/
> Fixes: 78521f3399ab ("scripts: add `rust_is_available.sh`")
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---
> [...]

Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>

Re: [PATCH v2 06/11] kbuild: rust_is_available: check that environment variables are set

[Martin Rodriguez Reboredo]

On 6/15/23 21:16, Miguel Ojeda wrote:
> Sometimes [1] users may attempt to setup the Rust support by
> checking what Kbuild does and they end up finding out about
> `scripts/rust_is_available.sh`. Inevitably, they run the script
> directly, but unless they setup the required variables,
> the result of the script is not meaningful.
> 
> We could add some defaults to the variables, but that could be
> confusing for those that may override the defaults (compared
> to their kernel builds), and `$CC` would not be a simple default
> in any case.
> 
> Therefore, instead, explicitly check whether the expected variables
> are set (`$RUSTC`, `$BINDGEN` and `$CC`). If not, print an explanation
> about the fact that the script is meant to be called from Kbuild,
> since that is the most likely cause for the variables not being set.
> 
> Link: https://lore.kernel.org/oe-kbuild-all/Y6r4mXz5NS0+HVXo@zn.tnic/ [1]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---
> [...]

Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>

Re: [PATCH v2 07/11] kbuild: rust_is_available: fix confusion when a version appears in the path

[Martin Rodriguez Reboredo]

On 6/15/23 21:16, Miguel Ojeda wrote:
> `bindgen`'s output for `libclang`'s version check contains paths, which
> in turn may contain strings that look like version numbers [1][2]:
> 
>      .../6.1.0-dev/.../rust_is_available_bindgen_libclang.h:2:9: warning: clang version 11.1.0  [-W#pragma-messages], err: false
> 
> which the script will pick up as the version instead of the latter.
> 
> It is also the case that versions may appear after the actual version
> (e.g. distribution's version text), which was the reason behind `head` [3]:
> 
>      .../rust-is-available-bindgen-libclang.h:2:9: warning: clang version 13.0.0 (Fedora 13.0.0-3.fc35) [-W#pragma-messages], err: false
> 
> Thus instead ask for a match after the `clang version` string.
> 
> Reported-by: Jordan Isaacs <mail@jdisaacs.com>
> Closes: https://github.com/Rust-for-Linux/linux/issues/942 [1]
> Reported-by: Ethan D. Twardy <ethan.twardy@gmail.com>
> Closes: https://lore.kernel.org/rust-for-linux/20230528131802.6390-2-ethan.twardy@gmail.com/ [2]
> Reported-by: Tiago Lam <tiagolam@gmail.com>
> Closes: https://github.com/Rust-for-Linux/linux/pull/789 [3]
> Fixes: 78521f3399ab ("scripts: add `rust_is_available.sh`")
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---
> [...]

Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>

Re: [PATCH v2 08/11] kbuild: rust_is_available: normalize version matching

[Martin Rodriguez Reboredo]

On 6/15/23 21:16, Miguel Ojeda wrote:
> In order to match the version string, `sed` is used in a couple
> cases, and `grep` and `head` in a couple others.
> 
> Make the script more consistent and easier to understand by
> using the same method, `sed`, for all of them.
> 
> This makes the version matching also a bit more strict for
> the changed cases, since the strings `rustc ` and `bindgen `
> will now be required, which should be fine since `rustc`
> complains if one attempts to call it with another program
> name, and `bindgen` uses a hardcoded string.
> 
> In addition, clarify why one of the existing `sed` commands
> does not provide an address like the others.
> 
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---
> [...]

Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>

Re: [PATCH v2 09/11] kbuild: rust_is_available: handle failures calling `$RUSTC`/`$BINDGEN`

[Martin Rodriguez Reboredo]

On 6/15/23 21:16, Miguel Ojeda wrote:
> The script already checks if `$RUSTC` and `$BINDGEN` exists via
> `command`, but the environment variables may point to a
> non-executable file, or the programs may fail for some other reason.
> While the script successfully exits with a failure as it should,
> the error given can be quite confusing depending on the shell and
> the behavior of its `command`. For instance, with `dash`:
> 
>      $ RUSTC=./mm BINDGEN=bindgen CC=clang scripts/rust_is_available.sh
>      scripts/rust_is_available.sh: 19: arithmetic expression: expecting primary: "100000 *  + 100 *  + "
> 
> Thus detect failure exit codes when calling `$RUSTC` and `$BINDGEN` and
> print a better message, in a similar way to what we do when extracting
> the `libclang` version found by `bindgen`.
> 
> Link: https://lore.kernel.org/rust-for-linux/CAK7LNAQYk6s11MASRHW6oxtkqF00EJVqhHOP=5rynWt-QDUsXw@mail.gmail.com/
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---
> [...]

Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>

Re: [PATCH v2 10/11] kbuild: rust_is_available: check that output looks as expected

[Martin Rodriguez Reboredo]

On 6/15/23 21:16, Miguel Ojeda wrote:
> The script already checks for `$RUSTC` and `$BINDGEN` existing
> and exiting without failure. However, one may still pass an
> unexpected binary that does not output what the later parsing
> expects. The script still successfully reports a failure as
> expected, but the error is confusing. For instance:
> 
>      $ RUSTC=true BINDGEN=bindgen CC=clang scripts/rust_is_available.sh
>      scripts/rust_is_available.sh: 19: arithmetic expression: expecting primary: "100000 *  + 100 *  + "
>      ***
>      *** Please see Documentation/rust/quick-start.rst for details
>      *** on how to set up the Rust support.
>      ***
> 
> Thus add an explicit check and a proper message for unexpected
> output from the called command.
> 
> Similarly, do so for the `libclang` version parsing, too.
> 
> Link: https://lore.kernel.org/rust-for-linux/CAK7LNAQYk6s11MASRHW6oxtkqF00EJVqhHOP=5rynWt-QDUsXw@mail.gmail.com/
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---
> [...]

Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>

Re: [PATCH v2 11/11] kbuild: rust_is_available: add test suite

[Martin Rodriguez Reboredo]

On 6/15/23 21:16, Miguel Ojeda wrote:
> The `rust_is_available.sh` script runs for everybody compiling the
> kernel, even if not using Rust. Therefore, it is important to ensure
> that the script is correct to avoid breaking people's compilation.
> 
> In addition, the script needs to be able to handle a set of subtle
> cases, including parsing version strings of different tools.
> 
> Therefore, maintenance of this script can be greatly eased with
> a set of tests.
> 
> Thus add a test suite to cover hopefully most of the setups that
> the script may encounter in the wild. Extra setups can be easily
> added later on if missing.
> 
> The script currently covers all the branches of the shell script,
> including several ways in which they may be entered.
> 
> Python is used for this script, since the script under test
> does not depend on Rust, thus hopefully making it easier for others
> to use if the need arises.
> 
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---
> [...]

Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>

Re: [PATCH v2 02/11] kbuild: rust_is_available: fix version check when CC has multiple arguments

[Nathan Chancellor]

On Fri, Jun 16, 2023 at 02:16:22AM +0200, Miguel Ojeda wrote:
> From: Russell Currey <ruscur@russell.cc>
> 
> rust_is_available.sh uses cc-version.sh to identify which C compiler is
> in use, as scripts/Kconfig.include does.  cc-version.sh isn't designed to
> be able to handle multiple arguments in one variable, i.e. "ccache clang".
> Its invocation in rust_is_available.sh quotes "$CC", which makes
> $1 == "ccache clang" instead of the intended $1 == ccache & $2 == clang.
> 
> cc-version.sh could also be changed to handle having "ccache clang" as one
> argument, but it only has the one consumer upstream, making it simpler to
> fix the caller here.
> 
> Signed-off-by: Russell Currey <ruscur@russell.cc>
> Fixes: 78521f3399ab ("scripts: add `rust_is_available.sh`")
> Link: https://github.com/Rust-for-Linux/linux/pull/873
> [ Reworded title prefix and reflow line to 75 columns. ]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  scripts/rust_is_available.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/rust_is_available.sh b/scripts/rust_is_available.sh
> index f43a010eaf30..0c9be438e4cd 100755
> --- a/scripts/rust_is_available.sh
> +++ b/scripts/rust_is_available.sh
> @@ -113,10 +113,10 @@ fi
>  #
>  # In the future, we might be able to perform a full version check, see
>  # https://github.com/rust-lang/rust-bindgen/issues/2138.
> -cc_name=$($(dirname $0)/cc-version.sh "$CC" | cut -f1 -d' ')
> +cc_name=$($(dirname $0)/cc-version.sh $CC | cut -f1 -d' ')
>  if [ "$cc_name" = Clang ]; then
>  	clang_version=$( \
> -		LC_ALL=C "$CC" --version 2>/dev/null \
> +		LC_ALL=C $CC --version 2>/dev/null \
>  			| sed -nE '1s:.*version ([0-9]+\.[0-9]+\.[0-9]+).*:\1:p'
>  	)
>  	if [ "$clang_version" != "$bindgen_libclang_version" ]; then
> -- 
> 2.41.0
> 

Re: [PATCH v2 03/11] docs: rust: add paragraph about finding a suitable `libclang`

[Nathan Chancellor]

On Fri, Jun 16, 2023 at 02:16:23AM +0200, Miguel Ojeda wrote:
> Sometimes users need to tweak the finding process of `libclang`
> for `bindgen` via the `clang-sys`-provided environment variables.
> 
> Thus add a paragraph to the setting up guide, including a reference
> to `clang-sys`'s relevant documentation.
> 
> Link: https://lore.kernel.org/rust-for-linux/CAKwvOdm5JT4wbdQQYuW+RT07rCi6whGBM2iUAyg8A1CmLXG6Nw@mail.gmail.com/
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  Documentation/rust/quick-start.rst | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/Documentation/rust/quick-start.rst b/Documentation/rust/quick-start.rst
> index 13b7744b1e27..a635be69e062 100644
> --- a/Documentation/rust/quick-start.rst
> +++ b/Documentation/rust/quick-start.rst
> @@ -100,6 +100,23 @@ Install it via (note that this will download and build the tool from source)::
>  
>  	cargo install --locked --version $(scripts/min-tool-version.sh bindgen) bindgen
>  
> +``bindgen`` needs to find a suitable ``libclang`` in order to work. If it is
> +not found (or a different ``libclang`` than the one found should be used),
> +the process can be tweaked using the environment variables understood by
> +``clang-sys`` (the Rust bindings crate that ``bindgen`` uses to access
> +``libclang``):
> +
> +* ``LLVM_CONFIG_PATH`` can be pointed to an ``llvm-config`` executable.
> +
> +* Or ``LIBCLANG_PATH`` can be pointed to a ``libclang`` shared library
> +  or to the directory containing it.
> +
> +* Or ``CLANG_PATH`` can be pointed to a ``clang`` executable.
> +
> +For details, please see ``clang-sys``'s documentation at:
> +
> +	https://github.com/KyleMayes/clang-sys#environment-variables
> +
>  
>  Requirements: Developing
>  ------------------------
> -- 
> 2.41.0
> 

Re: [PATCH v2 04/11] kbuild: rust_is_available: print docs reference

[Nathan Chancellor]

On Fri, Jun 16, 2023 at 02:16:24AM +0200, Miguel Ojeda wrote:
> People trying out the Rust support in the kernel may get
> warnings and errors from `scripts/rust_is_available.sh`
> from the `rustavailable` target or the build step.
> 
> Some of those users may be following the Quick Start guide,
> but others may not (likely those getting warnings from
> the build step instead of the target).
> 
> While the messages are fairly clear on what the problem is,
> it may not be clear how to solve the particular issue,
> especially for those not aware of the documentation.
> 
> We could add all sorts of details on the script for each one,
> but it is better to point users to the documentation instead,
> where it is easily readable in different formats. It also
> avoids duplication.
> 
> Thus add a reference to the documentation whenever the script
> fails or there is at least a warning.
> 
> Reviewed-by: Finn Behrens <fin@nyantec.com>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  scripts/rust_is_available.sh | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/scripts/rust_is_available.sh b/scripts/rust_is_available.sh
> index 0c9be438e4cd..6b8131d5b547 100755
> --- a/scripts/rust_is_available.sh
> +++ b/scripts/rust_is_available.sh
> @@ -19,6 +19,20 @@ get_canonical_version()
>  	echo $((100000 * $1 + 100 * $2 + $3))
>  }
>  
> +# Print a reference to the Quick Start guide in the documentation.
> +print_docs_reference()
> +{
> +	echo >&2 "***"
> +	echo >&2 "*** Please see Documentation/rust/quick-start.rst for details"
> +	echo >&2 "*** on how to set up the Rust support."
> +	echo >&2 "***"
> +}
> +
> +# If the script fails for any reason, or if there was any warning, then
> +# print a reference to the documentation on exit.
> +warning=0
> +trap 'if [ $? -ne 0 ] || [ $warning -ne 0 ]; then print_docs_reference; fi' EXIT
> +
>  # Check that the Rust compiler exists.
>  if ! command -v "$RUSTC" >/dev/null; then
>  	echo >&2 "***"
> @@ -60,6 +74,7 @@ if [ "$rust_compiler_cversion" -gt "$rust_compiler_min_cversion" ]; then
>  	echo >&2 "***   Your version:     $rust_compiler_version"
>  	echo >&2 "***   Expected version: $rust_compiler_min_version"
>  	echo >&2 "***"
> +	warning=1
>  fi
>  
>  # Check that the Rust bindings generator is suitable.
> @@ -87,6 +102,7 @@ if [ "$rust_bindings_generator_cversion" -gt "$rust_bindings_generator_min_cvers
>  	echo >&2 "***   Your version:     $rust_bindings_generator_version"
>  	echo >&2 "***   Expected version: $rust_bindings_generator_min_version"
>  	echo >&2 "***"
> +	warning=1
>  fi
>  
>  # Check that the `libclang` used by the Rust bindings generator is suitable.
> @@ -126,6 +142,7 @@ if [ "$cc_name" = Clang ]; then
>  		echo >&2 "***   libclang version: $bindgen_libclang_version"
>  		echo >&2 "***   Clang version:    $clang_version"
>  		echo >&2 "***"
> +		warning=1
>  	fi
>  fi
>  
> -- 
> 2.41.0
> 

Re: [PATCH v2 05/11] kbuild: rust_is_available: add check for `bindgen` invocation

[Nathan Chancellor]

On Fri, Jun 16, 2023 at 02:16:25AM +0200, Miguel Ojeda wrote:
> `scripts/rust_is_available.sh` calls `bindgen` with a special
> header in order to check whether the `libclang` version in use
> is suitable.
> 
> However, the invocation itself may fail if, for instance, `bindgen`
> cannot locate `libclang`. This is fine for Kconfig (since the
> script will still fail and therefore disable Rust as it should),
> but it is pretty confusing for users of the `rustavailable` target
> given the error will be unrelated:
> 
>     ./scripts/rust_is_available.sh: 21: arithmetic expression: expecting primary: "100000 *  + 100 *  + "
>     make: *** [Makefile:1816: rustavailable] Error 2
> 
> Instead, run the `bindgen` invocation independently in a previous
> step, saving its output and return code. If it fails, then show
> the user a proper error message. Otherwise, continue as usual
> with the saved output.
> 
> Since the previous patch we show a reference to the docs, and
> the docs now explain how `bindgen` looks for `libclang`,
> thus the error message can leverage the documentation, avoiding
> duplication here (and making users aware of the setup guide in
> the documentation).
> 
> Reported-by: Nick Desaulniers <ndesaulniers@google.com>
> Link: https://lore.kernel.org/rust-for-linux/CAKwvOdm5JT4wbdQQYuW+RT07rCi6whGBM2iUAyg8A1CmLXG6Nw@mail.gmail.com/
> Reported-by: François Valenduc <francoisvalenduc@gmail.com>
> Closes: https://github.com/Rust-for-Linux/linux/issues/934
> Reported-by: Alexandru Radovici <msg4alex@gmail.com>
> Closes: https://github.com/Rust-for-Linux/linux/pull/921
> Reported-by: Matthew Leach <dev@mattleach.net>
> Closes: https://lore.kernel.org/rust-for-linux/20230507084116.1099067-1-dev@mattleach.net/
> Fixes: 78521f3399ab ("scripts: add `rust_is_available.sh`")
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  scripts/rust_is_available.sh | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/rust_is_available.sh b/scripts/rust_is_available.sh
> index 6b8131d5b547..1bdff4472cbe 100755
> --- a/scripts/rust_is_available.sh
> +++ b/scripts/rust_is_available.sh
> @@ -106,8 +106,28 @@ if [ "$rust_bindings_generator_cversion" -gt "$rust_bindings_generator_min_cvers
>  fi
>  
>  # Check that the `libclang` used by the Rust bindings generator is suitable.
> +#
> +# In order to do that, first invoke `bindgen` to get the `libclang` version
> +# found by `bindgen`. This step may already fail if, for instance, `libclang`
> +# is not found, thus inform the user in such a case.
> +bindgen_libclang_output=$( \
> +	LC_ALL=C "$BINDGEN" $(dirname $0)/rust_is_available_bindgen_libclang.h 2>&1 >/dev/null
> +) || bindgen_libclang_code=$?
> +if [ -n "$bindgen_libclang_code" ]; then
> +	echo >&2 "***"
> +	echo >&2 "*** Running '$BINDGEN' to check the libclang version (used by the Rust"
> +	echo >&2 "*** bindings generator) failed with code $bindgen_libclang_code. This may be caused by"
> +	echo >&2 "*** a failure to locate libclang. See output and docs below for details:"
> +	echo >&2 "***"
> +	echo >&2 "$bindgen_libclang_output"
> +	echo >&2 "***"
> +	exit 1
> +fi
> +
> +# `bindgen` returned successfully, thus use the output to check that the version
> +# of the `libclang` found by the Rust bindings generator is suitable.
>  bindgen_libclang_version=$( \
> -	LC_ALL=C "$BINDGEN" $(dirname $0)/rust_is_available_bindgen_libclang.h 2>&1 >/dev/null \
> +	echo "$bindgen_libclang_output" \
>  		| grep -F 'clang version ' \
>  		| grep -oE '[0-9]+\.[0-9]+\.[0-9]+' \
>  		| head -n 1 \
> -- 
> 2.41.0
> 

Re: [PATCH v2 06/11] kbuild: rust_is_available: check that environment variables are set

[Nathan Chancellor]

On Fri, Jun 16, 2023 at 02:16:26AM +0200, Miguel Ojeda wrote:
> Sometimes [1] users may attempt to setup the Rust support by
> checking what Kbuild does and they end up finding out about
> `scripts/rust_is_available.sh`. Inevitably, they run the script
> directly, but unless they setup the required variables,
> the result of the script is not meaningful.
> 
> We could add some defaults to the variables, but that could be
> confusing for those that may override the defaults (compared
> to their kernel builds), and `$CC` would not be a simple default
> in any case.
> 
> Therefore, instead, explicitly check whether the expected variables
> are set (`$RUSTC`, `$BINDGEN` and `$CC`). If not, print an explanation
> about the fact that the script is meant to be called from Kbuild,
> since that is the most likely cause for the variables not being set.
> 
> Link: https://lore.kernel.org/oe-kbuild-all/Y6r4mXz5NS0+HVXo@zn.tnic/ [1]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  scripts/rust_is_available.sh | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/scripts/rust_is_available.sh b/scripts/rust_is_available.sh
> index 1bdff4472cbe..7e0368babe64 100755
> --- a/scripts/rust_is_available.sh
> +++ b/scripts/rust_is_available.sh
> @@ -28,11 +28,40 @@ print_docs_reference()
>  	echo >&2 "***"
>  }
>  
> +# Print an explanation about the fact that the script is meant to be called from Kbuild.
> +print_kbuild_explanation()
> +{
> +	echo >&2 "***"
> +	echo >&2 "*** This script is intended to be called from Kbuild."
> +	echo >&2 "*** Please use the 'rustavailable' target to call it instead."
> +	echo >&2 "*** Otherwise, the results may not be meaningful."
> +	exit 1
> +}
> +
>  # If the script fails for any reason, or if there was any warning, then
>  # print a reference to the documentation on exit.
>  warning=0
>  trap 'if [ $? -ne 0 ] || [ $warning -ne 0 ]; then print_docs_reference; fi' EXIT
>  
> +# Check that the expected environment variables are set.
> +if [ -z "${RUSTC+x}" ]; then
> +	echo >&2 "***"
> +	echo >&2 "*** Environment variable 'RUSTC' is not set."
> +	print_kbuild_explanation
> +fi
> +
> +if [ -z "${BINDGEN+x}" ]; then
> +	echo >&2 "***"
> +	echo >&2 "*** Environment variable 'BINDGEN' is not set."
> +	print_kbuild_explanation
> +fi
> +
> +if [ -z "${CC+x}" ]; then
> +	echo >&2 "***"
> +	echo >&2 "*** Environment variable 'CC' is not set."
> +	print_kbuild_explanation
> +fi
> +
>  # Check that the Rust compiler exists.
>  if ! command -v "$RUSTC" >/dev/null; then
>  	echo >&2 "***"
> -- 
> 2.41.0
> 

Re: [PATCH v2 07/11] kbuild: rust_is_available: fix confusion when a version appears in the path

[Nathan Chancellor]

On Fri, Jun 16, 2023 at 02:16:27AM +0200, Miguel Ojeda wrote:
> `bindgen`'s output for `libclang`'s version check contains paths, which
> in turn may contain strings that look like version numbers [1][2]:
> 
>     .../6.1.0-dev/.../rust_is_available_bindgen_libclang.h:2:9: warning: clang version 11.1.0  [-W#pragma-messages], err: false
> 
> which the script will pick up as the version instead of the latter.
> 
> It is also the case that versions may appear after the actual version
> (e.g. distribution's version text), which was the reason behind `head` [3]:
> 
>     .../rust-is-available-bindgen-libclang.h:2:9: warning: clang version 13.0.0 (Fedora 13.0.0-3.fc35) [-W#pragma-messages], err: false
> 
> Thus instead ask for a match after the `clang version` string.
> 
> Reported-by: Jordan Isaacs <mail@jdisaacs.com>
> Closes: https://github.com/Rust-for-Linux/linux/issues/942 [1]
> Reported-by: Ethan D. Twardy <ethan.twardy@gmail.com>
> Closes: https://lore.kernel.org/rust-for-linux/20230528131802.6390-2-ethan.twardy@gmail.com/ [2]
> Reported-by: Tiago Lam <tiagolam@gmail.com>
> Closes: https://github.com/Rust-for-Linux/linux/pull/789 [3]
> Fixes: 78521f3399ab ("scripts: add `rust_is_available.sh`")
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  scripts/rust_is_available.sh | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/scripts/rust_is_available.sh b/scripts/rust_is_available.sh
> index 7e0368babe64..810691af66eb 100755
> --- a/scripts/rust_is_available.sh
> +++ b/scripts/rust_is_available.sh
> @@ -157,9 +157,7 @@ fi
>  # of the `libclang` found by the Rust bindings generator is suitable.
>  bindgen_libclang_version=$( \
>  	echo "$bindgen_libclang_output" \
> -		| grep -F 'clang version ' \
> -		| grep -oE '[0-9]+\.[0-9]+\.[0-9]+' \
> -		| head -n 1 \
> +		| sed -nE 's:.*clang version ([0-9]+\.[0-9]+\.[0-9]+).*:\1:p'
>  )
>  bindgen_libclang_min_version=$($min_tool_version llvm)
>  bindgen_libclang_cversion=$(get_canonical_version $bindgen_libclang_version)
> -- 
> 2.41.0
> 

Re: [PATCH v2 08/11] kbuild: rust_is_available: normalize version matching

[Nathan Chancellor]

On Fri, Jun 16, 2023 at 02:16:28AM +0200, Miguel Ojeda wrote:
> In order to match the version string, `sed` is used in a couple
> cases, and `grep` and `head` in a couple others.
> 
> Make the script more consistent and easier to understand by
> using the same method, `sed`, for all of them.
> 
> This makes the version matching also a bit more strict for
> the changed cases, since the strings `rustc ` and `bindgen `
> will now be required, which should be fine since `rustc`
> complains if one attempts to call it with another program
> name, and `bindgen` uses a hardcoded string.
> 
> In addition, clarify why one of the existing `sed` commands
> does not provide an address like the others.
> 
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  scripts/rust_is_available.sh | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/scripts/rust_is_available.sh b/scripts/rust_is_available.sh
> index 810691af66eb..b7e0781fdea9 100755
> --- a/scripts/rust_is_available.sh
> +++ b/scripts/rust_is_available.sh
> @@ -83,8 +83,7 @@ fi
>  # Non-stable and distributions' versions may have a version suffix, e.g. `-dev`.
>  rust_compiler_version=$( \
>  	LC_ALL=C "$RUSTC" --version 2>/dev/null \
> -		| head -n 1 \
> -		| grep -oE '[0-9]+\.[0-9]+\.[0-9]+' \
> +		| sed -nE '1s:.*rustc ([0-9]+\.[0-9]+\.[0-9]+).*:\1:p'
>  )
>  rust_compiler_min_version=$($min_tool_version rustc)
>  rust_compiler_cversion=$(get_canonical_version $rust_compiler_version)
> @@ -111,8 +110,7 @@ fi
>  # Non-stable and distributions' versions may have a version suffix, e.g. `-dev`.
>  rust_bindings_generator_version=$( \
>  	LC_ALL=C "$BINDGEN" --version 2>/dev/null \
> -		| head -n 1 \
> -		| grep -oE '[0-9]+\.[0-9]+\.[0-9]+' \
> +		| sed -nE '1s:.*bindgen ([0-9]+\.[0-9]+\.[0-9]+).*:\1:p'
>  )
>  rust_bindings_generator_min_version=$($min_tool_version bindgen)
>  rust_bindings_generator_cversion=$(get_canonical_version $rust_bindings_generator_version)
> @@ -155,6 +153,9 @@ fi
>  
>  # `bindgen` returned successfully, thus use the output to check that the version
>  # of the `libclang` found by the Rust bindings generator is suitable.
> +#
> +# Unlike other version checks, note that this one does not necessarily appear
> +# in the first line of the output, thus no `sed` address is provided.
>  bindgen_libclang_version=$( \
>  	echo "$bindgen_libclang_output" \
>  		| sed -nE 's:.*clang version ([0-9]+\.[0-9]+\.[0-9]+).*:\1:p'
> -- 
> 2.41.0
> 

Re: [PATCH v2 09/11] kbuild: rust_is_available: handle failures calling `$RUSTC`/`$BINDGEN`

[Nathan Chancellor]

On Fri, Jun 16, 2023 at 02:16:29AM +0200, Miguel Ojeda wrote:
> The script already checks if `$RUSTC` and `$BINDGEN` exists via
> `command`, but the environment variables may point to a
> non-executable file, or the programs may fail for some other reason.
> While the script successfully exits with a failure as it should,
> the error given can be quite confusing depending on the shell and
> the behavior of its `command`. For instance, with `dash`:
> 
>     $ RUSTC=./mm BINDGEN=bindgen CC=clang scripts/rust_is_available.sh
>     scripts/rust_is_available.sh: 19: arithmetic expression: expecting primary: "100000 *  + 100 *  + "
> 
> Thus detect failure exit codes when calling `$RUSTC` and `$BINDGEN` and
> print a better message, in a similar way to what we do when extracting
> the `libclang` version found by `bindgen`.
> 
> Link: https://lore.kernel.org/rust-for-linux/CAK7LNAQYk6s11MASRHW6oxtkqF00EJVqhHOP=5rynWt-QDUsXw@mail.gmail.com/
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  scripts/rust_is_available.sh | 28 ++++++++++++++++++++++++++--
>  1 file changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/rust_is_available.sh b/scripts/rust_is_available.sh
> index b7e0781fdea9..da8296cd9b8d 100755
> --- a/scripts/rust_is_available.sh
> +++ b/scripts/rust_is_available.sh
> @@ -81,8 +81,20 @@ fi
>  # Check that the Rust compiler version is suitable.
>  #
>  # Non-stable and distributions' versions may have a version suffix, e.g. `-dev`.
> +rust_compiler_output=$( \
> +	LC_ALL=C "$RUSTC" --version 2>/dev/null
> +) || rust_compiler_code=$?
> +if [ -n "$rust_compiler_code" ]; then
> +	echo >&2 "***"
> +	echo >&2 "*** Running '$RUSTC' to check the Rust compiler version failed with"
> +	echo >&2 "*** code $rust_compiler_code. See output and docs below for details:"
> +	echo >&2 "***"
> +	echo >&2 "$rust_compiler_output"
> +	echo >&2 "***"
> +	exit 1
> +fi
>  rust_compiler_version=$( \
> -	LC_ALL=C "$RUSTC" --version 2>/dev/null \
> +	echo "$rust_compiler_output" \
>  		| sed -nE '1s:.*rustc ([0-9]+\.[0-9]+\.[0-9]+).*:\1:p'
>  )
>  rust_compiler_min_version=$($min_tool_version rustc)
> @@ -108,8 +120,20 @@ fi
>  # Check that the Rust bindings generator is suitable.
>  #
>  # Non-stable and distributions' versions may have a version suffix, e.g. `-dev`.
> +rust_bindings_generator_output=$( \
> +	LC_ALL=C "$BINDGEN" --version 2>/dev/null
> +) || rust_bindings_generator_code=$?
> +if [ -n "$rust_bindings_generator_code" ]; then
> +	echo >&2 "***"
> +	echo >&2 "*** Running '$BINDGEN' to check the Rust bindings generator version failed with"
> +	echo >&2 "*** code $rust_bindings_generator_code. See output and docs below for details:"
> +	echo >&2 "***"
> +	echo >&2 "$rust_bindings_generator_output"
> +	echo >&2 "***"
> +	exit 1
> +fi
>  rust_bindings_generator_version=$( \
> -	LC_ALL=C "$BINDGEN" --version 2>/dev/null \
> +	echo "$rust_bindings_generator_output" \
>  		| sed -nE '1s:.*bindgen ([0-9]+\.[0-9]+\.[0-9]+).*:\1:p'
>  )
>  rust_bindings_generator_min_version=$($min_tool_version bindgen)
> -- 
> 2.41.0
> 

Re: [PATCH v2 10/11] kbuild: rust_is_available: check that output looks as expected

[Nathan Chancellor]

On Fri, Jun 16, 2023 at 02:16:30AM +0200, Miguel Ojeda wrote:
> The script already checks for `$RUSTC` and `$BINDGEN` existing
> and exiting without failure. However, one may still pass an
> unexpected binary that does not output what the later parsing
> expects. The script still successfully reports a failure as
> expected, but the error is confusing. For instance:
> 
>     $ RUSTC=true BINDGEN=bindgen CC=clang scripts/rust_is_available.sh
>     scripts/rust_is_available.sh: 19: arithmetic expression: expecting primary: "100000 *  + 100 *  + "
>     ***
>     *** Please see Documentation/rust/quick-start.rst for details
>     *** on how to set up the Rust support.
>     ***
> 
> Thus add an explicit check and a proper message for unexpected
> output from the called command.
> 
> Similarly, do so for the `libclang` version parsing, too.
> 
> Link: https://lore.kernel.org/rust-for-linux/CAK7LNAQYk6s11MASRHW6oxtkqF00EJVqhHOP=5rynWt-QDUsXw@mail.gmail.com/
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  scripts/rust_is_available.sh | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/scripts/rust_is_available.sh b/scripts/rust_is_available.sh
> index da8296cd9b8d..117018946b57 100755
> --- a/scripts/rust_is_available.sh
> +++ b/scripts/rust_is_available.sh
> @@ -97,6 +97,15 @@ rust_compiler_version=$( \
>  	echo "$rust_compiler_output" \
>  		| sed -nE '1s:.*rustc ([0-9]+\.[0-9]+\.[0-9]+).*:\1:p'
>  )
> +if [ -z "$rust_compiler_version" ]; then
> +	echo >&2 "***"
> +	echo >&2 "*** Running '$RUSTC' to check the Rust compiler version did not return"
> +	echo >&2 "*** an expected output. See output and docs below for details:"
> +	echo >&2 "***"
> +	echo >&2 "$rust_compiler_output"
> +	echo >&2 "***"
> +	exit 1
> +fi
>  rust_compiler_min_version=$($min_tool_version rustc)
>  rust_compiler_cversion=$(get_canonical_version $rust_compiler_version)
>  rust_compiler_min_cversion=$(get_canonical_version $rust_compiler_min_version)
> @@ -136,6 +145,15 @@ rust_bindings_generator_version=$( \
>  	echo "$rust_bindings_generator_output" \
>  		| sed -nE '1s:.*bindgen ([0-9]+\.[0-9]+\.[0-9]+).*:\1:p'
>  )
> +if [ -z "$rust_bindings_generator_version" ]; then
> +	echo >&2 "***"
> +	echo >&2 "*** Running '$BINDGEN' to check the bindings generator version did not return"
> +	echo >&2 "*** an expected output. See output and docs below for details:"
> +	echo >&2 "***"
> +	echo >&2 "$rust_bindings_generator_output"
> +	echo >&2 "***"
> +	exit 1
> +fi
>  rust_bindings_generator_min_version=$($min_tool_version bindgen)
>  rust_bindings_generator_cversion=$(get_canonical_version $rust_bindings_generator_version)
>  rust_bindings_generator_min_cversion=$(get_canonical_version $rust_bindings_generator_min_version)
> @@ -184,6 +202,16 @@ bindgen_libclang_version=$( \
>  	echo "$bindgen_libclang_output" \
>  		| sed -nE 's:.*clang version ([0-9]+\.[0-9]+\.[0-9]+).*:\1:p'
>  )
> +if [ -z "$bindgen_libclang_version" ]; then
> +	echo >&2 "***"
> +	echo >&2 "*** Running '$BINDGEN' to check the libclang version (used by the Rust"
> +	echo >&2 "*** bindings generator) did not return an expected output. See output"
> +	echo >&2 "*** and docs below for details:"
> +	echo >&2 "***"
> +	echo >&2 "$bindgen_libclang_output"
> +	echo >&2 "***"
> +	exit 1
> +fi
>  bindgen_libclang_min_version=$($min_tool_version llvm)
>  bindgen_libclang_cversion=$(get_canonical_version $bindgen_libclang_version)
>  bindgen_libclang_min_cversion=$(get_canonical_version $bindgen_libclang_min_version)
> -- 
> 2.41.0
> 

Re: [PATCH v2 07/11] kbuild: rust_is_available: fix confusion when a version appears in the path

[Ethan D. Twardy]

On Thu Jun 15, 2023 at 7:16 PM CDT, Miguel Ojeda wrote:
> ---
>  scripts/rust_is_available.sh | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/scripts/rust_is_available.sh b/scripts/rust_is_available.sh
> index 7e0368babe64..810691af66eb 100755
> --- a/scripts/rust_is_available.sh
> +++ b/scripts/rust_is_available.sh
> @@ -157,9 +157,7 @@ fi
>  # of the `libclang` found by the Rust bindings generator is suitable.
>  bindgen_libclang_version=$( \
>  	echo "$bindgen_libclang_output" \
> -		| grep -F 'clang version ' \
> -		| grep -oE '[0-9]+\.[0-9]+\.[0-9]+' \
> -		| head -n 1 \
> +		| sed -nE 's:.*clang version ([0-9]+\.[0-9]+\.[0-9]+).*:\1:p'
>  )
>  bindgen_libclang_min_version=$($min_tool_version llvm)
>  bindgen_libclang_cversion=$(get_canonical_version $bindgen_libclang_version)

Reviewed-By: Ethan Twardy <ethan.twardy@gmail.com>
Tested-By: Ethan Twardy <ethan.twardy@gmail.com>

Re: [PATCH v2 05/11] kbuild: rust_is_available: add check for `bindgen` invocation

[Masahiro Yamada]

On Fri, Jun 16, 2023 at 9:17 AM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> `scripts/rust_is_available.sh` calls `bindgen` with a special
> header in order to check whether the `libclang` version in use
> is suitable.
>
> However, the invocation itself may fail if, for instance, `bindgen`
> cannot locate `libclang`. This is fine for Kconfig (since the
> script will still fail and therefore disable Rust as it should),
> but it is pretty confusing for users of the `rustavailable` target
> given the error will be unrelated:
>
>     ./scripts/rust_is_available.sh: 21: arithmetic expression: expecting primary: "100000 *  + 100 *  + "
>     make: *** [Makefile:1816: rustavailable] Error 2
>
> Instead, run the `bindgen` invocation independently in a previous
> step, saving its output and return code. If it fails, then show
> the user a proper error message. Otherwise, continue as usual
> with the saved output.
>
> Since the previous patch we show a reference to the docs, and
> the docs now explain how `bindgen` looks for `libclang`,
> thus the error message can leverage the documentation, avoiding
> duplication here (and making users aware of the setup guide in
> the documentation).
>
> Reported-by: Nick Desaulniers <ndesaulniers@google.com>
> Link: https://lore.kernel.org/rust-for-linux/CAKwvOdm5JT4wbdQQYuW+RT07rCi6whGBM2iUAyg8A1CmLXG6Nw@mail.gmail.com/
> Reported-by: François Valenduc <francoisvalenduc@gmail.com>
> Closes: https://github.com/Rust-for-Linux/linux/issues/934
> Reported-by: Alexandru Radovici <msg4alex@gmail.com>
> Closes: https://github.com/Rust-for-Linux/linux/pull/921
> Reported-by: Matthew Leach <dev@mattleach.net>
> Closes: https://lore.kernel.org/rust-for-linux/20230507084116.1099067-1-dev@mattleach.net/
> Fixes: 78521f3399ab ("scripts: add `rust_is_available.sh`")
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---


Reviewed-by: Masahiro Yamada <masahiroy@kernel.org>




>  scripts/rust_is_available.sh | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/rust_is_available.sh b/scripts/rust_is_available.sh
> index 6b8131d5b547..1bdff4472cbe 100755
> --- a/scripts/rust_is_available.sh
> +++ b/scripts/rust_is_available.sh
> @@ -106,8 +106,28 @@ if [ "$rust_bindings_generator_cversion" -gt "$rust_bindings_generator_min_cvers
>  fi
>
>  # Check that the `libclang` used by the Rust bindings generator is suitable.
> +#
> +# In order to do that, first invoke `bindgen` to get the `libclang` version
> +# found by `bindgen`. This step may already fail if, for instance, `libclang`
> +# is not found, thus inform the user in such a case.
> +bindgen_libclang_output=$( \
> +       LC_ALL=C "$BINDGEN" $(dirname $0)/rust_is_available_bindgen_libclang.h 2>&1 >/dev/null


Nit.

To avoid shellcheck warnings, you can quote as follows:

"$(dirname "$0")/rust_is_available_bindgen_libclang.h"


You can do shellcheck scanning to avoid new warnings.

(Of course, 02/11 intentionally unquote $CC, though)



> +) || bindgen_libclang_code=$?
> +if [ -n "$bindgen_libclang_code" ]; then
> +       echo >&2 "***"
> +       echo >&2 "*** Running '$BINDGEN' to check the libclang version (used by the Rust"
> +       echo >&2 "*** bindings generator) failed with code $bindgen_libclang_code. This may be caused by"
> +       echo >&2 "*** a failure to locate libclang. See output and docs below for details:"
> +       echo >&2 "***"
> +       echo >&2 "$bindgen_libclang_output"
> +       echo >&2 "***"
> +       exit 1
> +fi
> +
> +# `bindgen` returned successfully, thus use the output to check that the version
> +# of the `libclang` found by the Rust bindings generator is suitable.
>  bindgen_libclang_version=$( \
> -       LC_ALL=C "$BINDGEN" $(dirname $0)/rust_is_available_bindgen_libclang.h 2>&1 >/dev/null \
> +       echo "$bindgen_libclang_output" \
>                 | grep -F 'clang version ' \
>                 | grep -oE '[0-9]+\.[0-9]+\.[0-9]+' \
>                 | head -n 1 \
> --
> 2.41.0
>


-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2 06/11] kbuild: rust_is_available: check that environment variables are set

[Masahiro Yamada]

On Fri, Jun 16, 2023 at 9:17 AM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Sometimes [1] users may attempt to setup the Rust support by
> checking what Kbuild does and they end up finding out about
> `scripts/rust_is_available.sh`. Inevitably, they run the script
> directly, but unless they setup the required variables,
> the result of the script is not meaningful.
>
> We could add some defaults to the variables, but that could be
> confusing for those that may override the defaults (compared
> to their kernel builds), and `$CC` would not be a simple default
> in any case.
>
> Therefore, instead, explicitly check whether the expected variables
> are set (`$RUSTC`, `$BINDGEN` and `$CC`). If not, print an explanation
> about the fact that the script is meant to be called from Kbuild,
> since that is the most likely cause for the variables not being set.
>
> Link: https://lore.kernel.org/oe-kbuild-all/Y6r4mXz5NS0+HVXo@zn.tnic/ [1]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---
>  scripts/rust_is_available.sh | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>
> diff --git a/scripts/rust_is_available.sh b/scripts/rust_is_available.sh
> index 1bdff4472cbe..7e0368babe64 100755
> --- a/scripts/rust_is_available.sh
> +++ b/scripts/rust_is_available.sh
> @@ -28,11 +28,40 @@ print_docs_reference()
>         echo >&2 "***"
>  }
>
> +# Print an explanation about the fact that the script is meant to be called from Kbuild.
> +print_kbuild_explanation()
> +{
> +       echo >&2 "***"
> +       echo >&2 "*** This script is intended to be called from Kbuild."
> +       echo >&2 "*** Please use the 'rustavailable' target to call it instead."
> +       echo >&2 "*** Otherwise, the results may not be meaningful."
> +       exit 1
> +}
> +
>  # If the script fails for any reason, or if there was any warning, then
>  # print a reference to the documentation on exit.
>  warning=0
>  trap 'if [ $? -ne 0 ] || [ $warning -ne 0 ]; then print_docs_reference; fi' EXIT
>
> +# Check that the expected environment variables are set.
> +if [ -z "${RUSTC+x}" ]; then
> +       echo >&2 "***"
> +       echo >&2 "*** Environment variable 'RUSTC' is not set."
> +       print_kbuild_explanation
> +fi


So, you want to check whether RUSTC is set or unset.
(that is, you allow a case where RUSTC is set, but empty.
It will eventually fail with a different error message anyway.)




$ ./scripts/rust_is_available.sh
***
*** Environment variable 'RUSTC' is not set.
***
*** This script is intended to be called from Kbuild.
*** Please use the 'rustavailable' target to call it instead.
*** Otherwise, the results may not be meaningful.
***
*** Please see Documentation/rust/quick-start.rst for details
*** on how to set up the Rust support.
***

$ RUSTC= BINDGEN= CC= ./scripts/rust_is_available.sh
***
*** Rust compiler '' could not be found.
***
***
*** Please see Documentation/rust/quick-start.rst for details
*** on how to set up the Rust support.
***



I would rather check whether RUSTC is empty or not
with simpler code.


if [ -z "${RUSTC}" ]; then
         ...
fi





> +
> +if [ -z "${BINDGEN+x}" ]; then
> +       echo >&2 "***"
> +       echo >&2 "*** Environment variable 'BINDGEN' is not set."
> +       print_kbuild_explanation
> +fi
> +
> +if [ -z "${CC+x}" ]; then
> +       echo >&2 "***"
> +       echo >&2 "*** Environment variable 'CC' is not set."
> +       print_kbuild_explanation
> +fi
> +
>  # Check that the Rust compiler exists.
>  if ! command -v "$RUSTC" >/dev/null; then
>         echo >&2 "***"
> --
> 2.41.0
>


-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2 00/11] `scripts/rust_is_available.sh` improvements

[Masahiro Yamada]

On Fri, Jun 16, 2023 at 9:16 AM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> This is the patch series to improve `scripts/rust_is_available.sh`.
>
> The major addition in v2 is the test suite in the last commit. I added
> it because I wanted to have a proper way to test any further changes to
> it (such as the suggested `set --` idea to avoid forking by Masahiro),
> and so that adding new checks was easier to justify too (i.e. vs. the
> added complexity).
>
> In addition, there are also a few new checks in the script, to cover for
> even some more cases, which hopefully make problematic setups easier to
> identify and solve by users building the kernel. For instance, running
> the script externally gives:
>
>     $ scripts/rust_is_available.sh
>     ***
>     *** Environment variable 'RUSTC' is not set.
>     ***
>     *** This script is intended to be called from Kbuild.
>     *** Please use the 'rustavailable' target to call it instead.
>     *** Otherwise, the results may not be meaningful.
>     ***
>     *** Please see Documentation/rust/quick-start.rst for details
>     *** on how to set up the Rust support.
>     ***
>
> I also changed it to avoid setting `-e` as Masahiro suggested.
> Similarly, I now check for `$RUSTC`, `$BINDGEN` and `$CC`, instead of
> `$MAKEFLAGS`, as he also suggested (but I gave it their own error
> message rather than use the `${CC?: is not set}` approach. This goes in
> line with the reasons outlined above, i.e. trying to give users a clear
> error of what step exactly failed).
>
> In the test suite I included previously problematic compiler version
> strings we got reports for. The test suite covers all current branches
> in the script, and we should keep it that way in the future.
>
> The patch series also include Masahiro's patch to remove the `-v`
> option, as well as Russell's patch for supporting multiple arguments
> in `$CC`.
>
> All in all, this should solve all the issues we got so far (unless I
> have missed something) and improve things further with the new checks
> plus the test suite to hopefully have an easier time in the future.
>
> Testers for this one are appreciated, especially if you have uncommon or
> custom setups for building the kernel.
>
> This could go through either the Kbuild or the Rust tree.


Please feel free to apply it to the Rust tree.
Perhaps I may add minor comments, but it is up to you.





Best Regards
Masahiro Yamada

Re: [PATCH v2 08/11] kbuild: rust_is_available: normalize version matching

[Masahiro Yamada]

On Fri, Jun 16, 2023 at 9:17 AM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> In order to match the version string, `sed` is used in a couple
> cases, and `grep` and `head` in a couple others.
>
> Make the script more consistent and easier to understand by
> using the same method, `sed`, for all of them.
>
> This makes the version matching also a bit more strict for
> the changed cases, since the strings `rustc ` and `bindgen `
> will now be required, which should be fine since `rustc`
> complains if one attempts to call it with another program
> name, and `bindgen` uses a hardcoded string.
>
> In addition, clarify why one of the existing `sed` commands
> does not provide an address like the others.
>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>



Reviewed-by: Masahiro Yamada <masahiroy@kernel.org>




> ---
>  scripts/rust_is_available.sh | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/rust_is_available.sh b/scripts/rust_is_available.sh
> index 810691af66eb..b7e0781fdea9 100755
> --- a/scripts/rust_is_available.sh
> +++ b/scripts/rust_is_available.sh
> @@ -83,8 +83,7 @@ fi
>  # Non-stable and distributions' versions may have a version suffix, e.g. `-dev`.
>  rust_compiler_version=$( \
>         LC_ALL=C "$RUSTC" --version 2>/dev/null \
> -               | head -n 1 \
> -               | grep -oE '[0-9]+\.[0-9]+\.[0-9]+' \
> +               | sed -nE '1s:.*rustc ([0-9]+\.[0-9]+\.[0-9]+).*:\1:p'
>  )
>  rust_compiler_min_version=$($min_tool_version rustc)
>  rust_compiler_cversion=$(get_canonical_version $rust_compiler_version)
> @@ -111,8 +110,7 @@ fi
>  # Non-stable and distributions' versions may have a version suffix, e.g. `-dev`.
>  rust_bindings_generator_version=$( \
>         LC_ALL=C "$BINDGEN" --version 2>/dev/null \
> -               | head -n 1 \
> -               | grep -oE '[0-9]+\.[0-9]+\.[0-9]+' \
> +               | sed -nE '1s:.*bindgen ([0-9]+\.[0-9]+\.[0-9]+).*:\1:p'
>  )
>  rust_bindings_generator_min_version=$($min_tool_version bindgen)
>  rust_bindings_generator_cversion=$(get_canonical_version $rust_bindings_generator_version)
> @@ -155,6 +153,9 @@ fi
>
>  # `bindgen` returned successfully, thus use the output to check that the version
>  # of the `libclang` found by the Rust bindings generator is suitable.
> +#
> +# Unlike other version checks, note that this one does not necessarily appear
> +# in the first line of the output, thus no `sed` address is provided.
>  bindgen_libclang_version=$( \
>         echo "$bindgen_libclang_output" \
>                 | sed -nE 's:.*clang version ([0-9]+\.[0-9]+\.[0-9]+).*:\1:p'
> --
> 2.41.0
>


-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2 04/11] kbuild: rust_is_available: print docs reference

[Masahiro Yamada]

On Fri, Jun 16, 2023 at 9:17 AM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> People trying out the Rust support in the kernel may get
> warnings and errors from `scripts/rust_is_available.sh`
> from the `rustavailable` target or the build step.
>
> Some of those users may be following the Quick Start guide,
> but others may not (likely those getting warnings from
> the build step instead of the target).
>
> While the messages are fairly clear on what the problem is,
> it may not be clear how to solve the particular issue,
> especially for those not aware of the documentation.
>
> We could add all sorts of details on the script for each one,
> but it is better to point users to the documentation instead,
> where it is easily readable in different formats. It also
> avoids duplication.
>
> Thus add a reference to the documentation whenever the script
> fails or there is at least a warning.
>
> Reviewed-by: Finn Behrens <fin@nyantec.com>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---
>  scripts/rust_is_available.sh | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/scripts/rust_is_available.sh b/scripts/rust_is_available.sh
> index 0c9be438e4cd..6b8131d5b547 100755
> --- a/scripts/rust_is_available.sh
> +++ b/scripts/rust_is_available.sh
> @@ -19,6 +19,20 @@ get_canonical_version()
>         echo $((100000 * $1 + 100 * $2 + $3))
>  }
>
> +# Print a reference to the Quick Start guide in the documentation.
> +print_docs_reference()
> +{
> +       echo >&2 "***"
> +       echo >&2 "*** Please see Documentation/rust/quick-start.rst for details"
> +       echo >&2 "*** on how to set up the Rust support."
> +       echo >&2 "***"
> +}
> +
> +# If the script fails for any reason, or if there was any warning, then
> +# print a reference to the documentation on exit.
> +warning=0
> +trap 'if [ $? -ne 0 ] || [ $warning -ne 0 ]; then print_docs_reference; fi' EXIT


I confirmed that
pressing Ctrl-C while rust_is_available.sh is running
does not invoke print_docs_reference().
(and I believe that is the right behavior).

I also checked bash and dash work in the same way.
(I usually test both because the POSIX does not define the exact
condition when the EXIT handler is invoked.)


Reviewed-by: Masahiro Yamada <masahiroy@kernel.org>












> +
>  # Check that the Rust compiler exists.
>  if ! command -v "$RUSTC" >/dev/null; then
>         echo >&2 "***"
> @@ -60,6 +74,7 @@ if [ "$rust_compiler_cversion" -gt "$rust_compiler_min_cversion" ]; then
>         echo >&2 "***   Your version:     $rust_compiler_version"
>         echo >&2 "***   Expected version: $rust_compiler_min_version"
>         echo >&2 "***"
> +       warning=1
>  fi
>
>  # Check that the Rust bindings generator is suitable.
> @@ -87,6 +102,7 @@ if [ "$rust_bindings_generator_cversion" -gt "$rust_bindings_generator_min_cvers
>         echo >&2 "***   Your version:     $rust_bindings_generator_version"
>         echo >&2 "***   Expected version: $rust_bindings_generator_min_version"
>         echo >&2 "***"
> +       warning=1
>  fi
>
>  # Check that the `libclang` used by the Rust bindings generator is suitable.
> @@ -126,6 +142,7 @@ if [ "$cc_name" = Clang ]; then
>                 echo >&2 "***   libclang version: $bindgen_libclang_version"
>                 echo >&2 "***   Clang version:    $clang_version"
>                 echo >&2 "***"
> +               warning=1
>         fi
>  fi
>
> --
> 2.41.0
>


--
Best Regards
Masahiro Yamada

Re: [PATCH v2 00/11] `scripts/rust_is_available.sh` improvements

[Miguel Ojeda]

On Fri, Jun 16, 2023 at 2:16 AM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> This is the patch series to improve `scripts/rust_is_available.sh`.
>
> The major addition in v2 is the test suite in the last commit. I added
> it because I wanted to have a proper way to test any further changes to
> it (such as the suggested `set --` idea to avoid forking by Masahiro),
> and so that adding new checks was easier to justify too (i.e. vs. the
> added complexity).
>
> In addition, there are also a few new checks in the script, to cover for
> even some more cases, which hopefully make problematic setups easier to
> identify and solve by users building the kernel. For instance, running
> the script externally gives:
>
>     $ scripts/rust_is_available.sh
>     ***
>     *** Environment variable 'RUSTC' is not set.
>     ***
>     *** This script is intended to be called from Kbuild.
>     *** Please use the 'rustavailable' target to call it instead.
>     *** Otherwise, the results may not be meaningful.
>     ***
>     *** Please see Documentation/rust/quick-start.rst for details
>     *** on how to set up the Rust support.
>     ***
>
> I also changed it to avoid setting `-e` as Masahiro suggested.
> Similarly, I now check for `$RUSTC`, `$BINDGEN` and `$CC`, instead of
> `$MAKEFLAGS`, as he also suggested (but I gave it their own error
> message rather than use the `${CC?: is not set}` approach. This goes in
> line with the reasons outlined above, i.e. trying to give users a clear
> error of what step exactly failed).
>
> In the test suite I included previously problematic compiler version
> strings we got reports for. The test suite covers all current branches
> in the script, and we should keep it that way in the future.
>
> The patch series also include Masahiro's patch to remove the `-v`
> option, as well as Russell's patch for supporting multiple arguments
> in `$CC`.
>
> All in all, this should solve all the issues we got so far (unless I
> have missed something) and improve things further with the new checks
> plus the test suite to hopefully have an easier time in the future.
>
> Testers for this one are appreciated, especially if you have uncommon or
> custom setups for building the kernel.
>
> This could go through either the Kbuild or the Rust tree.

Applied to `rust-next` -- thanks everyone!

Cheers,
Miguel