Hi Daniel,
sorry for longer time to reply (vacation).
> 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).
command -v is supported in other common shells (busybox sh and dash),
I'll add it to the commit message.
IMHO it's non-POSIX extension, but because it's support we use it in LTP shell
API, where we expect very minimal shell tools (i.e. no bash, no core utils).
> It looks the bash dependency is not specified in the INSTALL file in
> "The Requirements" section. May I ask you to add it?
Due previous and the fact shebang in various scripts (bootstrap, geninit.sh,
docs/mdate-sh, grub-core/genemuinit.sh, ...) is /bin/sh and checkbashisms does
not complain it would be wrong to list bash as a dependency. IMHO Debian build
tools which use dash do not need any special patch (Colin Watson would know),
also Alpine which uses busybox sh does need any patch either.
> > Keep the possibility to define PYTHON.
> > Signed-off-by: Petr Vorel <pvo...@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?
Sure, sorry to overlook it.
> > + if command -v "$i" > /dev/null 2>&1; then
> Ditto and below please...
Sure.
> > + PYTHON="$i"
> > + echo "Using $PYTHON" >&2
> Please drop ">&2" redirection here.
IMHO there is no useful value for user to see "python not found", but no problem
to drop it.
> And I think it should be "Using $PYTHON...".
Sure.
> > + break
> > + fi
> > + done
> > +
> > + if [ -z "$PYTHON" ]; then
> > + echo "python not found" >&2
> s/found/found./
Sure.
Kind regards,
Petr
> > + exit 1
> > + fi
> > +fi
> Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
