diff --git a/.github/workflows/07-shellcheck.yml b/.github/workflows/07-shellcheck.yml new file mode 100644 index 0000000000..a5ddad6a36 --- /dev/null +++ b/.github/workflows/07-shellcheck.yml @@ -0,0 +1,49 @@ +# Static analysis of the shell scripts changed by a pull request with +# ShellCheck (https://www.shellcheck.net/). Many existing scripts have +# known findings, so this fails only for new problems of "warning" or +# "error" level (new "info" and "style" ones are shown as notices); the +# findings are shown as annotations on the changed lines, and listed in +# the job summary. Deliberate portability idioms of NUT scripts are +# disabled in .shellcheckrc file. +# Run tools/shellcheck-ci.pl locally for the same check. +# +# Runs on pull_request (not pull_request_target), so for PRs from forks +# it has a read-only token and runs the copy of this workflow in the PR, +# which is fine as it needs no write permissions. + +name: "GHA-07: ShellCheck" + +on: + pull_request: + +permissions: {} + +jobs: + shellcheck: + name: "ShellCheck new findings in changed scripts" + runs-on: ubuntu-24.04 + permissions: + contents: read + + steps: + # The pull_request checkout is a merge commit of the PR into its + # base branch, so its first parent is the base to compare with + - name: Checkout repository + uses: actions/checkout@v7 + with: + fetch-depth: 2 + persist-credentials: false + + # ShellCheck is usually pre-installed in GitHub-hosted Ubuntu runners; + # both base and PR versions are checked with the same one, so its + # exact version does not matter much + - name: Install ShellCheck + run: | + command -v shellcheck >/dev/null || { + sudo apt-get update && sudo apt-get install -y shellcheck + } + shellcheck --version + + - name: Run ShellCheck for changed scripts + run: | + perl tools/shellcheck-ci.pl HEAD^1 diff --git a/.shellcheckrc b/.shellcheckrc new file mode 100644 index 0000000000..d01c1afa7f --- /dev/null +++ b/.shellcheckrc @@ -0,0 +1,15 @@ +# ShellCheck (https://www.shellcheck.net/) settings for NUT scripts. +# Found by shellcheck in the directory of a checked script or its parents. +# +# NUT scripts deliberately stay portable to old Bourne-style /bin/sh +# implementations (e.g. on Solaris/illumos, AIX, HP-UX), so do not +# suggest modern replacements for these idioms: + +# Use $(...) notation instead of legacy backticks `...` +disable=SC2006 + +# Avoid x-prefix in comparisons as it no longer serves a purpose +disable=SC2268 + +# expr is antiquated, consider rewriting this using $((..)), ${} or [[ ]] +disable=SC2003 diff --git a/Makefile.am b/Makefile.am index a1b6b717a4..111cafb6e9 100644 --- a/Makefile.am +++ b/Makefile.am @@ -282,7 +282,7 @@ EXTRA_DIST += ci_build.sh ci_build.adoc \ EXTRA_DIST += autogen.sh # Help also the tarball users contribute properly: -EXTRA_DIST += .editorconfig +EXTRA_DIST += .editorconfig .shellcheckrc # automake xompilation wrapper EXTRA_DIST += compile @@ -844,8 +844,48 @@ $(SPELLCHECK_DIRS_LAST): prep-src-docs/docs/man prep-src-docs/docs $(SPELLCHECK_ # but using the correct make target for this goal: # FIXME: fanned-out recipes tend to fail early despite "make -ks", so for # now we retry with a not-fanned-out attempt to cover most touch-files +# In GitHub Actions, the docs recipes collect workflow commands (for error +# annotations) into SPELLCHECK_GHA_LOG, reported once at the end, because +# the same errors may be reported again by the retry and there is a limit +# on the count of annotations per step. They are also listed in the job +# summary (GITHUB_STEP_SUMMARY), if available, with links to the lines. +# Note: no hash characters in the awk program (octal escape is used). +SPELLCHECK_GHA_SUMMARY_AWK = { \ + s = $$0 ; sub(/^::error file=/, "", s) ; \ + f = s ; sub(/,line=.*$$/, "", f) ; \ + l = s ; sub(/^.*,line=/, "", l) ; sub(/,.*$$/, "", l) ; \ + c = s ; sub(/^.*,col=/, "", c) ; sub(/,.*$$/, "", c) ; \ + m = s ; sub(/^[^:]*::/, "", m) ; sub(/; fix it.*$$/, "", m) ; \ + gsub(/[|]/, "\\|", m) ; \ + loc = f ":" l ":" c ; \ + if (ENVIRON["GITHUB_SERVER_URL"] != "" && ENVIRON["GITHUB_REPOSITORY"] != "" && ENVIRON["GITHUB_SHA"] != "") { \ + loc = "[" loc "](" ENVIRON["GITHUB_SERVER_URL"] "/" ENVIRON["GITHUB_REPOSITORY"] "/blob/" ENVIRON["GITHUB_SHA"] "/" f "\043L" l ")" ; \ + } \ + print "| " loc " | " m " |" ; \ + } + spellcheck spellcheck-interactive: @dotMAKE@ +@SUBDIR_TGT_MAKEFLAGS="$${SUBDIR_TGT_MAKEFLAGS-} -k -s " ; export SUBDIR_TGT_MAKEFLAGS ; \ + if [ x"$${GITHUB_ACTIONS-}" = xtrue ] ; then \ + SPELLCHECK_GHA_LOG="$(abs_top_builddir)/gha-annotations-spellchecked" ; \ + rm -f "$${SPELLCHECK_GHA_LOG}" ; \ + export SPELLCHECK_GHA_LOG ; \ + fi ; \ + report_gha() { \ + if [ -n "$${SPELLCHECK_GHA_LOG-}" ] && [ -s "$${SPELLCHECK_GHA_LOG}" ] ; then \ + sort -u "$${SPELLCHECK_GHA_LOG}" ; \ + if [ -n "$${GITHUB_STEP_SUMMARY-}" ] ; then \ + { echo '### Spellcheck: unknown words' ; echo '' ; \ + echo '| Location | Problem |' ; echo '|---|---|' ; \ + sort -u "$${SPELLCHECK_GHA_LOG}" | $(AWK) '$(SPELLCHECK_GHA_SUMMARY_AWK)' ; \ + echo '' ; \ + echo 'Fix the spelling, or add valid words to `docs/nut.dict` (e.g. with `make spellcheck-interactive`, or edit it and run `make spellcheck-sortdict`).' ; \ + echo '' ; \ + } >> "$${GITHUB_STEP_SUMMARY}" ; \ + fi ; \ + fi ; \ + if [ -n "$${SPELLCHECK_GHA_LOG-}" ] ; then rm -f "$${SPELLCHECK_GHA_LOG}" ; fi ; \ + } ; \ if [ x"$(NUT_MAKE_SKIP_FANOUT)" = xtrue ] ; then \ RES=0 ; \ if [ x"$(SUBDIR_MAKE_VERBOSE)" != x0 ] ; then \ @@ -861,6 +901,7 @@ spellcheck spellcheck-interactive: @dotMAKE@ D="`echo \"$$D\" | sed 's,^spellcheck/,,'`" ; \ (cd "$(builddir)/$$D" && $(MAKE) $(AM_MAKEFLAGS) SPELLCHECK_REPORT_MAYBE_UPDATED_DICT=yes -k -s $@) || RES=$$? ; \ done ; \ + report_gha ; \ exit $$RES ; \ fi ; \ SUBDIR_MAKE_VERBOSE="$(SUBDIR_MAKE_VERBOSE)" ; \ @@ -871,7 +912,8 @@ spellcheck spellcheck-interactive: @dotMAKE@ ( $(MAKE) $(AM_MAKEFLAGS) SPELLCHECK_TGT='$@' SUBDIR_MAKE_VERBOSE="$${SUBDIR_MAKE_VERBOSE}" -k -s $(SPELLCHECK_DIRS) && exit ; \ echo "WARNING: FAILED fanned-out attempt in $@, retrying with NUT_MAKE_SKIP_FANOUT" >&2 ; \ $(MAKE) $(AM_MAKEFLAGS) NUT_MAKE_SKIP_FANOUT=true SPELLCHECK_TGT='$@' -k -s $(SPELLCHECK_DIRS) \ - ) || exit ; \ + ) || { RES=$$? ; report_gha ; exit $$RES ; } ; \ + report_gha ; \ if [ x'$@' = xspellcheck-interactive ] ; then \ echo "SUCCESS: $@: follow up with spellcheck-quick to revise and update timestamps"; \ $(MAKE) $(AM_MAKEFLAGS) spellcheck-quick ; \ @@ -1063,15 +1105,27 @@ dist-hook: ### 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. +### POSIX shell scripts are checked with each interpreter listed in the +### SHELL_PROGS envvar (system /bin/sh by default), like the NDE self-test +### below does; Bourne-Again shell scripts are always checked with bash. ### 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 $$(...)' + @echo 'NOTE: some shells (e.g. ksh93) complain about scripts using backticks (warning not error), which we ignore in NUT codebase for portability reasons, so such messages are hidden: `...` obsolete, use $$(...)' @RUNBASH=bash; if [ -x /bin/bash ] && /bin/bash -c 'echo $${BASH_VERSION}' | $(EGREP) '^[456789]\.' ; then RUNBASH=/bin/bash ; else if [ -x /usr/bin/env ] ; then RUNBASH="/usr/bin/env bash"; fi; fi ; \ + [ -n "$${SHELL_PROGS-}" ] || SHELL_PROGS="/bin/sh" ; \ + echo "Checking POSIX shell scripts with SHELL_PROGS='$$SHELL_PROGS', and Bourne-Again shell scripts with '$$RUNBASH'" ; \ + check_syntax() { \ + echo "+ $$1 -n $$F" ; \ + OUT="`$$1 -n \"$$F\" 2>&1`" ; RES=$$? ; \ + if [ -n "$$OUT" ] ; then printf '%s\n' "$$OUT" | $(EGREP) -v '`[.][.][.]` obsolete, use [$$][(][.][.][.][)]' >&2 ; fi ; \ + if [ "$$RES" != 0 ] ; then echo "ERROR: Syntax check failed for script file: $$F (with $$1)" >&2 ; return $$RES ; fi ; \ + } ; \ for F in `git ls-files || find . -type f` ; do \ case "`file \"$$F\"`" in \ - *"Bourne-Again shell script"*) ( set -x ; $$RUNBASH -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 ; } ; \ + *"Bourne-Again shell script"*) check_syntax "$$RUNBASH" || exit ;; \ + *"POSIX shell script"*|*"shell script"*) \ + for SH in $$SHELL_PROGS ; do check_syntax "$$SH" || exit ; done ;; \ + esac ; \ done @echo 'SUCCESS: Shell scripts syntax is acceptable, no fatal issues were found' diff --git a/NEWS.adoc b/NEWS.adoc index 7cca12228d..f310ff6d2d 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -997,6 +997,29 @@ https://github.com/networkupstools/nut/milestone/13 with both a current and an old (7.1, as on CentOS 7) version of the `aspell-en` dictionary. Local builds keep the default of `configure --enable-spellcheck=auto`. [PR #3752] + * The `make shellcheck` (and `make check-scripts-syntax`) recipe can now + check the POSIX shell scripts with each interpreter listed in the + `SHELL_PROGS` environment variable (default is still the system + `/bin/sh`), and hides warnings about the legacy command substitution + syntax (e.g. from `ksh93`) to avoid noise that obscured other problems. + [issues #1058, #1059] + * Added a "GHA-07: ShellCheck" GitHub Actions workflow which checks the + shell scripts changed by a pull request, and fails only for findings + of "warning" or "error" level that are new compared to the base branch + (many existing scripts have known findings), with annotations on the + changed lines and a list in the job summary. The same check can be run + locally with `perl tools/shellcheck-ci.pl`. A new `.shellcheckrc` file + disables the checks for deliberate portability idioms of NUT scripts + (backticks, `x"$var"` comparisons, `expr`). + * The `make spellcheck` recipes now report unknown words with the file + name, line and column where they were found (rather than byte offsets + in the `aspell` output), and in GitHub Actions also as annotations on + the source lines (once per unknown word, despite the retries) and in + the job summary. + * The `configure` script now also looks for `aspell` filter resources + under `lib64` (e.g. on CentOS 7) and in directories named by just the + major and minor version (e.g. `aspell-0.60` for `aspell` 0.60.6.1). + [issue #2068] Release notes for NUT 2.8.5 - what's new since 2.8.4 diff --git a/ci_build.sh b/ci_build.sh index 7a6e36292e..360706871f 100755 --- a/ci_build.sh +++ b/ci_build.sh @@ -2332,6 +2332,9 @@ default|default-alldrv|default-alldrv:no-distcheck|default-all-errors|default-al # Note: no PARMAKE_FLAGS here - better have this output readably # ordered in case of issues (in sequential replay below). ( echo "`date`: Starting the quiet build attempt for target $BUILD_TYPE..." >&2 + # Its output is discarded, so let the verbose attempt below + # make the GitHub Actions annotations and job summary + GITHUB_ACTIONS="" ; export GITHUB_ACTIONS $CI_TIME $MAKE $MAKE_FLAGS_QUIET SPELLCHECK_ERROR_FATAL=yes -k $PARMAKE_FLAGS "`echo \"$BUILD_TYPE\" | sed 's,^default-,,'`" >/dev/null 2>&1 \ && echo "`date`: SUCCEEDED the spellcheck" >&2 ) || \ @@ -2339,6 +2342,9 @@ default|default-alldrv|default-alldrv:no-distcheck|default-all-errors|default-al $CI_TIME $MAKE $MAKE_FLAGS_VERBOSE SPELLCHECK_ERROR_FATAL=yes spellcheck # Make end of log useful: echo "`date`: FAILED something in spellcheck above; re-starting a non-verbose build attempt to just summarize now:" >&2 + # The verbose attempt above already made the GitHub Actions + # annotations and job summary for the errors, do not repeat + GITHUB_ACTIONS="" ; export GITHUB_ACTIONS $CI_TIME $MAKE $MAKE_FLAGS_QUIET SPELLCHECK_ERROR_FATAL=yes spellcheck ) exit $? diff --git a/docs/Makefile.am b/docs/Makefile.am index 319b2939cf..67a7d38c76 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -885,6 +885,39 @@ ASPELL_ENV_LC_ALL = C ### ASPELL_ENV_LC_ALL = en_US.UTF-8 ASPELL_OUT_NOTERRORS = (^[ \t]*[\*\@]|^$$) +# Locate the errors in "aspell -a" output: it prints a banner line, and then +# for each input line its result lines (with a column offset for unknown +# words, which matches the original line since we prefix each line with a +# space), followed by an empty line. Reports the errors as "file:line:col:" +# prefixed aspell result lines, or as GitHub Actions workflow commands (for +# annotations on the changed lines) with `-v mode=gha`; the top-level +# spellcheck recipe collects these into SPELLCHECK_GHA_LOG, if set. +# No annotations are made with SPELLCHECK_ERROR_FATAL=no (e.g. in distcheck +# when the spelling is checked by another CI step). Note: no ternary +# operators nor hash characters here, for older awk and make implementations. +ASPELL_OUT_LOCATE_AWK = BEGIN { line = 1 } \ + /^@/ { next } \ + /^$$/ { line++ ; next } \ + /^[*+-]/ { next } \ + { \ + word = $$2 ; col = $$3 ; sugg = "" ; \ + if ($$1 == "&" || $$1 == "?") { \ + col = $$4 ; sub(/:$$/, "", col) ; \ + sugg = $$0 ; sub(/^[^:]*: /, "", sugg) ; \ + } \ + if (mode == "gha") { \ + n = split(sugg, s, ", ") ; hint = "" ; \ + for (i = 1; i <= n && i <= 5; i++) { \ + if (i > 1) { hint = hint ", " } \ + hint = hint s[i] ; \ + } \ + if (hint != "") { hint = " (suggestions: " hint ")" } \ + printf "::error file=%s,line=%d,col=%d,title=Spellcheck::Unknown word \"%s\"%s; fix it, or add it to docs/nut.dict if valid\n", src, line, col, word, hint ; \ + } else { \ + print src ":" line ":" col ": " $$0 ; \ + } \ + } + # WARNING: The percent wildcard is a GNU extension; otherwise we need # a ".txt.txt-spellchecked" type of rule and files like "README" all # renamed to *.txt, or lots of rules for files without the extensions. @@ -962,13 +995,24 @@ $(SPELLCHECK_BUILDDIR)/$(SPELLCHECK_SRC_ONE)-spellchecked: $(SPELLCHECK_SRCDIR)/ echo " ASPELL Spell checking on $${REPORT_PREFIX}$${REPORT_SRCDIR}$${REPORT_SRC_ONE}"; \ fi ; \ OUT="`(sed 's,^\(.*\)$$, \1,' | $(ASPELL) -a $(ASPELL_NUT_TEXMODE_ARGS) $(ASPELL_NUT_COMMON_ARGS) 2>&1) < '$(SPELLCHECK_SRCDIR)/$(SPELLCHECK_SRC_ONE)'`" \ - && { if test -n "$$OUT" ; then OUT="`echo \"$$OUT\" | $(EGREP) -b -v '$(ASPELL_OUT_NOTERRORS)' `" ; fi; \ - test -z "$$OUT" ; } \ + && { ERRS="" ; if test -n "$$OUT" ; then \ + printf '%s\n' "$$OUT" | $(AWK) -v src="$${REPORT_SRCDIR}$${REPORT_SRC_ONE}" '$(ASPELL_OUT_LOCATE_AWK)' > '$@.errs' ; \ + ERRS="`cat '$@.errs'`" ; \ + rm -f '$@.errs' ; \ + fi; \ + test -z "$$ERRS" ; } \ || { RES=$$? ; \ - echo "FAILED : Aspell reported errors here:" >&2 \ - && echo "----- vvv" >&2 \ - && echo "$$OUT" >&2 \ - && echo "----- ^^^" >&2 ; \ + echo "FAILED : Aspell reported errors here:" >&2 ; \ + echo "----- vvv" >&2 ; \ + if test -n "$$ERRS" ; then echo "$$ERRS" >&2 ; else echo "$$OUT" >&2 ; fi ; \ + echo "----- ^^^" >&2 ; \ + if test x"$${GITHUB_ACTIONS-}" = xtrue && test x"$(SPELLCHECK_ERROR_FATAL)" != xno && test -n "$$ERRS" ; then \ + if test -n "$${SPELLCHECK_GHA_LOG-}" ; then \ + printf '%s\n' "$$OUT" | $(AWK) -v src="$${REPORT_SRCDIR}$${REPORT_SRC_ONE}" -v mode=gha '$(ASPELL_OUT_LOCATE_AWK)' >> "$${SPELLCHECK_GHA_LOG}" ; \ + else \ + printf '%s\n' "$$OUT" | $(AWK) -v src="$${REPORT_SRCDIR}$${REPORT_SRC_ONE}" -v mode=gha '$(ASPELL_OUT_LOCATE_AWK)' ; \ + fi ; \ + fi ; \ exit $$RES; } ; \ if $(EGREP) '^[ *.-]*(NOTE|TIP|IMPORTANT|CAUTION|WARNING):[ \t]*[a-z]' '$(SPELLCHECK_SRCDIR)/$(SPELLCHECK_SRC_ONE)' ; then \ echo "FAILED: ASCIIDOC nuance: Admonition block should contain a sentence, not starting with a lower-case letter: '$${REPORT_PREFIX}$${REPORT_SRCDIR}$${REPORT_SRC_ONE}'" >&2 ; \ diff --git a/docs/nut.dict b/docs/nut.dict index bb627f72ad..1884d6f411 100644 --- a/docs/nut.dict +++ b/docs/nut.dict @@ -1,4 +1,4 @@ -personal_ws-1.1 en 3900 utf-8 +personal_ws-1.1 en 3904 utf-8 AAC AAS ABI @@ -1346,6 +1346,7 @@ Shambarger Shara ShareAlike Shaul +ShellCheck ShutDown Sibbald Sicon @@ -1798,6 +1799,7 @@ backgrounding backport backported backports +backticks backupspro bart baseurl @@ -2235,6 +2237,7 @@ executables executeCommand execve expandsize +expr extapi extern externalConsole @@ -3352,6 +3355,7 @@ sgml sgs sha shellcheck +shellcheckrc shellenv shm showinternals diff --git a/docs/qa-guide.adoc b/docs/qa-guide.adoc index e3345872d4..5e3aa84d80 100644 --- a/docs/qa-guide.adoc +++ b/docs/qa-guide.adoc @@ -199,15 +199,34 @@ Shell script checks The `make shellcheck` recipe finds files which the `file` tool determines to be POSIX or Bourne-Again shell scripts, and runs them through respective interpreter's (`bash` or system `/bin/sh`) test mode to validate the syntax -works. +works. POSIX shell scripts can be checked with other interpreters instead +of `/bin/sh` by listing them in the `SHELL_PROGS` environment variable, +e.g. `make shellcheck SHELL_PROGS="dash ksh bash"`. Warnings that some +shells (e.g. `ksh93`) emit about the legacy command substitution syntax +(which NUT scripts use deliberately for portability) are hidden. Given that the `/bin/sh` implementation varies wildly on different systems (e.g. Korn shell, BASH, DASH and many others), this goal performed by CI on a large number of available platforms makes sure that the lowest-denominator syntax we use is actually understood everywhere. -NOTE: At a later time additional tests, perhaps using the `shellcheck` tool, -can be introduced into the stack. +For pull requests, the "GHA-07: ShellCheck" GitHub Actions workflow also +runs the link:https://www.shellcheck.net/[ShellCheck] tool on the shell +scripts that they change. Many existing scripts have known findings, so it +fails only for findings of "warning" or "error" level that are new compared +to the base branch. The new findings are shown as annotations on the +changed lines, and listed in the job summary. You can run the same check +locally (with `perl` and `shellcheck` installed) before posting the +changes, e.g.: + +---- +:; perl tools/shellcheck-ci.pl origin/master +---- + +The `.shellcheckrc` file in the root of NUT sources disables the checks for +deliberate portability idioms of NUT scripts (e.g. the legacy command +substitution syntax), and is also used by `shellcheck` runs in your editor +or IDE. The `make shellcheck-nde` recipe calls `tests/nut-driver-enumerator-test.sh` to self-test the `scripts/upsdrvsvcctl/nut-driver-enumerator.sh.in` against @@ -243,6 +262,13 @@ For each checked file, a `*-spellchecked` touch-file is created in respective `${builddir}`, so it is not re-checked until the source document, the custom dictionary, or the `Makefile` recipe is updated. +Unknown words are reported with the file name, line and column where they +were found (relative to the root of NUT sources), followed by the `aspell` +report line with suggestions. When running in GitHub Actions (with the +`GITHUB_ACTIONS=true` environment variable), they are also reported as +annotations for the source lines, and listed in the job summary (unless +`SPELLCHECK_ERROR_FATAL=no` is set). + The ecosystem of `Makefile.am` files includes the following useful recipes: * `spellcheck`: passively check that all used words are in some dictionary diff --git a/m4/nut_check_aspell.m4 b/m4/nut_check_aspell.m4 index 2720ea6fac..14cb2881a9 100644 --- a/m4/nut_check_aspell.m4 +++ b/m4/nut_check_aspell.m4 @@ -35,6 +35,8 @@ AC_DEFUN([NUT_CHECK_ASPELL], AC_MSG_RESULT([${ASPELL_VERSION}]) ASPELL_VERSION_MINMAJ="`echo \"${ASPELL_VERSION}\" | sed 's,\.@<:@0-9@:>@@<:@0-9@:>@*$,,'`" + dnl # Resources are often in e.g. "aspell-0.60" even for a 0.60.6.1 + ASPELL_VERSION_MAJMIN="`echo \"${ASPELL_VERSION}\" | sed 's,^\(@<:@0-9@:>@*\.@<:@0-9@:>@*\).*$,\1,'`" dnl FIXME: Some systems have more complicated layouts, e.g. dnl /usr/lib/amd64/aspell-0.60/tex-filter.so @@ -45,42 +47,27 @@ AC_DEFUN([NUT_CHECK_ASPELL], dnl failed. This may need some more-direct addressing later. AC_MSG_CHECKING([for aspell "lib" filtering resources directory]) ASPELL_BINDIR="`dirname \"$ASPELL\"`" - AS_IF([test -d "${ASPELL_BINDIR}/../lib"], [ - AS_IF([test x"${ASPELL_VERSION}" != x"none" && test -d "${ASPELL_BINDIR}/../lib/aspell-${ASPELL_VERSION}"], [ - ASPELL_FILTER_LIB_PATH="`cd \"${ASPELL_BINDIR}/../lib/aspell-${ASPELL_VERSION}\" && pwd`" \ - || ASPELL_FILTER_LIB_PATH="${ASPELL_BINDIR}/../lib/aspell-${ASPELL_VERSION}" - ], [ - AS_IF([test x"${ASPELL_VERSION_MINMAJ}" != x"none" && test -d "${ASPELL_BINDIR}/../lib/aspell-${ASPELL_VERSION_MINMAJ}"], [ - ASPELL_FILTER_LIB_PATH="`cd \"${ASPELL_BINDIR}/../lib/aspell-${ASPELL_VERSION_MINMAJ}\" && pwd`" \ - || ASPELL_FILTER_LIB_PATH="${ASPELL_BINDIR}/../lib/aspell-${ASPELL_VERSION_MINMAJ}" - ], [ - AS_IF([test -d "${ASPELL_BINDIR}/../lib/aspell"], [ - ASPELL_FILTER_LIB_PATH="`cd \"${ASPELL_BINDIR}/../lib/aspell\" && pwd`" \ - || ASPELL_FILTER_LIB_PATH="${ASPELL_BINDIR}/../lib/aspell" - ]) + dnl # Some platforms (e.g. CentOS 7) keep the resources in "lib64" + for ASPELL_RESDIR in lib lib64 ; do + for ASPELL_RESSUBDIR in "aspell-${ASPELL_VERSION}" "aspell-${ASPELL_VERSION_MINMAJ}" "aspell-${ASPELL_VERSION_MAJMIN}" "aspell" ; do + AS_IF([test x"${ASPELL_FILTER_LIB_PATH}" = x"none" && test -d "${ASPELL_BINDIR}/../${ASPELL_RESDIR}/${ASPELL_RESSUBDIR}"], [ + ASPELL_FILTER_LIB_PATH="`cd \"${ASPELL_BINDIR}/../${ASPELL_RESDIR}/${ASPELL_RESSUBDIR}\" && pwd`" \ + || ASPELL_FILTER_LIB_PATH="${ASPELL_BINDIR}/../${ASPELL_RESDIR}/${ASPELL_RESSUBDIR}" ]) - ]) - ]) + done + done AC_MSG_RESULT([${ASPELL_FILTER_LIB_PATH}]) AC_MSG_CHECKING([for aspell "share" filtering resources directory]) ASPELL_BINDIR="`dirname \"$ASPELL\"`" - AS_IF([test -d "${ASPELL_BINDIR}/../share"], [ - AS_IF([test x"${ASPELL_VERSION}" != x"none" && test -d "${ASPELL_BINDIR}/../share/aspell-${ASPELL_VERSION}"], [ - ASPELL_FILTER_SHARE_PATH="`cd \"${ASPELL_BINDIR}/../share/aspell-${ASPELL_VERSION}\" && pwd`" \ - || ASPELL_FILTER_SHARE_PATH="${ASPELL_BINDIR}/../share/aspell-${ASPELL_VERSION}" - ], [ - AS_IF([test x"${ASPELL_VERSION_MINMAJ}" != x"none" && test -d "${ASPELL_BINDIR}/../share/aspell-${ASPELL_VERSION_MINMAJ}"], [ - ASPELL_FILTER_SHARE_PATH="`cd \"${ASPELL_BINDIR}/../share/aspell-${ASPELL_VERSION_MINMAJ}\" && pwd`" \ - || ASPELL_FILTER_SHARE_PATH="${ASPELL_BINDIR}/../share/aspell-${ASPELL_VERSION_MINMAJ}" - ], [ - AS_IF([test -d "${ASPELL_BINDIR}/../share/aspell"], [ - ASPELL_FILTER_SHARE_PATH="`cd \"${ASPELL_BINDIR}/../share/aspell\" && pwd`" \ - || ASPELL_FILTER_SHARE_PATH="${ASPELL_BINDIR}/../share/aspell" - ]) + for ASPELL_RESDIR in share ; do + for ASPELL_RESSUBDIR in "aspell-${ASPELL_VERSION}" "aspell-${ASPELL_VERSION_MINMAJ}" "aspell-${ASPELL_VERSION_MAJMIN}" "aspell" ; do + AS_IF([test x"${ASPELL_FILTER_SHARE_PATH}" = x"none" && test -d "${ASPELL_BINDIR}/../${ASPELL_RESDIR}/${ASPELL_RESSUBDIR}"], [ + ASPELL_FILTER_SHARE_PATH="`cd \"${ASPELL_BINDIR}/../${ASPELL_RESDIR}/${ASPELL_RESSUBDIR}\" && pwd`" \ + || ASPELL_FILTER_SHARE_PATH="${ASPELL_BINDIR}/../${ASPELL_RESDIR}/${ASPELL_RESSUBDIR}" ]) - ]) - ]) + done + done AC_MSG_RESULT([${ASPELL_FILTER_SHARE_PATH}]) AC_MSG_CHECKING([for aspell "tex" filtering resources directory]) diff --git a/tools/Makefile.am b/tools/Makefile.am index c6ff505852..b142b73a1e 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -27,7 +27,7 @@ PYTHON_DEFAULT = @PYTHON_DEFAULT@ EXTRA_DIST = nut-usbinfo.pl nut-recorder.sh nut-ddl-dump.sh nut-dumpdiff.sh \ gitlog2changelog.py.in gitlog2version.sh semver-compare.sh \ nut-snmpinfo.py.in driver-list-format.sh check-source-nonascii.pl \ - asciidoc-github-links.pl + asciidoc-github-links.pl shellcheck-ci.pl # These files are generated for nut-scanner builds (and cleaned as any others), # and can change as respective SNMP/USB subdriver sources are iterated by a diff --git a/tools/shellcheck-ci.pl b/tools/shellcheck-ci.pl new file mode 100644 index 0000000000..29321b1730 --- /dev/null +++ b/tools/shellcheck-ci.pl @@ -0,0 +1,321 @@ +#!/usr/bin/env perl + +# Report ShellCheck findings that a change adds to NUT shell scripts. +# +# Many existing scripts have known ShellCheck findings, so checking the +# whole tree would always fail. Instead, compare the shell scripts that +# changed since a base Git revision: run shellcheck on their base and +# current versions, and report only the findings that are new. +# +# Findings are matched by file, ShellCheck code and the text of the +# reported source line (not its number), so code moving around within +# a file does not count as new findings. +# +# Usage (from anywhere in the Git workspace): +# +# perl tools/shellcheck-ci.pl [BASE_REF] +# +# BASE_REF defaults to "origin/master". The current versions are taken +# from the working tree, so uncommitted changes are checked too. Both +# versions are checked with the current .shellcheckrc file. +# +# Exit code is 1 if new "error" or "warning" level findings were found, +# 2 for usage or tool problems, else 0. New "note" level findings (info +# and style in ShellCheck terms) are reported, but do not fail the check. +# +# In GitHub Actions (GITHUB_ACTIONS=true) findings are printed as workflow +# commands, so they show as annotations on the changed lines, and listed +# in the job summary (GITHUB_STEP_SUMMARY), if available. +# +# Only core Perl 5 modules are used, and the "gcc" output format of +# shellcheck is parsed, to not require a JSON module on older systems. +# +# Copyright (C) 2026 by nmbro +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. + +use strict; +use warnings; +use Cwd qw(getcwd); +use File::Basename qw(dirname); +use File::Path qw(mkpath); +use File::Temp qw(tempdir); + +my $base = 'origin/master'; +if (@ARGV > 1 || (@ARGV == 1 && $ARGV[0] =~ /^-/)) { + print STDERR "Usage: $0 [BASE_REF]\n"; + exit 2; +} +$base = $ARGV[0] if (@ARGV == 1); + +my $gha = (defined($ENV{GITHUB_ACTIONS}) && $ENV{GITHUB_ACTIONS} eq 'true'); + +# Run a command (list form, no shell), return its stdout and exit code +sub run_cmd { + my @cmd = @_; + my $out = ''; + my $pid = open(my $fh, '-|'); + if (!defined($pid)) { + die "ERROR: can not fork for: @cmd\n"; + } + if ($pid == 0) { + exec { $cmd[0] } @cmd or exit 127; + } + binmode($fh); + { local $/; $out = <$fh>; } + close($fh); + $out = '' unless defined($out); + return ($out, $? >> 8); +} + +sub have_tool { + my $tool = shift; + foreach my $dir (split(/:/, $ENV{PATH} || '')) { + return 1 if ($dir ne '' && -x "$dir/$tool"); + } + return 0; +} + +sub is_shell_script { + # Same detection as `make check-scripts-syntax` (see Makefile.am) + my ($out, $rc) = run_cmd('file', '-b', shift); + return ($rc == 0 && $out =~ /shell script/); +} + +# Return { current path => base path or undef } for added/changed files +sub changed_files { + my ($out, $rc) = run_cmd('git', 'diff', '--name-status', '-z', '-M', + '--diff-filter=AMR', $base, '--'); + die "ERROR: git diff failed\n" if ($rc != 0); + my @fields = split(/\0/, $out); + my %files; + while (@fields) { + my $status = shift(@fields); + last if ($status eq ''); + if ($status =~ /^R/) { + my $old = shift(@fields); + my $new = shift(@fields); + $files{$new} = $old; + } else { + my $path = shift(@fields); + $files{$path} = ($status eq 'A') ? undef : $path; + } + } + return %files; +} + +# Run shellcheck in a directory, return a list of finding hashes +sub run_shellcheck { + my ($dir, @paths) = @_; + return () unless (@paths); + my ($out, $rc); + { + my $cwd = getcwd(); + chdir($dir) or die "ERROR: can not chdir to $dir\n"; + ($out, $rc) = run_cmd('shellcheck', '-f', 'gcc', '--', @paths); + chdir($cwd) or die "ERROR: can not chdir back to $cwd\n"; + } + # 0 = no findings, 1 = findings, anything else = could not check + if ($rc != 0 && $rc != 1) { + print STDERR "ERROR: shellcheck failed with exit code $rc in $dir\n"; + exit 2; + } + my @findings; + foreach my $line (split(/\n/, $out)) { + if ($line =~ /^(.+?):(\d+):(\d+): (\w+): (.*) \[SC(\d+)\]$/) { + push(@findings, { + file => $1, line => $2, column => $3, + level => $4, message => $5, code => $6, + }); + } + } + return @findings; +} + +my %line_cache; +sub finding_key { + my ($dir, $f) = @_; + my $path = "$dir/$f->{file}"; + if (!exists($line_cache{$path})) { + my @lines; + if (open(my $fh, '<', $path)) { + binmode($fh); + @lines = <$fh>; + close($fh); + } + $line_cache{$path} = \@lines; + } + my $text = $line_cache{$path}->[$f->{line} - 1]; + $text = '' unless defined($text); + $text =~ s/^\s+//; + $text =~ s/\s+$//; + return join("\0", $f->{file}, $f->{code}, $text); +} + +sub escape_data { + my $s = shift; + $s =~ s/%/%25/g; + $s =~ s/\r/%0D/g; + $s =~ s/\n/%0A/g; + return $s; +} + +sub escape_property { + my $s = escape_data(shift); + $s =~ s/:/%3A/g; + $s =~ s/,/%2C/g; + return $s; +} + +sub escape_markdown { + my $s = shift; + $s =~ s/([\\`*_\[\]<>|])/\\$1/g; + return $s; +} + +sub report { + my $f = shift; + my $code = "SC$f->{code}"; + if ($gha) { + my $cmd = ($f->{level} eq 'error' || $f->{level} eq 'warning') + ? $f->{level} : 'notice'; + printf("::%s file=%s,line=%d,col=%d,title=%s::%s\n", + $cmd, escape_property($f->{file}), $f->{line}, $f->{column}, + escape_property("ShellCheck $code ($f->{level})"), + escape_data("$f->{message} See https://www.shellcheck.net/wiki/$code")); + } else { + printf("%s:%d:%d: %s: %s [%s]\n", $f->{file}, $f->{line}, + $f->{column}, $f->{level}, $f->{message}, $code); + } +} + +sub write_summary { + my $text = shift; + my $path = $ENV{GITHUB_STEP_SUMMARY}; + return unless ($gha && defined($path) && $path ne ''); + if (open(my $fh, '>>', $path)) { + print $fh $text; + close($fh); + } +} + +sub summary_table { + my @findings = @_; + # Link the locations to the checked out (PR merge) commit, if known + my $blob; + if ($ENV{GITHUB_SERVER_URL} && $ENV{GITHUB_REPOSITORY} && $ENV{GITHUB_SHA}) { + $blob = "$ENV{GITHUB_SERVER_URL}/$ENV{GITHUB_REPOSITORY}/blob/$ENV{GITHUB_SHA}/"; + } + my $text = sprintf("### ShellCheck: %d new finding(s) in changed shell scripts\n\n", + scalar(@findings)); + $text .= "| Location | Level | Code | Message |\n|---|---|---|---|\n"; + foreach my $f (@findings) { + my $loc = escape_markdown("$f->{file}:$f->{line}:$f->{column}"); + $loc = "[$loc]($blob$f->{file}#L$f->{line})" if (defined($blob)); + my $code = "SC$f->{code}"; + $text .= "| $loc | $f->{level} | [$code](https://www.shellcheck.net/wiki/$code) | " + . escape_markdown($f->{message}) . " |\n"; + } + $text .= "\nOnly new \"warning\" and \"error\" level findings fail the check." + . " Run `perl tools/shellcheck-ci.pl` locally to check before pushing" + . " changes.\n\n"; + return $text; +} + +my ($topdir, $rc) = run_cmd('git', 'rev-parse', '--show-toplevel'); +if ($rc != 0) { + print STDERR "ERROR: not in a Git workspace\n"; + exit 2; +} +chomp($topdir); +chdir($topdir) or die "ERROR: can not chdir to $topdir\n"; + +foreach my $tool ('git', 'shellcheck', 'file') { + if (!have_tool($tool)) { + print STDERR "ERROR: $tool is not installed\n"; + exit 2; + } +} + +(undef, $rc) = run_cmd('git', 'rev-parse', '--verify', '--quiet', "$base^{commit}"); +if ($rc != 0) { + print STDERR "ERROR: unknown base revision: $base\n"; + exit 2; +} + +my %all = changed_files(); +my %files; +foreach my $new (keys %all) { + $files{$new} = $all{$new} if (-f $new && is_shell_script($new)); +} +my @paths = sort(keys %files); +if (!@paths) { + print "No changed shell scripts since $base\n"; + write_summary("### ShellCheck: no changed shell scripts\n\n"); + exit 0; +} +printf("Checking %d changed shell script(s) since %s: %s\n", + scalar(@paths), $base, join(' ', @paths)); + +my @head = run_shellcheck($topdir, @paths); + +# Base versions go under their current names into a scratch tree, +# with the current settings file, so the findings are comparable +my %base_counts; +{ + my $basedir = tempdir('shellcheck-ci-XXXXXX', TMPDIR => 1, CLEANUP => 1); + if (-f '.shellcheckrc') { + open(my $in, '<', '.shellcheckrc') or die "ERROR: .shellcheckrc: $!\n"; + open(my $out, '>', "$basedir/.shellcheckrc") or die "ERROR: $!\n"; + print $out $_ while (<$in>); + close($in); + close($out); + } + my @base_paths; + foreach my $new (@paths) { + my $old = $files{$new}; + next unless (defined($old)); + my ($content, $grc) = run_cmd('git', 'show', "$base:$old"); + die "ERROR: git show $base:$old failed\n" if ($grc != 0); + mkpath(dirname("$basedir/$new")); + open(my $fh, '>', "$basedir/$new") or die "ERROR: $!\n"; + binmode($fh); + print $fh $content; + close($fh); + push(@base_paths, $new); + } + foreach my $f (run_shellcheck($basedir, @base_paths)) { + $base_counts{finding_key($basedir, $f)}++; + } +} + +my @new_findings; +foreach my $f (@head) { + my $key = finding_key($topdir, $f); + if ($base_counts{$key}) { + $base_counts{$key}--; + } else { + push(@new_findings, $f); + } +} + +report($_) foreach (@new_findings); + +my %levels; +$levels{$_->{level}}++ foreach (@new_findings); +my $summary = join(', ', map { "$levels{$_} $_" } + grep { $levels{$_} } ('error', 'warning', 'note')); +my $failed = ($levels{error} || $levels{warning}) ? 1 : 0; +printf("%s new ShellCheck finding(s) since %s: %s\n", + ($failed ? 'FAILED:' : 'OK:'), $base, ($summary ne '' ? $summary : 'none')); + +if (@new_findings) { + write_summary(summary_table(@new_findings)); +} else { + write_summary(sprintf("### ShellCheck: no new findings in %d changed shell script(s)\n\n", + scalar(@paths))); +} +exit($failed);