On Fri, Aug 7, 2020 at 8:24 PM Paul Spooren <[email protected]> wrote: > > > On 07.08.20 16:48, Rosen Penev wrote: > > On Fri, Aug 7, 2020 at 5:42 PM Paul Spooren <[email protected]> wrote: > >> > >> On 07.08.20 14:18, Rosen Penev wrote: > >>> On Fri, Aug 7, 2020 at 3:57 PM Paul Spooren <[email protected]> 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@dawn:~/src/openwrt/openwrt$ command -v git > >> /usr/bin/git > >> user@dawn:~/src/openwrt/openwrt$ which git > >> /usr/bin/git > >> user@dawn:~/src/openwrt/openwrt$ command -v git-nope > >> user@dawn:~/src/openwrt/openwrt$ echo $? > >> 1 > >> user@dawn:~/src/openwrt/openwrt$ which git-nope > >> user@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 <[email protected]> > >>>> --- > >>>> 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 > >>>> [email protected] > >>>> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
_______________________________________________ openwrt-devel mailing list [email protected] https://lists.openwrt.org/mailman/listinfo/openwrt-devel
