Skip to content

Commit

Permalink
Fix use of overly generic DEBUG env. var.
Browse files Browse the repository at this point in the history
It was an unfortunate mistake to name this variable as such.  It was
observed to collide with other non-conforming usages in downstream.
This was esp. leading to some difficult to debug situations, such as
containers/podman#13932  The common
automation library is used far/wide by many environments, which
unfortunately may also rely on a generic `$DEBUG`.  Fix the issue here,
by renaming the variable.

Let this serve as a warning to all downstream, everywhere: ***Avoid all
use of similar generic variable names, make them context-specific!***

Signed-off-by: Chris Evich <cevich@redhat.com>
  • Loading branch information
cevich committed Apr 20, 2022
1 parent 48039fb commit 7dfa5d1
Show file tree
Hide file tree
Showing 11 changed files with 35 additions and 34 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/cirrus-ci_retrospective.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ jobs:
- name: Load cirrus-ci_retrospective JSON and set action output variables
id: retro
env:
DEBUG: 1
A_DEBUG: 1
run: |
source ./main/$HELPER_LIB
load_ccir $GITHUB_WORKSPACE
Expand Down
8 changes: 4 additions & 4 deletions bin/install_automation.sh
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ INSTALLATION_SOURCE="${INSTALLATION_SOURCE:-}"
AUTOMATION_VERSION="$1"
shift || true # ignore if no more args
# Set non-zero to enable
DEBUG=${DEBUG:-0}
A_DEBUG=${A_DEBUG:-0}
# Save some output eyestrain (if script can be found)
OOE=$(realpath $(dirname "${BASH_SOURCE[0]}")/../common/bin/ooe.sh 2>/dev/null || echo "")
# Sentinel value representing whatever version is present in the local repository
Expand All @@ -42,7 +42,7 @@ _DEFAULT_MAGIC_JUJU=d41d844b68a14ee7b9e6a6bb88385b4d

msg() { echo -e "${1:-No Message given}"; }

dbg() { if ((DEBUG)); then msg "\n# $1"; fi }
dbg() { if ((A_DEBUG)); then msg "\n# $1"; fi }

