[Buildroot] [RFC PATCH 0/2] use `command -v' instead of `which'

[Buildroot] [RFC PATCH 0/2] use `command -v' instead of `which'

[Petr Vorel]

Hi,

I've tested the patchset on dash as the default shell. But it certainly
deserve more people to have look and test.

NOTE: If accepted, we might want to use `command -v' also in scripts:

$ git grep '$(which'
package/fakedate/fakedate:for date in $(which -a date |tac); do
package/transmission/S92transmission:DAEMON=$(which $NAME)

Kind regards,
Petr

Petr Vorel (2):
  make: support: use `command -v' instead of `which'
  support/dependencies: don't check for `which'

 Makefile                                      | 22 +++++++++----------
 package/Makefile.in                           |  8 +++----
 support/dependencies/check-host-bison-flex.mk |  4 ++--
 support/dependencies/check-host-cmake.sh      |  2 +-
 support/dependencies/check-host-gzip.sh       |  2 +-
 support/dependencies/check-host-lzip.sh       |  4 ++--
 support/dependencies/check-host-python3.sh    |  2 +-
 support/dependencies/check-host-tar.sh        |  4 ++--
 support/dependencies/check-host-xzcat.sh      |  4 ++--
 support/dependencies/dependencies.sh          | 18 +++++++--------
 .../pkg-toolchain-external.mk                 |  2 +-
 11 files changed, 35 insertions(+), 37 deletions(-)

-- 
2.33.0

_______________________________________________
buildroot mailing list
buildroot@lists.buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [RFC PATCH 1/2] make: support: use `command -v' instead of `which'

[Petr Vorel]

`which' has been discontinued after 2.21 release in 2015 due this (git
repository is empty [1]) and version shipped in Debian produces warning
[2]:

/usr/bin/which: this version of `which' is deprecated; use `command -v' in scripts instead.

