[LTP] [PATCH 0/3] shell: remove which, use type or command -v

[LTP] [PATCH 0/3] shell: remove which, use type or command -v

[Petr Vorel]

Petr Vorel (3):
  tst_test.sh: Simplify tst_cmd_available()
  binfmt_misc02.sh: Use "command -v" instead of "which"
  commands: Drop which01.sh

 runtest/commands                              |   1 -
 testcases/commands/which/Makefile             |  11 --
 testcases/commands/which/which01.sh           | 107 ------------------
 .../kernel/fs/binfmt_misc/binfmt_misc02.sh    |  14 +--
 testcases/lib/tst_test.sh                     |  13 +--
 5 files changed, 8 insertions(+), 138 deletions(-)
 delete mode 100644 testcases/commands/which/Makefile
 delete mode 100755 testcases/commands/which/which01.sh

-- 
2.32.0

[LTP] [PATCH 1/3] tst_test.sh: Simplify tst_cmd_available()

[Petr Vorel]

"command -v" [1] and "type" [2] are POSIX. They're supported in all
common shells (bash, zsh, dash, busybox sh, mksh). Thus we don't have to
fallback on "which", which has been discontinued after 2.21 release in
2015 due this (git repository is empty [3]).

Use "type" instead of "command -v" which is IMHO more known.

Also drop explicit return as the exit code is reliable an all
implementations.

[1] https://pubs.opengroup.org/onlinepubs/9699919799/utilities/command.html
[2] https://pubs.opengroup.org/onlinepubs/9699919799/utilities/type.html
[3] https://git.savannah.gnu.org/cgit/which.git

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/lib/tst_test.sh | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
index c6aa2c487..fa35a64f1 100644
--- a/testcases/lib/tst_test.sh
+++ b/testcases/lib/tst_test.sh
@@ -346,18 +346,7 @@ tst_virt_hyperv()
 
 tst_cmd_available()
 {
-	if type command > /dev/null 2>&1; then
-		command -v $1 > /dev/null 2>&1 || return 1
-	else
-		which $1 > /dev/null 2>&1
-		if [ $? -eq 0 ]; then
-			return 0
-		elif [ $? -eq 127 ]; then
-			tst_brk TCONF "missing which command"
-		else
-			return 1
-		fi
-	fi
+	type $1 >/dev/null 2>&1
 }
 
 tst_require_cmds()
-- 
2.32.0

[LTP] [PATCH 2/3] binfmt_misc02.sh: 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]) it provides warning on Debian/Ubuntu [2].

Use "command -v" which is POSIX [3] and supported on all common shells
(bash, zsh, dash, busybox sh, mksh).

[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 <pvorel@suse.cz>
---
 testcases/kernel/fs/binfmt_misc/binfmt_misc02.sh | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/testcases/kernel/fs/binfmt_misc/binfmt_misc02.sh b/testcases/kernel/fs/binfmt_misc/binfmt_misc02.sh
index 9dbcd68cc..cb4e770e3 100755
--- a/testcases/kernel/fs/binfmt_misc/binfmt_misc02.sh
+++ b/testcases/kernel/fs/binfmt_misc/binfmt_misc02.sh
@@ -19,7 +19,7 @@
 
 TST_CNT=6
 TST_TESTFUNC=do_test
-TST_NEEDS_CMDS="which cat head"
+TST_NEEDS_CMDS="cat head"
 
 . binfmt_misc_lib.sh
 
@@ -89,17 +89,17 @@ verify_binfmt_misc()
 do_test()
 {
 	case $1 in
-	1) verify_binfmt_misc ":textension:E::extension::$(which cat):" \
+	1) verify_binfmt_misc ":textension:E::extension::$(type cat):" \
 			      "$TST_DATAROOT/file.extension" "1";;
-	2) verify_binfmt_misc ":tmagic:M:1:This::$(which cat):" \
+	2) verify_binfmt_misc ":tmagic:M:1:This::$(type cat):" \
 			      "$TST_DATAROOT/file.magic" "1";;
