Merge pull request RIOT-OS#16776 from chrysn-pull-requests/shell-make…

…-fixes-2 Remove `which` from shell invocations
chrysn-pull-requests · Sep 3, 2021 · d4ed42a · d4ed42a
2 parents 2bf1202 + 8664eb8
commit d4ed42a
Show file tree

Hide file tree

Showing 7 changed files with 44 additions and 16 deletions.
diff --git a/Makefile.include b/Makefile.include
@@ -327,13 +327,13 @@ APPLICATION := $(strip $(APPLICATION))
 
 ifeq (,$(and $(DOWNLOAD_TO_STDOUT),$(DOWNLOAD_TO_FILE)))
   ifeq (,$(WGET))
-    ifeq (0,$(shell which wget > /dev/null 2>&1 ; echo $$?))
-      WGET = $(call memoized,WGET,$(shell which wget))
+    ifeq (,$(shell command -v wget))
+      WGET = $(call memoized,WGET,$(shell command -v wget))
     endif
   endif
   ifeq (,$(CURL))
-    ifeq (0,$(shell which curl > /dev/null 2>&1 ; echo $$?))
-      CURL = $(call memoized,CURL,$(shell which curl))
+    ifneq (,$(shell command -v curl))
+      CURL = $(call memoized,CURL,$(shell command -v curl))
     endif
   endif
   ifeq (,$(WGET)$(CURL))
@@ -349,11 +349,11 @@ ifeq (,$(and $(DOWNLOAD_TO_STDOUT),$(DOWNLOAD_TO_FILE)))
 endif
 
 ifeq (,$(UNZIP_HERE))
-  ifeq (0,$(shell which unzip > /dev/null 2>&1 ; echo $$?))
-    UNZIP_HERE = $(call memoized,UNZIP_HERE,$(shell which unzip) -q)
+  ifneq (,$(shell command -v unzip))
+    UNZIP_HERE = $(call memoized,UNZIP_HERE,$(shell command -v unzip) -q)
   else
-    ifeq (0,$(shell which 7z > /dev/null 2>&1 ; echo $$?))
-      UNZIP_HERE = $(call memoized,UNZIP_HERE,$(shell which 7z) x -bd)
+    ifneq (,$(shell command -v 7z))
+      UNZIP_HERE = $(call memoized,UNZIP_HERE,$(shell command -v 7z) x -bd)
     else
       $(warning Neither unzip nor 7z is installed.)
     endif

diff --git a/dist/testbed-support/makefile.iotlab.single.inc.mk b/dist/testbed-support/makefile.iotlab.single.inc.mk
@@ -57,7 +57,7 @@ endif
 IOTLAB_AUTH ?= $(HOME)/.iotlabrc
 IOTLAB_USER ?= $(shell cut -f1 -d: $(IOTLAB_AUTH))
 