`command is POSIX [3] and supported on all common shells (bash, zsh,
dash, busybox sh, mksh).

Patch tested on dash as the default shell.

[1] https://git.savannah.gnu.org/cgit/which.git
[2] https://salsa.debian.org/debian/debianutils/-/commit/3a8dd10b4502f7bae8fc6973c13ce23fc9da7efb
[3] https://pubs.opengroup.org/onlinepubs/9699919799/utilities/command.html

Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
---
 Makefile                                      | 22 +++++++++----------
 package/Makefile.in                           |  8 +++----
 support/dependencies/check-host-bison-flex.mk |  4 ++--
 support/dependencies/check-host-cmake.sh      |  2 +-
 support/dependencies/check-host-gzip.sh       |  2 +-
 support/dependencies/check-host-lzip.sh       |  4 ++--
 support/dependencies/check-host-python3.sh    |  2 +-
 support/dependencies/check-host-tar.sh        |  4 ++--
 support/dependencies/check-host-xzcat.sh      |  4 ++--
 support/dependencies/dependencies.sh          | 16 +++++++-------
 .../pkg-toolchain-external.mk                 |  2 +-
 11 files changed, 35 insertions(+), 35 deletions(-)

diff --git a/Makefile b/Makefile
index c960b53a6d..1b080bc10f 100644
--- a/Makefile
+++ b/Makefile
@@ -284,12 +284,12 @@ HOSTAS := as
 endif
 ifndef HOSTCC
 HOSTCC := gcc
-HOSTCC := $(shell which $(HOSTCC) || type -p $(HOSTCC) || echo gcc)
+HOSTCC := $(shell command -v $(HOSTCC) || type -p $(HOSTCC) || echo gcc)
 endif
 HOSTCC_NOCCACHE := $(HOSTCC)
 ifndef HOSTCXX
 HOSTCXX := g++
-HOSTCXX := $(shell which $(HOSTCXX) || type -p $(HOSTCXX) || echo g++)
+HOSTCXX := $(shell command -v $(HOSTCXX) || type -p $(HOSTCXX) || echo g++)
 endif
 HOSTCXX_NOCCACHE := $(HOSTCXX)
 ifndef HOSTCPP
@@ -310,15 +310,15 @@ endif
 ifndef HOSTRANLIB
 HOSTRANLIB := ranlib
 endif
-HOSTAR := $(shell which $(HOSTAR) || type -p $(HOSTAR) || echo ar)
-HOSTAS := $(shell which $(HOSTAS) || type -p $(HOSTAS) || echo as)
-HOSTCPP := $(shell which $(HOSTCPP) || type -p $(HOSTCPP) || echo cpp)
-HOSTLD := $(shell which $(HOSTLD) || type -p $(HOSTLD) || echo ld)
-HOSTLN := $(shell which $(HOSTLN) || type -p $(HOSTLN) || echo ln)
-HOSTNM := $(shell which $(HOSTNM) || type -p $(HOSTNM) || echo nm)
-HOSTOBJCOPY := $(shell which $(HOSTOBJCOPY) || type -p $(HOSTOBJCOPY) || echo objcopy)
-HOSTRANLIB := $(shell which $(HOSTRANLIB) || type -p $(HOSTRANLIB) || echo ranlib)
-SED := $(shell which sed || type -p sed) -i -e
+HOSTAR := $(shell command -v $(HOSTAR) || type -p $(HOSTAR) || echo ar)
+HOSTAS := $(shell command -v $(HOSTAS) || type -p $(HOSTAS) || echo as)
+HOSTCPP := $(shell command -v $(HOSTCPP) || type -p $(HOSTCPP) || echo cpp)
+HOSTLD := $(shell command -v $(HOSTLD) || type -p $(HOSTLD) || echo ld)
+HOSTLN := $(shell command -v $(HOSTLN) || type -p $(HOSTLN) || echo ln)
+HOSTNM := $(shell command -v $(HOSTNM) || type -p $(HOSTNM) || echo nm)
+HOSTOBJCOPY := $(shell command -v $(HOSTOBJCOPY) || type -p $(HOSTOBJCOPY) || echo objcopy)
+HOSTRANLIB := $(shell command -v $(HOSTRANLIB) || type -p $(HOSTRANLIB) || echo ranlib)
+SED := $(shell command -v sed || type -p sed) -i -e
 
 export HOSTAR HOSTAS HOSTCC HOSTCXX HOSTLD
 export HOSTCC_NOCCACHE HOSTCXX_NOCCACHE
diff --git a/package/Makefile.in b/package/Makefile.in
index 86db62ba5b..09092fad54 100644
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -4,7 +4,7 @@ endif
 ifndef HOSTMAKE
 HOSTMAKE = $(MAKE)
 endif
-HOSTMAKE := $(shell which $(HOSTMAKE) || type -p $(HOSTMAKE) || echo make)
+HOSTMAKE := $(shell command -v $(HOSTMAKE) || type -p $(HOSTMAKE) || echo make)
 
 # If BR2_JLEVEL is 0, scale the maximum concurrency with the number of
 # CPUs. An additional job is used in order to keep processors busy
@@ -222,8 +222,8 @@ else
 TARGET_STRIP = /bin/true
 STRIPCMD = $(TARGET_STRIP)
 endif
-INSTALL := $(shell which install || type -p install)
-UNZIP := $(shell which unzip || type -p unzip) -q
+INSTALL := $(shell command -v install || type -p install)
+UNZIP := $(shell command -v unzip || type -p unzip) -q
 
 APPLY_PATCHES = PATH=$(HOST_DIR)/bin:$$PATH support/scripts/apply-patches.sh $(if $(QUIET),-s)
 
@@ -237,7 +237,7 @@ HOST_LDFLAGS  += -L$(HOST_DIR)/lib -Wl,-rpath,$(HOST_DIR)/lib
 # the path to the system perl, before a host-perl built by Buildroot
 # might get installed into $(HOST_DIR)/bin and therefore appears
 # in our PATH. This system perl will be used as INTLTOOL_PERL.
-export PERL=$(shell which perl)
+export PERL=$(shell command -v perl)
 
 # host-intltool needs libxml-parser-perl, which Buildroot installs in
 # $(HOST_DIR)/lib/perl, so we must make sure that the system perl
diff --git a/support/dependencies/check-host-bison-flex.mk b/support/dependencies/check-host-bison-flex.mk
index 14a232fd44..e5db578941 100644
--- a/support/dependencies/check-host-bison-flex.mk
+++ b/support/dependencies/check-host-bison-flex.mk
@@ -5,10 +5,10 @@
 # that runs on host, e.g. Kconfig. To build code for target use plain
 # host-{bison,flex}.
 
-ifeq ($(shell which bison 2>/dev/null),)
+ifeq ($(shell command -v bison 2>/dev/null),)
 BR2_BISON_HOST_DEPENDENCY = host-bison
 endif
 
-ifeq ($(shell which flex 2>/dev/null),)
+ifeq ($(shell command -v flex 2>/dev/null),)
 BR2_FLEX_HOST_DEPENDENCY = host-flex
 endif
diff --git a/support/dependencies/check-host-cmake.sh b/support/dependencies/check-host-cmake.sh
index fadeae9f6b..f202d72a98 100755
--- a/support/dependencies/check-host-cmake.sh
+++ b/support/dependencies/check-host-cmake.sh
@@ -11,7 +11,7 @@ shift
 for candidate; do
 
     # Try to locate the candidate. Discard it if not located.
-    cmake=`which "${candidate}" 2>/dev/null`
+    cmake=`command -v "${candidate}" 2>/dev/null`
     [ -n "${cmake}" ] || continue
 
     # Extract version X.Y from versions in the form X.Y or X.Y.Z
diff --git a/support/dependencies/check-host-gzip.sh b/support/dependencies/check-host-gzip.sh
index 5f344c5f9b..4dbce72676 100755
--- a/support/dependencies/check-host-gzip.sh
+++ b/support/dependencies/check-host-gzip.sh
@@ -2,7 +2,7 @@
 
 candidate="$1" # ignored
 
-gzip="$(which gzip)"
+gzip="$(command -v gzip)"
 if [ ! -x "${gzip}" ]; then
     # echo nothing: no suitable gzip found
     exit 1
diff --git a/support/dependencies/check-host-lzip.sh b/support/dependencies/check-host-lzip.sh
index 4f8a2ba3de..ffd20e2809 100755
--- a/support/dependencies/check-host-lzip.sh
+++ b/support/dependencies/check-host-lzip.sh
@@ -2,9 +2,9 @@
 
 candidate="$1"
 
-lzip=`which $candidate 2>/dev/null`
+lzip=`command -v $candidate 2>/dev/null`
 if [ ! -x "$lzip" ]; then
-	lzip=`which lzip 2>/dev/null`
+	lzip=`command -v lzip 2>/dev/null`
 	if [ ! -x "$lzip" ]; then
 		# echo nothing: no suitable lzip found
 		exit 1
diff --git a/support/dependencies/check-host-python3.sh b/support/dependencies/check-host-python3.sh
index 17cafd2883..7b3f4f8451 100755
--- a/support/dependencies/check-host-python3.sh
+++ b/support/dependencies/check-host-python3.sh
@@ -14,7 +14,7 @@ shift
 # a more recent version.
 
 for candidate in "${@}" ; do
-	python3=`which $candidate 2>/dev/null`
+	python3=`command -v $candidate 2>/dev/null`
 	if [ ! -x "$python3" ]; then
 		continue
 	fi
diff --git a/support/dependencies/check-host-tar.sh b/support/dependencies/check-host-tar.sh
index b7d607a47a..369e1ae393 100755
--- a/support/dependencies/check-host-tar.sh
+++ b/support/dependencies/check-host-tar.sh
@@ -2,9 +2,9 @@
 
 candidate="$1"
 
-tar=`which $candidate`
+tar=`command -v $candidate`
 if [ ! -x "$tar" ]; then
-	tar=`which tar`
+	tar=`command -v tar`
 	if [ ! -x "$tar" ]; then
 		# echo nothing: no suitable tar found
 		exit 1
diff --git a/support/dependencies/check-host-xzcat.sh b/support/dependencies/check-host-xzcat.sh
index 10f1c4562a..1fbcecf0ff 100755
--- a/support/dependencies/check-host-xzcat.sh
+++ b/support/dependencies/check-host-xzcat.sh
@@ -2,9 +2,9 @@
 
 candidate="$1"
 
-xzcat=`which $candidate 2>/dev/null`
+xzcat=`command -v $candidate 2>/dev/null`
 if [ ! -x "$xzcat" ]; then
-	xzcat=`which xzcat 2>/dev/null`
+	xzcat=`command -v xzcat 2>/dev/null`
 	if [ ! -x "$xzcat" ]; then
 		# echo nothing: no suitable xzcat found
 		exit 1
diff --git a/support/dependencies/dependencies.sh b/support/dependencies/dependencies.sh
index c604a9efcc..c5dcb86231 100755
--- a/support/dependencies/dependencies.sh
+++ b/support/dependencies/dependencies.sh
@@ -58,7 +58,7 @@ fi
 check_prog_host()
 {
 	prog="$1"
-	if ! which $prog > /dev/null ; then
+	if ! command -v $prog > /dev/null ; then
 		echo >&2
 		echo "You must install '$prog' on your build machine" >&2
 		exit 1
@@ -75,7 +75,7 @@ check_prog_host "sed"
 check_prog_host "/usr/bin/file"
 
 # Check make
-MAKE=$(which make 2> /dev/null)
+MAKE=$(command -v make 2> /dev/null)
 if [ -z "$MAKE" ] ; then
 	echo
 	echo "You must install 'make' on your build machine";
@@ -96,9 +96,9 @@ if [ $MAKE_MAJOR -lt 3 ] || [ $MAKE_MAJOR -eq 3 -a $MAKE_MINOR -lt 81 ] ; then
 fi;
 
 # Check host gcc
-COMPILER=$(which $HOSTCC_NOCCACHE 2> /dev/null)
+COMPILER=$(command -v $HOSTCC_NOCCACHE 2> /dev/null)
 if [ -z "$COMPILER" ] ; then
-	COMPILER=$(which cc 2> /dev/null)
+	COMPILER=$(command -v cc 2> /dev/null)
 fi;
 if [ -z "$COMPILER" ] ; then
 	echo
@@ -122,9 +122,9 @@ if [ $COMPILER_MAJOR -lt 4 -o $COMPILER_MAJOR -eq 4 -a $COMPILER_MINOR -lt 8 ] ;
 fi;
 
 # check for host CXX
-CXXCOMPILER=$(which $HOSTCXX_NOCCACHE 2> /dev/null)
+CXXCOMPILER=$(command -v $HOSTCXX_NOCCACHE 2> /dev/null)
 if [ -z "$CXXCOMPILER" ] ; then
-	CXXCOMPILER=$(which c++ 2> /dev/null)
+	CXXCOMPILER=$(command -v c++ 2> /dev/null)
 fi
 
 if [ -z "$CXXCOMPILER" ] ; then
@@ -164,7 +164,7 @@ fi
 # Check that a few mandatory programs are installed
 missing_progs="no"
 for prog in perl tar wget cpio unzip rsync bc ${DL_TOOLS} ; do
-	if ! which $prog > /dev/null ; then
+	if ! command -v $prog > /dev/null ; then
 		echo "You must install '$prog' on your build machine";
 		missing_progs="yes"
 		if test $prog = "svn" ; then
@@ -198,7 +198,7 @@ if [ "${PATCH_MAJOR}" -lt 2 ] || [ "${PATCH_MAJOR}" -eq 2 -a "${PATCH_MINOR}" -l
 fi
 
 if grep ^BR2_NEEDS_HOST_UTF8_LOCALE=y $BR2_CONFIG > /dev/null; then
-	if ! which locale > /dev/null ; then
+	if ! command -v locale > /dev/null ; then
 		echo
 		echo "You need locale support on your build machine to build a toolchain supporting locales"
 		exit 1 ;
diff --git a/toolchain/toolchain-external/pkg-toolchain-external.mk b/toolchain/toolchain-external/pkg-toolchain-external.mk
index 68d7a3fe21..e3f61294f8 100644
--- a/toolchain/toolchain-external/pkg-toolchain-external.mk
+++ b/toolchain/toolchain-external/pkg-toolchain-external.mk
@@ -74,7 +74,7 @@ endif
 ifeq ($(TOOLCHAIN_EXTERNAL_INSTALL_DIR),)
 ifneq ($(TOOLCHAIN_EXTERNAL_PREFIX),)
 # if no path set, figure it out from path
-TOOLCHAIN_EXTERNAL_BIN := $(dir $(shell which $(TOOLCHAIN_EXTERNAL_PREFIX)-gcc))
+TOOLCHAIN_EXTERNAL_BIN := $(dir $(shell command -v $(TOOLCHAIN_EXTERNAL_PREFIX)-gcc))
 endif
 else
 TOOLCHAIN_EXTERNAL_REL_BIN_PATH = $(call qstrip,$(BR2_TOOLCHAIN_EXTERNAL_REL_BIN_PATH))
-- 
2.33.0

_______________________________________________
buildroot mailing list
buildroot@lists.buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [RFC PATCH 2/2] support/dependencies: don't check for `which'

