fix(installer): eight fixes from an overnight bug-hunt and its review

I squashed these because the per-bug reasoning lives in todo.org, which this commit carries.

Two could cost a machine. configure_initramfs_hook swapped the udev hook for systemd on a LUKS root, leaving a standalone encrypt hook under an init that never runs it. The rebuild succeeds and the installer exits clean, then the root won't unlock at the next boot. trim_firmware ran pacman -Rdd against twelve firmware packages behind a DMI gate reading product_name, where "Framework" never appears. That left it dead on the hardware it targets, and dangerous to fix the obvious way: this machine is a Framework Desktop whose Ryzen iGPU needs the amdgpu firmware. It refuses on PCI modalias evidence now.

wipedisk discarded before it checked. blkdiscard ran with -f, which disables the exclusive open, so picking the wrong disk destroyed a live filesystem and then reported that nothing had happened.

Four more are smaller. The NVIDIA preflight aborted dwm and headless installs over a driver floor they never need. zfs-replicate exited 0 after every dataset failed. Unattended installs blocked on two prompts, and the first fix for that inherited a [Y/n] default into passwordless console login. A fresh install left the dotfiles repo permanently dirty.

The review found a pattern worth more than any single fix. Helpers had thorough tests and none proved they were called. Deleting the call left five suites green, including the guard on that pacman -Rdd. CALL_SITES now pins nine caller/callee pairs.

The suite runs 341 tests at exit 0, with no new shellcheck findings. I proved every guard by deleting it and watching the intended test go red.

1 files changed, 216 insertions, 0 deletions

