From 67d0b6ea2e132a4ac9fd25bb977dbc4cb62b3230 Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Mon, 20 Jul 2026 16:00:39 -0500 Subject: refactor(installer): extract validate_yesno and a NVIDIA driver-floor constant Four near-identical blocks validated that AUTOLOGIN, NO_GPU_DRIVERS, INSTALL_CLAUDE_CODE, and INSTALL_DEVICE_UDEV_RULES were empty or exactly yes/no. They collapse into a validate_yesno helper the callers guard with `|| exit 1`. The NVIDIA driver floor of 535 was a magic literal in the comparison and three prose lines. It now lives in a NVIDIA_MIN_DRIVER constant, and the nvidia-preflight test sources that constant alongside the function it extracts. This is the safe, purely-testable slice of the refactor rollup. The structural extractions that touch the installer's system-mutation paths -- the GPU modalias scan, the stow and retry loops, the display-server/window-manager dispatch, and the snapper/fsck/GRUB/waybar sed blocks -- are split into a follow-up so they get characterization coverage first rather than a blind rewrite. --- archsetup | 41 +++++++++--------- tests/installer-steps/test_validate_yesno.py | 57 +++++++++++++++++++++++++ tests/nvidia-preflight/test_nvidia_preflight.py | 3 +- 3 files changed, 80 insertions(+), 21 deletions(-) create mode 100644 tests/installer-steps/test_validate_yesno.py diff --git a/archsetup b/archsetup index d7b84cb..f944334 100755 --- a/archsetup +++ b/archsetup @@ -167,6 +167,17 @@ validate_username() { done } +# Validate a tri-state config value: empty (the default) passes, otherwise it +# must be exactly "yes" or "no". Returns non-zero and names the offending +# variable on a bad value so validate_config can abort. +validate_yesno() { + local name="$1" value="$2" + if [[ -n "$value" && "$value" != "yes" && "$value" != "no" ]]; then + echo "ERROR: $name must be 'yes' or 'no'. Got: '$value'" >&2 + return 1 + fi +} + # Validate values pulled from --config-file, before anything is installed, so a # typo (DESKTOP_ENV=hyperland) fails immediately instead of partway through the # install. NOT a security boundary: load_config sources the config as bash, so a @@ -180,22 +191,10 @@ validate_config() { exit 1 fi - if [[ -n "$AUTOLOGIN" && "$AUTOLOGIN" != "yes" && "$AUTOLOGIN" != "no" ]]; then - echo "ERROR: AUTOLOGIN must be 'yes' or 'no'. Got: '$AUTOLOGIN'" >&2 - exit 1 - fi - if [[ -n "$NO_GPU_DRIVERS" && "$NO_GPU_DRIVERS" != "yes" && "$NO_GPU_DRIVERS" != "no" ]]; then - echo "ERROR: NO_GPU_DRIVERS must be 'yes' or 'no'. Got: '$NO_GPU_DRIVERS'" >&2 - exit 1 - fi - if [[ -n "$INSTALL_CLAUDE_CODE" && "$INSTALL_CLAUDE_CODE" != "yes" && "$INSTALL_CLAUDE_CODE" != "no" ]]; then - echo "ERROR: INSTALL_CLAUDE_CODE must be 'yes' or 'no'. Got: '$INSTALL_CLAUDE_CODE'" >&2 - exit 1 - fi - if [[ -n "$INSTALL_DEVICE_UDEV_RULES" && "$INSTALL_DEVICE_UDEV_RULES" != "yes" && "$INSTALL_DEVICE_UDEV_RULES" != "no" ]]; then - echo "ERROR: INSTALL_DEVICE_UDEV_RULES must be 'yes' or 'no'. Got: '$INSTALL_DEVICE_UDEV_RULES'" >&2 - exit 1 - fi + validate_yesno AUTOLOGIN "$AUTOLOGIN" || exit 1 + validate_yesno NO_GPU_DRIVERS "$NO_GPU_DRIVERS" || exit 1 + validate_yesno INSTALL_CLAUDE_CODE "$INSTALL_CLAUDE_CODE" || exit 1 + validate_yesno INSTALL_DEVICE_UDEV_RULES "$INSTALL_DEVICE_UDEV_RULES" || exit 1 if [[ -n "$locale" && ! "$locale" =~ ^[a-z]{2,3}(_[A-Z]{2})?(\.[A-Za-z0-9-]+)?(@[A-Za-z]+)?$ ]]; then echo "ERROR: LOCALE looks malformed: '$locale'. Expected e.g. en_US.UTF-8" >&2 @@ -253,6 +252,8 @@ packages_after="/var/log/archsetup-post-install-package-list.txt" archsetup_packages="/var/log/archsetup-installed-packages.txt" min_disk_space_gb=20 +# Minimum NVIDIA driver major version for usable Wayland/Hyprland. +NVIDIA_MIN_DRIVER=535 state_dir="/var/lib/archsetup/state" error_messages=() errors_encountered=0 @@ -464,7 +465,7 @@ nvidia_preflight_report() { [ "$found" = "true" ] || return 0 echo " [!!] NVIDIA GPU detected." - echo " Wayland/Hyprland on NVIDIA needs driver 535+ and explicit" + echo " Wayland/Hyprland on NVIDIA needs driver ${NVIDIA_MIN_DRIVER}+ and explicit" echo " environment variables; expect rougher edges than AMD/Intel:" echo " LIBVA_DRIVER_NAME=nvidia" echo " GBM_BACKEND=nvidia-drm" @@ -476,12 +477,12 @@ nvidia_preflight_report() { local ver major ver=$(pacman -Si nvidia-utils 2>/dev/null | awk '/^Version/ {print $3; exit}') major="${ver%%.*}" - if [[ "$major" =~ ^[0-9]+$ ]] && [ "$major" -ge 535 ]; then - echo " [OK] NVIDIA driver candidate: ${ver} (>= 535)" + if [[ "$major" =~ ^[0-9]+$ ]] && [ "$major" -ge "$NVIDIA_MIN_DRIVER" ]; then + echo " [OK] NVIDIA driver candidate: ${ver} (>= ${NVIDIA_MIN_DRIVER})" return 10 fi echo "ERROR: NVIDIA driver requirement not met" - echo " Required: driver 535 or newer for usable Wayland" + echo " Required: driver ${NVIDIA_MIN_DRIVER} or newer for usable Wayland" echo " Repo offers: ${ver:-unknown (pacman -Si nvidia-utils failed)}" echo " Fix: refresh the package database (pacman -Syy) and retry, or" echo " install with DESKTOP_ENV=dwm (X11) instead." diff --git a/tests/installer-steps/test_validate_yesno.py b/tests/installer-steps/test_validate_yesno.py new file mode 100644 index 0000000..da78fc5 --- /dev/null +++ b/tests/installer-steps/test_validate_yesno.py @@ -0,0 +1,57 @@ +"""Test the validate_yesno config-validation helper. + +The installer had four near-identical blocks validating that AUTOLOGIN, +NO_GPU_DRIVERS, INSTALL_CLAUDE_CODE, and INSTALL_DEVICE_UDEV_RULES are empty or +exactly yes/no. validate_yesno collapses them into one testable helper: empty +passes (the default), yes/no pass, anything else fails with a named error. + +Method: sed-extract validate_yesno from the real `archsetup` and drive it with +plain args. + +Run from repo root: + python3 -m unittest tests.installer-steps.test_validate_yesno +""" + +import os +import subprocess +import textwrap +import unittest + +REPO_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "..")) +ARCHSETUP = os.path.join(REPO_ROOT, "archsetup") + + +def run(name, value): + script = textwrap.dedent(f"""\ + source <(sed -n '/^validate_yesno() {{/,/^}}/p' "{ARCHSETUP}") + validate_yesno {name!r} {value!r} + echo "RC=$?" + """) + return subprocess.run( + ["bash", "-c", script], capture_output=True, text=True, timeout=10, + ) + + +class ValidateYesno(unittest.TestCase): + def test_yes_passes(self): + self.assertIn("RC=0", run("AUTOLOGIN", "yes").stdout) + + def test_no_passes(self): + self.assertIn("RC=0", run("AUTOLOGIN", "no").stdout) + + def test_empty_passes(self): + self.assertIn("RC=0", run("AUTOLOGIN", "").stdout) + + def test_other_value_fails_with_named_error(self): + r = run("NO_GPU_DRIVERS", "maybe") + self.assertNotIn("RC=0", r.stdout) + self.assertIn("NO_GPU_DRIVERS", r.stderr) + self.assertIn("maybe", r.stderr) + + def test_capitalized_yes_fails(self): + # The values are compared exactly; "Yes" is not accepted. + self.assertNotIn("RC=0", run("AUTOLOGIN", "Yes").stdout) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/nvidia-preflight/test_nvidia_preflight.py b/tests/nvidia-preflight/test_nvidia_preflight.py index bdacfd5..191e983 100644 --- a/tests/nvidia-preflight/test_nvidia_preflight.py +++ b/tests/nvidia-preflight/test_nvidia_preflight.py @@ -50,7 +50,8 @@ class NvidiaPreflightHarness(unittest.TestCase): "#!/bin/bash\n" 'ARCHSETUP="$1"; shift\n' "source <(sed -n " - "'/^nvidia_preflight_report() {/,/^}/p' \"$ARCHSETUP\")\n" + "'/^NVIDIA_MIN_DRIVER=/p;" + "/^nvidia_preflight_report() {/,/^}/p' \"$ARCHSETUP\")\n" "nvidia_preflight_report\n" ) os.chmod(self.wrapper, 0o755) -- cgit v1.2.3