[Petr Vorel]

It's requirement has been removed in previous commit.

Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
---
 support/dependencies/dependencies.sh | 2 --
 1 file changed, 2 deletions(-)

diff --git a/support/dependencies/dependencies.sh b/support/dependencies/dependencies.sh
index c5dcb86231..a3e0b3ecc4 100755
--- a/support/dependencies/dependencies.sh
+++ b/support/dependencies/dependencies.sh
@@ -65,8 +65,6 @@ check_prog_host()
 	fi
 }
 
-# Verify that which is installed
-check_prog_host "which"
 # Verify that sed is installed
 check_prog_host "sed"
 
-- 
2.33.0

_______________________________________________
buildroot mailing list
buildroot@lists.buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [RFC PATCH 0/2] use `command -v' instead of `which'

[Arnout Vandecappelle]

On 21/09/2021 22:51, Petr Vorel wrote:
> Hi,
> 
> I've tested the patchset on dash as the default shell. But it certainly
> deserve more people to have look and test.

  Well, as the commit message says: it's POSIX so it should be supported by 
everything. which has a much smaller chance of being supported.

> 
> NOTE: If accepted, we might want to use `command -v' also in scripts:
> 
> $ git grep '$(which'
> package/fakedate/fakedate:for date in $(which -a date |tac); do
> package/transmission/S92transmission:DAEMON=$(which $NAME)

  Also:

- remove it from the documentation (adding-packages-asciidoc.txt)
- remove it from docs/manual/prerequisite.txt
- it's still used in package/doc-asciidoc.mk
- package/libxml-parser-perl/libxml-parser-perl.mk
- utils/brmake

  But these two are already applied to master, thanks!

  Regards,
  Arnout

> 
> Kind regards,
> Petr
> 
> Petr Vorel (2):
>    make: support: use `command -v' instead of `which'
>    support/dependencies: don't check for `which'
> 
>   Makefile                                      | 22 +++++++++----------
>   package/Makefile.in                           |  8 +++----
>   support/dependencies/check-host-bison-flex.mk |  4 ++--
>   support/dependencies/check-host-cmake.sh      |  2 +-
>   support/dependencies/check-host-gzip.sh       |  2 +-
>   support/dependencies/check-host-lzip.sh       |  4 ++--
>   support/dependencies/check-host-python3.sh    |  2 +-
>   support/dependencies/check-host-tar.sh        |  4 ++--
>   support/dependencies/check-host-xzcat.sh      |  4 ++--
>   support/dependencies/dependencies.sh          | 18 +++++++--------
>   .../pkg-toolchain-external.mk                 |  2 +-
>   11 files changed, 35 insertions(+), 37 deletions(-)
> 
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [RFC PATCH 0/2] use `command -v' instead of `which'

