Skip to content

Commit

Permalink
Merge pull request #16776 from chrysn-pull-requests/shell-make-fixes-2
Browse files Browse the repository at this point in the history
Remove `which` from shell invocations
  • Loading branch information
chrysn authored Sep 3, 2021
2 parents 2bf1202 + 8664eb8 commit d4ed42a
Show file tree
Hide file tree
Showing 7 changed files with 44 additions and 16 deletions.
16 changes: 8 additions & 8 deletions Makefile.include
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions dist/testbed-support/makefile.iotlab.single.inc.mk
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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))
Expand Down
28 changes: 28 additions & 0 deletions dist/tools/buildsystem_sanity_check/check.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 ''
}
Expand All @@ -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() {
Expand Down
6 changes: 3 additions & 3 deletions dist/tools/openvisualizer/makefile.openvisualizer.inc.mk
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion makefiles/arch/riscv.inc.mk
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion makefiles/boards/sam0.inc.mk
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/flatbuffers/Makefile.include
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit d4ed42a

Please sign in to comment.