# On 5/14/2021 the default branch was renamed to 'main'.
# Since prior versions of the installer reference the old
Expand Down Expand Up @@ -194,7 +194,7 @@ exec_installer() {
# _MAGIC_JUJU set to signal actual installation work should commence
set -x
exec env \
DEBUG="$DEBUG" \
A_DEBUG="$A_DEBUG" \
INSTALLATION_SOURCE="$INSTALLATION_SOURCE" \
INSTALL_PREFIX="$INSTALL_PREFIX" \
AUTOMATION_REPO_URL="$AUTOMATION_REPO_URL" \
Expand Down Expand Up @@ -257,7 +257,7 @@ elif [[ "$_MAGIC_JUJU" == "$_DEFAULT_MAGIC_JUJU" ]]; then
env AUTOMATION_LIB_PATH=$AUTOMATION_LIB_PATH \
AUTOMATION_VERSION=$AUTOMATION_VERSION \
INSTALLATION_SOURCE=$INSTALLATION_SOURCE \
DEBUG=$DEBUG \
A_DEBUG=$A_DEBUG \
MAGIC_JUJU=$_MAGIC_JUJU \
/bin/bash $CHAIN_TO
msg "##### Installation complete for '$arg' subcomponent"
Expand Down
12 changes: 6 additions & 6 deletions build-push/test/testbuilds.sh
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ test_cmd "Confirm error output and exit(42) from --prepcmd" \

test_cmd "Confirm building native-arch test image w/ --nopush" \
0 "STEP 3/3: ENTRYPOINT /bin/false.+COMMIT" \
bash -c "DEBUG=1 $SUBJ_FILEPATH localhost/foo/bar $TEST_CONTEXT --nopush 2>&1"
bash -c "A_DEBUG=1 $SUBJ_FILEPATH localhost/foo/bar $TEST_CONTEXT --nopush 2>&1"

native_arch=$($RUNTIME info --format='{{.host.arch}}')
test_cmd "Confirm native_arch was set to non-empty string" \
Expand All @@ -46,20 +46,20 @@ test_cmd "Confirm built image manifest contains the native arch '$native_arch'"

test_cmd "Confirm rebuilding with same command uses cache" \
0 "STEP 3/3.+Using cache" \
bash -c "DEBUG=1 $SUBJ_FILEPATH localhost/foo/bar $TEST_CONTEXT --nopush 2>&1"
bash -c "A_DEBUG=1 $SUBJ_FILEPATH localhost/foo/bar $TEST_CONTEXT --nopush 2>&1"

test_cmd "Confirm manifest-list can be removed by name" \
0 "untagged: localhost/foo/bar:latest" \
$RUNTIME manifest rm containers-storage:localhost/foo/bar:latest

test_cmd "Verify expected partial failure when passing bogus architectures" \
125 "error creating build.+architecture staple" \
bash -c "DEBUG=1 $SUBJ_FILEPATH --arches=correct,horse,battery,staple localhost/foo/bar --nopush $TEST_CONTEXT 2>&1"
bash -c "A_DEBUG=1 $SUBJ_FILEPATH --arches=correct,horse,battery,staple localhost/foo/bar --nopush $TEST_CONTEXT 2>&1"

MODCMD='$RUNTIME tag $FQIN:latest $FQIN:9.8.7-testing'
test_cmd "Verify --modcmd is able to tag the manifest" \
0 "Executing mod-command" \
bash -c "DEBUG=1 $SUBJ_FILEPATH localhost/foo/bar $TEST_CONTEXT --nopush --modcmd='$MODCMD' 2>&1"
bash -c "A_DEBUG=1 $SUBJ_FILEPATH localhost/foo/bar $TEST_CONTEXT --nopush --modcmd='$MODCMD' 2>&1"

test_cmd "Verify the tagged manifest is also present" \
0 "[a-zA-Z0-9]+" \
Expand Down Expand Up @@ -95,7 +95,7 @@ $RUNTIME images && \
# - causing it to exit(0) as it should
test_cmd "Verify --modcmd can execute a long string with substitutions" \
125 "AllGone" \
bash -c "DEBUG=1 $SUBJ_FILEPATH --modcmd='$MODCMD' localhost/foo/bar --nopush $TEST_CONTEXT 2>&1"
bash -c "A_DEBUG=1 $SUBJ_FILEPATH --modcmd='$MODCMD' localhost/foo/bar --nopush $TEST_CONTEXT 2>&1"

test_cmd "Verify previous --modcmd removed the 'latest' tagged image" \
125 "image not known" \
Expand All @@ -111,7 +111,7 @@ MODCMD="set -ex;
\$RUNTIME manifest rm \$FQIN:latest;"
test_cmd "Verify e2e workflow w/ additional build-args" \
0 "Pushing $TEST_FQIN:$FAKE_VERSION" \
bash -c "env DEBUG=1 $SUBJ_FILEPATH \
bash -c "env A_DEBUG=1 $SUBJ_FILEPATH \
--prepcmd='touch $TEST_SOURCE_DIRPATH/test_context/Containerfile' \
--modcmd='$MODCMD' \
--arches=amd64,s390x,arm64,ppc64le \
Expand Down
2 changes: 1 addition & 1 deletion common/lib/console_output.sh
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ die() {
}

dbg() {
if ((DEBUG)); then
if ((A_DEBUG)); then
local shortest_source_path=$(_rel_path "${BASH_SOURCE[1]}")
(
echo
Expand Down
7 changes: 4 additions & 3 deletions common/lib/defaults.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@ CI="${CI:-false}" # true: _unlikely_ human-presence at the controls.
[[ $CI == "false" ]] || CI='true' # Err on the side of automation

# Default to NOT running in debug-mode unless set non-zero
DEBUG=${DEBUG:-0}
# Conditionals like ((DEBUG)) easier than checking "true"/"False"
( test "$DEBUG" -eq 0 || test "$DEBUG" -ne 0 ) &>/dev/null || DEBUG=1 # assume true when non-integer
A_DEBUG=${A_DEBUG:-0}
# Conditionals like ((A_DEBUG)) easier than checking "true"/"False"
( test "$A_DEBUG" -eq 0 || test "$A_DEBUG" -ne 0 ) &>/dev/null || \
A_DEBUG=1 # assume true when non-integer

# String prefixes to use when printing messages to the console
DEBUG_MSG_PREFIX="${DEBUG_MSG_PREFIX:-DEBUG:}"
Expand Down
4 changes: 2 additions & 2 deletions common/test/console_output_test_helper.sh
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ source "$AUTOMATION_LIB_PATH/common_lib.sh"
set +e

test_function() {
DEBUG=1 dbg "Test dbg message"
A_DEBUG=1 dbg "Test dbg message"
warn "Test warning message"
msg "Test msg message"
die "Test die message" 0
}

DEBUG=1 dbg "Test dbg message"
A_DEBUG=1 dbg "Test dbg message"
warn "Test warning message"
msg "Test msg message"
die "Test die message" 0
Expand Down
2 changes: 1 addition & 1 deletion common/test/testbin-install_automation.sh
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ load_example_environment() {
source "$INSTALL_PREFIX/automation/environment" || return 99
echo "AUTOMATION_LIB_PATH ==> ${AUTOMATION_LIB_PATH:-UNDEFINED}"
echo "PATH ==> ${PATH:-EMPTY}"
[[ -z "$_args" ]] || DEBUG=1 $_args
[[ -z "$_args" ]] || A_DEBUG=1 $_args
)
}

Expand Down
10 changes: 5 additions & 5 deletions common/test/testlib-console_output.sh
Original file line number Diff line number Diff line change
Expand Up @@ -58,20 +58,20 @@ test_cmd "The indent function indents it's own output" \
0 "$EXPECTED_SUM" \
bash -c "echo '$TEST_STRING' | indent | indent | sha256sum"

DEBUG=0
A_DEBUG=0
test_cmd \
"The dbg function has no output when \$DEBUG is zero and no message is given" \
"The dbg function has no output when \$A_DEBUG is zero and no message is given" \
0 "" \
dbg

test_cmd \
"The dbg function has no output when \$DEBUG is zero and a test message is given" \
"The dbg function has no output when \$A_DEBUG is zero and a test message is given" \
0 "" \
dbg "$test_message_text"

DEBUG=1
A_DEBUG=1
basic_tests dbg 0 DEBUG
DEBUG=0
A_DEBUG=0

test_cmd \
"All primary output functions include the expected context information" \
Expand Down
10 changes: 5 additions & 5 deletions common/test/testlib-defaults.sh
Original file line number Diff line number Diff line change
Expand Up @@ -20,24 +20,24 @@ test_ci() {
CI="$prev_CI"
}

# DEBUG must default to 0 or non-zero
# A_DEBUG must default to 0 or non-zero
# usage: <expected non-zero> [initial_value]
test_debug() {
local exp_non_zero=$1
local init_value="$2"
[[ -z "$init_value" ]] || \
DEBUG=$init_value
local desc_pfx="The \$DEBUG env. var initialized '$init_value', after loading library is"
A_DEBUG=$init_value
local desc_pfx="The \$A_DEBUG env. var initialized '$init_value', after loading library is"

source "$TEST_DIR"/"$SUBJ_FILENAME"
if ((exp_non_zero)); then
test_cmd "$desc_pfx non-zero" \
0 "" \
test "$DEBUG" -ne 0
test "$A_DEBUG" -ne 0
else
test_cmd "$desc_pfx zero" \
0 "" \
test "$DEBUG" -eq 0
test "$A_DEBUG" -eq 0
fi
}

Expand Down
4 changes: 2 additions & 2 deletions github/test/testlib-github.sh
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ test_cmd 'Default shell variables are initialized empty/false' \
echo -n "${prn}${tid}${sha}${tst}${was_pr}${do_intg}"

# Remaining tests all require debugging output to be enabled
DEBUG=1
A_DEBUG=1

test_cmd 'The debugging function does not throw any errors and uses special debug output' \
0 '::debug::' \
Expand Down Expand Up @@ -92,7 +92,7 @@ for regex in '"id": "10"' $MONITOR_TASK $ACTION_TASK '"branch": "pull/12"' \
done

# Remaining tests all require debugging output disabled
DEBUG=0
A_DEBUG=0

write_ccir 1 2 3 PAUSED COMPLETED
load_ccir "$TESTTEMPDIR"
Expand Down
8 changes: 4 additions & 4 deletions github/test/testlib-github_common.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,18 @@ test_cmd "The library $TEST_DIR/$SUBJ_FILENAME loads" \
0 '' \
source $TEST_DIR/$SUBJ_FILENAME

DEBUG=1
A_DEBUG=1
ACTIONS_STEP_DEBUG=true
# Should update $DEBUG value
# Should update $A_DEBUG value
source $TEST_DIR/$SUBJ_FILENAME || exit 1 # can't continue w/o loaded library

test_cmd "The debug message prefix is compatible with github actions commands" \
0 '::debug:: This is a test debug message' \
dbg 'This is a test debug message'

unset ACTIONS_STEP_DEBUG
unset DEBUG
# Should update $DEBUG value
unset A_DEBUG
# Should update $A_DEBUG value
source $TEST_DIR/$SUBJ_FILENAME

test_cmd "No debug message shows when ACTIONS_STEP_DEBUG is undefined" \
Expand Down

0 comments on commit 7dfa5d1

Please sign in to comment.