[Yann E. MORIN]

Petr, Arnout, All,

On 2021-09-26 23:32 +0200, Arnout Vandecappelle spake thusly:
> On 21/09/2021 22:51, Petr Vorel wrote:
> >I've tested the patchset on dash as the default shell. But it certainly
> >deserve more people to have look and test.
> 
>  Well, as the commit message says: it's POSIX so it should be supported by
> everything. which has a much smaller chance of being supported.

This is causing quite some issues.

First, 'command -v' does not behave the same way 'which' used to, when
passed more than one parameter, because some shells are not compliant to
POSIX (this might be a bug, but nonetheless it affects the most widely
used shell out there, bash) [0].

Second, this is causing a lot of error messages:

    $ make defconfig
    [...]
    $ make help
    make[1]: command: Command not found
    [...]

The original commit reports that 'which' is broken in Debian, but I was
not able to reproduce in Bullseye, where 'which' still works as expected
and does not emit any extra warning.

So, we are trying to fix something that is broken on a development
version of Debian, but that still works in all known released
distributions.

I usually am quite in favour of sticking to POSIX tools, but which has
been ubiquitous in the past 30 years or so, and I would consider that
Debian's which *is* broken for reporting such deprecation warnings.

So, I suggest that we do revert this patch, and work on a better
transition away from which, if at all. One very quick solution would be
to bundle our own which in Buildroot and then we'd have a quick way out
of that Debian's mess...

Anyway, I'd vote "revert".

Regards,
Yann E. MORIN.

[0] https://lore.kernel.org/buildroot/YVTIghzHs82uFBIe@pevik/T/#m95c17eb8374e4e3dd6eee700d397aa12cca0739e

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [RFC PATCH 0/2] use `command -v' instead of `which'

[Petr Vorel]

Hi Yann, all,

> Petr, Arnout, All,

> On 2021-09-26 23:32 +0200, Arnout Vandecappelle spake thusly:
> > On 21/09/2021 22:51, Petr Vorel wrote:
> > >I've tested the patchset on dash as the default shell. But it certainly
> > >deserve more people to have look and test.

> >  Well, as the commit message says: it's POSIX so it should be supported by
> > everything. which has a much smaller chance of being supported.

> This is causing quite some issues.

> First, 'command -v' does not behave the same way 'which' used to, when
> passed more than one parameter, because some shells are not compliant to
> POSIX (this might be a bug, but nonetheless it affects the most widely
> used shell out there, bash) [0].
Yes, there has been a proposal, how to fix this.

> Second, this is causing a lot of error messages:

>     $ make defconfig
>     [...]
>     $ make help
>     make[1]: command: Command not found
>     [...]
New error. But I was not able to reproduce it on x86_64 on current master
(5916cc5011). What am I missing to reproduce it?

> The original commit reports that 'which' is broken in Debian, but I was
> not able to reproduce in Bullseye, where 'which' still works as expected
> and does not emit any extra warning.
Yes, the waring is in debianutils 5.x, which is still in Debian unstable (not
even in the testing) [1].

> So, we are trying to fix something that is broken on a development
> version of Debian, but that still works in all known released
> distributions.
'command -v' and 'type' has been here quite long time as well.

> I usually am quite in favour of sticking to POSIX tools, but which has
> been ubiquitous in the past 30 years or so, and I would consider that
> Debian's which *is* broken for reporting such deprecation warnings.

> So, I suggest that we do revert this patch, and work on a better
> transition away from which, if at all. One very quick solution would be
> to bundle our own which in Buildroot and then we'd have a quick way out
> of that Debian's mess...
Sure, if it causes problems which are not easily fixed, I'm not against
reverting it. But I don't think that problem is that complex, that we'd need to
compile which. But I apologize for causing troubles.

Kind regards,
Petr

> Anyway, I'd vote "revert".

> Regards,
> Yann E. MORIN.

> [0] https://lore.kernel.org/buildroot/YVTIghzHs82uFBIe@pevik/T/#m95c17eb8374e4e3dd6eee700d397aa12cca0739e
[1] https://tracker.debian.org/pkg/debianutils
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [RFC PATCH 0/2] use `command -v' instead of `which'

[Yann E. MORIN]

Petr, All,

On 2021-09-30 22:16 +0200, Petr Vorel spake thusly:
> > Petr, Arnout, All,
> > On 2021-09-26 23:32 +0200, Arnout Vandecappelle spake thusly:
> > > On 21/09/2021 22:51, Petr Vorel wrote:
> > > >I've tested the patchset on dash as the default shell. But it certainly
> > > >deserve more people to have look and test.
> > >  Well, as the commit message says: it's POSIX so it should be supported by
> > > everything. which has a much smaller chance of being supported.
> > This is causing quite some issues.
> 
> > First, 'command -v' does not behave the same way 'which' used to, when
> > passed more than one parameter, because some shells are not compliant to
> > POSIX (this might be a bug, but nonetheless it affects the most widely
> > used shell out there, bash) [0].
> Yes, there has been a proposal, how to fix this.

The problem I see with that proposal, is that it catters for a few
variables only. We might not even notice until much later that something
else is borken in some weird ways... And we were lucky that Markus did
the investigation and was able to find the actual root cause.

> > Second, this is causing a lot of error messages:
> >     $ make defconfig
> >     [...]
> >     $ make help
> >     make[1]: command: Command not found
> >     [...]
> New error. But I was not able to reproduce it on x86_64 on current master
> (5916cc5011). What am I missing to reproduce it?

