diff --git a/migrations/1788009111.sh b/migrations/1788009111.sh index 1f0d67f5..fc2d262f 100644 --- a/migrations/1788009111.sh +++ b/migrations/1788009111.sh @@ -81,15 +81,28 @@ while IFS= read -r queue; do continue fi - # Removing a queue aborts what is printing on it. The implicitclass backend - # only needs cups-browsed to pick a destination, so a job already past that - # point finishes on its own even though the daemon has stopped -- and a job - # that has not is one this cannot route anyway. Either way the queue is left - # for the person whose job it is, and named so they know to remove it. + # Close the queue to new jobs before looking at what is on it. Otherwise a job + # submitted between the check and the removal -- the sudo below can sit at a + # password prompt for as long as someone takes to type it -- is cancelled by a + # deletion that decided the queue was empty. It also stops more jobs piling + # onto a queue that is being left behind and can no longer route them. + if ! sudo cupsreject -r "Printer discovery has been removed from Omarchy" "$queue"; then + echo " Could not stop $queue accepting new jobs, so it is being left alone." + unremoved=1 + continue + fi + + # Removing a queue cancels the jobs on it. implicitclass needs cups-browsed + # only to pick a destination, so a job already past that point finishes on its + # own; one still waiting cannot, because the daemon that would route it has + # stopped. Neither is this migration's to throw away, so the queue is left for + # whoever owns them, and what will and will not happen is said rather than + # implied. if job_report=$(LC_ALL=C lpstat -o "$queue" 2>/dev/null); then if [[ -n $job_report ]]; then - echo " $queue still has jobs, so it is being left alone." - echo " Once they finish or are cancelled, remove it in Print Settings; it cannot print again." + echo " $queue still has jobs and is no longer taking new ones, so it is being left alone." + echo " Anything already sent to the printer finishes; anything still waiting cannot be routed now." + echo " Cancel what is left and remove the queue in Print Settings." continue fi else @@ -97,9 +110,14 @@ while IFS= read -r queue; do continue fi + # A queue another administrator removed while this was running is a queue that + # is gone, which is the outcome wanted -- not a failure worth keeping the + # package for. if ! sudo lpadmin -x "$queue"; then - echo " Could not remove the queue $queue." - unremoved=1 + if LC_ALL=C lpstat -p "$queue" >/dev/null 2>&1; then + echo " Could not remove the queue $queue." + unremoved=1 + fi fi done <<<"$generated_queues" diff --git a/test/shell.d/cups-browsed-removal-migration-test.sh b/test/shell.d/cups-browsed-removal-migration-test.sh index 82797b4d..63f6a053 100644 --- a/test/shell.d/cups-browsed-removal-migration-test.sh +++ b/test/shell.d/cups-browsed-removal-migration-test.sh @@ -68,6 +68,11 @@ case $1 in printf '%s\n' "$BROWSED_QUEUES" | sed 's|^device for |Gerät für |' fi ;; + -p) + # A destination that no longer exists is unknown to lpstat. + [[ " $BROWSED_GONE_QUEUES " != *" $2 "* ]] + exit $? + ;; -o) [[ -z $BROWSED_LPSTAT_O_FAILS ]] || exit 1 # Jobs are listed per queue: "- ". @@ -83,6 +88,12 @@ printf 'lpadmin\t%s\n' "$*" >>"$BROWSED_LOG" [[ -z $BROWSED_LPADMIN_FAILS ]] SH +cat >"$mock_bin/cupsreject" <<'SH' +#!/bin/bash +printf 'cupsreject\t%s\n' "$*" >>"$BROWSED_LOG" +[[ -z $BROWSED_REJECT_FAILS ]] +SH + chmod +x "$mock_bin"/* log="$test_tmp/actions.log" @@ -107,6 +118,8 @@ run_migration() { BROWSED_LPSTAT_FAILS="${lpstat_fails:-}" \ BROWSED_LPADMIN_FAILS="${lpadmin_fails:-}" \ BROWSED_LPSTAT_O_FAILS="${lpstat_o_fails:-}" \ + BROWSED_REJECT_FAILS="${reject_fails:-}" \ + BROWSED_GONE_QUEUES="${gone_queues:-}" \ PATH="$mock_bin:$PATH" \ OMARCHY_PATH="$ROOT" \ OMARCHY_CUPS_BROWSED_REMOVAL_MARKER="${use_marker:-$marker}" \ @@ -262,6 +275,19 @@ if grep -q 'lpadmin -x Office' "$log"; then fi grep -qxF $'sudo\tlpadmin -x Spare' "$log" || fail "an idle generated queue is still removed" "$(cat "$log")" + +# A job submitted between the check and the deletion -- the sudo in between can +# sit at a password prompt -- would be cancelled by a deletion that had decided +# the queue was empty. +reject_line=$(grep -n 'cupsreject.*Spare' "$log" | head -1 | cut -d: -f1 || true) +probe_line=$(grep -n $'^lpstat\t-o Spare' "$log" | head -1 | cut -d: -f1 || true) +delete_line=$(grep -n 'lpadmin -x Spare' "$log" | head -1 | cut -d: -f1 || true) +[[ -n $reject_line && -n $probe_line && -n $delete_line ]] || + fail "a queue is closed, inspected and removed in that order" "$(cat "$log")" +(( reject_line < probe_line && probe_line < delete_line )) || + fail "a queue stops taking new jobs before it is inspected or removed" "$(cat "$log")" +pass "a queue stops taking new jobs before it is inspected or removed" + grep -q 'Office still has jobs' "$output" || fail "a queue left alone is named so it can be removed later" "$(cat "$output")" # One printer's job must not keep discovery on the machine. @@ -341,6 +367,52 @@ pass "a queue that will not go keeps the package and the marker back" lpadmin_fails="" +# --------------------------------------------- a queue removed by someone else + +# Another administrator deleting the queue mid-run is the outcome wanted, not a +# failure worth keeping the package installed for. +installed="cups-browsed" +enabled="cups-browsed.service" +active="cups-browsed.service cups.service" +blocked="" +queues=$'device for Office: implicitclass://Office/' +busy="" +lpstat_fails="" +lpstat_o_fails="" +lpadmin_fails="1" +gone_queues="Office" +use_marker="$test_tmp/var/lib/omarchy/migrations/vanished" +run_migration + +grep -qxF $'sudo\tpacman -R --noconfirm cups-browsed' "$log" || + fail "a queue that is already gone does not hold up the removal" "$(cat "$log")" +[[ -f $use_marker ]] || fail "a queue that is already gone still finishes the migration" +pass "a queue someone else removed counts as removed" + +lpadmin_fails="" +gone_queues="" + +# ------------------------------------------- a queue that will not stop taking jobs + +# Deleting a queue that is still accepting work races whatever arrives next. +installed="cups-browsed" +enabled="cups-browsed.service" +active="cups-browsed.service cups.service" +reject_fails="1" +use_marker="$test_tmp/var/lib/omarchy/migrations/openqueue" +run_migration + +if grep -q 'lpadmin -x' "$log"; then + fail "a queue still accepting jobs is not deleted" "$(cat "$log")" +fi +if grep -q $'^pacman\t-R --noconfirm' "$log"; then + fail "the package waits while a queue is still accepting jobs" "$(cat "$log")" +fi +[[ ! -e $use_marker ]] || fail "a queue still taking jobs is not recorded as done" +pass "a queue that will not stop taking jobs is neither inspected nor deleted" + +reject_fails="" + # -------------------------------------------------- a masked but running daemon installed="cups-browsed"