-	3) verify_binfmt_misc ".textension.E..extension..$(which cat)." \
+	3) verify_binfmt_misc ".textension.E..extension..$(type cat)." \
 			      "$TST_DATAROOT/file.extension" "1";;
-	4) verify_binfmt_misc ",tmagic,M,1,This,,$(which cat)," \
+	4) verify_binfmt_misc ",tmagic,M,1,This,,$(type cat)," \
 			      "$TST_DATAROOT/file.magic" "1";;
-	5) verify_binfmt_misc ":textension:E::ltp::$(which cat):" \
+	5) verify_binfmt_misc ":textension:E::ltp::$(type cat):" \
 			      "$TST_DATAROOT/file.extension" "0";;
-	6) verify_binfmt_misc ":tmagic:M:0:This::$(which cat):" \
+	6) verify_binfmt_misc ":tmagic:M:0:This::$(type cat):" \
 			      "$TST_DATAROOT/file.magic" "0";;
 	esac
 }
-- 
2.32.0

[LTP] [PATCH 3/3] commands: Drop which01.sh

[Petr Vorel]

"which" has been discontinued after 2.21 release in 2015 due this (git
repository is empty [1]).

[1] https://git.savannah.gnu.org/cgit/which.git

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 runtest/commands                    |   1 -
 testcases/commands/which/Makefile   |  11 ---
 testcases/commands/which/which01.sh | 107 ----------------------------
 3 files changed, 119 deletions(-)
 delete mode 100644 testcases/commands/which/Makefile
 delete mode 100755 testcases/commands/which/which01.sh

diff --git a/runtest/commands b/runtest/commands
index 8cfad0449..fc5c86684 100644
--- a/runtest/commands
+++ b/runtest/commands
@@ -32,7 +32,6 @@ mkfs01_msdos_sh mkfs01.sh -f msdos
 mkfs01_vfat_sh mkfs01.sh -f vfat
 mkfs01_ntfs_sh mkfs01.sh -f ntfs
 mkswap01_sh mkswap01.sh
-which01_sh which01.sh
 lsmod01_sh lsmod01.sh
 insmod01_sh insmod01.sh
 wc01_sh wc01.sh
