diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 929afad..75d6f5a 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -34,3 +34,19 @@ jobs: command -v xxd command -v bc - run: make -C tests test + # This job tests `cronic` under macOS's /bin/bash, which is bash 3.2, and + # macOS's BSD utilities. `cronic` and the bash scripts in its tests run + # under the first `bash` in PATH, so /bin is put first, ahead of any + # Homebrew bash. + cronic-macos: + runs-on: macos-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@v7 + with: + fetch-depth: 1 + show-progress: false + persist-credentials: false + - run: echo /bin >> "$GITHUB_PATH" + - run: bash --version + - run: make -C tests/cronic-test test diff --git a/cronic b/cronic index 7666461..49aba89 100755 --- a/cronic +++ b/cronic @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash # Cronic v2 - cron job report wrapper # Copyright 2007 Chuck Houpt @@ -14,8 +14,10 @@ # --duplicate-stderr options, which may be given in any order. # The options and the treatment of execution traces are documented in function # `cronic_help`; run `cronic --help`. -# This script runs under bash 3.2, which is /bin/bash on macOS, so it uses a -# feature of a later bash only after checking the version. +# This script runs under the first `bash` in PATH, which on macOS may be +# /bin/bash, bash 3.2. So it uses a feature of a later bash, such as +# BASH_XTRACEFD (bash 4.1), only after checking the version; see the +# BASH_VERSINFO check below. # Exit if a command exits with a non-zero status, and treat unset variables # as an error. @@ -174,6 +176,15 @@ set -e # Unsetting BASH_XTRACEFD also closes its file descriptor. unset BASH_XTRACEFD +# End stderr with a newline, if it is not empty. `grep` ends its output with a +# newline, so without this, the filtered stderr would differ from the +# unfiltered stderr even when `grep` removed no line, and the report would +# show the same output twice. The newline also keeps the last line of stderr +# off the line of the report's next heading. +if [ -s "$cronic_trace" ] && [ "$(tail -c 1 "$cronic_trace" | wc -l)" -eq 0 ]; then + echo >> "$cronic_trace" +fi + # Prints a basic regular expression that matches its argument literally. # The argument must not contain a backslash. cronic_bre_quote() { @@ -198,16 +209,26 @@ cronic_trace_pattern() { local LC_ALL=C [[ $PS4 =~ ^([$'\xc0'-$'\xff'][$'\x80'-$'\xbf']*|.) ]] local first=${BASH_REMATCH[0]} - printf '^\\(%s\\)\\+%s\n' "$(cronic_bre_quote "$first")" "$(cronic_bre_quote "${PS4#"$first"}")" + local rest=${PS4#"$first"} + # `\{1,\}` rather than GNU's `\+`, which is not a POSIX basic regular + # expression. + printf '^\\(%s\\)\\{1,\\}%s\n' "$(cronic_bre_quote "$first")" "$(cronic_bre_quote "$rest")" } +# Each `grep` that reads the wrapped command's output uses `-a`, so that it +# treats the output as text even if it contains a NUL byte or a byte that is +# invalid in the locale. Without `-a`, `grep` would treat such output as +# binary and print only a "Binary file ... matches" notice in place of the +# lines it selects, so that a real error could go unreported. cronic_pattern=$(cronic_trace_pattern) -if [ -n "$cronic_pattern" ] && grep -q "$cronic_pattern" "$cronic_trace"; then - # `|| true` because `grep -v` exits with status 1 when it selects no lines, - # which happens whenever stderr consists only of trace lines. Without it, - # `set -e` would abort the script here, before it produces its report, and - # it would exit with status 1 rather than with the wrapped command's status. - grep -v "$cronic_pattern" "$cronic_trace" > "$cronic_err" || true +if [ -n "$cronic_pattern" ]; then + # `|| [ $? -eq 1 ]` because `grep -v` exits with status 1 when it selects no + # lines, which happens whenever stderr consists only of trace lines. Without + # it, `set -e` would abort the script here, before it produces its report, + # and it would exit with status 1 rather than with the wrapped command's + # status. If `grep` fails (status 2), the unfiltered stderr is used, rather + # than an empty or truncated file cronic_err that could hide an error. + grep -a -v "$cronic_pattern" "$cronic_trace" > "$cronic_err" || [ $? -eq 1 ] || cronic_err=$cronic_trace else cronic_err=$cronic_trace fi @@ -216,11 +237,17 @@ fi # For example, git does this, and the POSIX standard requires it for make. # The full stderr output is shown, but file cronic_reduced_err is checked for # emptiness below. +# GNU make names itself by the name it was invoked as, without the directory, +# such as `make`, `gmake`, `gnumake` (on macOS), or `mingw32-make`. It quotes +# the directory as 'dir' from version 4.0, and as `dir' before then; macOS's +# /usr/bin/make is GNU make 3.81. # TODO: Expand this. -cat "$cronic_err" \ - | grep -v "^Everything up-to-date$" \ - | grep -v '^make\(\[[0-9]\+\]\)\?: \(Entering\|Leaving\) directory '"'"'.*'"'"'$' \ - > "$cronic_reduced_err" || true +# `|| [ $? -eq 1 ]` because `grep -v` exits with status 1 when it selects no +# lines. If `grep` fails (status 2), the unreduced stderr is used. +grep -a -E -v \ + -e '^Everything up-to-date$' \ + -e '^[[:alnum:]_.-]*make(\[[0-9]+\])?: (Entering|Leaving) directory '"[\`']"'.*'"'"'$' \ + "$cronic_err" > "$cronic_reduced_err" || [ $? -eq 1 ] || cronic_reduced_err=$cronic_err if [ $cronic_result -ne "$cronic_expected" ] || { [ $cronic_permitstderr -eq 0 ] && [ -s "$cronic_reduced_err" ]; }; then echo "Cronic detected failure or error output for the command:" @@ -240,7 +267,7 @@ if [ $cronic_result -ne "$cronic_expected" ] || { [ $cronic_permitstderr -eq 0 ] cat "$cronic_err" echo if [ "$cronic_debug" != "true" ]; then - if ! cmp --silent "$cronic_err" "$cronic_reduced_err"; then + if ! cmp -s "$cronic_err" "$cronic_reduced_err"; then echo "REDUCED ERROR OUTPUT:" cat "$cronic_reduced_err" echo @@ -248,7 +275,7 @@ if [ $cronic_result -ne "$cronic_expected" ] || { [ $cronic_permitstderr -eq 0 ] fi echo "STANDARD OUTPUT:" cat "$cronic_out" - if [ "$cronic_trace" != "$cronic_err" ]; then + if ! cmp -s "$cronic_trace" "$cronic_err"; then echo echo "TRACE-ERROR OUTPUT:" cat "$cronic_trace" diff --git a/tests/cronic-test/cronic-test.sh b/tests/cronic-test/cronic-test.sh index 874bf76..b17d3b5 100755 --- a/tests/cronic-test/cronic-test.sh +++ b/tests/cronic-test/cronic-test.sh @@ -10,6 +10,13 @@ # printed, and the exit status would be grep's 1 rather than the wrapped # command's. (The temporary files would still be removed, by `cronic`'s EXIT # trap, so the left-behind-files check in `check` does not detect that bug.) +# The checks that catch that bug are the "trace-only stderr" checks that use +# `stderr-lines` with a literal PS4, and the "nested trace in stderr" check, +# whose bash script writes its trace lines to stderr whatever the bash version. +# All of them run only when the test is not run as root. The checks that use +# `trace-only`, a bash script, catch it only under a bash earlier than 4.1: a +# later bash writes its trace lines to BASH_XTRACEFD, so `cronic` sees no trace +# lines in stderr and does not run `grep -v`. set -eu @@ -30,7 +37,7 @@ status=0 # A command whose stderr is nothing but trace lines. cat > "$work/trace-only" << 'EOF' -#!/bin/bash +#!/usr/bin/env bash set -x echo "the standard output" exit "$1" @@ -39,7 +46,7 @@ chmod +x "$work/trace-only" # A command that writes real error output in addition to trace lines. cat > "$work/trace-and-stderr" << 'EOF' -#!/bin/bash +#!/usr/bin/env bash set -x echo "a real error" 1>&2 exit "$1" @@ -47,12 +54,22 @@ EOF chmod +x "$work/trace-and-stderr" # A command whose stderr is nothing but `make` directory-change notices. Both -# the top-level form (`make:`) and the recursive form (`make[1]:`) appear; both -# must be filtered out of the reduced error output. +# the top-level form (`make:`) and the recursive form (`make[1]:`) appear, as do +# the quoting of GNU make 3.81 (macOS's /usr/bin/make) and the other names +# that GNU make runs under; all must be filtered out of the reduced error +# output. cat > "$work/make-noise" << 'EOF' #!/bin/sh echo "make: Entering directory '/tmp/x'" 1>&2 echo "make[1]: Entering directory '/tmp/x/sub'" 1>&2 +echo 'make[2]: Entering directory `/tmp/x/sub/old'"'" 1>&2 +echo 'make[2]: Leaving directory `/tmp/x/sub/old'"'" 1>&2 +echo "gmake[2]: Entering directory '/tmp/x/sub/g'" 1>&2 +echo "gmake[2]: Leaving directory '/tmp/x/sub/g'" 1>&2 +echo "gnumake[2]: Entering directory '/tmp/x/sub/gnu'" 1>&2 +echo "gnumake[2]: Leaving directory '/tmp/x/sub/gnu'" 1>&2 +echo "mingw32-make[2]: Entering directory '/tmp/x/sub/mingw'" 1>&2 +echo "mingw32-make[2]: Leaving directory '/tmp/x/sub/mingw'" 1>&2 echo "make[1]: Leaving directory '/tmp/x/sub'" 1>&2 echo "make: Leaving directory '/tmp/x'" 1>&2 exit "$1" @@ -71,6 +88,28 @@ exit "$1" EOF chmod +x "$work/make-noise-and-stderr" +# A command that writes a trace line and then an error line that contains a +# NUL byte and a byte that is invalid in UTF-8, so that `grep` would treat the +# output as binary. +cat > "$work/binary-stderr" << 'EOF' +#!/bin/sh +printf '[trace] echo hi\n' 1>&2 +printf 'bad \377\000 error\n' 1>&2 +EOF +chmod +x "$work/binary-stderr" + +# A bash command whose trace lines go to stderr, as under a bash earlier than +# 4.1, whatever the bash version. Its trace lines are nested 3 deep. +cat > "$work/nested-trace" << 'EOF' +#!/usr/bin/env bash +unset BASH_XTRACEFD +set -x +outer=$(inner=$(echo deepest); echo "$inner") +echo "$outer" +exit "$1" +EOF +chmod +x "$work/nested-trace" + # A command that writes, to stderr, lines that start with `+` but are not trace # lines. cat > "$work/plus-stderr" << 'EOF' @@ -96,9 +135,10 @@ temp_files() { # check DESCRIPTION EXPECTED-STATUS EXPECTED-OUTPUT COMMAND...: runs `cronic` # on COMMAND and checks its exit status, whether it printed a report, and that # it left no temporary files behind. EXPECTED-OUTPUT is "silent", "report", -# "report:TEXT", which also requires TEXT to appear in the report, or -# "message:TEXT", which requires TEXT to appear in output that need not be a -# report. +# "report:TEXT", which also requires TEXT to appear in the report, +# "report-once:TEXT", which also requires TEXT to appear on exactly one line of +# the report, or "message:TEXT", which requires TEXT to appear in output that +# need not be a report. check() { description="$1" expected_status="$2" @@ -136,6 +176,15 @@ check() { ;; esac case "$expected_output" in + report-once:*) + if [ "$(grep -c -e "${expected_output#*:}" "$work/output")" -ne 1 ]; then + echo "FAIL: $description: expected exactly one line of the output to" \ + "contain \"${expected_output#*:}\", but got:" + cat "$work/output" + status=1 + return + fi + ;; report:* | message:*) if ! grep -q -e "${expected_output#*:}" "$work/output"; then echo "FAIL: $description: expected the output to contain" \ @@ -183,6 +232,20 @@ check "make directory-change notices, exit 0" 0 silent "$work/make-noise" 0 check "make directory-change notices and real stderr, exit 0" 0 \ "report:^a real error$" "$work/make-noise-and-stderr" 0 +# Error output that does not end with a newline is shown once, on a line of +# its own, both when trace lines are filtered out of it and when they are not. +check "stderr without a final newline" 0 "report-once:^oops$" \ + sh -c 'printf oops >&2' +# shellcheck disable=SC2016 # The expansion is for the traced command to do. +PS4='+$X ' +export PS4 +check "stderr without a final newline, unfiltered" 0 "report-once:^oops$" \ + sh -c 'printf oops >&2' +unset PS4 + +# Error output that `grep` would treat as binary is still reported. +check "binary stderr" 0 "report:error$" "$work/binary-stderr" + # Without a command, `cronic` reports a usage error rather than aborting on an # unset variable. check "no arguments" 64 "message:^Usage: " @@ -306,42 +369,80 @@ check "caller's exported variables" 0 silent "$work/check-variables" $exported_n # shellcheck disable=SC2086 # each name is a separate argument. unset $exported_names -# Bash writes its trace lines to a separate file, so they are not error -# output, whatever PS4 is. +# A bash that supports BASH_XTRACEFD and the allocation of a file descriptor +# (bash 4.1 or later) writes its trace lines to a separate file, so they are +# not error output, whatever PS4 is. An earlier bash, such as macOS's +# /bin/bash, writes them to stderr, where `cronic` cannot recognize them when +# PS4 contains an expansion or an escape, so it reports them as error output. +# This tests for the features themselves, rather than for the bash version +# that `cronic` tests for, so that it detects a wrong version test in `cronic`. +# `cronic` and the bash scripts above all run under the first `bash` in PATH, +# so that is the bash tested here. The probe checks whether a trace line +# reaches the file, rather than whether bash writes to stderr, because bash can +# write an unrelated warning to stderr, such as for an uninstalled locale. +if bash -c 'exec {fd}> "$1" && BASH_XTRACEFD=$fd && set -x && :' \ + bash "$work/xtracefd-probe" 2> /dev/null && [ -s "$work/xtracefd-probe" ]; then + bash_has_xtracefd=true +else + bash_has_xtracefd=false +fi export PS4 # shellcheck disable=SC2016 # The expansion is for the traced command to do. PS4='+${LINENO}: ' -check "bash, PS4 with expansions, trace-only stderr" 0 silent \ - "$work/trace-only" 0 -check "bash, PS4 with expansions, trace in the report" 3 \ - "report:echo 'the standard output'$" "$work/trace-only" 3 +# The real error is reported, whether or not the trace lines are too. check "bash, PS4 with expansions, trace lines and real stderr" 0 \ "report:^a real error$" "$work/trace-and-stderr" 0 -PS4='+\D{%H}: ' -check "bash, PS4 with an escape, trace-only stderr" 0 silent \ - "$work/trace-only" 0 +check "bash, PS4 with expansions, trace in the report" 3 \ + "report:echo 'the standard output'$" "$work/trace-only" 3 +if [ "$bash_has_xtracefd" = true ]; then + check "bash, PS4 with expansions, trace-only stderr" 0 silent \ + "$work/trace-only" 0 + PS4='+\D{%H}: ' + check "bash, PS4 with an escape, trace-only stderr" 0 silent \ + "$work/trace-only" 0 +fi PS4='[trace] ' check "bash, PS4 with metacharacters, trace-only stderr" 0 silent \ "$work/trace-only" 0 -# The tests below concern trace lines that a shell other than bash writes to -# stderr; they are recognized by PS4. Bash running as root does not import -# PS4 from the environment, so `cronic` would see the default PS4, and these -# tests are skipped. +# The tests below concern trace lines in stderr, as written by a shell other +# than bash or by a bash that does not use BASH_XTRACEFD; they are recognized +# by PS4. Bash running as root does not import PS4 from the environment, so +# `cronic` would see the default PS4, and these tests are skipped. if [ "$(id -u)" -eq 0 ]; then echo "SKIP: tests of PS4 in stderr, because bash ignores PS4 when run as root" unset PS4 exit "$status" fi +# An earlier bash writes its trace lines to stderr, and when PS4 contains an +# expansion or an escape, they are reported. +if [ "$bash_has_xtracefd" = false ]; then + # shellcheck disable=SC2016 # The expansion is for the traced command to do. + PS4='+${LINENO}: ' + check "bash before 4.1, PS4 with expansions, trace-only stderr" 0 \ + "report:^+3: echo 'the standard output'$" "$work/trace-only" 0 + PS4='+\D{%H}: ' + check "bash before 4.1, PS4 with an escape, trace-only stderr" 0 \ + "report:^+[0-9][0-9]: echo 'the standard output'$" "$work/trace-only" 0 +fi + # Trace lines are recognized when PS4 contains regular-expression # metacharacters or quotes, and the metacharacters do not match other lines. PS4='[trace] ' check "PS4 with metacharacters, trace-only stderr" 0 silent \ - "$work/stderr-lines" "[trace] echo hi" "[[trace] echo nested" + "$work/stderr-lines" "[trace] echo hi" "[[trace] echo nested" \ + "[[[trace] echo nested more" +# The same, for the trace lines that bash writes to stderr. +check "bash, PS4 with metacharacters, nested trace in stderr" 0 silent \ + "$work/nested-trace" 0 +check "bash, PS4 with metacharacters, nested trace in the report" 3 \ + "report:^\[\[\[trace\] echo deepest$" "$work/nested-trace" 3 check "PS4 with metacharacters, trace lines and real stderr" 0 \ "report:^a real error$" \ "$work/stderr-lines" "[trace] echo hi" "a real error" +check "PS4 with metacharacters, trace lines and binary stderr" 0 \ + "report:error$" "$work/binary-stderr" PS4='+.* ' check "PS4 with metacharacters, error line that they would match" 0 \ "report:^+++ b/file$" "$work/stderr-lines" "+++ b/file" diff --git a/tests/cronic-test/temp-file-test.sh b/tests/cronic-test/temp-file-test.sh index a15f955..092677b 100755 --- a/tests/cronic-test/temp-file-test.sh +++ b/tests/cronic-test/temp-file-test.sh @@ -88,11 +88,13 @@ marker="cronic temp-file-test marker $work" # marker_files: prints the files that contain the marker. Files directly # within /tmp, or within a subdirectory of it, are searched, except for this -# test's own files. Errors, such as unreadable files, are ignored. +# test's own files. Errors, such as unreadable files, are ignored. `-H` +# because on macOS, /tmp is a symbolic link, which `find` otherwise does not +# follow. marker_files() { { find "$TMPDIR" -type f -exec grep -lF -- "$marker" {} + 2> /dev/null || true - find /tmp -maxdepth 2 -path "$work" -prune \ + find -H /tmp -maxdepth 2 -path "$work" -prune \ -o -type f -user "$(id -u)" -exec grep -lF -- "$marker" {} + \ 2> /dev/null || true } | sort -u