diff --git a/cronic b/cronic index 1bb3849..3187109 100755 --- a/cronic +++ b/cronic @@ -19,14 +19,22 @@ # BASH_XTRACEFD (bash 4.1), only after checking the version; see the # BASH_VERSINFO check below. -# The caller's shell options that this script changes, as the letters that `$-` -# uses for them, which this script restores for the command. They include any -# that the caller passed by exporting SHELLOPTS. This script turns xtrace off, -# so that its own commands are not traced to stderr, which would make every run -# produce output; the redirection discards the trace of `set +x` itself. It -# turns allexport off, so that its own variables are not exported to the -# command; `cronic_caller_flags`, which is assigned before that, is un-exported -# explicitly. +# Limitations, which `cronic --help` documents: every run produces output if +# the caller exports the verbose option or runs `bash -v cronic`, or if the file +# named by BASH_ENV writes to stderr. + +# Save the caller's settings of the options that this script changes, as the +# letters that `$-` uses for them, so that the command runs with them. The +# command inherits every other option, including every `shopt` option, +# unchanged. If this script ever changes another option, add its letter here. +# Then turn off xtrace, so that this script does not trace itself to stderr, +# which would make every run produce output, and allexport, so that this script +# does not export its variables to the command. `cronic_caller_flags`, which is +# assigned before that, is un-exported explicitly. The redirection discards +# the trace of `set +ax` itself. If the caller exports BASH_XTRACEFD, then +# bash writes that trace to the caller's trace descriptor instead. If that +# descriptor is not open, then bash reports so on stderr before this script +# runs, as for any bash script; cronic cannot prevent that. { cronic_caller_flags=${-//[^aeux]/} set +ax @@ -68,6 +76,12 @@ cronic_help() { echo "stderr, from which lines that start with PS4 are removed. That removal" echo "is skipped if PS4 contains \"\$\", a backquote, a backslash, or a newline," echo "because such a PS4 can expand differently on each line." + echo "Do not run \"bash -v cronic\", nor export the verbose option (\"set -v\") via" + echo "SHELLOPTS: bash would echo the lines of this script (and, if exported, of a" + echo "bash COMMAND) to stderr, so every run would produce output." + echo "Bash runs the file named by BASH_ENV, if any, before cronic's own code, so" + echo "cronic cannot capture that file's output, and exported shell options can make" + echo "that file write to stderr. To prevent that, unset BASH_ENV." exit 0 } @@ -178,16 +192,24 @@ fi # The command runs in a subshell rather than in this shell, so that a builtin # such as `exit` or `exec` cannot end this script before it reports. +# The subshell restores the caller's options that this script changed. +# Otherwise, if the caller exported SHELLOPTS, a bash script that reads an unset +# variable (as `/usr/bin/texi2dvi` does when /bin/sh is bash) would fail. Of +# those options, only nounset is on here, so a single `set` command turns it off +# and turns on the caller's, so that the restoring is not traced. The `:+` +# avoids a bare `-`, which has a special meaning to `set`. # `command` because a function of this script, such as `cronic_usage`, would -# otherwise run in place of a program of the same name. The redirections -# apply to the whole subshell, so that they also capture what it writes after -# the command finishes, such as the output of an EXIT trap that the command -# sets. The subshell restores the caller's `-a`, `-e`, `-u`, and `-x` options, -# replacing this script's, for a builtin and also for a bash script when the -# caller exported SHELLOPTS. It turns off this script's nounset (errexit is off -# already) in the same command that turns on the caller's options, so that only -# the command is traced. The `:+` avoids a bare `-`, which has a special -# meaning to `set`. +# otherwise run in place of a program of the same name. +# The redirections apply to the whole subshell, so that they also capture what +# it writes after the command finishes, such as the output of an EXIT trap that +# the command sets. +# The subshell is not the operand of `||` or the like, because then bash would +# ignore `set -e` in it and would remove "errexit" from the SHELLOPTS that the +# command inherits. +# Limitation: if the command is a builtin that runs code in the subshell, such +# as `source` or `eval`, that code runs with the caller's options even if the +# caller does not export SHELLOPTS. So if the caller does not use `set -u`, +# that code silently expands an unset variable to the empty string. set +e ( set +u${cronic_caller_flags:+ -$cronic_caller_flags} diff --git a/tests/cronic-test/cronic-test.sh b/tests/cronic-test/cronic-test.sh index 52f89ee..62da7be 100755 --- a/tests/cronic-test/cronic-test.sh +++ b/tests/cronic-test/cronic-test.sh @@ -20,6 +20,13 @@ set -eu +# Do not let the user's environment file run in `cronic` or in the bash commands +# below. It could write to stderr, especially when an exported SHELLOPTS turns +# on `-u` for it. +unset BASH_ENV +# Use the default PS4, which the goal files and the trace lines below assume. +unset PS4 + SCRIPT_DIR="$(CDPATH='' cd -- "$(dirname -- "$0")" && pwd -P)" CRONIC="$(CDPATH='' cd -- "${SCRIPT_DIR}/../.." && pwd -P)/cronic" @@ -127,11 +134,91 @@ printf '%s\n' "$@" 1>&2 EOF chmod +x "$work/stderr-lines" +# 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 + # temp_files: prints `cronic`'s temporary files, in a canonical order. temp_files() { find "$TMPDIR" -mindepth 1 -maxdepth 1 2> /dev/null | sort } +# run_cronic DESCRIPTION EXPECTED-STATUS COMMAND...: runs COMMAND, which runs +# `cronic`, with its standard output in "$work/output" and its error output in +# "$work/stderr". If COMMAND exits with a status other than EXPECTED-STATUS, +# or leaves temporary files behind, then reports a failure and returns 1. +run_cronic() { + description="$1" + expected_status="$2" + shift 2 + + before="$(temp_files)" + actual_status=0 + "$@" > "$work/output" 2> "$work/stderr" || actual_status=$? + after="$(temp_files)" + + if [ "$actual_status" != "$expected_status" ]; then + echo "FAIL: $description: exit status $actual_status, expected $expected_status" + cat "$work/output" "$work/stderr" + status=1 + return 1 + fi + if [ "$before" != "$after" ]; then + echo "FAIL: $description: temporary files were left behind:" + echo "$after" + status=1 + return 1 + fi +} + +# check_trace_section DESCRIPTION GOAL-FILE COMMAND...: runs COMMAND, which +# runs `cronic`, and checks that it exits with status 0, leaves no temporary +# files, writes nothing to stderr, and produces a complete report whose "TRACE +# OUTPUT" section, through the following empty line, is the contents of +# GOAL-FILE. +# Skips the check if bash lacks BASH_XTRACEFD, so that the report has no +# "TRACE OUTPUT". +check_trace_section() { + description="$1" + goal="$2" + shift 2 + + if [ "$bash_has_xtracefd" = false ]; then + echo "SKIP: $description, because bash lacks BASH_XTRACEFD" + return 0 + fi + + run_cronic "$description" 0 "$@" || return 0 + sed -n '/^TRACE OUTPUT:$/,/^$/p' "$work/output" > "$work/trace-section" + if cmp -s "$goal" "$work/trace-section" \ + && grep -q "^END OF CRONIC OUTPUT.$" "$work/output" \ + && [ ! -s "$work/stderr" ]; then + echo "PASS: $description" + else + echo "FAIL: $description: output:" + cat "$work/output" + echo "stderr:" + cat "$work/stderr" + echo "differences from the goal trace section:" + diff "$goal" "$work/trace-section" || true + status=1 + fi +} + # 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", @@ -145,17 +232,10 @@ check() { expected_output="$3" shift 3 - before="$(temp_files)" - actual_status=0 - "$CRONIC" "$@" > "$work/output" 2>&1 || actual_status=$? - after="$(temp_files)" - - if [ "$actual_status" != "$expected_status" ]; then - echo "FAIL: $description: exit status $actual_status, expected $expected_status" - cat "$work/output" - status=1 - return - fi + run_cronic "$description" "$expected_status" "$CRONIC" "$@" || return 0 + # The checks below examine both outputs, because `cronic` writes its usage + # messages to stderr. + cat "$work/stderr" >> "$work/output" case "$expected_output" in silent) @@ -196,13 +276,6 @@ check() { ;; esac - if [ "$before" != "$after" ]; then - echo "FAIL: $description: temporary files were left behind:" - echo "$after" - status=1 - return - fi - echo "PASS: $description" } @@ -340,74 +413,67 @@ check "EXIT trap writes to stdout" 0 silent trap 'echo "from the trap"' EXIT # shellcheck disable=SC2016 # The expansion is for the wrapped command to do. check "builtin with an unset variable" 0 silent \ eval ': "$cronic_test_unset_variable"' -# When the caller exports SHELLOPTS, a bash script runs with the caller's -# options rather than with `cronic`'s own `set -eu`. BASH_ENV is empty so that -# the script runs no startup file. -# Creates "$work/cronic-with-NAME", which runs `cronic` with SHELLOPTS exported, -# after `set OPTION` if OPTION is given. -make_cronic_wrapper() { # NAME [OPTION] - { - echo "#!/bin/bash" - if [ $# -gt 1 ]; then - echo "set $2" - fi - echo "export SHELLOPTS" - echo "exec \"$CRONIC\" \"\$@\"" - } > "$work/cronic-with-$1" - chmod +x "$work/cronic-with-$1" + +# When SHELLOPTS is exported, `cronic`'s own `-e` and `-u` options do not reach +# the wrapped command, but the caller's options do. +real_cronic="$CRONIC" +# cronic_with_shellopts SHELLOPTS ARG...: runs `cronic` on the ARGs, with +# SHELLOPTS exported. +# shellcheck disable=SC2329 # `check` calls it, as "$CRONIC". +cronic_with_shellopts() { + shellopts="$1" + shift + env SHELLOPTS="$shellopts" "$real_cronic" "$@" } -make_cronic_wrapper shellopts -make_cronic_wrapper nounset -u -make_cronic_wrapper errexit -e -make_cronic_wrapper allexport -a -# The wrapper's own trace goes to /dev/null, and `cronic` reads no startup -# file, so that any trace in the output comes from `cronic`. -cat > "$work/cronic-with-xtrace" << EOF -#!/bin/bash -unset BASH_ENV -exec 3>&2 2> /dev/null -set -x -export SHELLOPTS -exec "$CRONIC" "\$@" 2>&3 3>&- -EOF -chmod +x "$work/cronic-with-xtrace" -saved_cronic="$CRONIC" -CRONIC="$work/cronic-with-shellopts" +CRONIC=cronic_with_shellopts +default_shellopts=braceexpand:hashall:interactive-comments # shellcheck disable=SC2016 # The expansion is for the wrapped command to do. -check "bash script with an unset variable, SHELLOPTS exported" 0 silent \ - env BASH_ENV= bash -c ': "$cronic_test_unset_variable"' -check "bash script with a failing command, SHELLOPTS exported" 0 silent \ - env BASH_ENV= bash -c 'false; true' -CRONIC="$work/cronic-with-nounset" +check "exported SHELLOPTS, command reads an unset variable" 0 silent \ + "$default_shellopts" bash -c ': "$cronic_test_unset_variable"' +check "exported SHELLOPTS, command ignores a failure" 0 silent \ + "$default_shellopts" bash -c 'false; :' # shellcheck disable=SC2016 # The expansion is for the wrapped command to do. -check "bash script with an unset variable, caller's nounset exported" 127 \ - "report:cronic_test_unset_variable" \ - env BASH_ENV= bash -c ': "$cronic_test_unset_variable"' -CRONIC="$work/cronic-with-errexit" -check "bash script with a failing command, caller's errexit exported" 1 report \ - env BASH_ENV= bash -c 'false; true' -# When the caller exports allexport, `cronic` does not export its own variables -# to the command, but a variable that the command assigns is exported. -CRONIC="$work/cronic-with-allexport" -check "no cronic variables exported, caller's allexport exported" 0 silent \ - sh -c '! env | grep -q "^cronic_"' +check "exported SHELLOPTS with nounset, command reads an unset variable" 127 \ + "report:unbound variable" \ + "$default_shellopts:nounset" bash -c ': "$cronic_test_unset_variable"' +check "exported SHELLOPTS with errexit, command ignores a failure" 1 report \ + "$default_shellopts:errexit" bash -c 'false; :' +# With allexport exported, `cronic` does not export its own variables to the +# command, but a variable that the command assigns is exported. +check "exported SHELLOPTS with allexport, no cronic variables exported" 0 silent \ + "$default_shellopts:allexport" sh -c '! env | grep -q "^cronic_"' # shellcheck disable=SC2016 # The expansion is for the wrapped command to do. -check "command's variable exported, caller's allexport exported" 0 silent \ +check "exported SHELLOPTS with allexport, command's variable exported" 0 silent \ + "$default_shellopts:allexport" \ eval 'cronic_test_variable=1; sh -c "[ \"\$cronic_test_variable\" = 1 ]"' -# When the caller exports xtrace, `cronic` traces only the command, not itself. -CRONIC="$work/cronic-with-xtrace" -check "bash script, caller's xtrace exported" 0 silent \ - env BASH_ENV= bash -c 'true' -check "bash script that fails, caller's xtrace exported" 1 "report:^+ false$" \ - env BASH_ENV= bash -c 'false' -if grep -q cronic_ "$work/output"; then - echo "FAIL: caller's xtrace exported: the report traces cronic itself:" - cat "$work/output" - status=1 -else - echo "PASS: caller's xtrace exported: the report does not trace cronic itself" -fi -CRONIC="$saved_cronic" +# With xtrace exported, a successful command with only trace output on stderr +# produces no output at all: `cronic` does not trace itself. +check "exported SHELLOPTS with xtrace, trace-only stderr" 0 silent \ + "$default_shellopts:xtrace" "$work/trace-only" 0 +# With xtrace exported, the report's trace section is the trace of running the +# command, followed by the command's own trace, with nothing that `cronic` did +# to set up the command's options. Bash writes the trace via BASH_XTRACEFD, so +# it appears under "TRACE OUTPUT". +# Bash quotes a word in a trace line if it contains a space or a shell +# metacharacter, so let bash write the expected trace of the path, which is +# under the caller's TMPDIR. The trace goes to a separate file descriptor, +# and stderr is discarded, so that an unrelated warning from bash, such as for +# an uninstalled locale, is not part of the expected trace. +# shellcheck disable=SC2016 # The expansion is for bash to do. +traced_command=$(bash -c 'BASH_XTRACEFD=3; set -x; : -- "$1" 0' sh "$work/trace-and-stderr" 3>&1 2> /dev/null | sed 's/^+ : /+ command /') +cat > "$work/trace-section.goal" << EOF +TRACE OUTPUT: +$traced_command ++ set -x ++ echo 'a real error' ++ exit 0 + +EOF +check_trace_section "exported SHELLOPTS with xtrace, trace output" \ + "$work/trace-section.goal" \ + "$CRONIC" "$default_shellopts:xtrace" "$work/trace-and-stderr" 0 + +CRONIC="$real_cronic" # The wrapped command sees the caller's exported variables unchanged. For # each variable that `cronic` assigns, the caller exports that name in @@ -446,23 +512,20 @@ check "caller's exported variables" 0 silent "$work/check-variables" $exported_n # shellcheck disable=SC2086 # each name is a separate argument. unset $exported_names -# 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 +# `bash -x cronic` does not trace `cronic` itself, but does trace running the +# command. The command does not inherit xtrace, because SHELLOPTS is not +# exported, so its own `set -x` is not traced. +cat > "$work/bash-x-trace-section.goal" << EOF +TRACE OUTPUT: +$traced_command ++ echo 'a real error' ++ exit 0 + +EOF +check_trace_section "bash -x cronic, trace output" \ + "$work/bash-x-trace-section.goal" \ + bash -x "$CRONIC" "$work/trace-and-stderr" 0 + export PS4 # shellcheck disable=SC2016 # The expansion is for the traced command to do. PS4='+${LINENO}: '