Turn shellcheck into a normal make target. Fix new files it caught

This checks every file it checked (and a few more), but explicitly instead of "if it works it works" best-effort (which wasn't that good anyway) Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz> Closes openzfs#10512 Closes openzfs#12101
vpsfreecz · Jun 1, 2021 · c3ef9f7 · c3ef9f7
1 parent d3858ab
commit c3ef9f7
Show file tree

Hide file tree

Showing 40 changed files with 214 additions and 108 deletions.
diff --git a/Makefile.am b/Makefile.am
@@ -1,3 +1,5 @@
+include $(top_srcdir)/config/Shellcheck.am
+
 ACLOCAL_AMFLAGS = -I config
 
 SUBDIRS = include
@@ -123,17 +125,8 @@ cstyle:
 
 filter_executable = -exec test -x '{}' \; -print
 
-PHONY += shellcheck
-shellcheck:
-	@if type shellcheck > /dev/null 2>&1; then \
-		shellcheck --exclude=SC1090,SC1117,SC1091 --format=gcc \
-			$$(find ${top_srcdir} -name "config*" -prune -name tests -prune \
-				-o -name "*.sh" -o -name "*.sh.in" -type f) \
-			$$(find ${top_srcdir}/cmd/zpool/zpool.d/* \
-			-type f ${filter_executable}); \
-	else \
-		echo "skipping shellcheck because shellcheck is not installed"; \
-	fi
+SHELLCHECKDIRS = cmd contrib etc scripts tests
+SHELLCHECKSCRIPTS = autogen.sh
 
 PHONY += checkabi storeabi
 checkabi: lib

diff --git a/cmd/Makefile.am b/cmd/Makefile.am
@@ -1,17 +1,23 @@
+include $(top_srcdir)/config/Shellcheck.am
+
 SUBDIRS  = zfs zpool zdb zhack zinject zstream ztest
 SUBDIRS += fsck_zfs vdev_id raidz_test zfs_ids_to_path
 SUBDIRS += zpool_influxdb
 
 CPPCHECKDIRS  = zfs zpool zdb zhack zinject zstream ztest
 CPPCHECKDIRS += raidz_test zfs_ids_to_path zpool_influxdb
 
+# TODO: #12084: SHELLCHECKDIRS = fsck_zfs vdev_id zpool
+SHELLCHECKDIRS = fsck_zfs zpool
+
 if USING_PYTHON
 SUBDIRS += arcstat arc_summary dbufstat
 endif
 
 if BUILD_LINUX
 SUBDIRS += mount_zfs zed zgenhostid zvol_id zvol_wait
 CPPCHECKDIRS += mount_zfs zed zgenhostid zvol_id
+SHELLCHECKDIRS += zed
 endif
 
 PHONY = cppcheck

diff --git a/cmd/fsck_zfs/Makefile.am b/cmd/fsck_zfs/Makefile.am
@@ -1,4 +1,5 @@
 include $(top_srcdir)/config/Substfiles.am
+include $(top_srcdir)/config/Shellcheck.am
 
 dist_sbin_SCRIPTS = fsck.zfs
 

diff --git a/cmd/vdev_id/Makefile.am b/cmd/vdev_id/Makefile.am
@@ -1 +1,3 @@
+include $(top_srcdir)/config/Shellcheck.am
+
 dist_udev_SCRIPTS = vdev_id
diff --git a/cmd/zed/Makefile.am b/cmd/zed/Makefile.am
@@ -1,8 +1,10 @@
 include $(top_srcdir)/config/Rules.am
+include $(top_srcdir)/config/Shellcheck.am
 
 AM_CFLAGS += $(LIBUDEV_CFLAGS) $(LIBUUID_CFLAGS)
 
 SUBDIRS = zed.d
+SHELLCHECKDIRS = $(SUBDIRS)
 
 sbin_PROGRAMS = zed
 

diff --git a/cmd/zed/zed.d/Makefile.am b/cmd/zed/zed.d/Makefile.am
@@ -1,5 +1,6 @@
 include $(top_srcdir)/config/Rules.am
 include $(top_srcdir)/config/Substfiles.am
+include $(top_srcdir)/config/Shellcheck.am
 
 EXTRA_DIST += README
 

diff --git a/cmd/zpool/Makefile.am b/cmd/zpool/Makefile.am
@@ -1,4 +1,5 @@
 include $(top_srcdir)/config/Rules.am
+include $(top_srcdir)/config/Shellcheck.am
 
 AM_CFLAGS += $(LIBBLKID_CFLAGS) $(LIBUUID_CFLAGS)
 

diff --git a/cmd/zpool/zpool.d/smart b/cmd/zpool/zpool.d/smart
@@ -53,7 +53,7 @@ get_filename_from_dir()
 	num_files=$(find "$dir" -maxdepth 1 -type f | wc -l)
 	mod=$((pid % num_files))
 	i=0
-	find "$dir" -type f -printf "%f\n" | while read -r file ; do
+	find "$dir" -type f -printf '%f\n' | while read -r file ; do
 		if [ "$mod" = "$i" ] ; then
 			echo "$file"
 			break
@@ -231,11 +231,11 @@ esac
 with_vals=$(echo "$out" | grep -E "$scripts")
 if [ -n "$with_vals" ]; then
 	echo "$with_vals"
-	without_vals=$(echo "$scripts" | tr "|" "\n" |
+	without_vals=$(echo "$scripts" | tr '|' '\n' |
 		grep -v -E "$(echo "$with_vals" |
 		awk -F "=" '{print $1}')" | awk '{print $0"="}')
 else
-	without_vals=$(echo "$scripts" | tr "|" "\n" | awk '{print $0"="}')
+	without_vals=$(echo "$scripts" | tr '|' '\n' | awk '{print $0"="}')
 fi
 
 if [ -n "$without_vals" ]; then

diff --git a/cmd/zvol_wait/Makefile.am b/cmd/zvol_wait/Makefile.am
@@ -1 +1,3 @@
+include $(top_srcdir)/config/Shellcheck.am
+
 dist_bin_SCRIPTS = zvol_wait
diff --git a/config/Shellcheck.am b/config/Shellcheck.am
@@ -0,0 +1,8 @@
+.PHONY: shellcheck
+shellcheck: $(SCRIPTS) $(SHELLCHECKSCRIPTS)
+if HAVE_SHELLCHECK
+	[ -z "$(SCRIPTS)$(SHELLCHECKSCRIPTS)" ] && exit; shellcheck $$([ -n "$(SHELLCHECK_SHELL)" ] && echo "--shell=$(SHELLCHECK_SHELL)") --exclude=SC1090,SC1091$(SHELLCHECK_IGNORE) --format=gcc $(SCRIPTS) $(SHELLCHECKSCRIPTS)
+else
+	@[ -z "$(SCRIPTS)$(SHELLCHECKSCRIPTS)" ] && exit; echo "skipping shellcheck of" $(SCRIPTS) $(SHELLCHECKSCRIPTS) "because shellcheck is not installed"
+endif
+	@set -e; for dir in $(SHELLCHECKDIRS); do $(MAKE) -C $$dir shellcheck; done
diff --git a/config/always-shellcheck.m4 b/config/always-shellcheck.m4
@@ -0,0 +1,7 @@
+dnl #
+dnl # Check if shellcheck is available.
+dnl #
+AC_DEFUN([ZFS_AC_CONFIG_ALWAYS_SHELLCHECK], [
+	AC_CHECK_PROG([SHELLCHECK], [shellcheck], [yes])
+	AM_CONDITIONAL([HAVE_SHELLCHECK], [test "x$SHELLCHECK" = "xyes"])
+])
diff --git a/config/zfs-build.m4 b/config/zfs-build.m4
@@ -207,6 +207,7 @@ AC_DEFUN([ZFS_AC_CONFIG_ALWAYS], [
 	ZFS_AC_CONFIG_ALWAYS_PYZFS
 	ZFS_AC_CONFIG_ALWAYS_SED
 	ZFS_AC_CONFIG_ALWAYS_CPPCHECK
+	ZFS_AC_CONFIG_ALWAYS_SHELLCHECK
 ])
 
 AC_DEFUN([ZFS_AC_CONFIG], [

diff --git a/contrib/Makefile.am b/contrib/Makefile.am
@@ -1,3 +1,5 @@
+include $(top_srcdir)/config/Shellcheck.am
+
 SUBDIRS = bash_completion.d pyzfs zcp
 if BUILD_LINUX
 SUBDIRS += bpftrace dracut initramfs
@@ -6,3 +8,5 @@ if PAM_ZFS_ENABLED
 SUBDIRS += pam_zfs_key
 endif
 DIST_SUBDIRS = bash_completion.d bpftrace dracut initramfs pam_zfs_key pyzfs zcp
+
+SHELLCHECKDIRS = bash_completion.d bpftrace dracut initramfs
diff --git a/contrib/bash_completion.d/Makefile.am b/contrib/bash_completion.d/Makefile.am
@@ -1,8 +1,13 @@
 include $(top_srcdir)/config/Substfiles.am
+include $(top_srcdir)/config/Shellcheck.am
 
 bashcompletiondir = $(sysconfdir)/bash_completion.d
 
 noinst_DATA = zfs
 
 EXTRA_DIST += $(noinst_DATA)
 SUBSTFILES += $(noinst_DATA)
+
+SHELLCHECKSCRIPTS = $(noinst_DATA)
+SHELLCHECK_SHELL = bash
+SHELLCHECK_IGNORE = ,SC2207
diff --git a/contrib/bash_completion.d/zfs.in b/contrib/bash_completion.d/zfs.in
@@ -139,6 +139,7 @@ __zfs_match_multiple_snapshots()
             fi
             local range_start
             range_start="$(expr "$cur" : '\(.*%\)')"
+            # shellcheck disable=SC2016
             $__ZFS_CMD list -H -o name -s name -t snapshot -d 1 "$base_dataset" | sed 's$.*@$'"$range_start"'$g'
         fi
     else
@@ -163,7 +164,7 @@ __zfs_argument_chosen()
             then
                 return 0
             fi
-            for property in $@
+            for property in "$@"
             do
                 if [[ $prev == "$property"* ]]
                 then
@@ -181,6 +182,7 @@ __zfs_complete_ordered_arguments()
     local list2=$2
     local cur=$3
     local extra=$4
+    # shellcheck disable=SC2086
     if __zfs_argument_chosen $list1
     then
         COMPREPLY=($(compgen -W "$list2 $extra" -- "$cur"))
@@ -193,9 +195,10 @@ __zfs_complete_multiple_options()
 {
     local options=$1
     local cur=$2
+    local existing_opts
 
     COMPREPLY=($(compgen -W "$options" -- "${cur##*,}"))
-    local existing_opts=$(expr "$cur" : '\(.*,\)')
+    existing_opts=$(expr "$cur" : '\(.*,\)')
     if [[ $existing_opts ]]
     then
         COMPREPLY=( "${COMPREPLY[@]/#/${existing_opts}}" )
@@ -290,6 +293,7 @@ __zfs_complete()
                 *)
                     if ! __zfs_complete_switch "H,r,p,d,o,t,s"
                     then
+                        # shellcheck disable=SC2046
                         if __zfs_argument_chosen $(__zfs_get_properties)
                         then
                             COMPREPLY=($(compgen -W "$(__zfs_match_snapshot)" -- "$cur"))
@@ -303,7 +307,7 @@ __zfs_complete()
         inherit)
             if ! __zfs_complete_switch "r"
             then
-                __zfs_complete_ordered_arguments "$(__zfs_get_inheritable_properties)" "$(__zfs_match_snapshot)" $cur
+                __zfs_complete_ordered_arguments "$(__zfs_get_inheritable_properties)" "$(__zfs_match_snapshot)" "$cur"
             fi
             ;;
         list)
@@ -369,7 +373,7 @@ __zfs_complete()
             esac
             ;;
         set)
-            __zfs_complete_ordered_arguments "$(__zfs_get_editable_properties)" "$(__zfs_match_snapshot)" $cur
+            __zfs_complete_ordered_arguments "$(__zfs_get_editable_properties)" "$(__zfs_match_snapshot)" "$cur"
             __zfs_complete_nospace
             ;;
         upgrade)
@@ -388,7 +392,7 @@ __zfs_complete()
         destroy)
             if ! __zfs_complete_switch "d,f,n,p,R,r,v"
             then
-                __zfs_complete_multiple_options "$(__zfs_match_multiple_snapshots)" $cur
+                __zfs_complete_multiple_options "$(__zfs_match_multiple_snapshots)" "$cur"
                 __zfs_complete_nospace
             fi
             ;;
@@ -425,7 +429,7 @@ __zpool_list_pools()
 
 __zpool_complete()
 {
-    local cur prev cmd cmds
+    local cur prev cmd cmds pools
     COMPREPLY=()
     cur="${COMP_WORDS[COMP_CWORD]}"
     prev="${COMP_WORDS[COMP_CWORD-1]}"
@@ -440,7 +444,7 @@ __zpool_complete()
 
     case "${cmd}" in
         get)
-            __zfs_complete_ordered_arguments "$(__zpool_get_properties)" "$(__zpool_list_pools)" $cur
+            __zfs_complete_ordered_arguments "$(__zpool_get_properties)" "$(__zpool_list_pools)" "$cur"
             return 0
             ;;
         import)
@@ -453,12 +457,13 @@ __zpool_complete()
             return 0
             ;;
         set)
-            __zfs_complete_ordered_arguments "$(__zpool_get_editable_properties)" "$(__zpool_list_pools)" $cur
+            __zfs_complete_ordered_arguments "$(__zpool_get_editable_properties)" "$(__zpool_list_pools)" "$cur"
             __zfs_complete_nospace
             return 0
             ;;
         add|attach|clear|create|detach|offline|online|remove|replace)
-            local pools="$(__zpool_list_pools)"
+            pools="$(__zpool_list_pools)"
+            # shellcheck disable=SC2086
             if __zfs_argument_chosen $pools
             then
                 _filedir

diff --git a/contrib/bpftrace/Makefile.am b/contrib/bpftrace/Makefile.am
@@ -1,3 +1,7 @@
+include $(top_srcdir)/config/Shellcheck.am
+
 EXTRA_DIST = \
 	taskqlatency.bt \
 	zfs-trace.sh
+
+SHELLCHECKSCRIPTS = zfs-trace.sh
diff --git a/contrib/dracut/02zfsexpandknowledge/Makefile.am b/contrib/dracut/02zfsexpandknowledge/Makefile.am
@@ -1,4 +1,5 @@
 include $(top_srcdir)/config/Substfiles.am
+include $(top_srcdir)/config/Shellcheck.am
 
 pkgdracutdir = $(dracutdir)/modules.d/02zfsexpandknowledge
 pkgdracut_SCRIPTS = \

diff --git a/contrib/dracut/90zfs/Makefile.am b/contrib/dracut/90zfs/Makefile.am
@@ -1,4 +1,5 @@
 include $(top_srcdir)/config/Substfiles.am
+include $(top_srcdir)/config/Shellcheck.am
 
 pkgdracutdir = $(dracutdir)/modules.d/90zfs
 pkgdracut_SCRIPTS = \

diff --git a/contrib/dracut/90zfs/module-setup.sh.in b/contrib/dracut/90zfs/module-setup.sh.in
@@ -60,7 +60,7 @@ install() {
 	# shellcheck disable=SC2050
 	if [ @LIBFETCH_DYNAMIC@ != 0 ]; then
 		for d in $libdirs; do
-			[ -e "$d"/@LIBFETCH_SONAME@ ] && dracut_install "$d"/@LIBFETCH_SONAME@
+			[ -e "$d/"@LIBFETCH_SONAME@ ] && dracut_install "$d/"@LIBFETCH_SONAME@
 		done
 	fi
 	dracut_install @mounthelperdir@/mount.zfs

diff --git a/contrib/dracut/Makefile.am b/contrib/dracut/Makefile.am
@@ -1,3 +1,6 @@
+include $(top_srcdir)/config/Shellcheck.am
+
 SUBDIRS = 02zfsexpandknowledge 90zfs
+SHELLCHECKDIRS = $(SUBDIRS)
 
 EXTRA_DIST = README.dracut.markdown
diff --git a/contrib/initramfs/Makefile.am b/contrib/initramfs/Makefile.am
@@ -1,9 +1,12 @@
+include $(top_srcdir)/config/Shellcheck.am
+
 initrddir = /usr/share/initramfs-tools
 
 dist_initrd_SCRIPTS = \
        zfsunlock
 
 SUBDIRS = conf.d conf-hooks.d hooks scripts
+SHELLCHECKDIRS = hooks scripts
 
 EXTRA_DIST = \
 	README.initramfs.markdown
diff --git a/contrib/initramfs/hooks/Makefile.am b/contrib/initramfs/hooks/Makefile.am
@@ -1,4 +1,5 @@
 include $(top_srcdir)/config/Substfiles.am
+include $(top_srcdir)/config/Shellcheck.am
 
 hooksdir = /usr/share/initramfs-tools/hooks
 

diff --git a/contrib/initramfs/scripts/Makefile.am b/contrib/initramfs/scripts/Makefile.am
@@ -1,6 +1,11 @@
+include $(top_srcdir)/config/Shellcheck.am
+
 scriptsdir = /usr/share/initramfs-tools/scripts
 
-dist_scripts_DATA = \
+dist_scripts_SCRIPTS = \
 	zfs
 
 SUBDIRS = local-top
+
+SHELLCHECKDIRS = $(SUBDIRS)
+SHELLCHECK_SHELL = sh
diff --git a/contrib/initramfs/scripts/local-top/Makefile.am b/contrib/initramfs/scripts/local-top/Makefile.am
@@ -1,3 +1,5 @@
+include $(top_srcdir)/config/Shellcheck.am
+
 localtopdir = /usr/share/initramfs-tools/scripts/local-top
 
 dist_localtop_SCRIPTS = \
Original file line number	Diff line number	Diff line change
		@@ -1 +1,3 @@
		include $(top_srcdir)/config/Shellcheck.am

		dist_udev_SCRIPTS = vdev_id
Original file line number	Diff line number	Diff line change
		@@ -1 +1,3 @@
		include $(top_srcdir)/config/Shellcheck.am

		dist_bin_SCRIPTS = zvol_wait