From 42adb1beb21395ed28fb72bb77bf938eae419434 Mon Sep 17 00:00:00 2001 From: Michael Ernst Date: Thu, 8 Oct 2026 20:20:06 -0700 Subject: [PATCH 1/4] Prefix `cronic`'s variables with `CRONIC_`, so that it does not change the caller's exported variables --- cronic | 96 +++++++++++++++++--------------- tests/cronic-test/cronic-test.sh | 14 +++++ 2 files changed, 65 insertions(+), 45 deletions(-) diff --git a/cronic b/cronic index 70f35d7..87c8811 100755 --- a/cronic +++ b/cronic @@ -19,6 +19,11 @@ # as an error. set -eu +# The names of this script's functions start with "cronic_", and the names of +# its variables start with "CRONIC_", so that they are unlikely to be the names +# of the caller's exported variables. An assignment to a variable that the +# caller exported would change the value that the wrapped command sees. + # Prints the usage line. cronic_usage_line() { echo "Usage: $0 [--expected-status N] [--permit-stderr] [--duplicate-stderr] [--] COMMAND [ARG...]" @@ -47,23 +52,23 @@ cronic_help() { exit 0 } -# Sets EXPECTED to its argument, or reports a usage error if the argument is -# not a valid exit status. +# Sets CRONIC_EXPECTED to its argument, or reports a usage error if the +# argument is not a valid exit status. cronic_set_expected() { - EXPECTED=$1 + CRONIC_EXPECTED=$1 # No leading zeros, because `[[ ... -eq ... ]]` would read the value as # octal. At most 255, because no exit status is larger. - if ! [[ $EXPECTED =~ ^(0|[1-9][0-9]{0,2})$ ]] || [ "$EXPECTED" -gt 255 ]; then - echo "$0: --expected-status requires an integer from 0 to 255, got \"$EXPECTED\"" >&2 + if ! [[ $CRONIC_EXPECTED =~ ^(0|[1-9][0-9]{0,2})$ ]] || [ "$CRONIC_EXPECTED" -gt 255 ]; then + echo "$0: --expected-status requires an integer from 0 to 255, got \"$CRONIC_EXPECTED\"" >&2 cronic_usage fi } # The options are parsed before the temporary directory is created, so that # `--help` and a usage error neither create nor depend on it. -EXPECTED=0 -PERMITSTDERR=0 -DUPLICATE_STDERR=0 +CRONIC_EXPECTED=0 +CRONIC_PERMITSTDERR=0 +CRONIC_DUPLICATE_STDERR=0 while [ $# -gt 0 ]; do case $1 in --expected-status) @@ -81,11 +86,11 @@ while [ $# -gt 0 ]; do ;; --permit-stderr) shift - PERMITSTDERR=1 + CRONIC_PERMITSTDERR=1 ;; --duplicate-stderr) shift - DUPLICATE_STDERR=1 + CRONIC_DUPLICATE_STDERR=1 ;; -h | --help) cronic_help @@ -105,26 +110,26 @@ while [ $# -gt 0 ]; do done [ $# -gt 0 ] || cronic_usage -DEBUG=false -# DEBUG=true +CRONIC_DEBUG=false +# CRONIC_DEBUG=true # A private directory, rather than predictable `/tmp/cronic.*.$$` names: on a # shared machine, process ids are guessable and reused, so another user could # pre-create those paths as symlinks and have the wrapped command's output # written through them. CRONIC_TMPDIR=$(mktemp -d "${TMPDIR:-/tmp}/cronic.XXXXXX") || exit 1 -OUT=$CRONIC_TMPDIR/out -ERR=$CRONIC_TMPDIR/err -REDUCED_ERR=$CRONIC_TMPDIR/err.reduced -TRACE=$CRONIC_TMPDIR/trace +CRONIC_OUT=$CRONIC_TMPDIR/out +CRONIC_ERR=$CRONIC_TMPDIR/err +CRONIC_REDUCED_ERR=$CRONIC_TMPDIR/err.reduced +CRONIC_TRACE=$CRONIC_TMPDIR/trace # Remove the temporary files however this script terminates, including when it -# is interrupted before it reaches the end. When DEBUG is true, the files are -# retained; don't "echo" their location, because that would be appended to -# stderr. +# is interrupted before it reaches the end. When CRONIC_DEBUG is true, the +# files are retained; don't "echo" their location, because that would be +# appended to stderr. # shellcheck disable=SC2329 # the function is invoked from the trap handlers below. cronic_cleanup() { - [ "$DEBUG" = "true" ] || rm -rf "$CRONIC_TMPDIR" + [ "$CRONIC_DEBUG" = "true" ] || rm -rf "$CRONIC_TMPDIR" } trap cronic_cleanup EXIT @@ -144,65 +149,66 @@ set +e # `command` because the wrapped command runs in this shell, where a function of # this script, such as `cronic_usage`, would otherwise run in place of a # program of the same name. -command -- "$@" > "$OUT" 2> "$TRACE" -RESULT=$? +command -- "$@" > "$CRONIC_OUT" 2> "$CRONIC_TRACE" +CRONIC_RESULT=$? set -e -PATTERN="^${PS4:0:1}\\+${PS4:1}" -if grep -q "$PATTERN" "$TRACE"; then +CRONIC_PATTERN="^${PS4:0:1}\\+${PS4:1}" +if 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, # exits with the wrapped command's status, and removes its temporary files. - grep -v "$PATTERN" "$TRACE" > "$ERR" || true + grep -v "$CRONIC_PATTERN" "$CRONIC_TRACE" > "$CRONIC_ERR" || true else - ERR=$TRACE + CRONIC_ERR=$CRONIC_TRACE fi # Certain Unix programs send output to stderr even when a command is successful! # For example, git does this, and the POSIX standard requires it for make. -# The full stderr output is shown, but file REDUCED_ERR is checked for emptiness below. +# The full stderr output is shown, but file CRONIC_REDUCED_ERR is checked for +# emptiness below. # TODO: Expand this. -cat "$ERR" \ +cat "$CRONIC_ERR" \ | grep -v "^Everything up-to-date$" \ | grep -v '^make\(\[[0-9]\+\]\)\?: \(Entering\|Leaving\) directory '"'"'.*'"'"'$' \ - > "$REDUCED_ERR" || true + > "$CRONIC_REDUCED_ERR" || true -if [ $RESULT -ne "$EXPECTED" ] || { [ $PERMITSTDERR -eq 0 ] && [ -s "$REDUCED_ERR" ]; }; then +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:" echo "$@" echo - if [[ $EXPECTED -eq 0 ]]; then - if [[ $RESULT -eq 0 ]]; then - echo "RESULT CODE: $RESULT (success)" + if [[ $CRONIC_EXPECTED -eq 0 ]]; then + if [[ $CRONIC_RESULT -eq 0 ]]; then + echo "RESULT CODE: $CRONIC_RESULT (success)" else - echo "RESULT CODE: $RESULT (failure)" + echo "RESULT CODE: $CRONIC_RESULT (failure)" fi else - echo "RESULT CODE: $RESULT (expected: $EXPECTED)" + echo "RESULT CODE: $CRONIC_RESULT (expected: $CRONIC_EXPECTED)" fi echo echo "ERROR OUTPUT:" - cat "$ERR" + cat "$CRONIC_ERR" echo - if [ "$DEBUG" != "true" ]; then - if ! cmp --silent "$ERR" "$REDUCED_ERR"; then + if [ "$CRONIC_DEBUG" != "true" ]; then + if ! cmp --silent "$CRONIC_ERR" "$CRONIC_REDUCED_ERR"; then echo "REDUCED ERROR OUTPUT:" - cat "$REDUCED_ERR" + cat "$CRONIC_REDUCED_ERR" echo fi fi echo "STANDARD OUTPUT:" - cat "$OUT" - if [ "$TRACE" != "$ERR" ]; then + cat "$CRONIC_OUT" + if [ "$CRONIC_TRACE" != "$CRONIC_ERR" ]; then echo echo "TRACE-ERROR OUTPUT:" - cat "$TRACE" + cat "$CRONIC_TRACE" fi - if [ $DUPLICATE_STDERR -eq 1 ]; then + if [ $CRONIC_DUPLICATE_STDERR -eq 1 ]; then echo echo "ERROR OUTPUT:" - cat "$ERR" + cat "$CRONIC_ERR" fi echo echo "END OF CRONIC OUTPUT." @@ -210,4 +216,4 @@ fi # The temporary files are removed by the EXIT trap. -exit $RESULT +exit $CRONIC_RESULT diff --git a/tests/cronic-test/cronic-test.sh b/tests/cronic-test/cronic-test.sh index 35cc622..af823fc 100755 --- a/tests/cronic-test/cronic-test.sh +++ b/tests/cronic-test/cronic-test.sh @@ -245,4 +245,18 @@ for name in $function_names; do done PATH="$saved_path" +# `cronic` does not change a variable that the caller exported, even one whose +# name is a plain word such as `DEBUG` or `OUT`. +cat > "$work/check-variables" << 'EOF' +#!/bin/sh +[ "$DEBUG" = "caller's DEBUG" ] && [ "$OUT" = "caller's OUT" ] \ + && [ "$RESULT" = "caller's RESULT" ] || exit 3 +EOF +chmod +x "$work/check-variables" +DEBUG="caller's DEBUG" +OUT="caller's OUT" +RESULT="caller's RESULT" +export DEBUG OUT RESULT +check "caller's exported variables" 0 silent "$work/check-variables" + exit "$status" From 931c4bfbb84591724ff461162d4323f9a26ab052 Mon Sep 17 00:00:00 2001 From: Michael Ernst Date: Thu, 8 Oct 2026 20:32:54 -0700 Subject: [PATCH 2/4] Use lowercase names for `cronic`'s variables, and test every variable it assigns Environment variables are uppercase by convention, so lowercase names cannot overwrite a caller's exported variable (including `CRONIC_*` ones). The test now derives the variable names from `cronic` itself, and unsets the exported variables after the check. --- cronic | 95 ++++++++++++++++---------------- tests/cronic-test/cronic-test.sh | 38 ++++++++++--- 2 files changed, 77 insertions(+), 56 deletions(-) diff --git a/cronic b/cronic index 87c8811..80f4975 100755 --- a/cronic +++ b/cronic @@ -19,10 +19,11 @@ # as an error. set -eu -# The names of this script's functions start with "cronic_", and the names of -# its variables start with "CRONIC_", so that they are unlikely to be the names -# of the caller's exported variables. An assignment to a variable that the -# caller exported would change the value that the wrapped command sees. +# The names of this script's variables are lowercase, because an assignment to +# a variable that the caller exported would change the value that the wrapped +# command sees, and by convention exported (environment) variables are +# uppercase. The names of this script's functions and variables also start +# with "cronic_", to further reduce the chance of a clash. # Prints the usage line. cronic_usage_line() { @@ -52,23 +53,23 @@ cronic_help() { exit 0 } -# Sets CRONIC_EXPECTED to its argument, or reports a usage error if the +# Sets cronic_expected to its argument, or reports a usage error if the # argument is not a valid exit status. cronic_set_expected() { - CRONIC_EXPECTED=$1 + cronic_expected=$1 # No leading zeros, because `[[ ... -eq ... ]]` would read the value as # octal. At most 255, because no exit status is larger. - if ! [[ $CRONIC_EXPECTED =~ ^(0|[1-9][0-9]{0,2})$ ]] || [ "$CRONIC_EXPECTED" -gt 255 ]; then - echo "$0: --expected-status requires an integer from 0 to 255, got \"$CRONIC_EXPECTED\"" >&2 + if ! [[ $cronic_expected =~ ^(0|[1-9][0-9]{0,2})$ ]] || [ "$cronic_expected" -gt 255 ]; then + echo "$0: --expected-status requires an integer from 0 to 255, got \"$cronic_expected\"" >&2 cronic_usage fi } # The options are parsed before the temporary directory is created, so that # `--help` and a usage error neither create nor depend on it. -CRONIC_EXPECTED=0 -CRONIC_PERMITSTDERR=0 -CRONIC_DUPLICATE_STDERR=0 +cronic_expected=0 +cronic_permitstderr=0 +cronic_duplicate_stderr=0 while [ $# -gt 0 ]; do case $1 in --expected-status) @@ -86,11 +87,11 @@ while [ $# -gt 0 ]; do ;; --permit-stderr) shift - CRONIC_PERMITSTDERR=1 + cronic_permitstderr=1 ;; --duplicate-stderr) shift - CRONIC_DUPLICATE_STDERR=1 + cronic_duplicate_stderr=1 ;; -h | --help) cronic_help @@ -110,26 +111,26 @@ while [ $# -gt 0 ]; do done [ $# -gt 0 ] || cronic_usage -CRONIC_DEBUG=false -# CRONIC_DEBUG=true +cronic_debug=false +# cronic_debug=true # A private directory, rather than predictable `/tmp/cronic.*.$$` names: on a # shared machine, process ids are guessable and reused, so another user could # pre-create those paths as symlinks and have the wrapped command's output # written through them. -CRONIC_TMPDIR=$(mktemp -d "${TMPDIR:-/tmp}/cronic.XXXXXX") || exit 1 -CRONIC_OUT=$CRONIC_TMPDIR/out -CRONIC_ERR=$CRONIC_TMPDIR/err -CRONIC_REDUCED_ERR=$CRONIC_TMPDIR/err.reduced -CRONIC_TRACE=$CRONIC_TMPDIR/trace +cronic_tmpdir=$(mktemp -d "${TMPDIR:-/tmp}/cronic.XXXXXX") || exit 1 +cronic_out=$cronic_tmpdir/out +cronic_err=$cronic_tmpdir/err +cronic_reduced_err=$cronic_tmpdir/err.reduced +cronic_trace=$cronic_tmpdir/trace # Remove the temporary files however this script terminates, including when it -# is interrupted before it reaches the end. When CRONIC_DEBUG is true, the +# is interrupted before it reaches the end. When cronic_debug is true, the # files are retained; don't "echo" their location, because that would be # appended to stderr. # shellcheck disable=SC2329 # the function is invoked from the trap handlers below. cronic_cleanup() { - [ "$CRONIC_DEBUG" = "true" ] || rm -rf "$CRONIC_TMPDIR" + [ "$cronic_debug" = "true" ] || rm -rf "$cronic_tmpdir" } trap cronic_cleanup EXIT @@ -149,66 +150,66 @@ set +e # `command` because the wrapped command runs in this shell, where a function of # this script, such as `cronic_usage`, would otherwise run in place of a # program of the same name. -command -- "$@" > "$CRONIC_OUT" 2> "$CRONIC_TRACE" -CRONIC_RESULT=$? +command -- "$@" > "$cronic_out" 2> "$cronic_trace" +cronic_result=$? set -e -CRONIC_PATTERN="^${PS4:0:1}\\+${PS4:1}" -if grep -q "$CRONIC_PATTERN" "$CRONIC_TRACE"; then +cronic_pattern="^${PS4:0:1}\\+${PS4:1}" +if 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, # exits with the wrapped command's status, and removes its temporary files. - grep -v "$CRONIC_PATTERN" "$CRONIC_TRACE" > "$CRONIC_ERR" || true + grep -v "$cronic_pattern" "$cronic_trace" > "$cronic_err" || true else - CRONIC_ERR=$CRONIC_TRACE + cronic_err=$cronic_trace fi # Certain Unix programs send output to stderr even when a command is successful! # 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 +# The full stderr output is shown, but file cronic_reduced_err is checked for # emptiness below. # TODO: Expand this. -cat "$CRONIC_ERR" \ +cat "$cronic_err" \ | grep -v "^Everything up-to-date$" \ | grep -v '^make\(\[[0-9]\+\]\)\?: \(Entering\|Leaving\) directory '"'"'.*'"'"'$' \ - > "$CRONIC_REDUCED_ERR" || true + > "$cronic_reduced_err" || true -if [ $CRONIC_RESULT -ne "$CRONIC_EXPECTED" ] || { [ $CRONIC_PERMITSTDERR -eq 0 ] && [ -s "$CRONIC_REDUCED_ERR" ]; }; then +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:" echo "$@" echo - if [[ $CRONIC_EXPECTED -eq 0 ]]; then - if [[ $CRONIC_RESULT -eq 0 ]]; then - echo "RESULT CODE: $CRONIC_RESULT (success)" + if [[ $cronic_expected -eq 0 ]]; then + if [[ $cronic_result -eq 0 ]]; then + echo "RESULT CODE: $cronic_result (success)" else - echo "RESULT CODE: $CRONIC_RESULT (failure)" + echo "RESULT CODE: $cronic_result (failure)" fi else - echo "RESULT CODE: $CRONIC_RESULT (expected: $CRONIC_EXPECTED)" + echo "RESULT CODE: $cronic_result (expected: $cronic_expected)" fi echo echo "ERROR OUTPUT:" - cat "$CRONIC_ERR" + cat "$cronic_err" echo - if [ "$CRONIC_DEBUG" != "true" ]; then - if ! cmp --silent "$CRONIC_ERR" "$CRONIC_REDUCED_ERR"; then + if [ "$cronic_debug" != "true" ]; then + if ! cmp --silent "$cronic_err" "$cronic_reduced_err"; then echo "REDUCED ERROR OUTPUT:" - cat "$CRONIC_REDUCED_ERR" + cat "$cronic_reduced_err" echo fi fi echo "STANDARD OUTPUT:" - cat "$CRONIC_OUT" - if [ "$CRONIC_TRACE" != "$CRONIC_ERR" ]; then + cat "$cronic_out" + if [ "$cronic_trace" != "$cronic_err" ]; then echo echo "TRACE-ERROR OUTPUT:" - cat "$CRONIC_TRACE" + cat "$cronic_trace" fi - if [ $CRONIC_DUPLICATE_STDERR -eq 1 ]; then + if [ $cronic_duplicate_stderr -eq 1 ]; then echo echo "ERROR OUTPUT:" - cat "$CRONIC_ERR" + cat "$cronic_err" fi echo echo "END OF CRONIC OUTPUT." @@ -216,4 +217,4 @@ fi # The temporary files are removed by the EXIT trap. -exit $CRONIC_RESULT +exit $cronic_result diff --git a/tests/cronic-test/cronic-test.sh b/tests/cronic-test/cronic-test.sh index af823fc..7ddedd5 100755 --- a/tests/cronic-test/cronic-test.sh +++ b/tests/cronic-test/cronic-test.sh @@ -245,18 +245,38 @@ for name in $function_names; do done PATH="$saved_path" -# `cronic` does not change a variable that the caller exported, even one whose -# name is a plain word such as `DEBUG` or `OUT`. +# `cronic` does not change a variable that the caller exported. For each +# variable that `cronic` assigns, the caller exports that name in uppercase, +# both with and without any "cronic_" prefix (for example, `CRONIC_OUT` and +# `OUT`). `TMPDIR` is omitted, because `cronic` reads it. +variable_names=$(sed -n 's/^ *\([A-Za-z_][A-Za-z0-9_]*\)=.*/\1/p' "$CRONIC" | sort -u) +if [ -z "$variable_names" ]; then + echo "FAIL: found no variables in $CRONIC" + status=1 +fi +exported_names= +for name in $variable_names; do + upper=$(echo "$name" | tr '[:lower:]' '[:upper:]') + for exported in "$upper" "${upper#CRONIC_}"; do + [ "$exported" = TMPDIR ] && continue + exported_names="$exported_names $exported" + done +done cat > "$work/check-variables" << 'EOF' #!/bin/sh -[ "$DEBUG" = "caller's DEBUG" ] && [ "$OUT" = "caller's OUT" ] \ - && [ "$RESULT" = "caller's RESULT" ] || exit 3 +for name; do + eval "value=\${$name-}" + [ "$value" = "caller's $name" ] || exit 3 +done EOF chmod +x "$work/check-variables" -DEBUG="caller's DEBUG" -OUT="caller's OUT" -RESULT="caller's RESULT" -export DEBUG OUT RESULT -check "caller's exported variables" 0 silent "$work/check-variables" +for name in $exported_names; do + eval "$name=\"caller's \$name\"" + export "${name?}" +done +# shellcheck disable=SC2086 # each name is a separate argument. +check "caller's exported variables" 0 silent "$work/check-variables" $exported_names +# shellcheck disable=SC2086 # each name is a separate argument. +unset $exported_names exit "$status" From 391b40091642ab86bde5168a7c53cddbf6768bab Mon Sep 17 00:00:00 2001 From: Michael Ernst Date: Thu, 8 Oct 2026 20:39:54 -0700 Subject: [PATCH 3/4] Brevity --- cronic | 6 ------ 1 file changed, 6 deletions(-) diff --git a/cronic b/cronic index 80f4975..7b224bc 100755 --- a/cronic +++ b/cronic @@ -19,12 +19,6 @@ # as an error. set -eu -# The names of this script's variables are lowercase, because an assignment to -# a variable that the caller exported would change the value that the wrapped -# command sees, and by convention exported (environment) variables are -# uppercase. The names of this script's functions and variables also start -# with "cronic_", to further reduce the chance of a clash. - # Prints the usage line. cronic_usage_line() { echo "Usage: $0 [--expected-status N] [--permit-stderr] [--duplicate-stderr] [--] COMMAND [ARG...]" From 0c3b54069adc7b755e2ce16d680a7901d1eef2b2 Mon Sep 17 00:00:00 2001 From: Michael Ernst Date: Thu, 8 Oct 2026 20:42:59 -0700 Subject: [PATCH 4/4] Clarify what the exported-variables test of `cronic` checks --- tests/cronic-test/cronic-test.sh | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/tests/cronic-test/cronic-test.sh b/tests/cronic-test/cronic-test.sh index 7ddedd5..42048f3 100755 --- a/tests/cronic-test/cronic-test.sh +++ b/tests/cronic-test/cronic-test.sh @@ -245,10 +245,13 @@ for name in $function_names; do done PATH="$saved_path" -# `cronic` does not change a variable that the caller exported. For each -# variable that `cronic` assigns, the caller exports that name in uppercase, -# both with and without any "cronic_" prefix (for example, `CRONIC_OUT` and -# `OUT`). `TMPDIR` is omitted, because `cronic` reads it. +# The wrapped command sees the caller's exported variables unchanged. For +# each variable that `cronic` assigns, the caller exports that name in +# uppercase, both with and without any "cronic_" prefix (for example, +# `CRONIC_OUT` and `OUT`). `TMPDIR` is omitted, because `cronic` reads it. +# The lowercase names that `cronic` assigns, such as `cronic_out`, are not +# exported: `cronic` does overwrite them, but environment variables +# conventionally have uppercase names. variable_names=$(sed -n 's/^ *\([A-Za-z_][A-Za-z0-9_]*\)=.*/\1/p' "$CRONIC" | sort -u) if [ -z "$variable_names" ]; then echo "FAIL: found no variables in $CRONIC"