From 3bfea9b840a488129272e500dd988c8f6c9e5a3e Mon Sep 17 00:00:00 2001 From: Nille af Ekenstam Date: Fri, 7 Aug 2026 18:44:37 +0200 Subject: [PATCH] Fail loudly when a pre-update snapshot isn't actually created (#6580) * Fail the snapshot when Snapper is installed but has no configs omarchy-snapshot create loops over the configs snapper reports. With none, the loop body never runs, so it prints "Create system snapshot" and exits 0 without capturing anything. Every update then reports a snapshot it never took, and the absence only surfaces when a rollback is needed and the snapshot list turns out to be empty. * Say so when the update proceeds without a snapshot The update ignores exit 127 so a system without snapper updates quietly. Any other snapshot failure was being swallowed by the same expression, which let the update continue with no indication that it was now unprotected. Keep continuing, but say it out loud. * Point the snapshot repair hint at how the installer runs it Also hold the green header until a snapshot will actually be attempted, so the no-config failure doesn't open with a success banner. Co-Authored-By: Claude Fable 5 * Continue the quattro upgrade when the pre-upgrade snapshot fails The upgrade runs under set -e, so the new non-zero exit from an unconfigured Snapper would have aborted a re-run at the snapshot step instead of proceeding like omarchy-update does. Co-Authored-By: Claude Fable 5 --------- Co-authored-by: David Heinemeier Hansson Co-authored-by: Claude Fable 5 --- bin/omarchy-snapshot | 13 +++- bin/omarchy-update | 6 +- bin/omarchy-upgrade-to-quattro | 6 +- test/shell.d/snapshot-create-test.sh | 104 +++++++++++++++++++++++++++ 4 files changed, 125 insertions(+), 4 deletions(-) create mode 100644 test/shell.d/snapshot-create-test.sh diff --git a/bin/omarchy-snapshot b/bin/omarchy-snapshot index 261d1883..184d5892 100755 --- a/bin/omarchy-snapshot +++ b/bin/omarchy-snapshot @@ -22,11 +22,20 @@ create) # The description is just a label, so never let it abort the snapshot. DESC="$(omarchy-version 2>/dev/null || echo unknown)" - echo -e "\e[32mCreate system snapshot\e[0m" - # Get existing snapper config names from CSV output mapfile -t CONFIGS < <(sudo snapper --csvout list-configs | awk -F, 'NR>1 {print $1}') + # Snapper installed but unconfigured snapshots nothing. Staying quiet here + # reads as a successful snapshot, so the next update looks recoverable when + # nothing has ever been captured. + if (( ${#CONFIGS[@]} == 0 )); then + echo -e "\e[33mNo Snapper configs found, so no snapshot was created.\e[0m" >&2 + echo "Configure Snapper with: sudo bash -euo pipefail \"$OMARCHY_PATH/install/config/snapper.sh\"" >&2 + exit 1 + fi + + echo -e "\e[32mCreate system snapshot\e[0m" + for config in "${CONFIGS[@]}"; do sudo snapper -c "$config" create -c number -d "$DESC" sudo snapper -c "$config" cleanup number diff --git a/bin/omarchy-update b/bin/omarchy-update index ccf0884b..b45072aa 100755 --- a/bin/omarchy-update +++ b/bin/omarchy-update @@ -22,7 +22,11 @@ trap 'omarchy-update-stay-awake stop' EXIT omarchy-update-requires-free-space if [[ ${1:-} == "-y" ]] || omarchy-update-confirm; then - omarchy-snapshot create || (($? == 127)) + # 127 means Snapper is deliberately absent. Any other failure already said + # what went wrong, and a missing snapshot is not worth blocking an update + # over, but it must not pass for one either. + omarchy-snapshot create || (($? == 127)) || + echo -e "\e[33mContinuing the update without a snapshot.\e[0m" >&2 omarchy-update-stay-awake start diff --git a/bin/omarchy-upgrade-to-quattro b/bin/omarchy-upgrade-to-quattro index 974f956f..ef2cb509 100755 --- a/bin/omarchy-upgrade-to-quattro +++ b/bin/omarchy-upgrade-to-quattro @@ -610,7 +610,11 @@ configure_lock_authentication() { } create_pre_upgrade_snapshot() { - omarchy-snapshot create || (($? == 127)) + # 127 means Snapper is deliberately absent. Any other failure already said + # what went wrong, and a missing snapshot is not worth blocking the upgrade + # over, but it must not pass for one either. + omarchy-snapshot create || (($? == 127)) || + echo -e "\e[33mContinuing the upgrade without a snapshot.\e[0m" >&2 } remove_conflicting_legacy_packages() { diff --git a/test/shell.d/snapshot-create-test.sh b/test/shell.d/snapshot-create-test.sh new file mode 100644 index 00000000..befe64c0 --- /dev/null +++ b/test/shell.d/snapshot-create-test.sh @@ -0,0 +1,104 @@ +#!/bin/bash + +set -euo pipefail + +source "$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)/base-test.sh" + +snapshot="$ROOT/bin/omarchy-snapshot" + +test_tmp=$(mktemp -d) +trap 'rm -rf "$test_tmp"' EXIT + +fake_bin="$test_tmp/bin" +mkdir -p "$fake_bin" + +cat >"$fake_bin/sudo" <<'STUB' +#!/bin/bash +exec "$@" +STUB +chmod +x "$fake_bin/sudo" + +cat >"$fake_bin/omarchy-cmd-missing" <<'STUB' +#!/bin/bash +exit 1 +STUB +chmod +x "$fake_bin/omarchy-cmd-missing" + +cat >"$fake_bin/omarchy-version" <<'STUB' +#!/bin/bash +echo 4.0.0 +STUB +chmod +x "$fake_bin/omarchy-version" + +# Snapper with no configs: list-configs prints only the CSV header. +cat >"$fake_bin/snapper" <<'STUB' +#!/bin/bash +printf 'snapper %s\n' "$*" >>"$TEST_LOG" +if [[ "$*" == *"list-configs"* ]]; then + echo "config,subvolume" +fi +STUB +chmod +x "$fake_bin/snapper" + +# A snapshot that silently creates nothing reads as a successful snapshot, so +# an unconfigured Snapper has to fail loudly instead of passing for a backup. +: >"$test_tmp/calls.log" +set +e +stderr=$(TEST_LOG="$test_tmp/calls.log" PATH="$fake_bin:$PATH" \ + bash "$snapshot" create 2>&1 >/dev/null) +status=$? +set -e + +(( status != 0 )) || fail "snapshot create fails when Snapper has no configs" +grep -qF 'No Snapper configs found' <<<"$stderr" || + fail "snapshot create reports that no snapshot was created" "$stderr" +! grep -q '^snapper -c .* create ' "$test_tmp/calls.log" || + fail "snapshot create does not invent a config to snapshot" +pass "snapshot create fails loudly when Snapper is installed but unconfigured" + +cat >"$fake_bin/snapper" <<'STUB' +#!/bin/bash +printf 'snapper %s\n' "$*" >>"$TEST_LOG" +if [[ "$*" == *"list-configs"* ]]; then + echo "config,subvolume" + echo "root,/" +fi +STUB +chmod +x "$fake_bin/snapper" + +: >"$test_tmp/calls.log" +TEST_LOG="$test_tmp/calls.log" PATH="$fake_bin:$PATH" \ + bash "$snapshot" create >/dev/null + +grep -qFx 'snapper -c root create -c number -d 4.0.0' "$test_tmp/calls.log" || + fail "snapshot create snapshots each configured subvolume" "$(cat "$test_tmp/calls.log")" +grep -qFx 'snapper -c root cleanup number' "$test_tmp/calls.log" || + fail "snapshot create prunes older snapshots" +pass "snapshot create snapshots every configured Snapper config" + +# Snapper being deliberately absent is the one skip that stays quiet, and the +# update has to keep treating it as such. +cat >"$fake_bin/omarchy-cmd-missing" <<'STUB' +#!/bin/bash +exit 0 +STUB +chmod +x "$fake_bin/omarchy-cmd-missing" + +set +e +TEST_LOG="$test_tmp/calls.log" PATH="$fake_bin:$PATH" \ + bash "$snapshot" create >/dev/null 2>&1 +status=$? +set -e + +(( status == 127 )) || fail "snapshot create exits 127 without snapper" "got $status" +grep -qF 'omarchy-snapshot create || (($? == 127))' "$ROOT/bin/omarchy-update" || + fail "update ignores only the missing-snapper exit code" +pass "snapshot create keeps the quiet 127 path for systems without snapper" + +# The quattro upgrade runs under set -e, so a failed snapshot has to be warned +# past there too or it aborts the whole upgrade at the snapshot step. +grep -qF 'omarchy-snapshot create || (($? == 127))' "$ROOT/bin/omarchy-upgrade-to-quattro" || + fail "upgrade ignores only the missing-snapper exit code" +grep -qF 'Continuing the upgrade without a snapshot' "$ROOT/bin/omarchy-upgrade-to-quattro" || + fail "upgrade continues past a failed snapshot instead of aborting" +pass "upgrade to quattro survives a failed snapshot without passing it off"