[PATCH v2 0/1] Fix libclang version check for rustavailable

[PATCH v2 0/1] Fix libclang version check for rustavailable

[Ethan D. Twardy]

This series contains just one two-line patch, to fix a small issue
in scripts/rust_is_available.sh. This issue is currently affecting
kernel builds with Rust on Gentoo Linux (and potentially other
distributions/workflows where the kernel source path contains a
version string).

Changes from v1:
* Patch formatting. I believe there were some serious issues with the
  formatting that caused it to be lost/ignored by the mailing list.
  I sincerely hope that the formatting of this patch is acceptable,
  this is my first contribution to the kernel so I haven't mastered
  the patch workflow yet.

Ethan D. Twardy (1):
  scripts/rust_is_available: Fix clang version check

 scripts/rust_is_available.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)


base-commit: ac9a78681b921877518763ba0e89202254349d1b
-- 
2.40.1

[PATCH v2 1/1] scripts/rust_is_available: Fix clang version check

[Ethan D. Twardy]

During out-of-tree builds where the path to the kernel source tree
contains a version string, scripts/rust_is_available.sh incorrectly
identified the version string of libclang to be the version string in
the kernel sources path, resulting in CONFIG_RUST_IS_AVAILABLE
erroneously set to 'n'.

This issue was previously affecting builds on distributions, such as
Gentoo Linux, where the kernel source tree is under version control,
and placed under a path containing the current kernel version string
in /usr/src.

The fix is to take special care to match only the version string
following the string 'clang version' in the output.

To reproduce:

  $ cd ~/build && make -C ~/linux-6.2.0 O=$PWD LLVM=1 rustavailable
  [...]
  *** libclang (used by the Rust bindings generator 'bindgen') is too old.
  ***   Your version:    6.2.0
  ***   Minimum version: 11.0.0
  [...]

Fixes: 78521f3399ab ("scripts: add `rust_is_available.sh`")
Signed-off-by: Ethan D. Twardy <ethan.twardy@gmail.com>

diff --git a/scripts/rust_is_available.sh b/scripts/rust_is_available.sh
index aebbf1913970..e8a1439be9f8 100755
--- a/scripts/rust_is_available.sh
+++ b/scripts/rust_is_available.sh
@@ -102,8 +102,8 @@ fi
 # Check that the `libclang` used 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 \
-		| grep -F 'clang version ' \
-		| grep -oE '[0-9]+\.[0-9]+\.[0-9]+' \
+		| grep -oE 'clang version [0-9]+\.[0-9]+\.[0-9]+' \
+		| cut -d' ' -f3 \
 		| head -n 1 \
 )
 bindgen_libclang_min_version=$($min_tool_version llvm)
-- 
2.40.1

Re: [PATCH v2 1/1] scripts/rust_is_available: Fix clang version check

[Martin Rodriguez Reboredo]

On 5/28/23 10:18, Ethan D. Twardy wrote:
> [...]
> --- a/scripts/rust_is_available.sh
> +++ b/scripts/rust_is_available.sh
> @@ -102,8 +102,8 @@ fi
>   # Check that the `libclang` used 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 \
> -		| grep -F 'clang version ' \
> -		| grep -oE '[0-9]+\.[0-9]+\.[0-9]+' \
> +		| grep -oE 'clang version [0-9]+\.[0-9]+\.[0-9]+' \
> +		| cut -d' ' -f3 \
>   		| head -n 1 \
>   )
>   bindgen_libclang_min_version=$($min_tool_version llvm)

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

Re: [PATCH v2 1/1] scripts/rust_is_available: Fix clang version check

[Ethan D. Twardy]

Greetings, Martin!

Thanks very much for your testing and review. Do I need to take any
further action to make this patch ready for review? I sent out a v3
version of the patch, where the only change was adding Reviewed-By and
Tested-By tags to my git commit--since this is my first time
contributing to this community, I wasn't sure whether I needed to do
that. Thanks!

Sincerely,

Ethan Twardy

Re: [PATCH v2 1/1] scripts/rust_is_available: Fix clang version check

[Martin Rodriguez Reboredo]

On 6/14/23 09:08, Ethan D. Twardy wrote:
> Greetings, Martin!
> 
> Thanks very much for your testing and review. Do I need to take any
> further action to make this patch ready for review? I sent out a v3
> version of the patch, where the only change was adding Reviewed-By and
> Tested-By tags to my git commit--since this is my first time
> contributing to this community, I wasn't sure whether I needed to do
> that. Thanks!
> 
> Sincerely,
> 
> Ethan Twardy

Unless other reviewers requested changes, you have to rebase your
work or you want to make a modification, it is not necessary to send a
new version of the patch series, generally, maintainers will, and have
to, add any Reviewed-bys or Tested-bys prior to merging.

Re: [PATCH v2 1/1] scripts/rust_is_available: Fix clang version check

[Miguel Ojeda]

On Wed, Jun 14, 2023 at 2:08 PM Ethan D. Twardy <ethan.twardy@gmail.com> wrote:
>
> Thanks very much for your testing and review. Do I need to take any
> further action to make this patch ready for review? I sent out a v3
> version of the patch, where the only change was adding Reviewed-By and
> Tested-By tags to my git commit--since this is my first time
> contributing to this community, I wasn't sure whether I needed to do
> that. Thanks!

You did everything well! The patch looked fine to me, and the commit
message is detailed. The tags are all fine too. Thanks a lot for the
effort!

I had v2 of the script improvement patch series in my backlog, which
normalizes to `sed` (rather than use `grep`/`cut` etc.), adds a test
suite, and a few other things; but otherwise we could have easily
taken this one, because it is well done.

So, you did everything right, and you are more than welcome to send
more patches like that :)

Cheers,
Miguel

Re: [PATCH v2 1/1] scripts/rust_is_available: Fix clang version check

[Miguel Ojeda]

On Sun, May 28, 2023 at 3:21 PM Ethan D. Twardy <ethan.twardy@gmail.com> wrote:
>
> During out-of-tree builds where the path to the kernel source tree
> contains a version string, scripts/rust_is_available.sh incorrectly
> identified the version string of libclang to be the version string in
> the kernel sources path, resulting in CONFIG_RUST_IS_AVAILABLE
> erroneously set to 'n'.
>
> This issue was previously affecting builds on distributions, such as
> Gentoo Linux, where the kernel source tree is under version control,
> and placed under a path containing the current kernel version string
> in /usr/src.
>
> The fix is to take special care to match only the version string
> following the string 'clang version' in the output.
>
> To reproduce:
>
>   $ cd ~/build && make -C ~/linux-6.2.0 O=$PWD LLVM=1 rustavailable
>   [...]
>   *** libclang (used by the Rust bindings generator 'bindgen') is too old.
>   ***   Your version:    6.2.0
>   ***   Minimum version: 11.0.0
>   [...]
>
> Fixes: 78521f3399ab ("scripts: add `rust_is_available.sh`")
> Signed-off-by: Ethan D. Twardy <ethan.twardy@gmail.com>

Thanks a lot for this patch! This was previously reported and should
be fixed in this patch series (I added you as a reporter in v2):

    https://lore.kernel.org/rust-for-linux/20230616001631.463536-1-ojeda@kernel.org/

If you could please test the series, to make sure it solves your
issue, it would be great!

Cheers,
Miguel