From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1mGFg8-0005RO-4O for mharc-grub-devel@gnu.org; Wed, 18 Aug 2021 03:08:00 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:54108) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mGFg6-0005O8-Ga for grub-devel@gnu.org; Wed, 18 Aug 2021 03:07:58 -0400 Received: from smtp-out2.suse.de ([195.135.220.29]:44478) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mGFg3-0007Ct-Uy for grub-devel@gnu.org; Wed, 18 Aug 2021 03:07:58 -0400 Received: from imap1.suse-dmz.suse.de (imap1.suse-dmz.suse.de [192.168.254.73]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id A1EAB1FF82; Wed, 18 Aug 2021 07:07:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1629270471; h=from:from:reply-to:reply-to:date:date:message-id:message-id:to:to: cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=e8PBbIshxc7s5lYuC5rqoTRns+iWXiEJ8awjA6pT3cQ=; b=TrYVkSXodi83vwFCUMzPr/QY1/8Yu9A1dPln1lpZXxx6aRUTExE7s7dQmmr1zsBIvAEyKs U+yLGrdNclEjdvyGi+34rzwnJE0ussnkjsQMpEWHnUtAh/mrK2acP73H/w09dCHVXRLNVQ O+bZqNwXqxZDZd3a5LHou/dB1ocnauE= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1629270471; h=from:from:reply-to:reply-to:date:date:message-id:message-id:to:to: cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=e8PBbIshxc7s5lYuC5rqoTRns+iWXiEJ8awjA6pT3cQ=; b=Uy/0J1/jRiFL7Y9U+3or5tk/c2sEWDuQZtWjoTNRQTttMVukKaUWfX0uzGOfmKidPEh8E0 PIy9YVcQk6mw+vBw== Received: from imap1.suse-dmz.suse.de (imap1.suse-dmz.suse.de [192.168.254.73]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap1.suse-dmz.suse.de (Postfix) with ESMTPS id 767AB134B1; Wed, 18 Aug 2021 07:07:51 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap1.suse-dmz.suse.de with ESMTPSA id DXRzG8exHGFceQAAGKfGzw (envelope-from ); Wed, 18 Aug 2021 07:07:51 +0000 Date: Wed, 18 Aug 2021 09:07:49 +0200 From: Petr Vorel To: Daniel Kiper Cc: grub-devel@gnu.org Subject: Re: [PATCH 2/2] autogen.sh: Detect python Message-ID: Reply-To: Petr Vorel References: <20210806064508.4489-1-pvorel@suse.cz> <20210806064508.4489-2-pvorel@suse.cz> <20210809115229.toe34etovoul62ss@tomti.i.net-space.pl> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210809115229.toe34etovoul62ss@tomti.i.net-space.pl> Received-SPF: pass client-ip=195.135.220.29; envelope-from=pvorel@suse.cz; helo=smtp-out2.suse.de X-Spam_score_int: -43 X-Spam_score: -4.4 X-Spam_bar: ---- X-Spam_report: (-4.4 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 18 Aug 2021 07:07:58 -0000 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 > > --- > > 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