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.

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

Reply via email to