-ifneq (0,$(shell command -v iotlab-experiment -h 2>&1 > /dev/null ; echo $$?))
+ifeq (,$(shell command -v iotlab-experiment))
   $(info $(COLOR_RED)'iotlab-experiment' command is not available \
 	        please consider installing it from \
 	        https://pypi.python.org/pypi/iotlabcli$(COLOR_RESET))
@@ -66,7 +66,7 @@ endif
 
 ifeq (iotlab-a8-m3,$(BOARD))
   ifneq (,$(filter flash% reset,$(MAKECMDGOALS)))
-    ifneq (0,$(shell command -v iotlab-ssh -h 2>&1 > /dev/null ; echo $$?))
+    ifeq (,$(shell command -v iotlab-ssh))
       $(info $(COLOR_RED)'iotlab-ssh' command is not available \
 	          please consider installing it from \
 	          https://pypi.python.org/pypi/iotlabsshcli$(COLOR_RESET))

diff --git a/dist/tools/buildsystem_sanity_check/check.sh b/dist/tools/buildsystem_sanity_check/check.sh
@@ -334,6 +334,32 @@ check_no_pkg_source_local() {
         | error_with_message "Don't push PKG_SOURCE_LOCAL definitions upstream"
 }
 
+check_shell_which() {
+    local patterns=()
+    local pathspec=()
+
+    patterns+=(-e '(shell[[:blank:]]\+which')
+
+    pathspec+=('Makefile*')
+    pathspec+=('**/Makefile*')
+    pathspec+=('**/*.mk')
+    git -C "${RIOTBASE}" grep -n "${patterns[@]}" -- "${pathspec[@]}" \
+        | error_with_message "Don't use \`which\` in makefiles, use \`command -v\` instead."
+}
+
+check_stderr_null() {
+    local patterns=()
+    local pathspec=()
+
+    patterns+=(-e '2>[[:blank:]]*&1[[:blank:]]*>[[:blank:]]*/dev/null')
+
+    pathspec+=('Makefile*')
+    pathspec+=('**/Makefile*')
+    pathspec+=('**/*.mk')
+    git -C "${RIOTBASE}" grep -n "${patterns[@]}" -- "${pathspec[@]}" \
+        | error_with_message "Redirecting stderr and stdout to /dev/null is \`>/dev/null 2>&1\`; the other way round puts the old stderr to the new stdout."
+}
+
 error_on_input() {
     ! grep ''
 }
@@ -353,6 +379,8 @@ all_checks() {
     check_no_pseudomodules_in_makefile_dep
     check_no_usemodules_in_makefile_include
     check_no_pkg_source_local
+    check_shell_which
+    check_stderr_null
 }
 
 main() {

diff --git a/dist/tools/openvisualizer/makefile.openvisualizer.inc.mk b/dist/tools/openvisualizer/makefile.openvisualizer.inc.mk
@@ -34,9 +34,9 @@
 #
 
 # Use full path in case it needs to be run with sudo
-OPENV_SERVER_PATH := $(shell which openv-server)
-OPENV_CLIENT_PATH := $(shell which openv-client)
-OPENV_SERIAL_PATH := $(shell which openv-serial)
+OPENV_SERVER_PATH := $(shell command -v openv-server)
+OPENV_CLIENT_PATH := $(shell command -v openv-client)
+OPENV_SERIAL_PATH := $(shell command -v openv-serial)
 
 # Openvisualizer requires to know where openwsn-fw is located
 OPENV_OPENWSN_FW_PATH ?= --fw-path=$(RIOTBASE)/build/pkg/openwsn

diff --git a/makefiles/arch/riscv.inc.mk b/makefiles/arch/riscv.inc.mk
@@ -25,7 +25,7 @@ TARGET_ARCH_RISCV ?= \
     $(subst -gcc,,\
       $(notdir \
         $(word 1,\
-          $(foreach triple,$(_TRIPLES_TO_TEST),$(shell which $(triple)-gcc 2> /dev/null))))))
+          $(foreach triple,$(_TRIPLES_TO_TEST),$(shell command -v $(triple)-gcc))))))
 
 TARGET_ARCH ?= $(TARGET_ARCH_RISCV)
 

diff --git a/makefiles/boards/sam0.inc.mk b/makefiles/boards/sam0.inc.mk
@@ -29,7 +29,7 @@ ifeq ($(PROGRAMMER),)
     PROGRAMMER ?= edbg
   else ifeq ($(OPENOCD_DEBUG_ADAPTER),jlink)
     # only use JLinkExe if it's installed
-    ifneq (,$(shell which JLinkExe))
+    ifneq (,$(shell command -v JLinkExe))
       PROGRAMMER ?= jlink
     else
       PROGRAMMER ?= openocd

diff --git a/pkg/flatbuffers/Makefile.include b/pkg/flatbuffers/Makefile.include
@@ -2,7 +2,7 @@ INCLUDES += -I$(PKGDIRBASE)/flatbuffers/include
 
 FLATC ?= flatc
 
-ifneq (0,$(shell which flatc > /dev/null 2>&1 ; echo $$?))
+ifeq (,$(shell command -v flatc ))
   FLATC = $(RIOTTOOLS)/flatc/flatc
   $(call target-export-variables,all,FLATC)
 endif