I'm on a bog-down standard Ubuntu 20.04, prety up-to-date. My /bin/sh is
bash.

> > The original commit reports that 'which' is broken in Debian, but I was
> > not able to reproduce in Bullseye, where 'which' still works as expected
> > and does not emit any extra warning.
> Yes, the waring is in debianutils 5.x, which is still in Debian unstable (not
> even in the testing) [1].
> 
> > So, we are trying to fix something that is broken on a development
> > version of Debian, but that still works in all known released
> > distributions.
> 'command -v' and 'type' has been here quite long time as well.

I am not denying this. I'm just saying that we are trading a working
situation in all released distributions and a failure in Debian
unstable, for a failure in a quite a lot, in not most, of the previously
working cases.

> > I usually am quite in favour of sticking to POSIX tools, but which has
> > been ubiquitous in the past 30 years or so, and I would consider that
> > Debian's which *is* broken for reporting such deprecation warnings.
> 
> > So, I suggest that we do revert this patch, and work on a better
> > transition away from which, if at all. One very quick solution would be
> > to bundle our own which in Buildroot and then we'd have a quick way out
> > of that Debian's mess...
> Sure, if it causes problems which are not easily fixed, I'm not against
> reverting it. But I don't think that problem is that complex, that we'd need to
> compile which.

which is a shell script in Debian; we need not compile it at all. Let's
just grab the latest working which that did not have that warning:

    $ file /usr/bin/which
    /usr/bin/which: POSIX shell script, ASCII text executable

> But I apologize for causing troubles.

Do not apologise, this is not needed. The trouble (as you say) is
standard development issues that will eventually be resolved, one way or
another, no worries. ;-)

Thank you! :-)

Regards,
Yann E. MORIN.

> Kind regards,
> Petr
> 
> > Anyway, I'd vote "revert".
> 
> > Regards,
> > Yann E. MORIN.
> 
> > [0] https://lore.kernel.org/buildroot/YVTIghzHs82uFBIe@pevik/T/#m95c17eb8374e4e3dd6eee700d397aa12cca0739e
> [1] https://tracker.debian.org/pkg/debianutils

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [RFC PATCH 0/2] use `command -v' instead of `which'

[Yann E. MORIN]

Petr, Arnout, All,

On 2021-09-30 22:16 +0200, Petr Vorel spake thusly:
> > On 2021-09-26 23:32 +0200, Arnout Vandecappelle spake thusly:
> > > On 21/09/2021 22:51, Petr Vorel wrote:
> > This is causing quite some issues.
[--SNIP--]
> > Second, this is causing a lot of error messages:
> >     $ make defconfig
> >     [...]
> >     $ make help
> >     make[1]: command: Command not found
> >     [...]
> New error. But I was not able to reproduce it on x86_64 on current master
> (5916cc5011). What am I missing to reproduce it?

We've investigated with Arnout, who was also unable to reproduce, and we
eventually found the cause for this issue, which is two fold.

First, make, at least in some versions of make, will run commands that
it believes are "simple", directly with execve() (or any other exec*()
wrapper), instead of running it through a shell via system().

This is what happens when it see a command like this:

    export PERL=$(shell command -v perl)

which we have in package/Makefile.in@240. In this case, make will try to
run the command (split à-la python), as can be seen with strace:

    ['command', '-v', 'perl']

Second, the issue was invisible to Arnout, because the distribution he
uses, Fedora, provides /usr/bin/command, which is a simple shell script
that just basically does:

    #!/bin/sh
    builtin command "${@}"

However, this is provided by no package in the distribution I use,
Ubuntu 20.04.1 LTS (filtering to ignore /usr/bin/commander et al.):

    $ apt-file search bin/command |grep -E 'bin/command$'
    [nothing]

So, probably you did not see the error either, because your distribution
also provides command as an actual executable. Could you check that by
running:    which command     (Ahaha! :-])

Note: if we change the line above to

    export PERL=$(shell command -v perl 2>/dev/null)

then make no longer believes this is a simple command, and will execute
with system() and the warning goes away.

So, bottom line, there are more impacts than previously expected, and we
need to think the transition more carefully.

And to be extra clear: I am OK with transitionning away from which, or
at least from relying on which being provided by the distro.

> > So, I suggest that we do revert this patch, and work on a better
> > transition away from which, if at all. One very quick solution would be
> > to bundle our own which in Buildroot and then we'd have a quick way out
> > of that Debian's mess...
> Sure, if it causes problems which are not easily fixed, I'm not against
> reverting it. But I don't think that problem is that complex, that we'd need to
> compile which. But I apologize for causing troubles.

And again, I want to reiterate that: you have no reason to apologise. :-)
Your patch was reviewed and applied, and there was no way we could have
found the issues above without trying in the first place.

But now, we can't keep this in the current state so, after discussing
this with Arnout, I am going to revert the patch.

We can look at a better way to solve the Debian unstable issue about
which, probably the first being to open a bug with them, so that they
revert the warning, and second to find a way to no longer rely on which
from the distro (either by transitionning to something else, or by
bundling our own?).

Thanks for your work on Buildroot! :-)

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [RFC PATCH 0/2] use `command -v' instead of `which'

[Petr Vorel]

Hi Yann, Arnout, all,

> Petr, Arnout, All,

> On 2021-09-30 22:16 +0200, Petr Vorel spake thusly:
> > > On 2021-09-26 23:32 +0200, Arnout Vandecappelle spake thusly:
> > > > On 21/09/2021 22:51, Petr Vorel wrote:
> > > This is causing quite some issues.
> [--SNIP--]
> > > Second, this is causing a lot of error messages:
> > >     $ make defconfig
> > >     [...]
> > >     $ make help
> > >     make[1]: command: Command not found
> > >     [...]
> > New error. But I was not able to reproduce it on x86_64 on current master
> > (5916cc5011). What am I missing to reproduce it?

> We've investigated with Arnout, who was also unable to reproduce, and we
> eventually found the cause for this issue, which is two fold.
Thanks a lot for investigation!

> First, make, at least in some versions of make, will run commands that
> it believes are "simple", directly with execve() (or any other exec*()
> wrapper), instead of running it through a shell via system().

> This is what happens when it see a command like this:

