diff mbox series

[v2] treewide: replace `which` with `command -v`

Message ID 20200810013337.130195-1-mail@aparcar.org
State Accepted
Headers show
Series [v2] treewide: replace `which` with `command -v` | expand

Commit Message

Paul Spooren Aug. 10, 2020, 1:33 a.m. UTC
Fix shellcheck SC2230
> which is non-standard. Use builtin 'command -v' instead.

Using `command -v` is POSIX compliant while `which` is not.  Also to
mention, `command -v` is a shell builtin whereas `which` is a separate
busybox applet.

Once applied to everything concerning OpenWrt we can disable the busybox
feature `which` and save 3.8kB.

Acked-by: Stijn Tintel <stijn@linux-ipv6.be>
Signed-off-by: Paul Spooren <mail@aparcar.org>
---
 include/rootfs.mk                                    |  6 +++---
 package/base-files/files/lib/upgrade/stage2          |  2 +-
 .../kernel/broadcom-wl/files/lib/wifi/broadcom.sh    |  2 +-
 scripts/ipkg-build                                   | 12 ++++++------
 4 files changed, 11 insertions(+), 11 deletions(-)

Comments

Paul Oranje Aug. 15, 2020, 3:28 p.m. UTC | #1
Op 10 aug. 2020, om 03:33 heeft Paul Spooren <mail@aparcar.org> het volgende geschreven:
> 
> Fix shellcheck SC2230
>> which is non-standard. Use builtin 'command -v' instead.
> 
> Using `command -v` is POSIX compliant while `which` is not.  Also to
> mention, `command -v` is a shell builtin whereas `which` is a separate
> busybox applet.
> 
> Once applied to everything concerning OpenWrt we can disable the busybox
> feature `which` and save 3.8kB.
And, an alias which='command -v' in /etc/profile would be nice.

