2022-10-04 18:58:59

by Olof Johansson

[permalink] [raw]
Subject: [PATCH] scripts: rust_is_available.sh: Provide hints on how to fix missing pieces

This might be a bit bikesheddy, but it saves a few roundtrips to the
documentation when getting the `make LLVM=1 rustavailable` run to pass.

Stick to the rustup options to avoid too much verbosity.

Signed-off-by: Olof Johansson <[email protected]>
---
scripts/rust_is_available.sh | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/scripts/rust_is_available.sh b/scripts/rust_is_available.sh
index aebbf1913970..94e6a1ce1df3 100755
--- a/scripts/rust_is_available.sh
+++ b/scripts/rust_is_available.sh
@@ -59,6 +59,9 @@ if [ "$rust_compiler_cversion" -lt "$rust_compiler_min_cversion" ]; then
echo >&2 "*** Your version: $rust_compiler_version"
echo >&2 "*** Minimum version: $rust_compiler_min_version"
echo >&2 "***"
+ echo >&2 "*** To update to the expected version:"
+ echo >&2 "*** \$ rustup override set \$(scripts/min-tool-version.sh rustc)"
+ echo >&2 "***"
fi
exit 1
fi
@@ -68,6 +71,9 @@ if [ "$1" = -v ] && [ "$rust_compiler_cversion" -gt "$rust_compiler_min_cversion
echo >&2 "*** Your version: $rust_compiler_version"
echo >&2 "*** Expected version: $rust_compiler_min_version"
echo >&2 "***"
+ echo >&2 "*** To update to the expected version:"
+ echo >&2 "*** \$ rustup override set \$(scripts/min-tool-version.sh rustc)"
+ echo >&2 "***"
fi

# Check that the Rust bindings generator is suitable.
@@ -155,6 +161,9 @@ if [ ! -e "$rustc_src_core" ]; then
echo >&2 "*** Source code for the 'core' standard library could not be found"
echo >&2 "*** at '$rustc_src_core'."
echo >&2 "***"
+ echo >&2 "*** To install sources:"
+ echo >&2 "*** \$ rustup component add rust-src"
+ echo >&2 "***"
fi
exit 1
fi
--
2.30.2


2022-10-05 07:39:11

by Geert Stappers

[permalink] [raw]
Subject: Re: [PATCH] scripts: rust_is_available.sh: Provide hints on how to fix missing pieces

On Tue, Oct 04, 2022 at 11:46:25AM -0700, Olof Johansson wrote:
> This might be a bit bikesheddy, but it saves a few roundtrips to the
> documentation when getting the `make LLVM=1 rustavailable` run to pass.

Yeah, I have bin there :-)


> Stick to the rustup options to avoid too much verbosity.
>
> Signed-off-by: Olof Johansson <[email protected]>
> ---
> scripts/rust_is_available.sh | 9 +++++++++
> 1 file changed, 9 insertions(+)
>

Reviewed-by: Geert Stappers <[email protected]>

2022-10-13 17:23:13

by Geert Stappers

[permalink] [raw]
Subject: Re: [PATCH] scripts: rust_is_available.sh: Provide hints on how to fix missing pieces

On Wed, Oct 05, 2022 at 09:27:44AM +0200, Geert Stappers wrote:
> On Tue, Oct 04, 2022 at 11:46:25AM -0700, Olof Johansson wrote:
> > This might be a bit bikesheddy, but it saves a few roundtrips to the
> > documentation when getting the `make LLVM=1 rustavailable` run to pass.
>
> Yeah, I have bin there :-)
>
>
> > Stick to the rustup options to avoid too much verbosity.
> >
> > Signed-off-by: Olof Johansson <[email protected]>
> > ---
> > scripts/rust_is_available.sh | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
>
> Reviewed-by: Geert Stappers <[email protected]>


How to prevent that the patch gets lost in the mail?

And how to avoid that reminders like this get contra-productive?
(When to send the next "Please approve or reject the patch"?)


Groeten
Geert Stappers
--
Silence is hard to parse

2022-10-14 16:36:42

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH] scripts: rust_is_available.sh: Provide hints on how to fix missing pieces

On Tue, Oct 4, 2022 at 8:47 PM Olof Johansson <[email protected]> wrote:
>
> This might be a bit bikesheddy, but it saves a few roundtrips to the
> documentation when getting the `make LLVM=1 rustavailable` run to pass.

It is faster for someone that already knows how things work, but it
may make newcomers skip the docs and it duplicates the information
there. In addition, for the non-error case, it makes it more verbose
which may not be appreciated. So maybe we should point to the docs
instead? What do you think?

Also, the patch doesn't add instructions for all the cases, so
somebody that may have hit one of the documented ones + not have read
the docs may wonder where to find them the solution or why they are
missing.

Thanks!

Cheers,
Miguel

2022-10-14 16:53:55

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH] scripts: rust_is_available.sh: Provide hints on how to fix missing pieces

Hi Geert,

On Thu, Oct 13, 2022 at 7:01 PM Geert Stappers <[email protected]> wrote:
>
> How to prevent that the patch gets lost in the mail?

Do not worry, it is not lost :)

> And how to avoid that reminders like this get contra-productive?
> (When to send the next "Please approve or reject the patch"?)

The author of the patch would normally send a ping. If you are
curious, please see Documentation/process/submitting-patches.rst for
some guidelines.

In short, the authors would wait at least a week, and they would apply
their best judgment (e.g. whether the merge window is going on,
whether it is a new feature or a critical fix, whether
feedback/comments/tags are still coming, etc.).

Cheers,
Miguel

2022-10-14 19:21:50

by Olof Johansson

[permalink] [raw]
Subject: Re: [PATCH] scripts: rust_is_available.sh: Provide hints on how to fix missing pieces

On Fri, Oct 14, 2022 at 9:21 AM Miguel Ojeda
<[email protected]> wrote:
>
> On Tue, Oct 4, 2022 at 8:47 PM Olof Johansson <[email protected]> wrote:
> >
> > This might be a bit bikesheddy, but it saves a few roundtrips to the
> > documentation when getting the `make LLVM=1 rustavailable` run to pass.
>
> It is faster for someone that already knows how things work, but it
> may make newcomers skip the docs and it duplicates the information
> there. In addition, for the non-error case, it makes it more verbose
> which may not be appreciated. So maybe we should point to the docs
> instead? What do you think?

I don't really have a preference. This patch would have helped me, so
I figured I would post it. My interest isn't really high enough to
spend more effort on it at this time -- I got my setup working by now.

Refactoring the script to have a shared message on non-successful exit
with a reference to the doc would achieve what you're suggesting
though.


> Also, the patch doesn't add instructions for all the cases, so
> somebody that may have hit one of the documented ones + not have read
> the docs may wonder where to find them the solution or why they are
> missing.

Sure.


-Olof