From e1e9227cf4ea5b275cd81707507fb59640244c00 Mon Sep 17 00:00:00 2001 From: Shen Mintao Date: Mon, 19 Oct 2026 11:31:26 +0800 Subject: [PATCH] fix: handle installer errors and avoid Arch partial upgrades Install Arch dependencies using the existing package database and explain how to recover from stale package metadata. Avoid errexit on the first firmware count, handle Secure Boot prompt EOF as cancellation, propagate error traps into functions, and reject empty firmware sources before modifying installed firmware. Validation: eight isolated installer regression tests pass; the original installer fails the affected cases. Bash syntax and ShellCheck error-level checks pass. Fixes #100. Addresses the confirmed findings in #108; other audit suggestions remain separate. --- INSTALL_SCRIPT.md | 6 ++ install.sh | 33 ++++++-- tests/installer/test_install.py | 137 ++++++++++++++++++++++++++++++++ 3 files changed, 168 insertions(+), 8 deletions(-) create mode 100644 tests/installer/test_install.py diff --git a/INSTALL_SCRIPT.md b/INSTALL_SCRIPT.md index 8ed3c41..1a8f3f0 100644 --- a/INSTALL_SCRIPT.md +++ b/INSTALL_SCRIPT.md @@ -29,6 +29,12 @@ git clone https://github.com/shenmintao/aic8800d80.git && cd aic8800d80 && chmod sudo ./install.sh ``` +On Arch Linux and derivatives, the installer uses the existing pacman package +database. It does not refresh that database or upgrade the system. If pacman +cannot download dependencies because the database is stale, run +`sudo pacman -Syu` yourself, reboot if the kernel was updated, and rerun the +installer with headers matching the running kernel. + > For more information, please refer to the [README.md](README.md) file.
diff --git a/install.sh b/install.sh index 4ddad4c..15afaa9 100755 --- a/install.sh +++ b/install.sh @@ -9,7 +9,7 @@ # FIX: Corrected DKMS module configuration for aic8800_fdrv ############################################################################# -set -e # Exit on error +set -eE # Exit on error and run the ERR trap inside functions # Color definitions readonly RED='\033[0;31m' @@ -152,7 +152,8 @@ check_secure_boot() { echo " 2. Sign the module manually (advanced)" echo " 3. Continue installation anyway (module may fail to load)" echo "" - read -p "Continue installation? (y/N): " choice + local choice + read -r -p "Continue installation? (y/N): " choice || choice="n" if [[ ! "$choice" =~ ^[Yy]$ ]]; then print_info "Installation cancelled by user." exit 0 @@ -268,15 +269,15 @@ install_dependencies() { ;; pacman) - print_info "Syncing package database..." - run_logged "Package database sync" pacman -Sy --noconfirm - local -a packages=(dkms base-devel mokutil util-linux usb_modeswitch) if [ "$install_kernel_headers" = true ]; then packages+=(linux-headers) fi print_info "Installing: ${packages[*]}..." - run_logged "Dependency installation" pacman -S --needed --noconfirm "${packages[@]}" + if ! run_logged "Dependency installation" pacman -S --needed --noconfirm "${packages[@]}"; then + print_error "Check the pacman errors above. If the package database is stale, run 'sudo pacman -Syu', reboot if the kernel was updated, then rerun this installer." + return 1 + fi ;; zypper) @@ -390,6 +391,22 @@ install_firmware() { exit 1 fi + # Validate the source before changing any installed firmware or config. + local -a fw_dirs=() + local fw_dir + for fw_dir in "$fw_base"/aic8800*; do + [ -d "$fw_dir" ] || continue + if [ -z "$(find "$fw_dir" -type f -name '*.bin' -print -quit)" ]; then + print_error "No firmware binaries found in $fw_dir; existing firmware has not been changed." + exit 1 + fi + fw_dirs+=("$fw_dir") + done + if [ "${#fw_dirs[@]}" -eq 0 ]; then + print_error "No firmware directories found in $fw_base; existing firmware has not been changed." + exit 1 + fi + cleanup_legacy_bluetooth_config # Remove old firmware versions @@ -400,7 +417,7 @@ install_firmware() { # Copy all firmware variants print_info "Installing firmware for all chip variants..." - for fw_dir in "$fw_base"/aic8800*; do + for fw_dir in "${fw_dirs[@]}"; do if [ -d "$fw_dir" ]; then local fw_name=$(basename "$fw_dir") local fw_dest="/lib/firmware/$fw_name" @@ -713,7 +730,7 @@ verify_installation() { local fw_count=0 for fw_dir in /lib/firmware/aic8800*; do if [ -d "$fw_dir" ]; then - ((fw_count++)) + fw_count=$((fw_count + 1)) fi done if [ $fw_count -gt 0 ]; then diff --git a/tests/installer/test_install.py b/tests/installer/test_install.py new file mode 100644 index 0000000..4a9e408 --- /dev/null +++ b/tests/installer/test_install.py @@ -0,0 +1,137 @@ +#!/usr/bin/env python3 +"""Run installer regressions with mocked system commands and temporary firmware. + +Usage: python3 tests/installer/test_install.py [path/to/install.sh] +Requires Bash; does not install packages, load modules, or need root. +""" + +from pathlib import Path +import re +import shlex +import subprocess +import sys +import tempfile +import unittest + + +INSTALLER = Path(sys.argv.pop(1)) if len(sys.argv) > 1 else Path(__file__).resolve().parents[2] / "install.sh" +SOURCE = INSTALLER.read_text(encoding="utf-8") +SHELL_OPTIONS = next(line for line in SOURCE.splitlines() if line.startswith("set -")) + + +class InstallerTests(unittest.TestCase): + def setUp(self): + self.temp = tempfile.TemporaryDirectory(prefix="aic-installer-test-") + self.addCleanup(self.temp.cleanup) + self.root = Path(self.temp.name) + self.firmware = self.root / "firmware" + self.firmware.mkdir() + + def run_installer_function(self, name, stubs="", after=""): + # Extract only the functions under test; never execute main(). + names = ["run_logged", "cleanup_on_error", name] + functions = [] + for function in names: + match = re.search(r"(?ms)^" + function + r"\(\) \{.*?^\}", SOURCE) + self.assertIsNotNone(match, function) + functions.append(match.group()) + script = "\n".join(functions) + script = script.replace("/lib/firmware", str(self.firmware)) + script = script.replace("/sys/firmware/efi", str(self.root / "efi")) + setup = f""" +{SHELL_OPTIONS} +SCRIPT_DIR={shlex.quote(str(self.root))} +LOG_FILE="$SCRIPT_DIR/install.log" +DRV_NAME=aic8800 +DRV_VERSION=1.0.0 +MODULE_NAME=aic8800_fdrv +print_step() {{ echo "$*"; }} +print_info() {{ echo "$*"; }} +print_success() {{ echo "$*"; }} +print_warning() {{ echo "$*"; }} +print_error() {{ echo "$*"; }} +dkms() {{ echo 'aic8800/1.0.0: installed'; }} +lsmod() {{ :; }} +aic_zlp_target_present() {{ return 1; }} +iwconfig() {{ :; }} +bluetoothctl() {{ :; }} +kernel_build_tree_available() {{ return 0; }} +is_volumio() {{ return 1; }} +cleanup_legacy_bluetooth_config() {{ echo LEGACY_CLEANUP; }} +""" + return subprocess.run( + ["bash", "-c", setup + script + "\ntrap cleanup_on_error ERR\n" + stubs + "\n" + name + after], + input="", text=True, capture_output=True, timeout=15, + ) + + def test_verification_reaches_final_instructions(self): + for count in (0, 1, 3): + with self.subTest(firmware_count=count): + for index in range(count): + (self.firmware / f"aic8800-{index}").mkdir(exist_ok=True) + result = self.run_installer_function("verify_installation", after="\necho FINAL_INSTRUCTIONS") + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn("FINAL_INSTRUCTIONS", result.stdout) + if count: + self.assertIn(f"Firmware installed for {count} chip variant(s)", result.stdout) + + def test_pacman_uses_existing_database(self): + result = self.run_installer_function( + "install_dependencies", 'pacman() { echo "PACMAN $*"; }', " pacman", + ) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + calls = [line for line in result.stdout.splitlines() if line.startswith("PACMAN ")] + self.assertEqual(calls, ["PACMAN -S --needed --noconfirm dkms base-devel mokutil util-linux usb_modeswitch"]) + + def test_pacman_failure_stops_with_upgrade_guidance(self): + result = self.run_installer_function( + "install_dependencies", 'pacman() { return 7; }', " pacman\necho UNEXPECTED_SUCCESS", + ) + self.assertNotEqual(result.returncode, 0) + self.assertIn("sudo pacman -Syu", result.stdout) + self.assertNotIn("UNEXPECTED_SUCCESS", result.stdout) + self.assertIn("Installation failed", result.stdout) + + def test_secure_boot_eof_cancels_cleanly(self): + result = self.run_installer_function( + "check_secure_boot", 'mokutil() { echo "SecureBoot enabled"; }', "\necho UNEXPECTED_CONTINUE", + ) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn("Installation cancelled", result.stdout) + self.assertNotIn("UNEXPECTED_CONTINUE", result.stdout) + + def check_invalid_firmware(self): + old = self.firmware / "aic8800D80" / "old.bin" + old.parent.mkdir() + old.write_bytes(b"installed firmware") + result = self.run_installer_function("install_firmware") + self.assertNotEqual(result.returncode, 0) + self.assertEqual(old.read_bytes(), b"installed firmware") + self.assertNotIn("LEGACY_CLEANUP", result.stdout) + + def test_missing_firmware_preserves_installation(self): + self.check_invalid_firmware() + + def test_empty_firmware_tree_preserves_installation(self): + (self.root / "fw").mkdir() + self.check_invalid_firmware() + + def test_empty_variant_preserves_installation(self): + (self.root / "fw" / "aic8800D80").mkdir(parents=True) + self.check_invalid_firmware() + + def test_valid_firmware_replaces_old_variant(self): + old = self.firmware / "aic8800D80" / "old.bin" + old.parent.mkdir() + old.write_bytes(b"old") + new = self.root / "fw" / "aic8800D80" / "new.bin" + new.parent.mkdir(parents=True) + new.write_bytes(b"new") + result = self.run_installer_function("install_firmware") + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertFalse(old.exists()) + self.assertEqual((old.parent / "new.bin").read_bytes(), b"new") + + +if __name__ == "__main__": + unittest.main(verbosity=2)