ZacharyZhang-NY/omarchycn
3
4
Fork 0
Code Issues Pull Requests 2 Actions Packages Projects Releases 5 Wiki Activity

Close each discovered queue before inspecting or removing it

Browse Source 
Checking a queue for jobs and then deleting it leaves a window in between, and the sudo that does the deleting can sit at a password prompt for as long as someone takes to type. A job submitted in that window was cancelled by a deletion that had already decided the queue was empty. cupsreject closes the queue to new work first, which also stops more jobs piling onto one that is being left behind and can no longer route them. It comes from the cups package, which stays.

What a queue with jobs on it means is now said rather than implied: whatever is already at the printer finishes, whatever is still waiting cannot be routed with the daemon gone, and the person who owns them has to cancel what is left.

A queue that another administrator removed while this was running is the outcome wanted, not a failure to keep the package installed for, so a deletion that fails is checked against whether the destination is still there.

Co-Authored-By: Codex XHigh <noreply@openai.com>
This commit is contained in:
David Heinemeier Hansson
2026-08-29 17:44:31 +02:00
co-authored by Codex XHigh
parent 24c18df5b7
commit 1dbc7d5bce
2 changed files with 99 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
migrations/1788009111.sh
+27 -9
View File
@@ -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"
test/shell.d/cups-browsed-removal-migration-test.sh
+72
View File
@@ -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: "<queue>-<id> <user> <size>".
@@ -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"