>     export PERL=$(shell command -v perl)
Hm, that's bad. Can you post any version which is affected?
It looks like I tested it only on 4.3, but have plenty of VM with older
make releases.

> which we have in package/Makefile.in@240. In this case, make will try to
NOTE for myself (when debugging this later): package/Makefile.in (@240 is probably garbage)

> run the command (split à-la python), as can be seen with strace:

>     ['command', '-v', 'perl']

> Second, the issue was invisible to Arnout, because the distribution he
> uses, Fedora, provides /usr/bin/command, which is a simple shell script
> that just basically does:

>     #!/bin/sh
>     builtin command "${@}"

Hm, wrapping shell builtin into script is really strange.
BTW I was already thinking to add similar script to buildroot,
but I'm really surprised that Fedora added that to distro.

> However, this is provided by no package in the distribution I use,
> Ubuntu 20.04.1 LTS (filtering to ignore /usr/bin/commander et al.):

>     $ apt-file search bin/command |grep -E 'bin/command$'
>     [nothing]

This is probably Fedora/RHEL specific. I'll investigate which package it belongs
and ask Fedora maintainer for a reason (unfortunately Fedora does not have any
search like Debian https://packages.debian.org/ [1]).

> So, probably you did not see the error either, because your distribution
> also provides command as an actual executable. Could you check that by
> running:    which command     (Ahaha! :-])

> Note: if we change the line above to

>     export PERL=$(shell command -v perl 2>/dev/null)

> then make no longer believes this is a simple command, and will execute
> with system() and the warning goes away.
Hm, definitely worth to more investigate which make releases are affected and
report if not already fixed.

> So, bottom line, there are more impacts than previously expected, and we
> need to think the transition more carefully.
+1

> And to be extra clear: I am OK with transitionning away from which, or
> at least from relying on which being provided by the distro.
Thanks!

Also going back to the issue with bash 'command -v' implementation. First is it
also relevant to this issue or not?

You mentioned Ubuntu with bash as /bin/sh as a default shell in previous mail.
Testing 'make defconfig && make help 2>&1 |grep -i found' on bash as /bin/sh
does not trigger the error (make[1]: command: Command not found). Thus bash it's
not the problem (at least less problematic than make you mentioned).

Also, does anybody understand POSIX spec [2] whether only single command_name
can be used as Markus reported [3]? Should we report it to bash?

bash:
$ command -v ls uname
alias ls='ls --color=auto'
/usr/bin/uname

dash:
$ command -v ls uname
/usr/bin/ls

busybox sh
$ command -v ls uname
/usr/bin/ls

 I always used command -v with single argument in LTP and iputils, thus I
didn't get this problem.

[1] https://packages.debian.org/search?suite=default&section=all&arch=any&searchon=contents&keywords=%2Fusr%2Fbin%2Fcommand
[2] https://pubs.opengroup.org/onlinepubs/9699919799/utilities/command.html
[3] https://lore.kernel.org/buildroot/YVTIghzHs82uFBIe@pevik/T/#m95c17eb8374e4e3dd6eee700d397aa12cca0739e
[4] https://savannah.gnu.org/projects/which

> > > So, I suggest that we do revert this patch, and work on a better
> > > transition away from which, if at all. One very quick solution would be
> > > to bundle our own which in Buildroot and then we'd have a quick way out
> > > of that Debian's mess...
> > Sure, if it causes problems which are not easily fixed, I'm not against
> > reverting it. But I don't think that problem is that complex, that we'd need to
> > compile which. But I apologize for causing troubles.

> And again, I want to reiterate that: you have no reason to apologise. :-)
> Your patch was reviewed and applied, and there was no way we could have
> found the issues above without trying in the first place.
Yes, but next time I need testing on more distros, not just my laptop.

> But now, we can't keep this in the current state so, after discussing
> this with Arnout, I am going to revert the patch.
Sure, understand (I see you already reverted it).

> We can look at a better way to solve the Debian unstable issue about
> which, probably the first being to open a bug with them, so that they
> revert the warning, and second to find a way to no longer rely on which
> from the distro (either by transitionning to something else, or by
> bundling our own?).
I'll need to find more time to investigate the problem to suggest some
solution. I'd prefer to fix builtin shell than backport which (which is shell
just on Debian, there is also C version in upstream which discontinued [4]).


> Thanks for your work on Buildroot! :-)
yw :)

Kind regards,
Petr

> Regards,
> Yann E. MORIN.
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [RFC PATCH 0/2] use `command -v' instead of `which'

[Arnout Vandecappelle]

On 02/10/2021 21:22, Petr Vorel wrote:
> Hi Yann, Arnout, all,
> 
>> Petr, Arnout, All,
> 
>> On 2021-09-30 22:16 +0200, Petr Vorel spake thusly:
>>>> On 2021-09-26 23:32 +0200, Arnout Vandecappelle spake thusly:
>>>>> On 21/09/2021 22:51, Petr Vorel wrote:
>>>> This is causing quite some issues.
>> [--SNIP--]
>>>> Second, this is causing a lot of error messages:
>>>>      $ make defconfig
>>>>      [...]
>>>>      $ make help
>>>>      make[1]: command: Command not found
>>>>      [...]
>>> New error. But I was not able to reproduce it on x86_64 on current master
>>> (5916cc5011). What am I missing to reproduce it?
> 
>> We've investigated with Arnout, who was also unable to reproduce, and we
>> eventually found the cause for this issue, which is two fold.
> Thanks a lot for investigation!
> 
>> First, make, at least in some versions of make, will run commands that
>> it believes are "simple", directly with execve() (or any other exec*()
>> wrapper), instead of running it through a shell via system().
> 
>> This is what happens when it see a command like this:
> 
>>      export PERL=$(shell command -v perl)
> Hm, that's bad. Can you post any version which is affected?
> It looks like I tested it only on 4.3, but have plenty of VM with older
> make releases.
> 
>> which we have in package/Makefile.in@240. In this case, make will try to
> NOTE for myself (when debugging this later): package/Makefile.in (@240 is probably garbage)

  It *is* on line 240 in current master, and package/Makefile.in hasn't changed 
since July 4 (except for the command -v replacement, of course).

> 
>> run the command (split à-la python), as can be seen with strace:
> 
>>      ['command', '-v', 'perl']
> 
>> Second, the issue was invisible to Arnout, because the distribution he
>> uses, Fedora, provides /usr/bin/command, which is a simple shell script
>> that just basically does:
> 
>>      #!/bin/sh
>>      builtin command "${@}"
> 
> Hm, wrapping shell builtin into script is really strange.

  POSIX doesn't say anything about `command` etc. being shell builtins. Thus, a 