diff --git a/testcases/commands/which/Makefile b/testcases/commands/which/Makefile
deleted file mode 100644
index 1be02f7d7..000000000
--- a/testcases/commands/which/Makefile
+++ /dev/null
@@ -1,11 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0-or-later
-# Copyright (c) 2015 Fujitsu Ltd.
-# Author:Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
-
-top_srcdir		?= ../../..
-
-include $(top_srcdir)/include/mk/env_pre.mk
-
-INSTALL_TARGETS		:= which01.sh
-
-include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/commands/which/which01.sh b/testcases/commands/which/which01.sh
deleted file mode 100755
index dd6659ea0..000000000
--- a/testcases/commands/which/which01.sh
+++ /dev/null
@@ -1,107 +0,0 @@
-#!/bin/sh
-# SPDX-License-Identifier: GPL-2.0-or-later
-# Copyright (c) 2015 Fujitsu Ltd.
-# Author: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
-#
-# Test which command with some basic options.
-
-TST_CNT=10
-TST_SETUP=setup
-TST_TESTFUNC=do_test
-TST_NEEDS_TMPDIR=1
-TST_NEEDS_CMDS="which"
-. tst_test.sh
-
-setup()
-{
-	touch pname
-	chmod +x pname
-	PATH=$PATH:.
-
-	mkdir bin
-	touch bin/pname
-	chmod +x bin/pname
-	PATH=$PATH:./bin
-
-	alias pname='pname -i'
-}
-
-which_verify()
-{
-	local IFS i j
-	IFS="$IFS_FIRST_LEVEL"
-	for i in $1; do
-		found="no"
-		IFS="$IFS_SECOND_LEVEL"
-		for j in $i; do
-			if grep -F -q "$j" temp; then
-				found="yes"
-			fi
-		done
-		if [ "$found" != "yes" ]; then
-			echo "'$i' not found in:"
-			cat temp
-			echo
-			return 1
-		fi
-	done
-}
-
-which_test()
-{
-	local which_op=$1
-	local prog_name=$2
-
-	local which_cmd="which $which_op $prog_name"
-
-	if [ "$which_op" = "--read-alias" ] || [ "$which_op" = "-i" ] || \
-		[ "$which_op" = "--skip-alias" ]; then
-		which_cmd="alias | $which_cmd"
-	fi
-
-	eval ${which_cmd} >temp 2>&1
-	if [ $? -ne 0 ]; then
-		grep -q -E "unknown option|invalid option|Usage" temp
-		if [ $? -eq 0 ]; then
-			tst_res TCONF "'${which_cmd}' not supported."
-			return
-		fi
-
-		tst_res TFAIL "'${which_cmd}' failed."
-		cat temp
-		return
-	fi
-
-	if [ $# -gt 2 ]; then
-		shift 2
-		which_verify "$@"
-		if [ $? -ne 0 ]; then
-			tst_res TFAIL "'${which_cmd}' failed, not expected."
-			return
-		fi
-	fi
-
-	tst_res TPASS "'${which_cmd}' passed."
-}
-
-IFS_FIRST_LEVEL='^'
-IFS_SECOND_LEVEL='|'
-do_test()
-{
-	case $1 in
-	1) which_test "" "pname" "$PWD/pname|./pname";;
-	2) which_test "-all" "pname" "$PWD/bin/pname|./bin/pname^$PWD/pname|./pname";;
-	3) which_test "-a" "pname" "$PWD/bin/pname|./bin/pname^$PWD/pname|./pname";;
-	4) which_test "--read-alias" "pname" "pname='pname -i'^$PWD/pname";;
-	5) which_test "-i" "pname" "pname='pname -i'^$PWD/pname";;
-	6) alias which='which --read-alias';
-	   which_test "--skip-alias" "pname" "$PWD/pname";
-	   unalias which;;
-	7) which_test "--version";;
-	8) which_test "-v";;
-	9) which_test "-V";;
-	10) which_test "--help";;
-	esac
-}
-
-tst_run
-- 
2.32.0

[LTP] [PATCH 1/3] tst_test.sh: Simplify tst_cmd_available()

[Joerg Vehlow]

Hi Petr,

