From 218c6a4a82038dec8ad008bfdb8495b0fa300b2e Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Fri, 5 Dec 2025 22:33:49 +0100 Subject: [PATCH] Make pkgmgr installers fail hard and integrate Nix-based test pipeline (see https://chatgpt.com/share/69332bc4-a128-800f-a69c-fdc24c4cc7fe) --- Makefile | 30 ++++++---- flake.nix | 52 ++++++++-------- pkgmgr/install_repos.py | 11 ++-- pkgmgr/installers/__init__.py | 12 +++- pkgmgr/installers/ansible_requirements.py | 22 ++++--- pkgmgr/installers/aur.py | 43 +++++++++---- pkgmgr/installers/base.py | 11 +++- pkgmgr/installers/makefile.py | 11 ++-- pkgmgr/installers/nix_flake.py | 17 +++++- pkgmgr/installers/pkgbuild.py | 14 +++-- pkgmgr/installers/pkgmgr_manifest.py | 23 +++---- pkgmgr/installers/python.py | 60 ++++++++----------- ...test_integration_install_pkgmgr_shallow.py | 2 +- 13 files changed, 187 insertions(+), 121 deletions(-) diff --git a/Makefile b/Makefile index b2abb0f..4e7ed8b 100644 --- a/Makefile +++ b/Makefile @@ -1,8 +1,8 @@ .PHONY: install setup uninstall aur_builder_setup test # Local Nix cache directories in the repo -NIX_STORE_DIR := .nix/store -NIX_CACHE_DIR := .nix/cache +NIX_STORE_VOLUME := pkgmgr_nix_store +NIX_CACHE_VOLUME := pkgmgr_nix_cache setup: install @echo "Running pkgmgr setup via main.py..." @@ -14,20 +14,28 @@ setup: install python3 main.py install; \ fi -test: - @echo "Ensuring local Nix cache directories exist..." - @mkdir -p "$(NIX_STORE_DIR)" "$(NIX_CACHE_DIR)" + +build-no-cache: + @echo "Building test image 'package-manager-test' with no cache..." + docker build --no-cache -t package-manager-test . + +build: @echo "Building test image 'package-manager-test'..." docker build -t package-manager-test . - @echo "Running tests inside Nix devShell with local cache..." + + +test: build + @echo "Ensuring Docker Nix volumes exist (auto-created if missing)..." + @echo "Running tests inside Nix devShell with cached store..." docker run --rm \ - -v "$$(pwd)/$(NIX_STORE_DIR):/nix" \ - -v "$$(pwd)/$(NIX_CACHE_DIR):/root/.cache/nix" \ + -v "$$(pwd):/src" \ + -v "$(NIX_STORE_VOLUME):/nix" \ + -v "$(NIX_CACHE_VOLUME):/root/.cache/nix" \ --workdir /src \ - --entrypoint nix \ + --entrypoint bash \ package-manager-test \ - develop .#default --no-write-lock-file -c \ - python -m unittest discover -s tests -p "test_*.py" + -c 'git config --global --add safe.directory /src && nix develop .#default --no-write-lock-file -c python3 -m unittest discover -s tests -p "test_*.py"' + install: @echo "Making 'main.py' executable..." diff --git a/flake.nix b/flake.nix index 4289366..c385ea1 100644 --- a/flake.nix +++ b/flake.nix @@ -47,33 +47,37 @@ ); # Packages: nix build .#pkgmgr / .#default - packages = forAllSystems (system: - let - pkgs = nixpkgs.legacyPackages.${system}; - python = pkgs.python311; - pypkgs = pkgs.python311Packages; + packages = forAllSystems (system: + let + pkgs = nixpkgs.legacyPackages.${system}; + python = pkgs.python311; + pypkgs = pkgs.python311Packages; - # Be robust: ansible-core if available, otherwise ansible. - ansiblePkg = - if pkgs ? ansible-core then pkgs.ansible-core - else pkgs.ansible; - in - rec { - pkgmgr = pypkgs.buildPythonApplication { - pname = "package-manager"; - version = "0.1.0"; - src = ./.; + # Be robust: ansible-core if available, otherwise ansible. + ansiblePkg = + if pkgs ? ansible-core then pkgs.ansible-core + else pkgs.ansible; + in + rec { + pkgmgr = pypkgs.buildPythonApplication { + pname = "package-manager"; + version = "0.1.0"; + src = ./.; - propagatedBuildInputs = [ - pypkgs.pyyaml - ansiblePkg - ]; - }; + pyproject = true; + build-system = [ pypkgs.setuptools ]; + + propagatedBuildInputs = [ + pypkgs.pyyaml + ansiblePkg + ]; + }; + + # default package just points to pkgmgr + default = pkgmgr; + } + ); - # default package just points to pkgmgr - default = pkgmgr; - } - ); # Apps: nix run .#pkgmgr / .#default apps = forAllSystems (system: diff --git a/pkgmgr/install_repos.py b/pkgmgr/install_repos.py index 8321c80..0c46669 100644 --- a/pkgmgr/install_repos.py +++ b/pkgmgr/install_repos.py @@ -11,14 +11,14 @@ This module orchestrates the installation of repositories by: 3. Creating executable links using create_ink(). 4. Running a sequence of modular installer components that handle specific technologies or manifests (pkgmgr.yml, PKGBUILD, Nix, - Ansible requirements, Python, Makefile). + Ansible requirements, Python, Makefile, AUR). The goal is to keep this file thin and delegate most logic to small, focused installer classes. """ import os -from typing import List, Dict, Any, Tuple +from typing import List, Dict, Any from pkgmgr.get_repo_identifier import get_repo_identifier from pkgmgr.get_repo_dir import get_repo_dir @@ -38,7 +38,7 @@ from pkgmgr.installers.makefile import MakefileInstaller from pkgmgr.installers.aur import AurInstaller -# Ordered list of installers to apply to each repository +# Ordered list of installers to apply to each repository. INSTALLERS = [ PkgmgrManifestInstaller(), PkgbuildInstaller(), @@ -159,7 +159,10 @@ def install_repos( """ Install repositories by creating symbolic links and processing standard manifest files (pkgmgr.yml, PKGBUILD, flake.nix, Ansible requirements, - Python manifests, Makefile) via dedicated installer components. + Python manifests, Makefile, AUR) via dedicated installer components. + + Any installer failure (SystemExit) is treated as fatal and will abort + the current installation. """ for repo in selected_repos: identifier = get_repo_identifier(repo, all_repos) diff --git a/pkgmgr/installers/__init__.py b/pkgmgr/installers/__init__.py index e1d84df..54fcb2a 100644 --- a/pkgmgr/installers/__init__.py +++ b/pkgmgr/installers/__init__.py @@ -5,5 +5,15 @@ Installer package for pkgmgr. Each installer implements a small, focused step in the repository -installation pipeline (e.g. PKGBUILD dependencies, Nix flakes, Python, etc.). +installation pipeline (e.g. PKGBUILD dependencies, Nix flakes, Python, +Ansible requirements, pkgmgr.yml, Makefile, AUR). """ + +from pkgmgr.installers.base import BaseInstaller # noqa: F401 +from pkgmgr.installers.pkgmgr_manifest import PkgmgrManifestInstaller # noqa: F401 +from pkgmgr.installers.pkgbuild import PkgbuildInstaller # noqa: F401 +from pkgmgr.installers.nix_flake import NixFlakeInstaller # noqa: F401 +from pkgmgr.installers.ansible_requirements import AnsibleRequirementsInstaller # noqa: F401 +from pkgmgr.installers.python import PythonInstaller # noqa: F401 +from pkgmgr.installers.makefile import MakefileInstaller # noqa: F401 +from pkgmgr.installers.aur import AurInstaller # noqa: F401 diff --git a/pkgmgr/installers/ansible_requirements.py b/pkgmgr/installers/ansible_requirements.py index d729966..cfb23b1 100644 --- a/pkgmgr/installers/ansible_requirements.py +++ b/pkgmgr/installers/ansible_requirements.py @@ -8,6 +8,7 @@ This installer installs collections and roles via ansible-galaxy when found. """ import os +import shutil import tempfile from typing import Any, Dict, List @@ -50,24 +51,30 @@ class AnsibleRequirementsInstaller(BaseInstaller): return "" def _load_requirements(self, req_path: str, identifier: str) -> Dict[str, Any]: + """ + Load requirements.yml. + + Any parsing error is treated as fatal (SystemExit). + """ try: with open(req_path, "r", encoding="utf-8") as f: return yaml.safe_load(f) or {} except Exception as exc: print(f"Error loading {self.REQUIREMENTS_FILE} in {identifier}: {exc}") - return {} + raise SystemExit( + f"{self.REQUIREMENTS_FILE} parsing failed for {identifier}: {exc}" + ) def _validate_requirements(self, requirements: Dict[str, Any], identifier: str) -> None: """ Validate the requirements.yml structure. + Raises SystemExit on any validation error. """ - errors: List[str] = [] if not isinstance(requirements, dict): errors.append("Top-level structure must be a mapping.") - else: allowed_keys = {"collections", "roles"} unknown_keys = set(requirements.keys()) - allowed_keys @@ -88,19 +95,19 @@ class AnsibleRequirementsInstaller(BaseInstaller): for idx, entry in enumerate(value): if isinstance(entry, str): - # short form "community.docker" etc. + # Short form "community.docker", etc. continue if isinstance(entry, dict): - # Collections: brauchen zwingend 'name' if section == "collections": + # Collections require 'name' if not entry.get("name"): errors.append( f"Entry #{idx} in '{section}' is a mapping " f"but has no 'name' key." ) else: - # Roles: 'name' ODER 'src' sind ok (beides gängig) + # Roles: 'name' OR 'src' are acceptable. if not (entry.get("name") or entry.get("src")): errors.append( f"Entry #{idx} in '{section}' is a mapping but " @@ -127,7 +134,7 @@ class AnsibleRequirementsInstaller(BaseInstaller): if not requirements: return - # Validate structure before doing anything dangerous + # Validate structure before doing anything dangerous. self._validate_requirements(requirements, ctx.identifier) if "collections" not in requirements and "roles" not in requirements: @@ -166,4 +173,3 @@ class AnsibleRequirementsInstaller(BaseInstaller): print(f"Ansible roles found in {ctx.identifier}, installing...") cmd = f"{galaxy_cmd} role install -r {tmp_filename}" run_command(cmd, cwd=ctx.repo_dir, preview=ctx.preview) - diff --git a/pkgmgr/installers/aur.py b/pkgmgr/installers/aur.py index e52f977..a8cb362 100644 --- a/pkgmgr/installers/aur.py +++ b/pkgmgr/installers/aur.py @@ -1,10 +1,24 @@ -# pkgmgr/installers/aur.py +#!/usr/bin/env python3 +# -*- coding: utf-8 -*- + +""" +Installer for Arch AUR dependencies declared in an `aur.yml` file. + +This installer is: + - Arch-only (requires `pacman`) + - helper-driven (yay/paru/..) + - safe to ignore on non-Arch systems + +Config parsing errors are treated as fatal to avoid silently ignoring +broken configuration. +""" import os import shutil -import yaml from typing import List +import yaml + from pkgmgr.installers.base import BaseInstaller from pkgmgr.context import RepoContext from pkgmgr.run_command import run_command @@ -16,11 +30,6 @@ AUR_CONFIG_FILENAME = "aur.yml" class AurInstaller(BaseInstaller): """ Installer for Arch AUR dependencies declared in an `aur.yml` file. - - This installer is: - - Arch-only (requires `pacman`) - - optional helper-driven (yay/paru/..) - - safe to ignore on non-Arch systems """ def _is_arch_like(self) -> bool: @@ -30,6 +39,12 @@ class AurInstaller(BaseInstaller): return os.path.join(ctx.repo_dir, AUR_CONFIG_FILENAME) def _load_config(self, ctx: RepoContext) -> dict: + """ + Load and validate aur.yml. + + Any parsing error or invalid top-level structure is treated as fatal + (SystemExit). + """ path = self._config_path(ctx) if not os.path.exists(path): return {} @@ -38,12 +53,12 @@ class AurInstaller(BaseInstaller): with open(path, "r", encoding="utf-8") as f: data = yaml.safe_load(f) or {} except Exception as exc: - print(f"[Warning] Failed to load AUR config from '{path}': {exc}") - return {} + print(f"[Error] Failed to load AUR config from '{path}': {exc}") + raise SystemExit(f"AUR config '{path}' could not be parsed: {exc}") if not isinstance(data, dict): - print(f"[Warning] AUR config '{path}' is not a mapping. Ignoring.") - return {} + print(f"[Error] AUR config '{path}' is not a mapping.") + raise SystemExit(f"AUR config '{path}' must be a mapping at top level.") return data @@ -85,6 +100,8 @@ class AurInstaller(BaseInstaller): - We are on an Arch-like system (pacman available), - An aur.yml exists, - That aur.yml declares at least one package. + + An invalid aur.yml will raise SystemExit during config loading. """ if not self._is_arch_like(): return False @@ -99,6 +116,9 @@ class AurInstaller(BaseInstaller): def run(self, ctx: RepoContext) -> None: """ Install AUR packages using the configured helper (default: yay). + + Missing helper is treated as non-fatal (warning), everything else + that fails in run_command() is fatal. """ if not self._is_arch_like(): print("AUR installer skipped: not an Arch-like system.") @@ -127,5 +147,4 @@ class AurInstaller(BaseInstaller): print(f"Installing AUR packages via '{helper}': {pkg_list_str}") cmd = f"{helper} -S --noconfirm {pkg_list_str}" - # We respect preview mode to allow dry runs. run_command(cmd, preview=ctx.preview) diff --git a/pkgmgr/installers/base.py b/pkgmgr/installers/base.py index e0c83f8..933af93 100644 --- a/pkgmgr/installers/base.py +++ b/pkgmgr/installers/base.py @@ -6,6 +6,7 @@ Base interface for all installer components in the pkgmgr installation pipeline. """ from abc import ABC, abstractmethod + from pkgmgr.context import RepoContext @@ -14,7 +15,7 @@ class BaseInstaller(ABC): A single step in the installation pipeline for a repository. Implementations should be small and focused on one technology or manifest - type (e.g. PKGBUILD, Nix, Python, Ansible). + type (e.g. PKGBUILD, Nix, Python, Ansible, pkgmgr.yml). """ @abstractmethod @@ -22,6 +23,9 @@ class BaseInstaller(ABC): """ Return True if this installer should run for the given repository context. This is typically based on file existence or platform checks. + + Implementations must never swallow critical errors silently; if a + configuration is broken, they should raise SystemExit. """ raise NotImplementedError @@ -29,6 +33,9 @@ class BaseInstaller(ABC): def run(self, ctx: RepoContext) -> None: """ Execute the installer logic for the given repository context. - Implementations may raise SystemExit via run_command() on errors. + + Implementations are allowed to raise SystemExit (for example via + run_command()) on errors. Such failures are considered fatal for + the installation pipeline. """ raise NotImplementedError diff --git a/pkgmgr/installers/makefile.py b/pkgmgr/installers/makefile.py index a389d6d..d0fbb80 100644 --- a/pkgmgr/installers/makefile.py +++ b/pkgmgr/installers/makefile.py @@ -25,8 +25,11 @@ class MakefileInstaller(BaseInstaller): return os.path.exists(makefile_path) def run(self, ctx: RepoContext) -> None: + """ + Execute `make install` in the repository directory. + + Any failure in `make install` is treated as a fatal error and will + propagate as SystemExit from run_command(). + """ cmd = "make install" - try: - run_command(cmd, cwd=ctx.repo_dir, preview=ctx.preview) - except SystemExit as exc: - print(f"[Warning] Failed to run '{cmd}' for {ctx.identifier}: {exc}") + run_command(cmd, cwd=ctx.repo_dir, preview=ctx.preview) diff --git a/pkgmgr/installers/nix_flake.py b/pkgmgr/installers/nix_flake.py index 318659f..3b265b1 100644 --- a/pkgmgr/installers/nix_flake.py +++ b/pkgmgr/installers/nix_flake.py @@ -5,7 +5,7 @@ Installer for Nix flakes. If a repository contains flake.nix and the 'nix' command is available, this -installer will try to install a profile output from the flake. +installer will try to install profile outputs from the flake. """ import os @@ -22,12 +22,22 @@ class NixFlakeInstaller(BaseInstaller): FLAKE_FILE = "flake.nix" def supports(self, ctx: RepoContext) -> bool: + """ + Only support repositories that: + - Have a flake.nix + - And have the `nix` command available. + """ if shutil.which("nix") is None: return False flake_path = os.path.join(ctx.repo_dir, self.FLAKE_FILE) return os.path.exists(flake_path) def run(self, ctx: RepoContext) -> None: + """ + Install Nix flake profile outputs (pkgmgr, default). + + Any failure in `nix profile install` is treated as fatal (SystemExit). + """ flake_path = os.path.join(ctx.repo_dir, self.FLAKE_FILE) if not os.path.exists(flake_path): return @@ -43,5 +53,6 @@ class NixFlakeInstaller(BaseInstaller): run_command(cmd, cwd=ctx.repo_dir, preview=ctx.preview) print(f"Nix flake output '{output}' successfully installed.") except SystemExit as e: - print(f"[Warning] Failed to install Nix flake output '{output}': {e}") - + print(f"[Error] Failed to install Nix flake output '{output}': {e}") + # Hard fail: a broken flake is considered a fatal error. + raise diff --git a/pkgmgr/installers/pkgbuild.py b/pkgmgr/installers/pkgbuild.py index 8978999..5166eb5 100644 --- a/pkgmgr/installers/pkgbuild.py +++ b/pkgmgr/installers/pkgbuild.py @@ -32,10 +32,8 @@ class PkgbuildInstaller(BaseInstaller): def _extract_pkgbuild_array(self, ctx: RepoContext, var_name: str) -> List[str]: """ Extract a Bash array (depends/makedepends) from PKGBUILD using bash itself. - Returns a list of package names or an empty list on error. - Uses a minimal shell environment (no profile/rc) to avoid noise from MOTD - or interactive shell banners polluting the output. + Any failure in sourcing or extracting the variable is treated as fatal. """ pkgbuild_path = os.path.join(ctx.repo_dir, self.PKGBUILD_NAME) if not os.path.exists(pkgbuild_path): @@ -48,8 +46,14 @@ class PkgbuildInstaller(BaseInstaller): cwd=ctx.repo_dir, text=True, ) - except Exception: - return [] + except Exception as exc: + print( + f"[Error] Failed to extract '{var_name}' from PKGBUILD in " + f"{ctx.identifier}: {exc}" + ) + raise SystemExit( + f"PKGBUILD parsing failed for '{var_name}' in {ctx.identifier}: {exc}" + ) packages: List[str] = [] for line in output.splitlines(): diff --git a/pkgmgr/installers/pkgmgr_manifest.py b/pkgmgr/installers/pkgmgr_manifest.py index 2c49bd5..9c491c7 100644 --- a/pkgmgr/installers/pkgmgr_manifest.py +++ b/pkgmgr/installers/pkgmgr_manifest.py @@ -28,12 +28,19 @@ class PkgmgrManifestInstaller(BaseInstaller): return os.path.exists(manifest_path) def _load_manifest(self, manifest_path: str) -> Dict[str, Any]: + """ + Load the pkgmgr.yml manifest. + + Any parsing error is treated as a fatal error (SystemExit). + """ try: with open(manifest_path, "r", encoding="utf-8") as f: return yaml.safe_load(f) or {} except Exception as exc: print(f"Error loading {self.MANIFEST_NAME} in '{manifest_path}': {exc}") - return {} + raise SystemExit( + f"{self.MANIFEST_NAME} parsing failed for '{manifest_path}': {exc}" + ) def _collect_dependency_ids(self, dependencies: List[Dict[str, Any]]) -> List[str]: ids: List[str] = [] @@ -70,14 +77,12 @@ class PkgmgrManifestInstaller(BaseInstaller): dep_repo_ids = self._collect_dependency_ids(dependencies) + # Optionally pull dependencies if requested. if ctx.update_dependencies and dep_repo_ids: cmd_pull = "pkgmgr pull " + " ".join(dep_repo_ids) - try: - run_command(cmd_pull, preview=ctx.preview) - except SystemExit as exc: - print(f"Warning: 'pkgmgr pull' for dependencies failed (exit code {exc}).") + run_command(cmd_pull, preview=ctx.preview) - # Install dependencies one by one + # Install dependencies one by one. for dep in dependencies: if not isinstance(dep, dict): continue @@ -108,7 +113,5 @@ class PkgmgrManifestInstaller(BaseInstaller): if ctx.clone_mode: cmd += f" --clone-mode {ctx.clone_mode}" - try: - run_command(cmd, preview=ctx.preview) - except SystemExit as exc: - print(f"[Warning] Failed to install dependency '{repo_id}': {exc}") + # Dependency installation failures are fatal. + run_command(cmd, preview=ctx.preview) diff --git a/pkgmgr/installers/python.py b/pkgmgr/installers/python.py index e1cb324..03ba22c 100644 --- a/pkgmgr/installers/python.py +++ b/pkgmgr/installers/python.py @@ -1,31 +1,35 @@ +#!/usr/bin/env python3 +# -*- coding: utf-8 -*- + +""" +Installer for Python projects based on pyproject.toml and/or requirements.txt. + +Strategy: + - Determine a pip command in this order: + 1. $PKGMGR_PIP (explicit override, e.g. ~/.venvs/pkgmgr/bin/pip) + 2. sys.executable -m pip (current interpreter) + 3. "pip" from PATH as last resort + - If pyproject.toml exists: pip install . + - If requirements.txt exists: pip install -r requirements.txt + +All installation failures are treated as fatal errors (SystemExit). +""" + import os import sys -from .base import BaseInstaller +from pkgmgr.installers.base import BaseInstaller from pkgmgr.run_command import run_command class PythonInstaller(BaseInstaller): - """ - Install Python projects based on pyproject.toml and/or requirements.txt. - - Strategy: - - Determine a pip command in this order: - 1. $PKGMGR_PIP (explicit override, e.g. ~/.venvs/pkgmgr/bin/pip) - 2. sys.executable -m pip (current interpreter) - 3. "pip" from PATH as last resort - - If pyproject.toml exists: pip install . - - If requirements.txt exists: pip install -r requirements.txt - """ + """Install Python projects and dependencies via pip.""" name = "python" def supports(self, ctx) -> bool: """ Return True if this installer should handle the given repository. - - ctx must provide: - - repo_dir: filesystem path to the repository """ repo_dir = ctx.repo_dir return ( @@ -37,24 +41,20 @@ class PythonInstaller(BaseInstaller): """ Resolve the pip command to use. """ - # 1) Explicit override via environment variable explicit = os.environ.get("PKGMGR_PIP", "").strip() if explicit: return explicit - # 2) Current Python interpreter (works well in Nix/dev shells) if sys.executable: return f"{sys.executable} -m pip" - # 3) Fallback to plain pip return "pip" def run(self, ctx) -> None: """ - ctx must provide: - - repo_dir: path to repository - - identifier: human readable name - - preview: bool + Install Python project (pyproject.toml) and/or requirements.txt. + + Any pip failure is propagated as SystemExit. """ pip_cmd = self._pip_cmd() @@ -65,12 +65,7 @@ class PythonInstaller(BaseInstaller): f"installing Python project..." ) cmd = f"{pip_cmd} install ." - try: - run_command(cmd, cwd=ctx.repo_dir, preview=ctx.preview) - except SystemExit as exc: - print( - f"[Warning] Failed to install Python project in {ctx.identifier}: {exc}" - ) + run_command(cmd, cwd=ctx.repo_dir, preview=ctx.preview) req_txt = os.path.join(ctx.repo_dir, "requirements.txt") if os.path.exists(req_txt): @@ -79,11 +74,4 @@ class PythonInstaller(BaseInstaller): f"installing Python dependencies..." ) cmd = f"{pip_cmd} install -r requirements.txt" - try: - run_command(cmd, cwd=ctx.repo_dir, preview=ctx.preview) - except SystemExit as exc: - print( - f"[Warning] Failed to install Python dependencies in {ctx.identifier}: {exc}" - ) - - + run_command(cmd, cwd=ctx.repo_dir, preview=ctx.preview) diff --git a/tests/integration/test_integration_install_pkgmgr_shallow.py b/tests/integration/test_integration_install_pkgmgr_shallow.py index 8e04329..7aa578c 100644 --- a/tests/integration/test_integration_install_pkgmgr_shallow.py +++ b/tests/integration/test_integration_install_pkgmgr_shallow.py @@ -16,7 +16,7 @@ import unittest class TestIntegrationInstallAllShallow(unittest.TestCase): - def test_install_all_repositories_shallow(self): + def test_install_pkgmgr_self_install(self): """ Run: pkgmgr install --all --clone-mode shallow --no-verification