diff --git a/CHANGELOG.md b/CHANGELOG.md index a4036ef..23fab35 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,13 @@ and versions are tracked in the repo-root `VERSION` file. ## [Unreleased] +### Fixed + +- Hardened `std_run --timeout` retry internals so timeout discovery is cached + per call, fallback setup failures return a generic error, and fallback timer + cleanup cannot remove the timeout marker before it is observed. +- Clarified `update_file_section` logging when appending a new managed section. + ### Removed - Removed early compatibility aliases `run`, `std_run_with_timeout`, and diff --git a/lib/bash/file/lib_file.sh b/lib/bash/file/lib_file.sh index 0f5d750..6757bef 100644 --- a/lib/bash/file/lib_file.sh +++ b/lib/bash/file/lib_file.sh @@ -240,7 +240,11 @@ update_file_section() { __file_remove_temp_paths__ "$current_content_file" fi - log_info "Updating '$target_file'" + if [[ "$section_exists" == true ]]; then + log_info "Updating '$target_file'" + else + log_info "Adding section to '$target_file'" + fi if ! __file_make_target_temp__ temp_file "$target_file"; then log_error "Failed to create temporary file for '$target_file'." __file_remove_temp_paths__ "$new_content_file" diff --git a/lib/bash/file/tests/lib_file.bats b/lib/bash/file/tests/lib_file.bats index 8464b4e..4e404a3 100644 --- a/lib/bash/file/tests/lib_file.bats +++ b/lib/bash/file/tests/lib_file.bats @@ -33,6 +33,26 @@ file_mode() { [ "$(cat "$target")" = $'line-one\n# BEGIN\nfirst\nsecond\n# END' ] } +@test "update_file_section logs adding when markers are absent" { + local script="$TEST_TMPDIR/add-log.sh" + local target="$TEST_TMPDIR/config.txt" + cat > "$script" < "\$1" +update_file_section "\$1" "# BEGIN" "# END" "first" +EOF + chmod +x "$script" + + bats_run bash "$script" "$target" + + [ "$status" -eq 0 ] + [[ "$output" == *"Adding section to '$target'"* ]] + [[ "$output" != *"Updating '$target'"* ]] + [ "$(cat "$target")" = $'line-one\n# BEGIN\nfirst\n# END' ] +} + @test "update_file_section appends to an empty file without a leading blank line" { local target="$TEST_TMPDIR/config.txt" touch "$target" diff --git a/lib/bash/std/lib_std.sh b/lib/bash/std/lib_std.sh index 67d3d1c..04ac4ed 100644 --- a/lib/bash/std/lib_std.sh +++ b/lib/bash/std/lib_std.sh @@ -799,12 +799,11 @@ __std_join_run_policy__() { } __std_run_once__() { - local timeout_seconds="$1" - shift - local timeout_path="" + local timeout_seconds="$1" timeout_path="$2" + shift 2 if [[ -n "$timeout_seconds" ]]; then - if std_command_path timeout_path timeout || std_command_path timeout_path gtimeout; then + if [[ -n "$timeout_path" ]]; then "$timeout_path" "$timeout_seconds" "$@" else __std_run_with_timeout_fallback__ "$timeout_seconds" "$@" @@ -885,7 +884,7 @@ __std_run_status_message__() { __std_run_impl__() { local helper_name="$1" shift - local exit_on_failure=1 quiet=0 timeout_seconds="" max_attempts=1 retry_delay=0 + local exit_on_failure=1 quiet=0 timeout_seconds="" timeout_path="" max_attempts=1 retry_delay=0 # Parse optional run flags before the command. while (($#)); do @@ -969,9 +968,13 @@ __std_run_impl__() { # Execute the command. Using "$@" is the key. It expands each argument # as a separate, quoted string, preserving spaces and special characters. # This is the safe, modern alternative to using `eval`. + if [[ -n "$timeout_seconds" ]]; then + std_command_path timeout_path timeout || std_command_path timeout_path gtimeout || timeout_path="" + fi + local attempt=1 exit_code=0 message while ((attempt <= max_attempts)); do - if __std_run_once__ "$timeout_seconds" "$@"; then + if __std_run_once__ "$timeout_seconds" "$timeout_path" "$@"; then return 0 else exit_code=$? @@ -1031,7 +1034,7 @@ __std_run_with_timeout_fallback__() { local timeout_marker command_pid timer_pid command_status local kill_grace_seconds=1 - std_make_temp_file timeout_marker base-bash-libs-timeout || return 127 + std_make_temp_file timeout_marker base-bash-libs-timeout || return 1 "$@" & command_pid=$! @@ -1049,7 +1052,7 @@ __std_run_with_timeout_fallback__() { command_status=$? if kill -0 "$timer_pid" 2>/dev/null; then - kill "$timer_pid" 2>/dev/null || true + kill -KILL "$timer_pid" 2>/dev/null || true fi wait "$timer_pid" 2>/dev/null || true diff --git a/lib/bash/std/tests/lib_std.bats b/lib/bash/std/tests/lib_std.bats index 9304c9a..a0a427e 100644 --- a/lib/bash/std/tests/lib_std.bats +++ b/lib/bash/std/tests/lib_std.bats @@ -974,16 +974,25 @@ EOF } @test "std_run --timeout returns 124 when the command times out" { + local script="$TEST_TMPDIR/run-timeout.sh" local stderr_file="$TEST_TMPDIR/run-timeout.err" - local rc + local rc_file="$TEST_TMPDIR/run-timeout.rc" - if std_run --no-exit --quiet --timeout 1 sleep 2 2>"$stderr_file"; then - rc=0 - else - rc=$? - fi + create_script "$script" <"$stderr_file"; then + printf '0\n' > "$rc_file" +else + printf '%s\n' "\$?" > "$rc_file" +fi +EOF - [ "$rc" -eq 124 ] + bats_run bash "$script" + + [ "$status" -eq 0 ] + [ "$output" = "" ] + [ "$(cat "$rc_file")" = "124" ] [ ! -s "$stderr_file" ] } @@ -1086,6 +1095,95 @@ EOF [ "$(cat "$output_file")" = "ok" ] } +@test "std_run discovers timeout binary once across retries" { + local fake_bin="$TEST_TMPDIR/timeout-bin" + local lookup_file="$TEST_TMPDIR/timeout-lookups.txt" + local counter_file="$TEST_TMPDIR/timeout-discovery-count.txt" + local script="$TEST_TMPDIR/timeout-discovery-command.sh" + local rc + local -a lookups=() + + mkdir -p "$fake_bin" + create_script "$fake_bin/timeout" <<'EOF' +#!/usr/bin/env bash +shift +"$@" +EOF + create_script "$script" <<'EOF' +#!/usr/bin/env bash +count=0 +[[ -f "$1" ]] && count="$(cat "$1")" +count=$((count + 1)) +printf '%s\n' "$count" > "$1" +((count >= 3)) +EOF + + eval "$(declare -f std_command_path | sed '1s/std_command_path/__orig_std_command_path/')" + std_command_path() { + if [[ "${2-}" == "timeout" || "${2-}" == "gtimeout" ]]; then + printf '%s\n' "$2" >> "$lookup_file" + fi + __orig_std_command_path "$@" + } + + PATH="$fake_bin:$PATH" std_run --no-exit --quiet --timeout 5 --max-attempts 3 /bin/bash "$script" "$counter_file" + rc=$? + unset -f std_command_path __orig_std_command_path + + [ "$rc" -eq 0 ] + mapfile -t lookups < "$lookup_file" + [ "${#lookups[@]}" -eq 1 ] + [ "${lookups[0]}" = "timeout" ] + [ "$(cat "$counter_file")" = "3" ] +} + +@test "std_run fallback timeout returns 1 when marker creation fails" { + local fake_bin="$TEST_TMPDIR/no-timeout-bin" + local stderr_file="$TEST_TMPDIR/timeout-marker-failure.err" + local rc + + mkdir -p "$fake_bin" + + eval "$(declare -f std_make_temp_file | sed '1s/std_make_temp_file/__orig_std_make_temp_file/')" + std_make_temp_file() { + return 1 + } + + if PATH="$fake_bin" std_run --no-exit --quiet --timeout 5 /bin/echo fallback 2>"$stderr_file"; then + rc=0 + else + rc=$? + fi + unset -f std_make_temp_file __orig_std_make_temp_file + + [ "$rc" -eq 1 ] +} + +@test "std_run fallback timeout kills timer with SIGKILL after command exits" { + local fake_bin="$TEST_TMPDIR/no-timeout-bin" + local kill_log="$TEST_TMPDIR/timeout-kill.log" + local output_file="$TEST_TMPDIR/timeout-kill-output.txt" + local rc + + mkdir -p "$fake_bin" + ln -s "$(command -v mktemp)" "$fake_bin/mktemp" + ln -s "$(command -v rm)" "$fake_bin/rm" + ln -s "$(command -v sleep)" "$fake_bin/sleep" + + kill() { + printf '%s\n' "$*" >> "$kill_log" + builtin kill "$@" + } + + PATH="$fake_bin" std_run --no-exit --quiet --timeout 5 /bin/echo fallback > "$output_file" + rc=$? + unset -f kill + + [ "$rc" -eq 0 ] + [ "$(cat "$output_file")" = "fallback" ] + [[ "$(cat "$kill_log")" == *"-KILL "* ]] +} + @test "std_run rejects invalid execution policy options" { local stderr_file="$TEST_TMPDIR/run-policy-invalid.err" local rc