On 8/18/2021 11:12 AM, Petr Vorel wrote:
> "command -v" [1] and "type" [2] are POSIX. They're supported in all
> common shells (bash, zsh, dash, busybox sh, mksh). Thus we don't have to
> fallback on "which", which has been discontinued after 2.21 release in
> 2015 due this (git repository is empty [3]).
>
> Use "type" instead of "command -v" which is IMHO more known.
>
> Also drop explicit return as the exit code is reliable an all
> implementations.
>
> [1] https://pubs.opengroup.org/onlinepubs/9699919799/utilities/command.html
> [2] https://pubs.opengroup.org/onlinepubs/9699919799/utilities/type.html
> [3] https://git.savannah.gnu.org/cgit/which.git
>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
>   testcases/lib/tst_test.sh | 13 +------------
>   1 file changed, 1 insertion(+), 12 deletions(-)
>
> diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
> index c6aa2c487..fa35a64f1 100644
> --- a/testcases/lib/tst_test.sh
> +++ b/testcases/lib/tst_test.sh
> @@ -346,18 +346,7 @@ tst_virt_hyperv()
>   
>   tst_cmd_available()
>   {
> -	if type command > /dev/null 2>&1; then
> -		command -v $1 > /dev/null 2>&1 || return 1
> -	else
> -		which $1 > /dev/null 2>&1
> -		if [ $? -eq 0 ]; then
> -			return 0
> -		elif [ $? -eq 127 ]; then
> -			tst_brk TCONF "missing which command"
> -		else
> -			return 1
> -		fi
> -	fi
> +	type $1 >/dev/null 2>&1
I guess there was a reason, why command was used here in the first place.
Iirc type is often a shell builtin, that can have different behavior, 
while command -v is posix and should be extremely portable.
So maybe it is better to use "command -v" instead of type here. I hope 
most distributions have a command-command...

Joerg

[LTP] [PATCH 2/3] binfmt_misc02.sh: Use "command -v" instead of "which"

[Joerg Vehlow]

Hi Petr,

On 8/18/2021 11:12 AM, Petr Vorel wrote:
> "which" has been discontinued after 2.21 release in 2015 due this (git
> repository is empty [1]) it provides warning on Debian/Ubuntu [2].
>
> Use "command -v" which is POSIX [3] and supported on all common shells
> (bash, zsh, dash, busybox sh, mksh).
>
> [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 <pvorel@suse.cz>
> ---
>   testcases/kernel/fs/binfmt_misc/binfmt_misc02.sh | 14 +++++++-------
>   1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/testcases/kernel/fs/binfmt_misc/binfmt_misc02.sh b/testcases/kernel/fs/binfmt_misc/binfmt_misc02.sh
> index 9dbcd68cc..cb4e770e3 100755
> --- a/testcases/kernel/fs/binfmt_misc/binfmt_misc02.sh
> +++ b/testcases/kernel/fs/binfmt_misc/binfmt_misc02.sh
> @@ -19,7 +19,7 @@
>   
>   TST_CNT=6
>   TST_TESTFUNC=do_test
> -TST_NEEDS_CMDS="which cat head"
> +TST_NEEDS_CMDS="cat head"
>   
>   . binfmt_misc_lib.sh
>   
> @@ -89,17 +89,17 @@ verify_binfmt_misc()
>   do_test()
>   {
>   	case $1 in
> -	1) verify_binfmt_misc ":textension:E::extension::$(which cat):" \
> +	1) verify_binfmt_misc ":textension:E::extension::$(type cat):" \
>   			      "$TST_DATAROOT/file.extension" "1";;
> -	2) verify_binfmt_misc ":tmagic:M:1:This::$(which cat):" \
> +	2) verify_binfmt_misc ":tmagic:M:1:This::$(type cat):" \
>   			      "$TST_DATAROOT/file.magic" "1";;
> -	3) verify_binfmt_misc ".textension.E..extension..$(which cat)." \
> +	3) verify_binfmt_misc ".textension.E..extension..$(type cat)." \
>   			      "$TST_DATAROOT/file.extension" "1";;
> -	4) verify_binfmt_misc ",tmagic,M,1,This,,$(which cat)," \
> +	4) verify_binfmt_misc ",tmagic,M,1,This,,$(type cat)," \
>   			      "$TST_DATAROOT/file.magic" "1";;
> -	5) verify_binfmt_misc ":textension:E::ltp::$(which cat):" \
> +	5) verify_binfmt_misc ":textension:E::ltp::$(type cat):" \
>   			      "$TST_DATAROOT/file.extension" "0";;
> -	6) verify_binfmt_misc ":tmagic:M:0:This::$(which cat):" \
> +	6) verify_binfmt_misc ":tmagic:M:0:This::$(type cat):" \
>   			      "$TST_DATAROOT/file.magic" "0";;
This does not match the description, you say you replace which with 
command, but actually you replaced it with type.

>   	esac
>   }

[LTP] [PATCH 3/3] commands: Drop which01.sh

[Joerg Vehlow]

Hi Petr,

On 8/18/2021 11:12 AM, Petr Vorel wrote:
> "which" has been discontinued after 2.21 release in 2015 due this (git
> repository is empty [1]).
I am a bit against dropping this. which is widely used and I think as 
long as it is available, it should behave as expected.

Joerg