> 
> Acked-by: Stijn Tintel <stijn@linux-ipv6.be>
> Signed-off-by: Paul Spooren <mail@aparcar.org>
> ---
> include/rootfs.mk                                    |  6 +++---
> package/base-files/files/lib/upgrade/stage2          |  2 +-
> .../kernel/broadcom-wl/files/lib/wifi/broadcom.sh    |  2 +-
> scripts/ipkg-build                                   | 12 ++++++------
> 4 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/include/rootfs.mk b/include/rootfs.mk
> index b6775c7e15..18ada3cd43 100644
> --- a/include/rootfs.mk
> +++ b/include/rootfs.mk
> @@ -69,7 +69,7 @@ define prepare_rootfs
> 	@( \
> 		cd $(1); \
> 		for script in ./usr/lib/opkg/info/*.postinst; do \
> -			IPKG_INSTROOT=$(1) $$(which bash) $$script; \
> +			IPKG_INSTROOT=$(1) $$(command -v bash) $$script; \
> 			ret=$$?; \
> 			if [ $$ret -ne 0 ]; then \
> 				echo "postinst script $$script has failed with exit code $$ret" >&2; \
> @@ -79,10 +79,10 @@ define prepare_rootfs
> 		for script in ./etc/init.d/*; do \
> 			grep '#!/bin/sh /etc/rc.common' $$script >/dev/null || continue; \
> 			if ! echo " $(3) " | grep -q " $$(basename $$script) "; then \
> -				IPKG_INSTROOT=$(1) $$(which bash) ./etc/rc.common $$script enable; \
> +				IPKG_INSTROOT=$(1) $$(command -v bash) ./etc/rc.common $$script enable; \
> 				echo "Enabling" $$(basename $$script); \
> 			else \
> -				IPKG_INSTROOT=$(1) $$(which bash) ./etc/rc.common $$script disable; \
> +				IPKG_INSTROOT=$(1) $$(command -v bash) ./etc/rc.common $$script disable; \
> 				echo "Disabling" $$(basename $$script); \
> 			fi; \
> 		done || true \
> diff --git a/package/base-files/files/lib/upgrade/stage2 b/package/base-files/files/lib/upgrade/stage2
> index dbb33e8958..a4fef42134 100755
> --- a/package/base-files/files/lib/upgrade/stage2
> +++ b/package/base-files/files/lib/upgrade/stage2
> @@ -45,7 +45,7 @@ switch_to_ramfs() {
> 		snapshot snapshot_tool					\
> 		$RAMFS_COPY_BIN
> 	do
> -		local file="$(which "$binary" 2>/dev/null)"
> +		local file="$(command -v "$binary" 2>/dev/null)"
> 		[ -n "$file" ] && install_bin "$file"
> 	done
> 	install_file /etc/resolv.conf /lib/*.sh /lib/functions/*.sh /lib/upgrade/*.sh /lib/upgrade/do_stage2 /usr/share/libubox/jshn.sh $RAMFS_COPY_DATA
> diff --git a/package/kernel/broadcom-wl/files/lib/wifi/broadcom.sh b/package/kernel/broadcom-wl/files/lib/wifi/broadcom.sh
> index 33447341b2..352c365f27 100644
> --- a/package/kernel/broadcom-wl/files/lib/wifi/broadcom.sh
> +++ b/package/kernel/broadcom-wl/files/lib/wifi/broadcom.sh
> @@ -223,7 +223,7 @@ enable_broadcom() {
> 	}
> 
> 	local _c=0
> -	local nas="$(which nas)"
> +	local nas="$(command -v nas)"
> 	local if_pre_up if_up nas_cmd
> 	local vif vif_pre_up vif_post_up vif_do_up vif_txpower
> 	local bssmax=$(wlc ifname "$device" bssmax)
> diff --git a/scripts/ipkg-build b/scripts/ipkg-build
> index 21127f3391..6e027bc546 100755
> --- a/scripts/ipkg-build
> +++ b/scripts/ipkg-build
> @@ -10,10 +10,10 @@
> set -e
> 
> version=1.0
> -FIND="$(which find)"
> -FIND="${FIND:-$(which gfind)}"
> -TAR="${TAR:-$(which tar)}"
> -GZIP="$(which gzip)"
> +FIND="$(command -v find)"
> +FIND="${FIND:-$(command -v gfind)}"
> +TAR="${TAR:-$(command -v tar)}"
> +GZIP="$(command -v gzip)"
> 
> # try to use fixed source epoch
> if [ -n "$SOURCE_DATE_EPOCH" ]; then
> @@ -21,10 +21,10 @@ if [ -n "$SOURCE_DATE_EPOCH" ]; then
> 
> # look up date of last commit
> elif [ -d "$TOPDIR/.git" ]; then
> -	GIT="$(which git)"
> +	GIT="$(command -v git)"
> 	TIMESTAMP=$(cd $TOPDIR; $GIT log -1 -s --format=%ci)
> elif [ -d "$TOPDIR/.svn" ]; then
> -	SVN="$(which svn)"
> +	SVN="$(command -v svn)"
> 	TIMESTAMP=$($SVN info "$TOPDIR" | sed -n "s/^Last Changed Date: \(.*\)/\1/p")
> else
> 	TIMESTAMP=$(date)
> -- 
> 2.25.1
> 
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Adrian Schmutzler Aug. 15, 2020, 4:57 p.m. UTC | #2
> -----Original Message-----
> From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org]
> On Behalf Of Paul Oranje
> Sent: Samstag, 15. August 2020 17:29
> To: Paul Spooren <mail@aparcar.org>
> Cc: Stijn Tintel <stijn@linux-ipv6.be>; OpenWrt Development List <openwrt-
> devel@lists.openwrt.org>
> Subject: Re: [PATCH v2] treewide: replace `which` with `command -v`
> 
> Op 10 aug. 2020, om 03:33 heeft Paul Spooren <mail@aparcar.org> het
> volgende geschreven:
> >
> > Fix shellcheck SC2230
> >> which is non-standard. Use builtin 'command -v' instead.
> >
> > Using `command -v` is POSIX compliant while `which` is not.  Also to
> > mention, `command -v` is a shell builtin whereas `which` is a separate
> > busybox applet.
> >
> > Once applied to everything concerning OpenWrt we can disable the
> > busybox feature `which` and save 3.8kB.
> And, an alias which='command -v' in /etc/profile would be nice.

I'm not sure whether that's a good idea. After all, which and command -v have tiny differences in what they do, and hiding this might be misleading in some corner cases.

Best

Adrian

> 
> >
> > Acked-by: Stijn Tintel <stijn@linux-ipv6.be>
> > Signed-off-by: Paul Spooren <mail@aparcar.org>
> > ---
> > include/rootfs.mk                                    |  6 +++---
> > package/base-files/files/lib/upgrade/stage2          |  2 +-
> > .../kernel/broadcom-wl/files/lib/wifi/broadcom.sh    |  2 +-
> > scripts/ipkg-build                                   | 12 ++++++------
> > 4 files changed, 11 insertions(+), 11 deletions(-)
> >
> > diff --git a/include/rootfs.mk b/include/rootfs.mk index
> > b6775c7e15..18ada3cd43 100644
> > --- a/include/rootfs.mk
> > +++ b/include/rootfs.mk
> > @@ -69,7 +69,7 @@ define prepare_rootfs
> > 	@( \
> > 		cd $(1); \
> > 		for script in ./usr/lib/opkg/info/*.postinst; do \
> > -			IPKG_INSTROOT=$(1) $$(which bash) $$script; \
> > +			IPKG_INSTROOT=$(1) $$(command -v bash) $$script;
> \
> > 			ret=$$?; \
> > 			if [ $$ret -ne 0 ]; then \
> > 				echo "postinst script $$script has failed with
> exit code $$ret"
> > >&2; \ @@ -79,10 +79,10 @@ define prepare_rootfs
> > 		for script in ./etc/init.d/*; do \
> > 			grep '#!/bin/sh /etc/rc.common' $$script >/dev/null
> || continue; \
> > 			if ! echo " $(3) " | grep -q " $$(basename $$script) ";
> then \
> > -				IPKG_INSTROOT=$(1) $$(which bash)
> ./etc/rc.common $$script enable; \
> > +				IPKG_INSTROOT=$(1) $$(command -v bash)
> ./etc/rc.common $$script
> > +enable; \
> > 				echo "Enabling" $$(basename $$script); \
> > 			else \
> > -				IPKG_INSTROOT=$(1) $$(which bash)
> ./etc/rc.common $$script disable; \
> > +				IPKG_INSTROOT=$(1) $$(command -v bash)
> ./etc/rc.common $$script
> > +disable; \
> > 				echo "Disabling" $$(basename $$script); \
> > 			fi; \
> > 		done || true \
> > diff --git a/package/base-files/files/lib/upgrade/stage2
> > b/package/base-files/files/lib/upgrade/stage2
> > index dbb33e8958..a4fef42134 100755
> > --- a/package/base-files/files/lib/upgrade/stage2
> > +++ b/package/base-files/files/lib/upgrade/stage2
> > @@ -45,7 +45,7 @@ switch_to_ramfs() {
> > 		snapshot snapshot_tool
> 	\
> > 		$RAMFS_COPY_BIN
> > 	do
> > -		local file="$(which "$binary" 2>/dev/null)"
> > +		local file="$(command -v "$binary" 2>/dev/null)"
> > 		[ -n "$file" ] && install_bin "$file"
> > 	done
> > 	install_file /etc/resolv.conf /lib/*.sh /lib/functions/*.sh
> > /lib/upgrade/*.sh /lib/upgrade/do_stage2 /usr/share/libubox/jshn.sh
> > $RAMFS_COPY_DATA diff --git
> > a/package/kernel/broadcom-wl/files/lib/wifi/broadcom.sh
> > b/package/kernel/broadcom-wl/files/lib/wifi/broadcom.sh
> > index 33447341b2..352c365f27 100644
> > --- a/package/kernel/broadcom-wl/files/lib/wifi/broadcom.sh
> > +++ b/package/kernel/broadcom-wl/files/lib/wifi/broadcom.sh
> > @@ -223,7 +223,7 @@ enable_broadcom() {
> > 	}
> >
> > 	local _c=0
> > -	local nas="$(which nas)"
> > +	local nas="$(command -v nas)"
> > 	local if_pre_up if_up nas_cmd
> > 	local vif vif_pre_up vif_post_up vif_do_up vif_txpower
> > 	local bssmax=$(wlc ifname "$device" bssmax) diff --git
> > a/scripts/ipkg-build b/scripts/ipkg-build index 21127f3391..6e027bc546
> > 100755
> > --- a/scripts/ipkg-build
> > +++ b/scripts/ipkg-build
> > @@ -10,10 +10,10 @@
> > set -e
> >
> > version=1.0
> > -FIND="$(which find)"
> > -FIND="${FIND:-$(which gfind)}"
> > -TAR="${TAR:-$(which tar)}"
> > -GZIP="$(which gzip)"
> > +FIND="$(command -v find)"
> > +FIND="${FIND:-$(command -v gfind)}"
> > +TAR="${TAR:-$(command -v tar)}"
> > +GZIP="$(command -v gzip)"
> >
> > # try to use fixed source epoch
> > if [ -n "$SOURCE_DATE_EPOCH" ]; then
> > @@ -21,10 +21,10 @@ if [ -n "$SOURCE_DATE_EPOCH" ]; then
> >
> > # look up date of last commit
> > elif [ -d "$TOPDIR/.git" ]; then
> > -	GIT="$(which git)"
> > +	GIT="$(command -v git)"
> > 	TIMESTAMP=$(cd $TOPDIR; $GIT log -1 -s --format=%ci) elif [ -d
> > "$TOPDIR/.svn" ]; then
> > -	SVN="$(which svn)"
> > +	SVN="$(command -v svn)"
> > 	TIMESTAMP=$($SVN info "$TOPDIR" | sed -n "s/^Last Changed Date:
> > \(.*\)/\1/p") else
> > 	TIMESTAMP=$(date)
> > --
> > 2.25.1
> >
> >
> > _______________________________________________
> > openwrt-devel mailing list
> > openwrt-devel@lists.openwrt.org
> > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
> 
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
diff mbox series

Patch

diff --git a/include/rootfs.mk b/include/rootfs.mk
index b6775c7e15..18ada3cd43 100644
--- a/include/rootfs.mk
+++ b/include/rootfs.mk
@@ -69,7 +69,7 @@  define prepare_rootfs
 	@( \
 		cd $(1); \
 		for script in ./usr/lib/opkg/info/*.postinst; do \
-			IPKG_INSTROOT=$(1) $$(which bash) $$script; \
+			IPKG_INSTROOT=$(1) $$(command -v bash) $$script; \
 			ret=$$?; \
 			if [ $$ret -ne 0 ]; then \
 				echo "postinst script $$script has failed with exit code $$ret" >&2; \
@@ -79,10 +79,10 @@  define prepare_rootfs
 		for script in ./etc/init.d/*; do \
 			grep '#!/bin/sh /etc/rc.common' $$script >/dev/null || continue; \
 			if ! echo " $(3) " | grep -q " $$(basename $$script) "; then \
-				IPKG_INSTROOT=$(1) $$(which bash) ./etc/rc.common $$script enable; \
+				IPKG_INSTROOT=$(1) $$(command -v bash) ./etc/rc.common $$script enable; \
 				echo "Enabling" $$(basename $$script); \
 			else \
-				IPKG_INSTROOT=$(1) $$(which bash) ./etc/rc.common $$script disable; \
+				IPKG_INSTROOT=$(1) $$(command -v bash) ./etc/rc.common $$script disable; \
 				echo "Disabling" $$(basename $$script); \
 			fi; \
 		done || true \
diff --git a/package/base-files/files/lib/upgrade/stage2 b/package/base-files/files/lib/upgrade/stage2
index dbb33e8958..a4fef42134 100755
--- a/package/base-files/files/lib/upgrade/stage2
+++ b/package/base-files/files/lib/upgrade/stage2
@@ -45,7 +45,7 @@  switch_to_ramfs() {
 		snapshot snapshot_tool					\
 		$RAMFS_COPY_BIN
 	do
-		local file="$(which "$binary" 2>/dev/null)"
+		local file="$(command -v "$binary" 2>/dev/null)"
 		[ -n "$file" ] && install_bin "$file"
 	done
 	install_file /etc/resolv.conf /lib/*.sh /lib/functions/*.sh /lib/upgrade/*.sh /lib/upgrade/do_stage2 /usr/share/libubox/jshn.sh $RAMFS_COPY_DATA
diff --git a/package/kernel/broadcom-wl/files/lib/wifi/broadcom.sh b/package/kernel/broadcom-wl/files/lib/wifi/broadcom.sh
index 33447341b2..352c365f27 100644
--- a/package/kernel/broadcom-wl/files/lib/wifi/broadcom.sh
+++ b/package/kernel/broadcom-wl/files/lib/wifi/broadcom.sh
@@ -223,7 +223,7 @@  enable_broadcom() {
 	}
 
 	local _c=0
-	local nas="$(which nas)"
+	local nas="$(command -v nas)"
 	local if_pre_up if_up nas_cmd
 	local vif vif_pre_up vif_post_up vif_do_up vif_txpower
 	local bssmax=$(wlc ifname "$device" bssmax)
diff --git a/scripts/ipkg-build b/scripts/ipkg-build
index 21127f3391..6e027bc546 100755
--- a/scripts/ipkg-build
+++ b/scripts/ipkg-build
@@ -10,10 +10,10 @@ 
 set -e
 
 version=1.0
-FIND="$(which find)"
-FIND="${FIND:-$(which gfind)}"
-TAR="${TAR:-$(which tar)}"
-GZIP="$(which gzip)"
+FIND="$(command -v find)"
+FIND="${FIND:-$(command -v gfind)}"
+TAR="${TAR:-$(command -v tar)}"
+GZIP="$(command -v gzip)"
 
 # try to use fixed source epoch
 if [ -n "$SOURCE_DATE_EPOCH" ]; then
@@ -21,10 +21,10 @@  if [ -n "$SOURCE_DATE_EPOCH" ]; then
 
 # look up date of last commit
 elif [ -d "$TOPDIR/.git" ]; then
-	GIT="$(which git)"
+	GIT="$(command -v git)"
 	TIMESTAMP=$(cd $TOPDIR; $GIT log -1 -s --format=%ci)
 elif [ -d "$TOPDIR/.svn" ]; then
-	SVN="$(which svn)"
+	SVN="$(command -v svn)"
 	TIMESTAMP=$($SVN info "$TOPDIR" | sed -n "s/^Last Changed Date: \(.*\)/\1/p")
 else
 	TIMESTAMP=$(date)