Introduce shell script syntax checks (#675)

* Introduce shell script syntax checks

* Makefile.am : do not shellcheck/spellcheck by default as it can fail for external circumstances (e.g. missing tools)

* Makefile.am : visibly separate the "check-scripts-syntax" implementation (with current system shells) vs "shellcheck" which may get impemented differently (with external tools) in later iterations

* Makefile.am : update the check-scripts-syntax comment (clarify text, fix typo)

* Makefile.am : clarify the check-scripts-syntax vs shellcheck comment

* .travis.yml and ci_build.sh : leave TBDs for future shellcheck tool integration

* ci_build.sh : call both shellcheck and check-scripts-syntax targets; it does not duplicate the work and reports while one calls another for now - and will do two tests when they become independent

.travis.yml

@@ -98,6 +98,14 @@ matrix:
         packages: &deps_aspell
         - aspell
         - aspell-en
+  - env: BUILD_TYPE=default-shellcheck
+    os: linux
+    addons:
+      apt:
+        packages:
+        - coreutils
+        - file
+        #TBD# - shellcheck
   - env: BUILD_TYPE=default-tgt:distcheck-valgrind
     os: linux
     sudo: false

Makefile.am

@@ -57,6 +57,37 @@ spellcheck spellcheck-interactive:
 doc spellcheck-sortdict:
 	cd $(srcdir)/docs && $(MAKE) $@
 
+# This target adds syntax-checking for committed shell script files,
+# to avoid surprises and delays in finding fatal typos after packaging
+###
+### Note: currently, shellcheck target calls check-scripts-syntax
+### so when both are invoked at once, in the end the check is only
+### executed once. Later it is anticipated that shellcheck would
+### be implemented by requiring, configuring and calling the tool
+### named "shellcheck" for even more code inspection and details.
+### Still, there remains value in also checking the script syntax
+### by the very version of the shell interpreter that would run
+### these scripts in production usage of the resulting packages.
+###
+check-scripts-syntax:
+	@echo 'NOTE: modern bash complains about scripts using backticks (warning not error), which we ignore in NUT codebase for portability reasons: `...` obsolete, use $$(...)'
+	for F in `git ls-files || find . -type f` ; do \
+	    case "`file "$$F"`" in \
+	        *"Bourne-Again shell script"*) ( set -x ; /bin/bash -n "$$F" ; ) ;; \
+	        *"POSIX shell script"*|*"shell script"*) ( set -x ; /bin/sh -n "$$F" ; ) ;; \
+	    esac || { RES=$$? ; echo "ERROR: Syntax check failed for script file: $$F" >&2 ; exit $$RES ; } ; \
+	done
+	@echo 'SUCCESS: Shell scripts syntax is acceptable, no fatal issues were found'
+
+shellcheck-disclaimer:
+	@echo "==============================================================================="
+	@echo "NOTICE: 'make shellcheck' is currently an alias for 'make check-scripts-syntax'"
+	@echo "Later it may become a call to the real shellcheck tool (if available on the"
+	@echo "build system during the configure phase)"
+	@echo "==============================================================================="
+
+shellcheck: shellcheck-disclaimer check-scripts-syntax
+
 # ----------------------------------------------------------------------
 # Automatically generate the ChangeLog from Git logs:
 MAINTAINERCLEAN_FILES = ChangeLog

ci_build.sh

@@ -25,7 +25,7 @@ case "$CI_TRACE" in
 esac
 
 case "$BUILD_TYPE" in
-default|default-alldrv|default-spellcheck|default-nodoc|default-withdoc|"default-tgt:"*)
+default|default-alldrv|default-spellcheck|default-shellcheck|default-nodoc|default-withdoc|"default-tgt:"*)
     LANG=C
     LC_ALL=C
     export LANG LC_ALL
@@ -111,10 +111,11 @@ default|default-alldrv|default-spellcheck|default-nodoc|default-withdoc|"default
             CONFIG_OPTS+=("--with-doc=no")
             DO_DISTCHECK=no
             ;;
-        "default-spellcheck")
+        "default-spellcheck"|"default-shellcheck")
             CONFIG_OPTS+=("--with-all=no")
             CONFIG_OPTS+=("--with-libltdl=no")
             CONFIG_OPTS+=("--with-doc=man=skip")
+            #TBD# CONFIG_OPTS+=("--with-shellcheck=yes")
             DO_DISTCHECK=no
             ;;
         "default-withdoc")
@@ -227,6 +228,19 @@ default|default-alldrv|default-spellcheck|default-nodoc|default-withdoc|"default
             ( $CI_TIME make VERBOSE=1 SPELLCHECK_ERROR_FATAL=yes spellcheck )
             exit 0
             ;;
+        "default-shellcheck")
+            [ -z "$CI_TIME" ] || echo "`date`: Trying to check shell script syntax validity of the currently tested project..."
+            ### Note: currently, shellcheck target calls check-scripts-syntax
+            ### so when both are invoked at once, in the end the check is only
+            ### executed once. Later it is anticipated that shellcheck would
+            ### be implemented by requiring, configuring and calling the tool
+            ### named "shellcheck" for even more code inspection and details.
+            ### Still, there remains value in also checking the script syntax
+            ### by the very version of the shell interpreter that would run
+            ### these scripts in production usage of the resulting packages.
+            ( $CI_TIME make VERBOSE=1 shellcheck check-scripts-syntax )
+            exit $?
+            ;;
     esac
 
     ( echo "`date`: Starting the parallel build attempt..."; \