diff --git a/tests/installer-steps/test_mark_volatile_configs.py b/tests/installer-steps/test_mark_volatile_configs.py
new file mode 100644
index 0000000..27eac61
--- /dev/null
+++ b/tests/installer-steps/test_mark_volatile_configs.py
@@ -0,0 +1,216 @@
+"""Tests for mark_volatile_configs, the tail of the dotfiles install.
+
+Four stowed configs get rewritten in place by the apps that own them (btop,
+qalculate, calibre, waypaper). The dotfiles repo handles that with git's
+skip-worktree bit, applied by its own `skip-volatile` make target, which
+`make stow` runs as its last step.
+
+archsetup stows inline rather than through that Makefile, and it never ran the
+step -- so a machine installed by archsetup alone starts with a dotfiles repo
+that goes dirty as soon as those apps run, and every later `git pull --ff-only`
+trips over paths the user never edited. Both daily drivers carry the bits
+today, but archsetup contains no code that sets them: they were applied by a
+hand-run `make stow`.
+
+The fix calls the dotfiles Makefile's target rather than reimplementing the
+logic here, so the volatile list stays in one place. That is what these tests
+pin: that archsetup delegates, and that it degrades quietly when the checkout
+it was handed cannot answer.
+
+These tests exercise the REAL function body, extracted from the `archsetup`
+script at run time (not a copy), against a fixture git repo, with sudo stubbed
+on PATH.
+
+Run from repo root:
+    python3 -m unittest tests.installer-steps.test_mark_volatile_configs
+"""
+
+import os
+import shutil
+import subprocess
+import tempfile
+import textwrap
+import unittest
+
+
+REPO_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", ".."))
+ARCHSETUP = os.path.join(REPO_ROOT, "archsetup")
+
+# The real target, copied from the dotfiles Makefile. The point of the fixture
+# is that archsetup calls *a* skip-volatile target rather than doing the work
+# itself; the body here mirrors the real one so the assertions are meaningful.
+MAKEFILE = textwrap.dedent("""\
+    DOTFILES := $(shell pwd)
+    VOLATILE := $(shell sed 's/#.*//' $(DOTFILES)/volatile-configs 2>/dev/null)
+
+    skip-volatile:
+    \t@for f in $(VOLATILE); do \\
+    \t\tif git -C $(DOTFILES) ls-files --error-unmatch "$$f" >/dev/null 2>&1; then \\
+    \t\t\tgit -C $(DOTFILES) update-index --skip-worktree "$$f" && echo "  skip-worktree: $$f"; \\
+    \t\telse \\
+    \t\t\techo "  WARN: not tracked, skipping: $$f"; \\
+    \t\tfi; \\
+    \tdone
+""")
+
+# sudo isn't available (and isn't wanted) in a unit test. Drop `-u <user>` and
+# exec the rest, so the function's real command line is what runs.
+SUDO_STUB = textwrap.dedent("""\
+    #!/bin/bash
+    # Record the argv archsetup handed sudo, then drop `-u <user>` and exec the
+    # rest, so the function's real command line is what runs.
+    printf '%s\\n' "$*" >> "$SUDO_CALLS"
+    if [ "$1" = "-u" ]; then shift 2; fi
+    exec "$@"
+""")
+
+
+class MarkVolatileConfigsHarness(unittest.TestCase):
+    """Source mark_volatile_configs out of the real archsetup script."""
+
+    def setUp(self):
+        self.tmp = tempfile.mkdtemp(prefix="volatile-test-")
+        self.repo = os.path.join(self.tmp, "dotfiles")
+        os.mkdir(self.repo)
+        self.bindir = os.path.join(self.tmp, "bin")
+        os.mkdir(self.bindir)
+        sudo = os.path.join(self.bindir, "sudo")
+        with open(sudo, "w") as f:
+            f.write(SUDO_STUB)
+        os.chmod(sudo, 0o755)
+        self.logfile = os.path.join(self.tmp, "install.log")
+        self.sudo_calls = os.path.join(self.tmp, "sudo-calls")
+        self.wrapper = os.path.join(self.tmp, "run.sh")
+        with open(self.wrapper, "w") as f:
+            f.write(
+                "#!/bin/bash\n"
+                'ARCHSETUP="$1"; shift\n'
+                "source <(sed -n "
+                "'/^mark_volatile_configs() {/,/^}/p' \"$ARCHSETUP\")\n"
+                'display() { echo "DISPLAY:$2"; }\n'
+                'error_warn() { echo "WARN:$1"; return 1; }\n'
+                'mark_volatile_configs "$REPO" testuser\n'
+                'echo "RETURNED=$?"\n'
+            )
+        os.chmod(self.wrapper, 0o755)
+
+    def tearDown(self):
+        shutil.rmtree(self.tmp, ignore_errors=True)
+
+    def git(self, *args):
+        return subprocess.run(["git", "-C", self.repo, *args],
+                              capture_output=True, text=True)
+
+    def make_repo(self, tracked=("common/.config/btop/btop.conf",),
+                  volatile=None, makefile: "str | None" = MAKEFILE):
+        self.git("init", "-q")
+        self.git("config", "user.email", "t@example.com")
+        self.git("config", "user.name", "t")
+        for rel in tracked:
+            path = os.path.join(self.repo, rel)
+            os.makedirs(os.path.dirname(path), exist_ok=True)
+            with open(path, "w") as f:
+                f.write("original\n")
+        listed = tracked if volatile is None else volatile
+        with open(os.path.join(self.repo, "volatile-configs"), "w") as f:
+            f.write("# apps rewrite these in place\n")
+            f.write("".join(rel + "\n" for rel in listed))
+        if makefile is not None:
+            with open(os.path.join(self.repo, "Makefile"), "w") as f:
+                f.write(makefile)
+        self.git("add", "-A")
+        self.git("commit", "-q", "-m", "fixture")
+
+    def run_step(self):
+        env = dict(os.environ)
+        env["PATH"] = self.bindir + os.pathsep + env["PATH"]
+        env["REPO"] = self.repo
+        env["logfile"] = self.logfile
+        env["SUDO_CALLS"] = self.sudo_calls
+        return subprocess.run(["bash", self.wrapper, ARCHSETUP],
+                              capture_output=True, text=True, env=env)
+
+    def sudo_argv(self):
+        """The argument lines sudo was called with, one per invocation."""
+        if not os.path.exists(self.sudo_calls):
+            return []
+        with open(self.sudo_calls) as f:
+            return [line.rstrip("\n") for line in f if line.strip()]
+
+    def skipped(self):
+        """Paths carrying the skip-worktree bit."""
+        r = self.git("ls-files", "-v")
+        return sorted(line[2:] for line in r.stdout.splitlines()
+                      if line.startswith("S "))
+
+    # ---------------------------------------------------------- normal ----
+    def test_sets_skip_worktree_on_every_volatile_path(self):
+        paths = ["common/.config/btop/btop.conf",
+                 "hyprland/.config/waypaper/config.ini"]
+        self.make_repo(tracked=paths)
+        self.assertEqual(self.skipped(), [])   # the bit is not set by default
+        r = self.run_step()
+        self.assertIn("RETURNED=0", r.stdout,
+                      "stdout=%r stderr=%r" % (r.stdout, r.stderr))
+        self.assertEqual(self.skipped(), sorted(paths))
+
+    def test_runs_make_as_the_user_not_as_root(self):
+        # archsetup runs as root against a user-owned checkout. Letting root
+        # write .git/index leaves it root-owned, and the user's next git
+        # command then cannot update the index at all.
+        self.make_repo()
+        self.run_step()
+        self.assertEqual(self.sudo_argv(),
+                         ["-u testuser make -C %s skip-volatile" % self.repo])
+
+    def test_leaves_non_volatile_tracked_files_alone(self):
+        self.make_repo(tracked=["common/.config/btop/btop.conf",
+                                "common/.bashrc"],
+                       volatile=["common/.config/btop/btop.conf"])
+        self.run_step()
+        self.assertEqual(self.skipped(), ["common/.config/btop/btop.conf"])
+
+    # -------------------------------------------------------- boundary ----
+    def test_no_makefile_is_a_quiet_no_op(self):
+        # DOTFILES_DIR can point at a checkout that predates the target, or at
+        # a fork. Nothing to delegate to is not an error.
+        self.make_repo(makefile=None)
+        r = self.run_step()
+        self.assertIn("RETURNED=0", r.stdout)
+        self.assertNotIn("WARN:", r.stdout)
+        self.assertEqual(self.skipped(), [])
+
+    def test_empty_volatile_list_sets_nothing_and_does_not_warn(self):
+        self.make_repo(tracked=["common/.config/btop/btop.conf"], volatile=[])
+        r = self.run_step()
+        self.assertIn("RETURNED=0", r.stdout)
+        self.assertNotIn("WARN:", r.stdout)
+        self.assertEqual(self.skipped(), [])
+
+    def test_untracked_path_in_the_list_does_not_fail_the_step(self):
+        # The Makefile warns and carries on; a stale entry must not take the
+        # install down, and the tracked entries still get their bit.
+        self.make_repo(tracked=["common/.config/btop/btop.conf"],
+                       volatile=["common/.config/btop/btop.conf",
+                                 "common/.config/gone/removed.conf"])
+        r = self.run_step()
+        self.assertIn("RETURNED=0", r.stdout)
+        self.assertEqual(self.skipped(), ["common/.config/btop/btop.conf"])
+
+    # ----------------------------------------------------------- error ----
+    def test_makefile_without_the_target_warns_and_returns(self):
+        self.make_repo(makefile="all:\n\t@true\n")
+        r = self.run_step()
+        self.assertIn("WARN:", r.stdout)
+        self.assertEqual(self.skipped(), [])
+
+    def test_make_output_goes_to_the_logfile_not_the_console(self):
+        self.make_repo(tracked=["common/.config/btop/btop.conf"])
+        r = self.run_step()
+        self.assertNotIn("skip-worktree:", r.stdout)
+        with open(self.logfile) as f:
+            self.assertIn("skip-worktree:", f.read())
+
+
+if __name__ == "__main__":
+    unittest.main()