>
> [1] https://git.savannah.gnu.org/cgit/which.git
>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
>   runtest/commands                    |   1 -
>   testcases/commands/which/Makefile   |  11 ---
>   testcases/commands/which/which01.sh | 107 ----------------------------
>   3 files changed, 119 deletions(-)
>   delete mode 100644 testcases/commands/which/Makefile
>   delete mode 100755 testcases/commands/which/which01.sh
>
> diff --git a/runtest/commands b/runtest/commands
> index 8cfad0449..fc5c86684 100644
> --- a/runtest/commands
> +++ b/runtest/commands
> @@ -32,7 +32,6 @@ mkfs01_msdos_sh mkfs01.sh -f msdos
>   mkfs01_vfat_sh mkfs01.sh -f vfat
>   mkfs01_ntfs_sh mkfs01.sh -f ntfs
>   mkswap01_sh mkswap01.sh
> -which01_sh which01.sh
>   lsmod01_sh lsmod01.sh
>   insmod01_sh insmod01.sh
>   wc01_sh wc01.sh
> diff --git a/testcases/commands/which/Makefile b/testcases/commands/which/Makefile
> deleted file mode 100644
> index 1be02f7d7..000000000
> --- a/testcases/commands/which/Makefile
> +++ /dev/null
> @@ -1,11 +0,0 @@
> -# SPDX-License-Identifier: GPL-2.0-or-later
> -# Copyright (c) 2015 Fujitsu Ltd.
> -# Author:Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
> -
> -top_srcdir		?= ../../..
> -
> -include $(top_srcdir)/include/mk/env_pre.mk
> -
> -INSTALL_TARGETS		:= which01.sh
> -
> -include $(top_srcdir)/include/mk/generic_leaf_target.mk
> diff --git a/testcases/commands/which/which01.sh b/testcases/commands/which/which01.sh
> deleted file mode 100755
> index dd6659ea0..000000000
> --- a/testcases/commands/which/which01.sh
> +++ /dev/null
> @@ -1,107 +0,0 @@
> -#!/bin/sh
> -# SPDX-License-Identifier: GPL-2.0-or-later
> -# Copyright (c) 2015 Fujitsu Ltd.
> -# Author: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
> -#
> -# Test which command with some basic options.
> -
> -TST_CNT=10
> -TST_SETUP=setup
> -TST_TESTFUNC=do_test
> -TST_NEEDS_TMPDIR=1
> -TST_NEEDS_CMDS="which"
> -. tst_test.sh
> -
> -setup()
> -{
> -	touch pname
> -	chmod +x pname
> -	PATH=$PATH:.
> -
> -	mkdir bin
> -	touch bin/pname
> -	chmod +x bin/pname
> -	PATH=$PATH:./bin
> -
> -	alias pname='pname -i'
> -}
> -
> -which_verify()
> -{
> -	local IFS i j
> -	IFS="$IFS_FIRST_LEVEL"
> -	for i in $1; do
> -		found="no"
> -		IFS="$IFS_SECOND_LEVEL"
> -		for j in $i; do
> -			if grep -F -q "$j" temp; then
> -				found="yes"
> -			fi
> -		done
> -		if [ "$found" != "yes" ]; then
> -			echo "'$i' not found in:"
> -			cat temp
> -			echo
> -			return 1
> -		fi
> -	done
> -}
> -
> -which_test()
> -{
> -	local which_op=$1
> -	local prog_name=$2
> -
> -	local which_cmd="which $which_op $prog_name"
> -
> -	if [ "$which_op" = "--read-alias" ] || [ "$which_op" = "-i" ] || \
> -		[ "$which_op" = "--skip-alias" ]; then
> -		which_cmd="alias | $which_cmd"
> -	fi
> -
> -	eval ${which_cmd} >temp 2>&1
> -	if [ $? -ne 0 ]; then
> -		grep -q -E "unknown option|invalid option|Usage" temp
> -		if [ $? -eq 0 ]; then
> -			tst_res TCONF "'${which_cmd}' not supported."
> -			return
> -		fi
> -
> -		tst_res TFAIL "'${which_cmd}' failed."
> -		cat temp
> -		return
> -	fi
> -
> -	if [ $# -gt 2 ]; then
> -		shift 2
> -		which_verify "$@"
> -		if [ $? -ne 0 ]; then
> -			tst_res TFAIL "'${which_cmd}' failed, not expected."
> -			return
> -		fi
> -	fi
> -
> -	tst_res TPASS "'${which_cmd}' passed."
> -}
> -
> -IFS_FIRST_LEVEL='^'
> -IFS_SECOND_LEVEL='|'
> -do_test()
> -{
> -	case $1 in
> -	1) which_test "" "pname" "$PWD/pname|./pname";;
> -	2) which_test "-all" "pname" "$PWD/bin/pname|./bin/pname^$PWD/pname|./pname";;
> -	3) which_test "-a" "pname" "$PWD/bin/pname|./bin/pname^$PWD/pname|./pname";;
> -	4) which_test "--read-alias" "pname" "pname='pname -i'^$PWD/pname";;
> -	5) which_test "-i" "pname" "pname='pname -i'^$PWD/pname";;
> -	6) alias which='which --read-alias';
> -	   which_test "--skip-alias" "pname" "$PWD/pname";
> -	   unalias which;;
> -	7) which_test "--version";;
> -	8) which_test "-v";;
> -	9) which_test "-V";;
> -	10) which_test "--help";;
> -	esac
> -}
> -
> -tst_run

