From f9da097042a28c0bdff037b0b53ae20d567cd5c4 Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Tue, 21 Jul 2026 06:50:12 -0500 Subject: fix(installer): merge GRUB cmdline instead of overwriting it configure_grub replaced the whole GRUB_CMDLINE_LINUX_DEFAULT line with a fixed string. A base install that had set cryptdevice=, resume=, zfs=, or any other boot-critical parameter lost it, and the grub-mkconfig that follows baked an unbootable config. update_grub_cmdline now reads the current value and merges: every existing token survives, archsetup's tokens are added, and where both set the same key archsetup's value wins. A safety check refuses to write when any existing token's key would vanish from the merge, and the write goes through awk + mv so paths with slashes can't break the substitution. The block also backs up /etc/default/grub before editing, which the other system-file edits already did. --- archsetup | 61 ++++++++++++++- tests/installer-steps/test_grub_cmdline.py | 121 +++++++++++++++++++++++++++++ 2 files changed, 181 insertions(+), 1 deletion(-) create mode 100644 tests/installer-steps/test_grub_cmdline.py diff --git a/archsetup b/archsetup index b1fc44b..be5b3f4 100755 --- a/archsetup +++ b/archsetup @@ -3163,6 +3163,64 @@ trim_firmware() { } +# Merge an existing GRUB cmdline with archsetup's desired tokens. Every +# existing token survives (this is what keeps cryptdevice=/resume=/zfs= and +# any other boot-critical parameter alive); where both set the same key, +# archsetup's value wins. Args: $1 existing value, $2 desired tokens. Prints +# the merged value. +merge_grub_cmdline() { + local existing="$1" desired="$2" + local out="" tok key dtok dkey replaced + for tok in $existing; do + key="${tok%%=*}" + replaced="" + for dtok in $desired; do + dkey="${dtok%%=*}" + [ "$key" = "$dkey" ] && { replaced="$dtok"; break; } + done + out="$out ${replaced:-$tok}" + done + for dtok in $desired; do + dkey="${dtok%%=*}" + case " $out " in + *" $dkey "*|*" $dkey="*) ;; + *) out="$out $dtok" ;; + esac + done + printf '%s\n' "${out# }" +} + +# Rewrite GRUB_CMDLINE_LINUX_DEFAULT in $1 (default /etc/default/grub) as the +# merge of its current value and archsetup's tokens. The old code replaced the +# whole line with a fixed string, which dropped a base install's boot-critical +# parameters and let grub-mkconfig bake an unbootable config. A safety check +# refuses to write if any existing token's key would vanish from the merge. +update_grub_cmdline() { + local file="${1:-/etc/default/grub}" + local desired="rw loglevel=2 rd.systemd.show_status=auto rd.udev.log_level=2 nvme.noacpi=1 mem_sleep_default=deep nowatchdog random.trust_cpu=off quiet splash" + local existing merged tok key + existing=$(sed -n 's/^[[:space:]]*GRUB_CMDLINE_LINUX_DEFAULT="\{0,1\}\([^"]*\)"\{0,1\}[[:space:]]*$/\1/p' "$file" | head -1) + merged=$(merge_grub_cmdline "$existing" "$desired") + for tok in $existing; do + key="${tok%%=*}" + case " $merged " in + *" $key "*|*" $key="*) ;; + *) error_warn "GRUB cmdline merge would drop '$tok'; leaving $file untouched" "1" + return 1 ;; + esac + done + if grep -q "^[[:space:]]*GRUB_CMDLINE_LINUX_DEFAULT=" "$file"; then + awk -v v="$merged" ' + /^[[:space:]]*GRUB_CMDLINE_LINUX_DEFAULT=/ { + print "GRUB_CMDLINE_LINUX_DEFAULT=\"" v "\""; next + } + { print }' "$file" > "$file.archsetup-tmp" \ + && mv "$file.archsetup-tmp" "$file" + else + printf 'GRUB_CMDLINE_LINUX_DEFAULT="%s"\n' "$merged" >> "$file" + fi +} + configure_grub() { # GRUB: reset timeouts, adjust log levels, larger menu for HiDPI screens, and show splashscreen # Note: nvme.noacpi=1 disables NVMe ACPI power management to prevent freezes on some drives. @@ -3171,12 +3229,13 @@ configure_grub() { action="configuring boot menu for silence and bootsplash" && display "task" "$action" if [ -f /etc/default/grub ]; then action="resetting timeouts and adjusting log levels on grub boot" && display "task" "$action" + backup_system_file /etc/default/grub sed -i "s/.*GRUB_TIMEOUT=.*/GRUB_TIMEOUT=2/g" /etc/default/grub sed -i "s/.*GRUB_DEFAULT=.*/GRUB_DEFAULT=0/g" /etc/default/grub sed -i 's/.*GRUB_TERMINAL_OUTPUT=console/GRUB_TERMINAL_OUTPUT=gfxterm/' /etc/default/grub sed -i 's/.*GRUB_GFXMODE=auto/GRUB_GFXMODE=1024x768/' /etc/default/grub sed -i "s/.*GRUB_RECORDFAIL_TIMEOUT=.*/GRUB_RECORDFAIL_TIMEOUT=2/g" /etc/default/grub - sed -i "s/.*GRUB_CMDLINE_LINUX_DEFAULT=.*/GRUB_CMDLINE_LINUX_DEFAULT=\"rw loglevel=2 rd.systemd.show_status=auto rd.udev.log_level=2 nvme.noacpi=1 mem_sleep_default=deep nowatchdog random.trust_cpu=off quiet splash\"/g" /etc/default/grub + update_grub_cmdline /etc/default/grub fi # Regenerate GRUB config after all modifications diff --git a/tests/installer-steps/test_grub_cmdline.py b/tests/installer-steps/test_grub_cmdline.py new file mode 100644 index 0000000..8c0b5b0 --- /dev/null +++ b/tests/installer-steps/test_grub_cmdline.py @@ -0,0 +1,121 @@ +"""Test the GRUB cmdline merge. + +Bug (P2): configure_grub rewrote the whole GRUB_CMDLINE_LINUX_DEFAULT line +with a fixed string. A base install that had set cryptdevice=/resume=/zfs= +(or any other boot-critical token) lost it, and the following grub-mkconfig +baked an unbootable config. + +The fix is a merge: every pre-existing token survives, archsetup's tokens are +added, and where both set the same key archsetup's value wins. A safety +assert refuses to write if any existing token's key would vanish. + +Method: sed-extract merge_grub_cmdline (pure) and update_grub_cmdline +(file-level, run against temp grub files with a fake error_warn). + +Run from repo root: + python3 -m unittest tests.installer-steps.test_grub_cmdline +""" + +import os +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") + +EXTRACT = ( + "source <(sed -n '/^merge_grub_cmdline() {{/,/^}}/p;" + "/^update_grub_cmdline() {{/,/^}}/p' \"{a}\")" +).format(a=ARCHSETUP) + + +def run(body): + script = f"logfile=/dev/null\nerror_warn() {{ echo \"WARN: $1\"; return 1; }}\n{EXTRACT}\n{body}\n" + return subprocess.run(["bash", "-c", script], + capture_output=True, text=True, timeout=10) + + +def merge(existing, desired): + r = run(f'merge_grub_cmdline "{existing}" "{desired}"') + return r.stdout.strip() + + +class MergeGrubCmdline(unittest.TestCase): + DESIRED = "rw loglevel=2 quiet splash" + + def test_empty_existing_yields_desired(self): + self.assertEqual(merge("", self.DESIRED), self.DESIRED) + + def test_boot_critical_tokens_survive(self): + out = merge("cryptdevice=UUID=abc:root resume=/dev/nvme0n1p3 root=/dev/mapper/root", + self.DESIRED) + for tok in ("cryptdevice=UUID=abc:root", "resume=/dev/nvme0n1p3", + "root=/dev/mapper/root", "loglevel=2", "quiet", "splash"): + self.assertIn(tok, out.split(), f"{tok} missing from: {out}") + + def test_same_key_desired_value_wins_once(self): + out = merge("loglevel=7 quiet", self.DESIRED).split() + self.assertIn("loglevel=2", out) + self.assertNotIn("loglevel=7", out) + self.assertEqual(out.count("loglevel=2"), 1) + self.assertEqual(out.count("quiet"), 1) + + def test_zfs_token_survives(self): + out = merge("zfs=zroot/ROOT/default", self.DESIRED).split() + self.assertIn("zfs=zroot/ROOT/default", out) + + +class UpdateGrubCmdline(unittest.TestCase): + def run_update(self, grub_body): + with tempfile.TemporaryDirectory() as d: + path = os.path.join(d, "grub") + with open(path, "w") as f: + f.write(grub_body) + r = run(f'update_grub_cmdline "{path}"; echo "RC=$?"') + with open(path) as f: + return r, f.read() + + def line(self, content): + return [ln for ln in content.splitlines() + if ln.startswith('GRUB_CMDLINE_LINUX_DEFAULT=')] + + def test_existing_tokens_preserved_in_file(self): + _, content = self.run_update(textwrap.dedent("""\ + GRUB_TIMEOUT=5 + GRUB_CMDLINE_LINUX_DEFAULT="loglevel=3 cryptdevice=UUID=abc:root resume=/dev/sda2" + GRUB_DISABLE_RECOVERY=true + """)) + lines = self.line(content) + self.assertEqual(len(lines), 1) + val = lines[0] + self.assertIn("cryptdevice=UUID=abc:root", val) + self.assertIn("resume=/dev/sda2", val) + self.assertIn("loglevel=2", val) # archsetup's value wins + self.assertNotIn("loglevel=3", val) + self.assertIn("quiet", val) + # other lines untouched + self.assertIn("GRUB_TIMEOUT=5", content) + self.assertIn("GRUB_DISABLE_RECOVERY=true", content) + + def test_quoting_stays_a_single_pair(self): + _, content = self.run_update('GRUB_CMDLINE_LINUX_DEFAULT="quiet"\n') + val = self.line(content)[0] + self.assertEqual(val.count('"'), 2) + self.assertRegex(val, r'^GRUB_CMDLINE_LINUX_DEFAULT="[^"]*"$') + + def test_commented_line_gets_active_line_appended(self): + _, content = self.run_update('#GRUB_CMDLINE_LINUX_DEFAULT="quiet"\n') + self.assertEqual(len(self.line(content)), 1) + self.assertIn('#GRUB_CMDLINE_LINUX_DEFAULT="quiet"', content) + + def test_idempotent_on_rerun(self): + body = 'GRUB_CMDLINE_LINUX_DEFAULT="cryptdevice=UUID=abc:root"\n' + _, once = self.run_update(body) + _, twice = self.run_update(once) + self.assertEqual(once, twice) + + +if __name__ == "__main__": + unittest.main() -- cgit v1.2.3