program written for POSIX may assume that it can execvp("command", ...) (i.e. 
without going through the shell).

  I assume that someone in RedHat encountered such an issue and that that was 
their solution.

> BTW I was already thinking to add similar script to buildroot,

  Right - instead of providing our on `which`, we can also provide our own 
`command`!

  One small caveat though: we currently don't add anything to PATH. Adding our 
own thing (whether it's which or command) will require us to add something to 
PATH, which by itself can have surprising effects as well.

> but I'm really surprised that Fedora added that to distro.
> 
>> However, this is provided by no package in the distribution I use,
>> Ubuntu 20.04.1 LTS (filtering to ignore /usr/bin/commander et al.):
> 
>>      $ apt-file search bin/command |grep -E 'bin/command$'
>>      [nothing]
> 
> This is probably Fedora/RHEL specific. I'll investigate which package it belongs

  Yes it is. It's provided by the bash package.

> and ask Fedora maintainer for a reason (unfortunately Fedora does not have any
> search like Debian https://packages.debian.org/ [1]).
> 
>> So, probably you did not see the error either, because your distribution
>> also provides command as an actual executable. Could you check that by
>> running:    which command     (Ahaha! :-])
> 
>> Note: if we change the line above to
> 
>>      export PERL=$(shell command -v perl 2>/dev/null)
> 
>> then make no longer believes this is a simple command, and will execute
>> with system() and the warning goes away.

  However, a future version of GNU make may very well decide that '2>/dev/null' 
is something it can handle itself, and bypass the shell as well.

> Hm, definitely worth to more investigate which make releases are affected and
> report if not already fixed.
> 
>> So, bottom line, there are more impacts than previously expected, and we
>> need to think the transition more carefully.
> +1
> 
>> And to be extra clear: I am OK with transitionning away from which, or
>> at least from relying on which being provided by the distro.
> Thanks!
> 
> Also going back to the issue with bash 'command -v' implementation. First is it
> also relevant to this issue or not?

  It's an issue if we pass several parameters to `command -v`. We probably 
should simply never do that - the case that Markus found is a bug anyway.


> You mentioned Ubuntu with bash as /bin/sh as a default shell in previous mail.
> Testing 'make defconfig && make help 2>&1 |grep -i found' on bash as /bin/sh
> does not trigger the error (make[1]: command: Command not found). Thus bash it's
> not the problem (at least less problematic than make you mentioned).
> 
> Also, does anybody understand POSIX spec [2] whether only single command_name
> can be used as Markus reported [3]? Should we report it to bash?

  The POSIX spec says:

command [-p][-v|-V] command_name

So it's only specified with a single argument. The POSIX spec is pretty vague 
about what to do with stuff that is not specified, like a second argument. So I 
guess implementations can do what they want...


  By the way, the differences between which and command -v are a bit larger than 
this. command -v also works for shell builtins, while which doesn't - unless 
which is defined as a function, which is the case for the `which` package in 
Fedora...

$ command -v ls
alias ls='ls -AFh'
$ which ls
alias ls='ls -AFh'
         /usr/bin/ls
$ /bin/which ls
/usr/bin/ls
$ type which
which is a function
which ()
{
     ( alias;
     eval ${which_declare} ) | /usr/bin/which --tty-only --read-alias 
--read-functions --show-tilde --show-dot "$@"
}


  I think we can safely say: this is a mess :-)


  From this, I actually conclude that it is indeed safer to go to command -v, 
with a wrapper script that we add to $PATH to handle the case where make tries 
to bypass the shell.


  Regards,
  Arnout


> 
> bash:
> $ command -v ls uname
> alias ls='ls --color=auto'
> /usr/bin/uname
> 
> dash:
> $ command -v ls uname
> /usr/bin/ls
> 
> busybox sh
> $ command -v ls uname
> /usr/bin/ls
> 
>   I always used command -v with single argument in LTP and iputils, thus I
> didn't get this problem.
> 
> [1] https://packages.debian.org/search?suite=default&section=all&arch=any&searchon=contents&keywords=%2Fusr%2Fbin%2Fcommand
> [2] https://pubs.opengroup.org/onlinepubs/9699919799/utilities/command.html
> [3] https://lore.kernel.org/buildroot/YVTIghzHs82uFBIe@pevik/T/#m95c17eb8374e4e3dd6eee700d397aa12cca0739e
> [4] https://savannah.gnu.org/projects/which
> 
>>>> So, I suggest that we do revert this patch, and work on a better
>>>> transition away from which, if at all. One very quick solution would be
>>>> to bundle our own which in Buildroot and then we'd have a quick way out
>>>> of that Debian's mess...
>>> Sure, if it causes problems which are not easily fixed, I'm not against
>>> reverting it. But I don't think that problem is that complex, that we'd need to
>>> compile which. But I apologize for causing troubles.
> 
>> And again, I want to reiterate that: you have no reason to apologise. :-)
>> Your patch was reviewed and applied, and there was no way we could have
>> found the issues above without trying in the first place.
> Yes, but next time I need testing on more distros, not just my laptop.
> 
>> But now, we can't keep this in the current state so, after discussing
>> this with Arnout, I am going to revert the patch.
> Sure, understand (I see you already reverted it).
> 
>> We can look at a better way to solve the Debian unstable issue about
>> which, probably the first being to open a bug with them, so that they
>> revert the warning, and second to find a way to no longer rely on which
>> from the distro (either by transitionning to something else, or by
>> bundling our own?).
> I'll need to find more time to investigate the problem to suggest some
> solution. I'd prefer to fix builtin shell than backport which (which is shell
> just on Debian, there is also C version in upstream which discontinued [4]).
> 
> 
>> Thanks for your work on Buildroot! :-)
> yw :)
> 
> Kind regards,
> Petr
> 
>> Regards,
>> Yann E. MORIN.
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [RFC PATCH 0/2] use `command -v' instead of `which'

[Petr Vorel]

Hi Arnout, Yann, all,

<snip>

> > > which we have in package/Makefile.in@240. In this case, make will try to
> > NOTE for myself (when debugging this later): package/Makefile.in (@240 is probably garbage)