[LTP] [PATCH 1/3] tst_test.sh: Simplify tst_cmd_available()

[Petr Vorel]

Hi Joerg,

...
> > +	type $1 >/dev/null 2>&1
> I guess there was a reason, why command was used here in the first place.
> Iirc type is often a shell builtin, that can have different behavior, while
> command -v is posix and should be extremely portable.
> So maybe it is better to use "command -v" instead of type here. I hope most
> distributions have a command-command...
Well, I wrote that code, in dba1d50cb :). IMHO both are POSIX and both are shell
builtin.

I tested it on all implementations and the only difference is that both "type"
and "command -v" on dash and busybox sh returns 127 on missing command, the rest
return 1.

Kind regards,
Petr

[LTP] [PATCH 2/3] binfmt_misc02.sh: Use "command -v" instead of "which"

[Petr Vorel]

Hi,

...
> > -	6) verify_binfmt_misc ":tmagic:M:0:This::$(which cat):" \
> > +	6) verify_binfmt_misc ":tmagic:M:0:This::$(type cat):" \
> >   			      "$TST_DATAROOT/file.magic" "0";;
> This does not match the description, you say you replace which with command,
> but actually you replaced it with type.
Good catch, thanks! Actually, this commit is wrong, it *must* use
"command -v".

Kind regards,
Petr

[LTP] [PATCH 3/3] commands: Drop which01.sh

[Petr Vorel]

Hi Joerg,

> Hi Petr,

> On 8/18/2021 11:12 AM, Petr Vorel wrote:
> > "which" has been discontinued after 2.21 release in 2015 due this (git
> > repository is empty [1]).
> I am a bit against dropping this. which is widely used and I think as long
> as it is available, it should behave as expected.
First, thanks for your review of all patchset.

I should have marked this commit as RFC. I have no problem to postpone
deleting this test fairly long time (even for 1-2 years).

But, if we really want to test basic shell commands (IMHO they should be part of
particular shell implementation code and most of these projects have it: e.g.
bash, busybox, coreutils, util-linux, ...), in this particular test it'd be more
useful to test "type" or "command" than "which", which code hasn't been changed
since 2015. Looking into openSUSE code [1], there is no patch on "which"
command.

Also, I might be a bit too aggressive when removing tests for legacy code, but
in this case Debian has deprecated "which" from it's basic tools [2], proving
there is a push to use "type" / "command -v" at least in some distros.

Kind regards,
Petr

[1] https://build.opensuse.org/package/show/Base:System/which
[2] https://salsa.debian.org/debian/debianutils/-/commit/3a8dd10b4502f7bae8fc6973c13ce23fc9da7efb

