diff options
| -rwxr-xr-x | bin/shuriken | 103 | ||||
| -rw-r--r-- | src/lib/archive.source.sh | 23 | ||||
| -rw-r--r-- | src/lib/bootstrap.source.sh | 42 | ||||
| -rw-r--r-- | src/lib/config.sync.source.sh | 24 | ||||
| -rw-r--r-- | src/lib/config.validate.source.sh | 14 | ||||
| -rwxr-xr-x | tests/cli.sh | 31 |
6 files changed, 157 insertions, 80 deletions
diff --git a/bin/shuriken b/bin/shuriken index 2efd49a..75f8fd3 100755 --- a/bin/shuriken +++ b/bin/shuriken @@ -144,6 +144,48 @@ log_warning() { printf 'WARNING: %s\n' "$*" >&2 } +# Read a configuration value into the named array, accepting both Bash array +# and whitespace-separated scalar declarations of the same variable. +# This is shared by resolve_tar_opts and resolve_sync_destinations so the +# "array or scalar config" parsing lives in exactly one place. +# +# Arguments: +# $1 name of the source config variable (e.g. TAR_OPTS) +# $2 name of the destination array variable (nameref) +# Returns: +# 0 if the variable was declared (the destination may still be empty), +# 1 if the variable was not declared at all (lets callers apply defaults). +resolve_config_array() { + local -r config_var="$1"; shift + local -n config_array_ref="$1"; shift + local config_decl + + config_array_ref=() + + # declare -p fails when the variable was never set; callers use the + # non-zero return to distinguish "unset" from "set but empty". + if ! config_decl=$(declare -p "$config_var" 2>/dev/null); then + return 1 + fi + + case "$config_decl" in + declare\ -a*\ "$config_var"=*) + # Already a real array: copy it element by element. + local -n config_source_ref="$config_var" + # shellcheck disable=SC2034 + config_array_ref=("${config_source_ref[@]}") + ;; + *) + # Scalar string: word-split it into the destination array. + local -n config_scalar_ref="$config_var" + if [ -n "${config_scalar_ref:-}" ]; then + # shellcheck disable=SC2034 + read -r -a config_array_ref <<< "$config_scalar_ref" + fi + ;; + esac +} + resolve_default_rc_file() { local source_root @@ -402,27 +444,14 @@ tarball() { resolve_tar_opts() { local -n options_ref="$1"; shift - local tar_opts_decl - - options_ref=() - if ! tar_opts_decl=$(declare -p TAR_OPTS 2>/dev/null); then - options_ref=(-c) - return - fi - - case "$tar_opts_decl" in - declare\ -a*\ TAR_OPTS=*) - options_ref=("${TAR_OPTS[@]}") - ;; - *) - if [ -n "${TAR_OPTS:-}" ]; then - read -r -a options_ref <<< "$TAR_OPTS" - fi - ;; - esac + # Parse TAR_OPTS (array or scalar) via the shared config-array helper, + # then fall back to a plain "-c" whenever no options were configured, + # whether TAR_OPTS was unset or set to an empty value. + resolve_config_array TAR_OPTS options_ref if (( ${#options_ref[@]} == 0 )); then + # shellcheck disable=SC2034 options_ref=(-c) fi } @@ -2758,26 +2787,14 @@ print_config() { # Inlined from src/lib/config.sync.source.sh resolve_sync_destinations() { + # destinations_ref is a nameref output filled by resolve_config_array. + # shellcheck disable=SC2034 local -n destinations_ref="$1"; shift - local destinations_decl - - destinations_ref=() - if ! destinations_decl=$(declare -p SYNC_DESTINATIONS 2>/dev/null); then - return - fi - - case "$destinations_decl" in - declare\ -a*\ SYNC_DESTINATIONS=*) - destinations_ref=("${SYNC_DESTINATIONS[@]}") - ;; - *) - if [ -n "${SYNC_DESTINATIONS:-}" ]; then - # shellcheck disable=SC2034 - read -r -a destinations_ref <<< "$SYNC_DESTINATIONS" - fi - ;; - esac + # Parse SYNC_DESTINATIONS (array or scalar) via the shared config-array + # helper. Unlike TAR_OPTS there is no default: an unset or empty value + # simply yields an empty destinations array for callers to validate. + resolve_config_array SYNC_DESTINATIONS destinations_ref } sync_dist() { @@ -3170,10 +3187,16 @@ validate_refresh_splash_config() { } validate_imagemagick() { - if command -v magick >/dev/null 2>&1; then - return - fi - if command -v convert >/dev/null 2>&1; then + # Reuse the canonical detection in resolve_imagemagick_command instead of + # duplicating the magick/convert probing here. Its own error message is + # suppressed so we report the failure through config_error, keeping the + # validation output consistent (single "ERROR: ..." line, return code 1). + # imagemagick_command is a nameref output filled by the resolver; we only + # care about the exit status here, not the resolved command. + # shellcheck disable=SC2034 + local -a imagemagick_command=() + + if resolve_imagemagick_command convert imagemagick_command 2>/dev/null; then return fi diff --git a/src/lib/archive.source.sh b/src/lib/archive.source.sh index 127a0c5..3b173b7 100644 --- a/src/lib/archive.source.sh +++ b/src/lib/archive.source.sh @@ -26,27 +26,14 @@ tarball() { resolve_tar_opts() { local -n options_ref="$1"; shift - local tar_opts_decl - options_ref=() - - if ! tar_opts_decl=$(declare -p TAR_OPTS 2>/dev/null); then - options_ref=(-c) - return - fi - - case "$tar_opts_decl" in - declare\ -a*\ TAR_OPTS=*) - options_ref=("${TAR_OPTS[@]}") - ;; - *) - if [ -n "${TAR_OPTS:-}" ]; then - read -r -a options_ref <<< "$TAR_OPTS" - fi - ;; - esac + # Parse TAR_OPTS (array or scalar) via the shared config-array helper, + # then fall back to a plain "-c" whenever no options were configured, + # whether TAR_OPTS was unset or set to an empty value. + resolve_config_array TAR_OPTS options_ref if (( ${#options_ref[@]} == 0 )); then + # shellcheck disable=SC2034 options_ref=(-c) fi } diff --git a/src/lib/bootstrap.source.sh b/src/lib/bootstrap.source.sh index 244cd55..67ba090 100644 --- a/src/lib/bootstrap.source.sh +++ b/src/lib/bootstrap.source.sh @@ -60,6 +60,48 @@ log_warning() { printf 'WARNING: %s\n' "$*" >&2 } +# Read a configuration value into the named array, accepting both Bash array +# and whitespace-separated scalar declarations of the same variable. +# This is shared by resolve_tar_opts and resolve_sync_destinations so the +# "array or scalar config" parsing lives in exactly one place. +# +# Arguments: +# $1 name of the source config variable (e.g. TAR_OPTS) +# $2 name of the destination array variable (nameref) +# Returns: +# 0 if the variable was declared (the destination may still be empty), +# 1 if the variable was not declared at all (lets callers apply defaults). +resolve_config_array() { + local -r config_var="$1"; shift + local -n config_array_ref="$1"; shift + local config_decl + + config_array_ref=() + + # declare -p fails when the variable was never set; callers use the + # non-zero return to distinguish "unset" from "set but empty". + if ! config_decl=$(declare -p "$config_var" 2>/dev/null); then + return 1 + fi + + case "$config_decl" in + declare\ -a*\ "$config_var"=*) + # Already a real array: copy it element by element. + local -n config_source_ref="$config_var" + # shellcheck disable=SC2034 + config_array_ref=("${config_source_ref[@]}") + ;; + *) + # Scalar string: word-split it into the destination array. + local -n config_scalar_ref="$config_var" + if [ -n "${config_scalar_ref:-}" ]; then + # shellcheck disable=SC2034 + read -r -a config_array_ref <<< "$config_scalar_ref" + fi + ;; + esac +} + resolve_default_rc_file() { local source_root diff --git a/src/lib/config.sync.source.sh b/src/lib/config.sync.source.sh index 68bb759..1707fe6 100644 --- a/src/lib/config.sync.source.sh +++ b/src/lib/config.sync.source.sh @@ -1,24 +1,12 @@ resolve_sync_destinations() { + # destinations_ref is a nameref output filled by resolve_config_array. + # shellcheck disable=SC2034 local -n destinations_ref="$1"; shift - local destinations_decl - destinations_ref=() - - if ! destinations_decl=$(declare -p SYNC_DESTINATIONS 2>/dev/null); then - return - fi - - case "$destinations_decl" in - declare\ -a*\ SYNC_DESTINATIONS=*) - destinations_ref=("${SYNC_DESTINATIONS[@]}") - ;; - *) - if [ -n "${SYNC_DESTINATIONS:-}" ]; then - # shellcheck disable=SC2034 - read -r -a destinations_ref <<< "$SYNC_DESTINATIONS" - fi - ;; - esac + # Parse SYNC_DESTINATIONS (array or scalar) via the shared config-array + # helper. Unlike TAR_OPTS there is no default: an unset or empty value + # simply yields an empty destinations array for callers to validate. + resolve_config_array SYNC_DESTINATIONS destinations_ref } sync_dist() { diff --git a/src/lib/config.validate.source.sh b/src/lib/config.validate.source.sh index 7988821..b858144 100644 --- a/src/lib/config.validate.source.sh +++ b/src/lib/config.validate.source.sh @@ -152,10 +152,16 @@ validate_refresh_splash_config() { } validate_imagemagick() { - if command -v magick >/dev/null 2>&1; then - return - fi - if command -v convert >/dev/null 2>&1; then + # Reuse the canonical detection in resolve_imagemagick_command instead of + # duplicating the magick/convert probing here. Its own error message is + # suppressed so we report the failure through config_error, keeping the + # validation output consistent (single "ERROR: ..." line, return code 1). + # imagemagick_command is a nameref output filled by the resolver; we only + # care about the exit status here, not the resolved command. + # shellcheck disable=SC2034 + local -a imagemagick_command=() + + if resolve_imagemagick_command convert imagemagick_command 2>/dev/null; then return fi diff --git a/tests/cli.sh b/tests/cli.sh index d6fbbd4..b8b56f6 100755 --- a/tests/cli.sh +++ b/tests/cli.sh @@ -1677,6 +1677,34 @@ test_print_config_normalizes_scalar_and_array_tar_opts() { test::teardown } +test_print_config_empty_tar_opts_falls_back_to_default() { + local config_file + local empty_array_output + local empty_scalar_output + + test::setup + config_file="$TEST_TMPDIR/shuriken.conf" + test::write_album_config \ + "$config_file" "$TEST_TMPDIR/incoming" "$TEST_TMPDIR/dist" \ + 'Empty tar opts' 40 + + # An empty scalar TAR_OPTS must fall back to the default "-c" just like an + # unset value; the shared resolve_config_array helper yields an empty array + # and resolve_tar_opts supplies the default. + printf 'TAR_OPTS=%q\n' '' >> "$config_file" + empty_scalar_output=$("$TEST_SHURIKEN" --print-config --config "$config_file") + test::assert_contains 'TAR_OPTS=( -c )' "$empty_scalar_output" + + # An empty array declaration must behave identically. + test::write_album_config \ + "$config_file" "$TEST_TMPDIR/incoming" "$TEST_TMPDIR/dist" \ + 'Empty array tar opts' 40 + printf 'TAR_OPTS=()\n' >> "$config_file" + empty_array_output=$("$TEST_SHURIKEN" --print-config --config "$config_file") + test::assert_contains 'TAR_OPTS=( -c )' "$empty_array_output" + test::teardown +} + test_print_config_quiet_and_verbose_keep_machine_output() { local config_file local plain_output @@ -4856,6 +4884,9 @@ main() { '--print-config normalizes scalar and array TAR_OPTS' \ test_print_config_normalizes_scalar_and_array_tar_opts test::run_case \ + '--print-config empty TAR_OPTS falls back to default' \ + test_print_config_empty_tar_opts_falls_back_to_default + test::run_case \ '--print-config quiet and verbose keep machine output' \ test_print_config_quiet_and_verbose_keep_machine_output test::run_case \ |