Repository navigation
Conversation
|
A ZIP file with standard source tarball and another tarball with pre-built docs for commit 1eac126 is temporarily available: NUT-tarballs-PR-4.zip. |
Let "make shellcheck" (check-scripts-syntax) check the POSIX shell scripts with each interpreter listed in the SHELL_PROGS envvar, like the NDE self-test already does, rather than only the system /bin/sh. Bourne-Again shell scripts are still checked with bash. The shell is named in the error message, so a failure is easy to tell apart. Also hide the "`...` obsolete, use $(...)" warnings that e.g. ksh93 (the /bin/sh on illumos and Solaris) prints in syntax-check mode for each use of backticks, which NUT scripts use deliberately for portability: on current master, "ksh -n" printed 1442 of these for 47 scripts, which buried the 15 other warnings it had. The output is captured before filtering, so the syntax check exit code is kept. The filter uses $(EGREP) with bracket expressions, rather than "grep -F", for older grep implementations. Checked with SHELL_PROGS="dash ksh bash" (ksh 93u+m/1.0.8): all scripts pass, no backticks warnings are left, the other ksh warnings are still shown; and that syntax errors in a POSIX or a Bourne-Again shell script fail the check, naming the file and the shell. Spellcheck and distcheck-light pass. Closes: networkupstools#1058 Closes: networkupstools#1059 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: nmbro <nmbro@users.noreply.github.com>
Add a "GHA-07: ShellCheck" workflow for pull requests, which runs ShellCheck on the shell scripts changed by the PR. The existing scripts have many known findings (about 2900 in the tree, 28 of them errors), so checking the whole tree would always fail. Instead, the new tools/shellcheck-ci.pl helper checks both the base and the PR versions of the changed scripts, and reports only new findings, matched by file, ShellCheck code and source line text (so moved code does not count as new). It fails for new "warning" or "error" level findings; new "note" level (info and style) ones are shown as notices. In GitHub Actions, the findings are shown as annotations on the changed lines and listed in the job summary, with links to the lines and the ShellCheck wiki. It can be run locally against any base revision, including uncommitted changes. The helper is in Perl, like tools/check-source-nonascii.pl (Perl is already needed for autogen.sh in Git workspaces). It only uses core Perl 5 modules, and parses the "gcc" output format of shellcheck, so no JSON module is needed on older systems. The workflow uses only actions/checkout (no new third-party actions) and compares the PR merge commit with its first parent. A new .shellcheckrc file disables the checks for deliberate portability idioms of NUT scripts: backticks (SC2006), x"$var" comparisons (SC2268) and expr (SC2003). It is distributed like .editorconfig, to help the tarball users contribute too. Describe both in docs/qa-guide.adoc, replacing the note that the tool could be introduced at a later time. The .shellcheckrc backticks setting does for ShellCheck what the previous commit did for "make shellcheck" (issue networkupstools#1058). Checked with the "build: keep full git version out of config.h" series (PR networkupstools#3759): its first version is reported for one new SC2166 warning ([ p -a q ]) and the final one passes. Checked renamed, added, removed and non-shell files, a bad base revision, and the workflow with actionlint and zizmor. In a test PR on a fork, deliberate findings showed as annotations on the changed lines. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: nmbro <nmbro@users.noreply.github.com>
The spellcheck recipe reported unknown words as the aspell pipe-mode result lines, prefixed by "egrep -b" byte offsets into the aspell output, so finding the place in the source document took a search. Count the input lines in the aspell output instead (it ends the results for each line with an empty line), and report each unknown word as "file:line:col: <aspell result line>", with the file name relative to the root of NUT sources. The column offset aspell gives already matches the original line, as the recipe prefixes each line with a space. When running in GitHub Actions (GITHUB_ACTIONS=true), also print the errors as workflow commands, so they show as annotations on the source lines, with a few suggestions. The top-level spellcheck recipe collects them into a log file, reported once at the end, because the sequential retry after a failed parallel attempt reports the same errors again, and GitHub limits the count of annotations per step. Likewise, the summary re-run in ci_build.sh does not repeat them. No annotations are made with SPELLCHECK_ERROR_FATAL=no, as used by the distcheck steps after the spelling was checked by another step. Also list the unknown words in the job summary (GITHUB_STEP_SUMMARY), with links to the source lines and a hint on updating docs/nut.dict. The ci_build.sh quiet first attempt (whose output is discarded) does not make annotations nor the summary either, so only its verbose attempt reports them. The awk program avoids ternary operators and hash characters, for older awk and make implementations. Checked with typos in NEWS.adoc, docs/man/upsc.txt and scripts/upsdrvsvcctl/README.adoc: correct file, line and column, each annotation and summary table row printed once (also via BUILD_TYPE=default-spellcheck ci_build.sh), none with SPELLCHECK_ERROR_FATAL=no or without GITHUB_ACTIONS. A clean spellcheck passes, and distcheck-light passes with GITHUB_ACTIONS=true and a typo (as the dist worker runs it). In a test PR on a fork, typos showed as annotations on the changed lines, also in the old aspell-en 7.1 job. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: nmbro <nmbro@users.noreply.github.com>
NUT_CHECK_ASPELL looked for the filter resources (to pass the location of "tex.amf" via --filter-path) only under "../lib" and "../share" from the aspell binary, in "aspell-<version>", "aspell-<version minus the last component>" or "aspell" subdirectories. On CentOS 7 the resources are in /usr/lib64/aspell-0.60 for aspell 0.60.6.1, so neither the "lib64" base nor the "aspell-0.60" subdirectory was tried, and the check fell back to the aspell built-in paths. Also try "../lib64" (after "../lib"), and the major.minor version subdirectory (before the plain "aspell" one), in loops rather than nested conditions. The order of the other candidates is not changed. Checked with configure: on Ubuntu with aspell 0.60.8.1 the detected paths are unchanged; with a fake CentOS 7 layout (aspell reporting 0.60.6.1, an empty "lib", resources in "lib64/aspell-0.60") the lib and tex paths are found where master found "none", and make spellcheck passes using that --filter-path. The other part of the issue, to enable the spellcheck only if aspell actually works, was done by the live check from commit e519320: with a missing "en" dictionary (via ASPELL_CONF dict-dir), configure reports "no" for "if we have all the tools mandatory for documentation spell checks". Closes: networkupstools#2068 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: nmbro <nmbro@users.noreply.github.com>
Do not merge: checks that the CI annotations show up. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: nmbro <nmbro@users.noreply.github.com>
nmbro
force-pushed
the
ci-script-doc-checks-test
branch
from
October 15, 2026 16:11
5acf8be to
1eac126
Compare
Owner
Author
|
Test PR done: annotations and job summaries verified; real PR is networkupstools#3773. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fork-internal test PR, do not merge. Checks that the new GHA-07 ShellCheck workflow and the spellcheck annotations (GHA-01 dist worker) report the deliberate findings in the last commit as annotations.
Real changes: branch
ci-script-doc-checks.🤖 Generated with Claude Code