| Message ID | 20230109204520.539080-2-ojeda@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/6] docs: rust: add paragraph about finding a suitable `libclang` | expand |
On 9 Jan 2023, at 21:45, 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. As I always use my systems rustc/bindgen, I always get the warning, which already clutters the build output a bit. But I see why it is helpful, so not a fan, but this patch is reasonable. > > Signed-off-by: Miguel Ojeda <ojeda@kernel.org> Reviewed-by: Finn Behrens <fin@nyantec.com> Regards, Finn > --- > Note that is based on top of patch "kbuild: rust: remove -v > option of scripts/rust_is_available.sh" applied on v6.2-rc3: > https://lore.kernel.org/rust-for-linux/20230109061436.3146442-1-masahiroy@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 eaeafebf8572..c907cf881c2c 100755 > --- a/scripts/rust_is_available.sh > +++ b/scripts/rust_is_available.sh > @@ -21,6 +21,20 @@ get_canonical_version() > echo $((100000 * $1 + 100 * $2 + $3)) > } > > +# Print a reference to the setup 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 setup 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 "***" > @@ -62,6 +76,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. > @@ -89,6 +104,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. > @@ -128,6 +144,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.39.0
On Tue, Jan 10, 2023 at 11:16 AM Finn Behrens <fin@nyantec.com> wrote: > > As I always use my systems rustc/bindgen, I always get the warning, which already clutters the build output a bit. But I see why it is helpful, so not a fan, but this patch is reasonable. Indeed, if one uses a different version, it may end up becoming too annoying when running it during build -- it is something I worried about when adding it back then in commit 11c0cf1e8c06 ("rust: run rust-is-available on build") in our repository. I think, for a while, until more people is accustomed to dealing with Rust, it may be worth the pain for some of us in order to help to catch bad setups, since otherwise users may not attempt to check with the `rustavailable` target themselves. In any case, of course, the "too new" warnings will go away when we reach a stable version situation since they will not be needed anymore (but we can also do it sooner than that, for the build step especially). Thanks for the review! Cheers, Miguel
diff --git a/scripts/rust_is_available.sh b/scripts/rust_is_available.sh index eaeafebf8572..c907cf881c2c 100755 --- a/scripts/rust_is_available.sh +++ b/scripts/rust_is_available.sh @@ -21,6 +21,20 @@ get_canonical_version() echo $((100000 * $1 + 100 * $2 + $3)) } +# Print a reference to the setup 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 setup 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 "***" @@ -62,6 +76,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. @@ -89,6 +104,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. @@ -128,6 +144,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
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. Signed-off-by: Miguel Ojeda <ojeda@kernel.org> --- Note that is based on top of patch "kbuild: rust: remove -v option of scripts/rust_is_available.sh" applied on v6.2-rc3: https://lore.kernel.org/rust-for-linux/20230109061436.3146442-1-masahiroy@kernel.org/ scripts/rust_is_available.sh | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)