| Message ID | 20230616001631.463536-7-ojeda@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | `scripts/rust_is_available.sh` improvements | expand |
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>
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 >
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 >
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 "***"
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(+)