[PATCH] treewide: replace `which` with `command -v`
Rosen Penev
rosenp at gmail.com
Sat Aug 8 01:07:18 EDT 2020
On Fri, Aug 7, 2020 at 8:24 PM Paul Spooren <mail at aparcar.org> wrote:
>
>
> On 07.08.20 16:48, Rosen Penev wrote:
> > On Fri, Aug 7, 2020 at 5:42 PM Paul Spooren <mail at aparcar.org> wrote:
> >>
> >> On 07.08.20 14:18, Rosen Penev wrote:
> >>> On Fri, Aug 7, 2020 at 3:57 PM Paul Spooren <mail at aparcar.org> wrote:
> >>>> Fix shellcheck SC2230
> >>>>> which is non-standard. Use builtin 'command -v' instead.
> >>>> Once applied to everything concerning OpenWrt we can disable the busybox
> >>>> feature `which` and save 3.8kB.
> >>> which and command -v seem to not be the same. See
> >>> https://github.com/openwrt/openwrt/commit/8242c6de29951fbb549355770cd685ffe3ac9c54
> >> Afaik ldir uses a Mac which may has a different build in implementation
> >> of `command`.
> >>
> >> Testing it locally on a Linux machine they behave very much the same.
> >>
> >> user at dawn:~/src/openwrt/openwrt$ command -v git
> >> /usr/bin/git
> >> user at dawn:~/src/openwrt/openwrt$ which git
> >> /usr/bin/git
> >> user at dawn:~/src/openwrt/openwrt$ command -v git-nope
> >> user at dawn:~/src/openwrt/openwrt$ echo $?
> >> 1
> >> user at dawn:~/src/openwrt/openwrt$ which git-nope
> >> user at dawn:~/src/openwrt/openwrt$ echo $?
> >> 1
> >>
> >> I'd image the *command* `command` doesn't exist on a Mac and it would
> >> actually execute the *command* `git` there, which obviously pollutes the
> >> output.
> > command is a shell builtin. macOS used bash and now zsh. I doubt
> > that's the problem.
>
> All right, I found the issue. The problem is the following commit:
>
> https://github.com/openwrt/openwrt/commit/56f813674a912490df327304033bf667b285930a
>
> It replaces the function without migrating the 2>/dev/null. It's
> therefore neither the fault of MacOS nor command.
Good catch.
>
> I therefore assume my proposed patch is fine.
>
> >> Seems like a problem of Mac and not the bash/sh included `command`.
> >> Thinking about that, it seems fairly dangerous: If you'd run `command -v
> >> nuke_harddrive > /dev/null` it would actually run whatever command silently.
> >>
> >> There is a wrapper called `./scripts/md5sum` which calls `md5`, the Mac
> >> tool to get a MD5 checksum.
> >>
> >> I guess we could create a wrapper like ./scripts/command which contains
> >> the following line:
> >>
> >> which $2 # skipping the -v arg of command
> >>
> >> Ultimately it's about freeing up busybox-which, which is independent of
> >> any Mac ideas. For that rootfs.mk and ipkg-build could be left untouched.
> >>
> >>>> Signed-off-by: Paul Spooren <mail at 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 at lists.openwrt.org
> >>>> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
More information about the openwrt-devel
mailing list