[LTP] [PATCH 1/3] tst_test.sh: Simplify tst_cmd_available()

[Li Wang]

On Wed, Aug 18, 2021 at 5:41 PM Petr Vorel <pvorel@suse.cz> wrote:

> Hi Joerg,
>
> ...
> > > +   type $1 >/dev/null 2>&1
> > I guess there was a reason, why command was used here in the first place.
> > Iirc type is often a shell builtin, that can have different behavior,
> while
> > command -v is posix and should be extremely portable.
> > So maybe it is better to use "command -v" instead of type here. I hope
> most
> > distributions have a command-command...
> Well, I wrote that code, in dba1d50cb :). IMHO both are POSIX and both are
> shell
> builtin.
>

Another reason, I guess, is `command -v ` looks to match the function name
more:).
But it depends on you, I'm OK with any of them.

Reviewed-by: Li Wang <liwang@redhat.com>



>
> I tested it on all implementations and the only difference is that both
> "type"
> and "command -v" on dash and busybox sh returns 127 on missing command,
> the rest
> return 1.
>

Sounds great!

-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20210819/e923ea43/attachment.htm>

[LTP] [PATCH 3/3] commands: Drop which01.sh

[Li Wang]

On Wed, Aug 18, 2021 at 6:01 PM Petr Vorel <pvorel@suse.cz> wrote:

> Hi Joerg,
>
> > Hi Petr,
>
> > On 8/18/2021 11:12 AM, Petr Vorel wrote:
> > > "which" has been discontinued after 2.21 release in 2015 due this (git
> > > repository is empty [1]).
> > I am a bit against dropping this. which is widely used and I think as
> long
> > as it is available, it should behave as expected.
> First, thanks for your review of all patchset.
>
> I should have marked this commit as RFC. I have no problem to postpone
> deleting this test fairly long time (even for 1-2 years).
>

I just have a look at which01.sh that is written by the new API and so far
so good.
Maybe we can reserve it for a bit long time.



>
> But, if we really want to test basic shell commands (IMHO they should be
> part of
> particular shell implementation code and most of these projects have it:
> e.g.
> bash, busybox, coreutils, util-linux, ...), in this particular test it'd
> be more
> useful to test "type" or "command" than "which", which code hasn't been
> changed
> since 2015. Looking into openSUSE code [1], there is no patch on "which"
> command.
>
> Also, I might be a bit too aggressive when removing tests for legacy code,
> but
> in this case Debian has deprecated "which" from it's basic tools [2],
> proving
> there is a push to use "type" / "command -v" at least in some distros.
>
> Kind regards,
> Petr
>
> [1] https://build.opensuse.org/package/show/Base:System/which
> [2]
> https://salsa.debian.org/debian/debianutils/-/commit/3a8dd10b4502f7bae8fc6973c13ce23fc9da7efb
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>
>

-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20210819/a65632c2/attachment.htm>

[LTP] [PATCH 3/3] commands: Drop which01.sh

[Petr Vorel]

Hi Li, all,

> I just have a look at which01.sh that is written by the new API and so far
> so good.
> Maybe we can reserve it for a bit long time.

OK, two people asking to keep it, lets keep it :).

Kind regards,
Petr

[LTP] [PATCH 1/3] tst_test.sh: Simplify tst_cmd_available()

[Petr Vorel]

Hi Li,

...
> Another reason, I guess, is `command -v ` looks to match the function name
> more:).

> But it depends on you, I'm OK with any of them.

Sure, no problem to keep "command -v".
Thanks for your review.

Kind regards,
Petr

> Reviewed-by: Li Wang <liwang@redhat.com>

[LTP] [PATCH 1/3] tst_test.sh: Simplify tst_cmd_available()

[Petr Vorel]

Hi Li, all,

...
> Another reason, I guess, is `command -v ` looks to match the function name
> more:).
> But it depends on you, I'm OK with any of them.
I merged it with "command -v" and second commit.

Kind regards,
Petr