diff options
| -rwxr-xr-x | bin/photoalbum | 76 | ||||
| -rwxr-xr-x | src/photoalbum.sh | 76 | ||||
| -rwxr-xr-x | tests/cli.sh | 59 | ||||
| -rwxr-xr-x | tests/helpers.sh | 48 |
4 files changed, 247 insertions, 12 deletions
diff --git a/bin/photoalbum b/bin/photoalbum index cb629bb..a55646e 100755 --- a/bin/photoalbum +++ b/bin/photoalbum @@ -914,17 +914,81 @@ wait_for_image_job_pid() { return "$status" } -wait_for_image_job_slot() { +image_job_pid_is_running() { + local -r pid="$1"; shift + local running_pid + + for running_pid in "$@"; do + if [ "$running_pid" = "$pid" ]; then + return 0 + fi + done + + return 1 +} + +reap_finished_image_jobs() { + local -r image_job_pids_name="$1"; shift # shellcheck disable=SC2178 - local -n image_job_pids_ref="$1"; shift + local -n image_job_pids_ref="$image_job_pids_name" local -n failed_ref="$1"; shift - local -r max_jobs="${IMAGE_JOBS:-3}" + local -i reaped_status="$1"; shift + local -i has_reaped_status=1 + local -i status=0 + local -a remaining_pids=() + local -a running_pids=() + local pid - while (( ${#image_job_pids_ref[@]} >= max_jobs )); do - if ! wait_for_image_job_pid "${image_job_pids_ref[0]}"; then + mapfile -t running_pids < <(jobs -rp) + + for pid in "${image_job_pids_ref[@]}"; do + if image_job_pid_is_running "$pid" "${running_pids[@]}"; then + remaining_pids+=("$pid") + continue + fi + + if wait_for_image_job_pid "$pid"; then + status=0 + else + status=$? + fi + if (( status == 127 && has_reaped_status != 0 )); then + status=$reaped_status + has_reaped_status=0 + fi + + if (( status != 0 )); then failed_ref=1 fi - image_job_pids_ref=("${image_job_pids_ref[@]:1}") + done + + image_job_pids_ref=("${remaining_pids[@]}") +} + +wait_for_next_image_job() { + local -r image_job_pids_name="$1"; shift + local -r failed_name="$1"; shift + # shellcheck disable=SC2178 + local -n image_job_pids_ref="$image_job_pids_name" + local -i status=0 + + set +e + wait -n "${image_job_pids_ref[@]}" + status=$? + set -e + + reap_finished_image_jobs "$image_job_pids_name" "$failed_name" "$status" +} + +wait_for_image_job_slot() { + local -r image_job_pids_name="$1"; shift + local -r failed_name="$1"; shift + # shellcheck disable=SC2178 + local -n image_job_pids_ref="$image_job_pids_name" + local -r max_jobs="${IMAGE_JOBS:-3}" + + while (( ${#image_job_pids_ref[@]} >= max_jobs )); do + wait_for_next_image_job "$image_job_pids_name" "$failed_name" done } diff --git a/src/photoalbum.sh b/src/photoalbum.sh index 32a38ee..5d7776b 100755 --- a/src/photoalbum.sh +++ b/src/photoalbum.sh @@ -914,17 +914,81 @@ wait_for_image_job_pid() { return "$status" } -wait_for_image_job_slot() { +image_job_pid_is_running() { + local -r pid="$1"; shift + local running_pid + + for running_pid in "$@"; do + if [ "$running_pid" = "$pid" ]; then + return 0 + fi + done + + return 1 +} + +reap_finished_image_jobs() { + local -r image_job_pids_name="$1"; shift # shellcheck disable=SC2178 - local -n image_job_pids_ref="$1"; shift + local -n image_job_pids_ref="$image_job_pids_name" local -n failed_ref="$1"; shift - local -r max_jobs="${IMAGE_JOBS:-3}" + local -i reaped_status="$1"; shift + local -i has_reaped_status=1 + local -i status=0 + local -a remaining_pids=() + local -a running_pids=() + local pid - while (( ${#image_job_pids_ref[@]} >= max_jobs )); do - if ! wait_for_image_job_pid "${image_job_pids_ref[0]}"; then + mapfile -t running_pids < <(jobs -rp) + + for pid in "${image_job_pids_ref[@]}"; do + if image_job_pid_is_running "$pid" "${running_pids[@]}"; then + remaining_pids+=("$pid") + continue + fi + + if wait_for_image_job_pid "$pid"; then + status=0 + else + status=$? + fi + if (( status == 127 && has_reaped_status != 0 )); then + status=$reaped_status + has_reaped_status=0 + fi + + if (( status != 0 )); then failed_ref=1 fi - image_job_pids_ref=("${image_job_pids_ref[@]:1}") + done + + image_job_pids_ref=("${remaining_pids[@]}") +} + +wait_for_next_image_job() { + local -r image_job_pids_name="$1"; shift + local -r failed_name="$1"; shift + # shellcheck disable=SC2178 + local -n image_job_pids_ref="$image_job_pids_name" + local -i status=0 + + set +e + wait -n "${image_job_pids_ref[@]}" + status=$? + set -e + + reap_finished_image_jobs "$image_job_pids_name" "$failed_name" "$status" +} + +wait_for_image_job_slot() { + local -r image_job_pids_name="$1"; shift + local -r failed_name="$1"; shift + # shellcheck disable=SC2178 + local -n image_job_pids_ref="$image_job_pids_name" + local -r max_jobs="${IMAGE_JOBS:-3}" + + while (( ${#image_job_pids_ref[@]} >= max_jobs )); do + wait_for_next_image_job "$image_job_pids_name" "$failed_name" done } diff --git a/tests/cli.sh b/tests/cli.sh index 975b724..63383fa 100755 --- a/tests/cli.sh +++ b/tests/cli.sh @@ -868,6 +868,62 @@ test_generate_image_jobs_limits_parallel_imagemagick() { test::teardown } +test_generate_image_jobs_waits_for_any_finished_imagemagick() { + local config_file + local fake_bin + local finish_01_line + local lock_file + local log_file + local log_output + local start_03_line + + test::setup + fake_bin="$TEST_TMPDIR/bin" + config_file="$TEST_TMPDIR/photoalbum.conf" + lock_file="$TEST_TMPDIR/wait-n.lock" + log_file="$TEST_TMPDIR/wait-n.log" + + test::install_wait_n_imagemagick_spy "$fake_bin" + mkdir -p "$TEST_TMPDIR/incoming" + printf 'fake image\n' > "$TEST_TMPDIR/incoming/01.jpg" + printf 'fake image\n' > "$TEST_TMPDIR/incoming/02.jpg" + printf 'fake image\n' > "$TEST_TMPDIR/incoming/03.jpg" + printf 'fake image\n' > "$TEST_TMPDIR/incoming/04.jpg" + test::write_album_config \ + "$config_file" "$TEST_TMPDIR/incoming" "$TEST_TMPDIR/dist" \ + 'Wait n image jobs album' 40 + + ( + cd "$TEST_TMPDIR" + PATH="$fake_bin:$PATH" \ + TEST_WAIT_N_MAGICK_LOCK="$lock_file" \ + TEST_WAIT_N_MAGICK_LOG="$log_file" \ + "$TEST_PHOTOALBUM" --image-jobs 2 --generate + ) + + log_output=$(<"$log_file") + test::assert_contains 'start photos/03.jpg' "$log_output" + test::assert_contains 'finish photos/01.jpg' "$log_output" + start_03_line=$( + grep -n '^start photos/03\.jpg$' "$log_file" \ + | head -n 1 \ + | cut -d: -f1 + ) + finish_01_line=$( + grep -n '^finish photos/01\.jpg$' "$log_file" \ + | head -n 1 \ + | cut -d: -f1 + ) + + if (( start_03_line >= finish_01_line )); then + echo 'FAIL: expected photos/03.jpg to start before photos/01.jpg finished' >&2 + cat "$log_file" >&2 + exit 1 + fi + + test::teardown +} + test_repeated_output_flags_use_last_value() { local config_file local fake_bin @@ -2967,6 +3023,9 @@ main() { '--generate --image-jobs limits ImageMagick parallelism' \ test_generate_image_jobs_limits_parallel_imagemagick test::run_case \ + '--generate --image-jobs waits for any finished ImageMagick job' \ + test_generate_image_jobs_waits_for_any_finished_imagemagick + test::run_case \ 'repeated output flags use last value' \ test_repeated_output_flags_use_last_value test::run_case \ diff --git a/tests/helpers.sh b/tests/helpers.sh index 6306004..96f0ab4 100755 --- a/tests/helpers.sh +++ b/tests/helpers.sh @@ -272,6 +272,54 @@ MAGICK cp "$bin_dir/magick" "$bin_dir/convert" } +test::install_wait_n_imagemagick_spy() { + local -r bin_dir="$1"; shift + + mkdir -p "$bin_dir" + + cat > "$bin_dir/magick" <<'MAGICK' +#!/usr/bin/env bash +set -euo pipefail + +if [ "${1:-}" = identify ]; then + exit 0 +fi + +lock_file="${TEST_WAIT_N_MAGICK_LOCK:?}" +log_file="${TEST_WAIT_N_MAGICK_LOG:?}" +dest="${@: -1}" +dest_name="$(basename "$dest")" +dest_dir="$(basename "$(dirname "$dest")")" +dest_label="$dest_dir/$dest_name" + +( + flock 9 + printf 'start %s\n' "$dest_label" >> "$log_file" +) 9>"$lock_file" + +if [ "$dest_label" = 'photos/01.jpg' ]; then + sleep "${TEST_WAIT_N_SLOW_SECONDS:-0.4}" +else + sleep "${TEST_WAIT_N_FAST_SECONDS:-0.02}" +fi + +mkdir -p "$(dirname "$dest")" +{ + printf 'fake image\n' + printf 'args:' + printf ' %q' "$@" + printf '\n' +} > "$dest" + +( + flock 9 + printf 'finish %s\n' "$dest_label" >> "$log_file" +) 9>"$lock_file" +MAGICK + chmod 0755 "$bin_dir/magick" + cp "$bin_dir/magick" "$bin_dir/convert" +} + test::install_failing_imagemagick() { local -r bin_dir="$1"; shift |