From 3becfac66dc569e71d0f6085fb56e92cfa6d626a Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Fri, 24 Jul 2026 12:19:04 -0500 Subject: 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. --- .../test_framework_firmware_trim.py | 233 +++++++++++++++++++++ 1 file changed, 233 insertions(+) create mode 100644 tests/installer-steps/test_framework_firmware_trim.py (limited to 'tests/installer-steps/test_framework_firmware_trim.py') diff --git a/tests/installer-steps/test_framework_firmware_trim.py b/tests/installer-steps/test_framework_firmware_trim.py new file mode 100644 index 0000000..cdb6a60 --- /dev/null +++ b/tests/installer-steps/test_framework_firmware_trim.py @@ -0,0 +1,233 @@ +"""Tests for is_framework_intel_laptop, the gate on trim_firmware. + +trim_firmware runs `pacman -Rdd` against twelve linux-firmware packages, +keeping only the three a Framework 13 Intel needs (intel, atheros, realtek). +It is the most destructive conditional in the installer, so the condition is +what these tests pin. + +The gate has to be exact in both directions. Too narrow and the step is dead +code: "framework" lives in DMI's sys_vendor, never in product_name, so a gate +reading product_name alone matches no Framework machine at all. Too wide and it +removes linux-firmware-amdgpu from a Framework Desktop, whose Ryzen AI Max iGPU +needs it to bring up a display. Both machines are real: velox is +Framework/"Laptop (13th Gen Intel Core)" and must trim; ratio is +Framework/"Desktop (AMD Ryzen AI Max 300 Series)" and must not. + +These tests exercise the REAL function body, extracted from the `archsetup` +script at run time (not a copy), against a fixture DMI directory. + +Run from repo root: + python3 -m unittest tests.installer-steps.test_framework_firmware_trim +""" + +import os +import shutil +import subprocess +import tempfile +import unittest + + +REPO_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "..")) +ARCHSETUP = os.path.join(REPO_ROOT, "archsetup") + + +class FrameworkIntelGateHarness(unittest.TestCase): + """Source is_framework_intel_laptop out of the real archsetup script.""" + + def setUp(self): + self.tmp = tempfile.mkdtemp(prefix="fw-trim-test-") + self.dmi = os.path.join(self.tmp, "dmi") + os.mkdir(self.dmi) + 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 " + "'/^is_framework_intel_laptop() {/,/^}/p' \"$ARCHSETUP\")\n" + 'if is_framework_intel_laptop "$DMI_DIR"; then echo "TRIM=yes"; ' + 'else echo "TRIM=no"; fi\n' + ) + os.chmod(self.wrapper, 0o755) + + def tearDown(self): + shutil.rmtree(self.tmp, ignore_errors=True) + + def dmi_write(self, **fields): + for name, value in fields.items(): + with open(os.path.join(self.dmi, name), "w") as f: + f.write(value + "\n") + + def trims(self, dmi_dir=None): + env = dict(os.environ) + env["DMI_DIR"] = self.dmi if dmi_dir is None else dmi_dir + r = subprocess.run(["bash", self.wrapper, ARCHSETUP], + capture_output=True, text=True, env=env) + if "TRIM=yes" in r.stdout: + return True + if "TRIM=no" in r.stdout: + return False + self.fail("no TRIM line; stdout=%r stderr=%r" % (r.stdout, r.stderr)) + + # ---------------------------------------------------------- normal ---- + def test_velox_framework_13_intel_trims(self): + # The exact strings read off velox on 2026-07-24. + self.dmi_write(sys_vendor="Framework", + product_name="Laptop (13th Gen Intel Core)") + self.assertTrue(self.trims()) + + def test_framework_12_intel_trims(self): + self.dmi_write(sys_vendor="Framework", + product_name="Laptop 12 (13th Gen Intel Core)") + self.assertTrue(self.trims()) + + # -------------------------------------------------------- boundary ---- + def test_ratio_framework_desktop_amd_does_not_trim(self): + # The exact strings read off ratio on 2026-07-24. Trimming here would + # remove linux-firmware-amdgpu from the machine whose iGPU needs it. + self.dmi_write(sys_vendor="Framework", + product_name="Desktop (AMD Ryzen AI Max 300 Series)") + self.assertFalse(self.trims()) + + def test_framework_16_amd_laptop_does_not_trim(self): + self.dmi_write(sys_vendor="Framework", + product_name="Laptop 16 (AMD Ryzen 7040 Series)") + self.assertFalse(self.trims()) + + def test_non_framework_intel_laptop_does_not_trim(self): + # The firmware set is only known for Framework hardware. The model + # deliberately carries both "Laptop" and "Intel", so the vendor check + # is the only condition that can reject it -- a fixture that failed the + # model checks too would pass no matter what the vendor check did. + self.dmi_write(sys_vendor="Dell Inc.", + product_name="Laptop 15 (Intel Core i7)") + self.assertFalse(self.trims()) + + def test_framework_desktop_naming_an_intel_cpu_does_not_trim(self): + # No such machine ships today -- the Framework Desktop is AMD -- but + # "an Intel Framework desktop will never exist" is exactly the kind of + # assumption that made the old gate wrong. The model must say Laptop, + # so a desktop is refused on the model rather than on its CPU vendor. + self.dmi_write(sys_vendor="Framework", + product_name="Desktop (Intel Core Ultra)") + self.assertFalse(self.trims()) + + def test_vendor_match_is_case_insensitive(self): + self.dmi_write(sys_vendor="framework", + product_name="Laptop (13th Gen intel Core)") + self.assertTrue(self.trims()) + + def test_vendor_with_surrounding_whitespace_still_matches(self): + # DMI strings are vendor-supplied and often padded. + self.dmi_write(sys_vendor=" Framework ", + product_name="Laptop (13th Gen Intel Core)") + self.assertTrue(self.trims()) + + # ----------------------------------------------------------- error ---- + def test_missing_dmi_directory_does_not_trim(self): + self.assertFalse(self.trims(dmi_dir=os.path.join(self.tmp, "nope"))) + + def test_missing_product_name_does_not_trim(self): + self.dmi_write(sys_vendor="Framework") + self.assertFalse(self.trims()) + + def test_missing_sys_vendor_does_not_trim(self): + self.dmi_write(product_name="Laptop (13th Gen Intel Core)") + self.assertFalse(self.trims()) + + def test_empty_dmi_values_do_not_trim(self): + self.dmi_write(sys_vendor="", product_name="") + self.assertFalse(self.trims()) + +class GpuVendorEvidence(unittest.TestCase): + """trim_firmware deletes GPU firmware, so what it needs to know is which + GPU is physically present -- and the kernel already answers that. + + The DMI model string cannot. It is a marketing name: it does not know a + swapped card, and it cannot see hardware the allowlist has never met. The + same file already reads PCI modalias twice (install_gpu_drivers, and + nvidia_preflight_report, which parameterises its globs for exactly this + kind of test). detect_gpu_vendors is that read, made reusable, so the trim + can refuse on evidence rather than on a name. + """ + + def setUp(self): + self.tmp = tempfile.mkdtemp(prefix="gpu-vendor-test-") + self.drm = os.path.join(self.tmp, "drm") + self.pci = os.path.join(self.tmp, "pci") + os.makedirs(self.drm) + os.makedirs(self.pci) + 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 " + "'/^detect_gpu_vendors() {/,/^}/p' \"$ARCHSETUP\")\n" + 'detect_gpu_vendors "$DRM_GLOB" "$PCI_GLOB"\n' + ) + os.chmod(self.wrapper, 0o755) + + def tearDown(self): + shutil.rmtree(self.tmp, ignore_errors=True) + + def card(self, root, name, modalias): + d = os.path.join(root, name, "device") + os.makedirs(d, exist_ok=True) + with open(os.path.join(d, "modalias"), "w") as f: + f.write(modalias + "\n") + + def vendors(self): + env = dict(os.environ) + env["DRM_GLOB"] = os.path.join(self.drm, "card*", "device", "modalias") + env["PCI_GLOB"] = os.path.join(self.pci, "*", "device", "modalias") + r = subprocess.run(["bash", self.wrapper, ARCHSETUP], + capture_output=True, text=True, env=env) + return sorted(v for v in r.stdout.split() if v) + + # ---------------------------------------------------------- normal ---- + def test_intel_igpu_detected(self): + self.card(self.drm, "card0", "pci:v00008086d00007D55sv00001414bc03sc00i00") + self.assertEqual(self.vendors(), ["intel"]) + + def test_amd_igpu_detected(self): + self.card(self.drm, "card0", "pci:v00001002d0000150Esv00001414bc03sc00i00") + self.assertEqual(self.vendors(), ["amd"]) + + # -------------------------------------------------------- boundary ---- + def test_nvidia_detected_in_either_hex_case(self): + self.card(self.drm, "card0", "pci:v000010DEd00002684bc03sc00i00") + self.assertEqual(self.vendors(), ["nvidia"]) + shutil.rmtree(os.path.join(self.drm, "card0")) + self.card(self.drm, "card0", "pci:v000010ded00002684bc03sc00i00") + self.assertEqual(self.vendors(), ["nvidia"]) + + def test_hybrid_machine_reports_both(self): + self.card(self.drm, "card0", "pci:v00008086d00007D55bc03sc00i00") + self.card(self.drm, "card1", "pci:v000010DEd00002684bc03sc00i00") + self.assertEqual(self.vendors(), ["intel", "nvidia"]) + + def test_falls_back_to_pci_when_drm_is_empty(self): + # Early boot / chroot: no DRM nodes yet, same ruling install_gpu_drivers + # already makes. + self.card(self.pci, "0000:00:02.0", "pci:v00001002d0000150Ebc03sc00i00") + self.assertEqual(self.vendors(), ["amd"]) + + def test_pci_fallback_ignores_non_display_devices(self): + # An NVIDIA audio function on the same card is bc04, not bc03. + self.card(self.pci, "0000:01:00.1", "pci:v000010DEd000022BAbc04sc03i00") + self.assertEqual(self.vendors(), []) + + # ----------------------------------------------------------- error ---- + def test_no_gpu_anywhere_reports_nothing(self): + self.assertEqual(self.vendors(), []) + + def test_unreadable_modalias_is_skipped_not_fatal(self): + d = os.path.join(self.drm, "card0", "device") + os.makedirs(d) + os.mkdir(os.path.join(d, "modalias")) # a directory, not a file + self.assertEqual(self.vendors(), []) + + +if __name__ == "__main__": + unittest.main() -- cgit v1.2.3