Re: [PATCH 2/2] autogen.sh: Detect python

[Daniel Kiper]

On Fri, Aug 06, 2021 at 08:45:08AM +0200, Petr Vorel wrote:
> It help to avoid error on distros which has only python3 binary:
> ./autogen.sh: line 20: python: command not found
>
> Using bash builtin 'command -v' to avoid requiring which as extra
> dependency (usable on containers).

It looks the bash dependency is not specified in the INSTALL file in
"The Requirements" section. May I ask you to add it?

> Keep the possibility to define PYTHON.
>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
>  autogen.sh | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/autogen.sh b/autogen.sh
> index 31b0ced7e..46f9e1a6d 100755
> --- a/autogen.sh
> +++ b/autogen.sh
> @@ -7,8 +7,21 @@ if [ ! -e grub-core/lib/gnulib/stdlib.in.h ]; then
>    exit 1
>  fi
>
> -# Set ${PYTHON} to plain 'python' if not set already
> -: ${PYTHON:=python}
> +# Detect python
> +if [ -z "$PYTHON" ]; then
> +     for i in python python3 python2; do

May I ask you to use (multiple of) 2 space indention as it is done in
most of this file?

> +             if command -v "$i" > /dev/null 2>&1; then

Ditto and below please...

> +                     PYTHON="$i"
> +                     echo "Using $PYTHON" >&2

Please drop ">&2" redirection here.
And I think it should be "Using $PYTHON...".

> +                     break
> +             fi
> +     done
> +
> +     if [ -z "$PYTHON" ]; then
> +             echo "python not found" >&2

s/found/found./

> +             exit 1
> +     fi
> +fi

Daniel