>  It *is* on line 240 in current master, and package/Makefile.in hasn't
> changed since July 4 (except for the command -v replacement, of course).

+1, thanks.

> > > run the command (split à-la python), as can be seen with strace:

> > >      ['command', '-v', 'perl']

> > > Second, the issue was invisible to Arnout, because the distribution he
> > > uses, Fedora, provides /usr/bin/command, which is a simple shell script
> > > that just basically does:

> > >      #!/bin/sh
> > >      builtin command "${@}"

> > Hm, wrapping shell builtin into script is really strange.

>  POSIX doesn't say anything about `command` etc. being shell builtins. Thus,
> a program written for POSIX may assume that it can execvp("command", ...)
> (i.e. without going through the shell).
Good point. Obviously POSIX didn't realize problems we encountered. IMHO it'd be
great to specify this precisely in POSIX.

>  I assume that someone in RedHat encountered such an issue and that that was
> their solution.
I wonder if they tried to contribute it to coreutils. It'd be great to have
upstream solution.

> > BTW I was already thinking to add similar script to buildroot,

>  Right - instead of providing our on `which`, we can also provide our own
> `command`!
Yes (fairly smaller amount of code, which likely solve all our problems).
I'll also test "builtin command" in Makefile (I suppose it will be the same:
"Command not found").

>  One small caveat though: we currently don't add anything to PATH. Adding
> our own thing (whether it's which or command) will require us to add
> something to PATH, which by itself can have surprising effects as well.
Good point. IMHO we'll have to go this way (and solve any problem with PATH)
even if we wan to take 'which'. Sooner or later which version disappear from
Debian. GNU which is discontinued, but I suppose distros which ship it now (RHEL/Fedora and SLES/Tumbleweed and probably many others) will keep it for long time.

> > but I'm really surprised that Fedora added that to distro.

> > > However, this is provided by no package in the distribution I use,
> > > Ubuntu 20.04.1 LTS (filtering to ignore /usr/bin/commander et al.):

> > >      $ apt-file search bin/command |grep -E 'bin/command$'
> > >      [nothing]

> > This is probably Fedora/RHEL specific. I'll investigate which package it belongs

>  Yes it is. It's provided by the bash package.
+1

> > and ask Fedora maintainer for a reason (unfortunately Fedora does not have any
> > search like Debian https://packages.debian.org/ [1]).

> > > So, probably you did not see the error either, because your distribution
> > > also provides command as an actual executable. Could you check that by
> > > running:    which command     (Ahaha! :-])

> > > Note: if we change the line above to

> > >      export PERL=$(shell command -v perl 2>/dev/null)

> > > then make no longer believes this is a simple command, and will execute
> > > with system() and the warning goes away.

>  However, a future version of GNU make may very well decide that
> '2>/dev/null' is something it can handle itself, and bypass the shell as
> well.
Maybe it'd be good to teach make always run 'command' via system().

> > Hm, definitely worth to more investigate which make releases are affected and
> > report if not already fixed.

> > > So, bottom line, there are more impacts than previously expected, and we
> > > need to think the transition more carefully.
> > +1

> > > And to be extra clear: I am OK with transitionning away from which, or
> > > at least from relying on which being provided by the distro.
> > Thanks!

> > Also going back to the issue with bash 'command -v' implementation. First is it
> > also relevant to this issue or not?

>  It's an issue if we pass several parameters to `command -v`. We probably
> should simply never do that - the case that Markus found is a bug anyway.
+1

> > You mentioned Ubuntu with bash as /bin/sh as a default shell in previous mail.
> > Testing 'make defconfig && make help 2>&1 |grep -i found' on bash as /bin/sh
> > does not trigger the error (make[1]: command: Command not found). Thus bash it's
> > not the problem (at least less problematic than make you mentioned).

> > Also, does anybody understand POSIX spec [2] whether only single command_name
> > can be used as Markus reported [3]? Should we report it to bash?

>  The POSIX spec says:

> command [-p][-v|-V] command_name

> So it's only specified with a single argument. The POSIX spec is pretty
Yep, I understood it that way.
> vague about what to do with stuff that is not specified, like a second
> argument. So I guess implementations can do what they want...
+1

>  By the way, the differences between which and command -v are a bit larger
> than this. command -v also works for shell builtins, while which doesn't -
> unless which is defined as a function, which is the case for the `which`
> package in Fedora...

> $ command -v ls
> alias ls='ls -AFh'
> $ which ls
> alias ls='ls -AFh'
>         /usr/bin/ls
> $ /bin/which ls
> /usr/bin/ls
> $ type which
> which is a function
> which ()
> {
>     ( alias;
>     eval ${which_declare} ) | /usr/bin/which --tty-only --read-alias
> --read-functions --show-tilde --show-dot "$@"
> }

Yes, 'type' is quite different than which/command -v. More verbose, thus I
understand why 'type' is POSIX extension and 'command -v' mandatory.

>  I think we can safely say: this is a mess :-)


>  From this, I actually conclude that it is indeed safer to go to command -v,
> with a wrapper script that we add to $PATH to handle the case where make
> tries to bypass the shell.
+1. I'll try to have look into it.

Thanks a lot to all for your comments.

Kind regards,
Petr

>  Regards,
>  Arnout

<snip>
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [RFC PATCH 0/2] use `command -v' instead of `which'

[Peter Korsgaard]

>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:

Hi,

 >  From this, I actually conclude that it is indeed safer to go to
 > command -v, with a wrapper script that we add to $PATH to handle the
 > case where make tries to bypass the shell.

If command is always a shell builtin and we already do the dance to find
bash, how about just doing something like:

$(shell $(SHELL) -c "command -v ...")?

-- 
Bye, Peter Korsgaard
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [RFC PATCH 0/2] use `command -v' instead of `which'

[Petr Vorel]

Hi all,

> >>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:

> Hi,

>  >  From this, I actually conclude that it is indeed safer to go to
>  > command -v, with a wrapper script that we add to $PATH to handle the
>  > case where make tries to bypass the shell.

> If command is always a shell builtin and we already do the dance to find
> bash, how about just doing something like:

> $(shell $(SHELL) -c "command -v ...")?
Peter, good point, thanks! That would safe us to modify path.
This works on make 4.3 on Debian with /bin/sh, I'll try to test it on more VMs
(+ find one where plain $(shell command -v ...) does not work).

Kind regards,
Petr
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot