From ebf189545b0eee851fefa23dc1db901c023a5cc2 Mon Sep 17 00:00:00 2001 From: Keith Raghubar Date: Sat, 24 Oct 2026 10:59:04 -0400 Subject: [PATCH 01/11] docs(config): shipped config comments match the code (3.5.0-DOC15) Audit of etc/sysforge/*.toml for statements the code contradicts: the optimized profile's linker note, kernel.toml's scripts/config and "no kernel PGO" claims, bootstrap.toml's nonexistent stage names, a per-user sysforge.toml that is never read, and DESIGN.md pointers the package does not install. Internal stage and pass numbering fixed in the design docs and sysforge/CLAUDE.md. DOC scope now covers shipped config comments. Files 3.5.0-B139, B140 and DEV23. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 6 ++- DESIGN.md | 10 ++-- ROADMAP.md | 77 ++++++++++++++++++++++++++- docs/design/10-flag-profile-system.md | 6 +-- docs/design/14-hardware-detection.md | 2 +- docs/design/18-reconverge.md | 2 +- docs/release-notes/unreleased.md | 4 ++ etc/sysforge/bootstrap.toml | 12 ++--- etc/sysforge/kernel.toml | 23 ++++---- etc/sysforge/packages.toml | 17 +++--- etc/sysforge/profiles.toml | 30 +++++------ etc/sysforge/sysforge.toml | 17 +++--- etc/sysforge/toolchain.toml | 30 +++++------ sysforge/CLAUDE.md | 4 +- 14 files changed, 160 insertions(+), 80 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index e1709318..646d6480 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -79,12 +79,14 @@ Shipped-file edits must pass `make check-shipped`; doc/design edits `make check- vocabulary outside the closing "In sysforge" section, upstream sources cited, no numbers measured on one machine (`tests/test_concept_pages.py` checks the closing sections, the upstream links and the vocabulary); guides link them (and the glossary) rather than re-explaining. -- **`DOC` IDs are for user-facing docs only** (`docs/guides/`, `docs/concepts/`, README, man page) and file under +- **`DOC` IDs are for user-facing docs only** (`docs/guides/`, `docs/concepts/`, README, man page, + comments in shipped `etc/sysforge/*.toml`) and file under `## Changed` (patch; lint-enforced). Internal docs (design, CLAUDE.md, ROADMAP) get no ID; wrong `--help`/man output is a `B`, since it is CLI behaviour. - **`DEV` IDs are for contributor-only work** (only `tests/`, `tools/`, `Makefile`, `.claude/`, dev docs) and file their release-note entry under `## Changed` only, like `DOC` (lint-enforced, - `3.4.0-STD1`). Touching `sysforge/` or a shipped file makes it `B`/`F`. + `3.4.0-STD1`). Touching `sysforge/` or a shipped file makes it `B`/`F` (a comment-only edit to a + shipped config is `DOC`). - **Completions stay in lockstep with the CLI** (`completions/_sysforge` + bash) in the same change. - **A user-facing change regenerates `tests/data/surface.json`** (`make surface`) and names what changed in the release notes; `make check-surface` (a required CI gate) enforces both. diff --git a/DESIGN.md b/DESIGN.md index 3b66f6c8..1878893b 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -2963,11 +2963,11 @@ toolchain = "gcc" # global package-compiler default (see "Toolchain field [profiles.bare] # Fallback profile, no flags +# No profile names a compiler or linker: both come from `toolchain` (see +# "Toolchain field"). Override individual CC/CXX/AR/… keys to win over it. + [profiles.standard] extends = "bare" -# Compiler comes from the `toolchain` field (defaults to "gcc" via [defaults]). -# Override individual CC/CXX/AR/… keys to win over the bundle, or set -# `toolchain = "llvm"` here / in a rule to switch the whole bundle. CFLAGS = "-march=native -O2 -pipe" CXXFLAGS = "$CFLAGS" LDFLAGS = "-Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now" @@ -3861,7 +3861,7 @@ This gives hand-crafted prose with OPTIONS that stay automatically in sync with --- ## Hardware Detection -Pipeline stage 3. Probes the running system via `/proc/cpuinfo` and `lspci`, emits `hardware_profile.toml` to `state_dir`. The file feeds kconfig automation (kernel stage) and is shown in the reconfigure config review. +Pipeline stage 2. Probes the running system via `/proc/cpuinfo` and `lspci`, emits `hardware_profile.toml` to `state_dir`. The file feeds kconfig automation (kernel stage) and is shown in the reconfigure config review. **Detections and kconfig output:** @@ -4231,7 +4231,7 @@ Escape hatches: **Config defaults.** All three drift-rebuild flags (`--rebuild-on-drift`, `--rebuild-on-toolchain-drift`, `--rebuild-on-flag-drift`) fall back to a sysforge.toml `[update]` default (`rebuild_on_drift`, `rebuild_on_toolchain_drift`, `rebuild_on_flag_drift` respectively) via `_resolve_drift_axes`, which resolves each axis through `config.resolve_flag_default`. The CLI flag always wins when explicitly passed; the config value only supplies the default when it isn't. -**PGO toolchain packages** (`build_mode = "pgo_llvm_toolchain"`) are handled specially during update. `makepkg_wrapper.run()` reads `toolchain.toml → pgo_store`, checks for a saved `clang.profdata` and its `clang.profdata.version` sidecar, and compares the sidecar's LLVM major version against the PKGBUILD's `pkgver` major. If they match, `-fprofile-use=` is injected and the build proceeds as a PGO-optimised build. If profdata is absent or version-mismatched (e.g. after a major LLVM bump), the user is prompted: **[p]lain build or [s]kip (default: skip)**. In non-interactive mode the build is skipped automatically. Skipped packages are counted separately in the update summary and do not count as failures. To rebuild profdata after a major version bump, run `sysforge run toolchain`. The toolchain stage itself also reuses compatible profdata — see the **Profdata reuse** section under stage 6. +**PGO toolchain packages** (`build_mode = "pgo_llvm_toolchain"`) are handled specially during update. `makepkg_wrapper.run()` reads `toolchain.toml → pgo_store`, checks for a saved `clang.profdata` and its `clang.profdata.version` sidecar, and compares the sidecar's LLVM major version against the PKGBUILD's `pkgver` major. If they match, `-fprofile-use=` is injected and the build proceeds as a PGO-optimised build. If profdata is absent or version-mismatched (e.g. after a major LLVM bump), the user is prompted: **[p]lain build or [s]kip (default: skip)**. In non-interactive mode the build is skipped automatically. Skipped packages are counted separately in the update summary and do not count as failures. To rebuild profdata after a major version bump, run `sysforge run toolchain`. The toolchain stage itself also reuses compatible profdata — see the **Profdata reuse** section under stage 5. **Stale-profraw post-build check.** After every non-PGO-managed build, `makepkg_wrapper.run()` globs `pgo_store` for `*.profraw` files. Any file with `mtime >= build_start - 1s` is treated as **fresh** — it was written by the build just completed, which means an instrumented LLVM is still installed on the system and the build was leaking profile data. The wrapper fatals, telling the user to reinstall `llvm`/`llvm-libs` or run `sysforge run toolchain`. Files strictly older than `build_start` are **orphans** left behind by a prior failed or partial toolchain run whose instrumented binaries the user has since cleaned up; these are unlinked in place and an info line is logged. The split makes the safety net self-healing: once the system is clean, the next build purges the residue automatically instead of requiring manual cleanup of `pgo_store`. **Neither verdict applies while a toolchain PGO run is in progress** (`3.3.0-B8`): if `build_lock.is_held(makepkg_pgo.resolve_pgo_lock_path(tcfg))` reports the `sysforge-pgo.lock` held, the store is that run's live Pass-3 training data, so a fresh file is not a leak and an "orphan" is data its merge monitor has not read yet. The check logs that it is deferring and touches nothing. Before this, a `sysforge build` started alongside `run toolchain` either fatally aborted on the training data or deleted whatever the monitor had not yet merged. `is_held` is a read-only probe: it never creates the lock file or overwrites the holder's PID. diff --git a/ROADMAP.md b/ROADMAP.md index 394d480d..90b2f5a6 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -44,7 +44,8 @@ neighbour by a `---` rule, so a single item is readable on its own rather than running into the next one. `docs/release-notes/` entries use the same two rules. **`DOC` covers user-facing documentation only** — guides under `docs/guides/`, concept -pages under `docs/concepts/`, `README.md`, the man page — so doc work stays visible in release notes without +pages under `docs/concepts/`, `README.md`, the man page, and the comments in shipped config files +(`etc/sysforge/*.toml`) — so doc work stays visible in release notes without being forced into `F` or `B`. A `DOC` entry always lands under `## Changed`: the section drives the derived SemVer bump, and documentation changes no API (a `DOC` under `## Added` would demand a minor release). `make check-standards` @@ -111,12 +112,86 @@ canonical ordering. | ID | Item | Priority | Effort | Bump | |----|------|----------|--------|------| +| `3.5.0-B139` | a package entry's cache = false is recorded but never applied | med | small | patch | +| `3.5.0-B140` | sysforge.toml [failure_handling] is never read | med | medium | patch | +| `3.5.0-DEV23` | widen the end-to-end tier beyond build, update and setup | med | large | patch | ### Features ### Bugs +- **`3.5.0-B139` — a package entry's `cache = false` is recorded but never applied.** + `sysforge packages add --no-cache` writes `cache = false` to the `[[package]]` entry (a + `[group.*]` table accepts it as a member default too), `packages list` shows it, and + `verbs.shared.OVERRIDE_FIELDS` counts it as a behaviour-changing override, which silences + `update`'s inert-entry warning. No build path reads it: `build_core`, `makepkg_wrapper` and + `makepkg_invoke` never consult the entry, so ccache and sccache stay on for that package. The + shipped `packages.toml` and the `--no-cache` help both call it "required for PGO", which is + doubtful as well: neither cache sits between an instrumented binary and the profraw it writes, + and the toolchain stage already sets `CCACHE_DISABLE`/`SCCACHE_DISABLE` itself where it needs + to. Fix by honouring it (the same two env vars, plus dropping `RUSTC_WRAPPER`, for that + package's makepkg env) and correcting the "required for PGO" wording in `packages.toml`, the + help text and the man page. Retiring the key instead would be a `major` removal through + `tools/retired_names.toml` and `config_schema.RETIRED_KEYS`. Found in the shipped-config audit + of 2026-10-24 (`3.5.0-DOC15`). + *Priority: med · Effort: small · Bump: patch* — a documented switch that silently does + nothing; anyone relying on it gets cached builds. + +- **`3.5.0-B140` — `sysforge.toml [failure_handling]` is never read.** + The shipped `sysforge.toml` documents `[failure_handling]` and `config_schema.KNOWN_SECTIONS` + lists it there, but both readers take it from the `profiles.toml` dict: + `makepkg_wrapper.run` calls `load_config()` (the profiles loader) and passes that `config` to + the auto-repair driver (`_failure_cfg`, the `srcinfo_drift` preflight and `apply_first_match`), + to `failure.handle_failure` for `pkgbuild_unparseable` and `tempfile_write_failed`, and to + `dep_analysis.run_dep_analysis` for `abi_mismatch` and `makedep_probe_failed`. Editing the + documented section therefore changes nothing; the same table in `profiles.toml` would be + honoured, but the schema calls it an unknown section there. Today's behaviour is the hardcoded + defaults, which happen to be the safe ones. Three further faults sit in the same seam: + (1) **two vocabularies, two fallbacks.** The four auto-repair keys take + `auto_repair`/`auto_repair_with_warning`/`prompt_user`/… and an unknown value falls back to + `auto_repair` (`_normalise_behaviour`), so a typo in `checksum_mismatch` selects the + silent-repair behaviour the shipped comment says must never be relaxed. The `failure.py` + scenarios take only `abort`/`error`/`warn_and_fallback`/`fallback`, and an unknown value + aborts. The shipped comment lists all seven values for every key. (2) **the + `checksum_mismatch` guard is not where the config says.** It is safe only because + `_repair_checksum_mismatch` always prompts, whatever the behaviour. Under `auto_repair` the + `batch` short-circuit (which only checks for `prompt_user`) is skipped, so a `batch = true` + profile run at a terminal still stops to ask. (3) **undocumented and dead scenarios.** + `_FAILURE_DEFAULTS` has nine keys; four have callers (`pkgbuild_unparseable`, `abi_mismatch`, + `makedep_probe_failed`, and `tempfile_write_failed`, which always aborts), and none of the four + is in the shipped file. `no_rule_matched`, `profile_missing`, `profile_cycle`, `env_conflict` + and `dep_unsatisfied` are never raised. `failure_handling` is also still in + `profile.SYSFORGE_KEYS`, left from when it was a per-profile key. Fix: one resolver in + `primitives/config.py` reads the table from `load_sysforge_toml()` and validates each key against + its own vocabulary (an unknown value takes that scenario's safe default, never `auto_repair`); + pass it to all three readers; make `checksum_mismatch` refuse anything but `prompt_user` or + `abort`; document the four live `failure.py` keys and each key's values in `sysforge.toml`; drop + the dead defaults and the `SYSFORGE_KEYS` entry. Found while auditing the shipped configs on + 2026-10-24 (`3.5.0-DOC15`). + *Priority: med · Effort: medium · Bump: patch* — a documented, security-flagged setting that + silently does nothing; the defaults keep it safe today, but any edit is ignored and a typo in the + right file would weaken the checksum guard. + ### Developer tooling +- **`3.5.0-DEV23` — widen the end-to-end tier beyond `build`, `update` and setup.** + The tier (`3.5.0-DEV21`, `tools/e2e/`) has four scenarios that call 4 of the 41 commands in + `tests/data/surface.json` (`build`, `update`, `run reconfigure`, `run hardware`), all on the + shipped config except one `profiles.toml` edit and the `packages.toml` opt-in write. Add + scenarios, local `pkgs/` packages and no network wherever possible, in this order: (1) the + mutating commands that cross the user/root seam the tier exists for: `uninstall`, + `revert-to-stock`, `state forget`, `packages add`/`remove`/`add-group`, `config reset`/`clean`, + `artifact adopt`/`deploy`; (2) read-only commands against the state the earlier scenarios + leave: `state list`/`failed`/`orphans`, `doctor system`/`pkg`, `config check`, `resolve`, `log`; + (3) `run toolchain` and `run kernel` as `--dry-run` passes with non-default `toolchain.toml` + and `kernel.toml` (a real run needs hours or a reboot), plus `sysforge.toml` keys that change a + path a scenario takes (`[security] freeze_sources`, `[aur] enabled = false`). Pair it with a + coverage check that lists the commands no scenario calls, so a new verb shows up as uncovered + instead of silently absent. Separately, decide whether `e2e` becomes a required status check + beside `gates` on the `main` ruleset; today a red run does not block a merge, and + `10-quickstart` is networked, so requiring it puts AUR and mirror outages on the merge path. + *Priority: med · Effort: large · Bump: patch* — most commands have no test of their real + root/user, terminal and cross-command behaviour, the seams where the 2026-10-23 bugs lived. + ### Open questions diff --git a/docs/design/10-flag-profile-system.md b/docs/design/10-flag-profile-system.md index 82a73775..aa34a921 100644 --- a/docs/design/10-flag-profile-system.md +++ b/docs/design/10-flag-profile-system.md @@ -10,11 +10,11 @@ toolchain = "gcc" # global package-compiler default (see "Toolchain field [profiles.bare] # Fallback profile, no flags +# No profile names a compiler or linker: both come from `toolchain` (see +# "Toolchain field"). Override individual CC/CXX/AR/… keys to win over it. + [profiles.standard] extends = "bare" -# Compiler comes from the `toolchain` field (defaults to "gcc" via [defaults]). -# Override individual CC/CXX/AR/… keys to win over the bundle, or set -# `toolchain = "llvm"` here / in a rule to switch the whole bundle. CFLAGS = "-march=native -O2 -pipe" CXXFLAGS = "$CFLAGS" LDFLAGS = "-Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now" diff --git a/docs/design/14-hardware-detection.md b/docs/design/14-hardware-detection.md index c912909b..861015f8 100644 --- a/docs/design/14-hardware-detection.md +++ b/docs/design/14-hardware-detection.md @@ -1,6 +1,6 @@ ## Hardware Detection -Pipeline stage 3. Probes the running system via `/proc/cpuinfo` and `lspci`, emits `hardware_profile.toml` to `state_dir`. The file feeds kconfig automation (kernel stage) and is shown in the reconfigure config review. +Pipeline stage 2. Probes the running system via `/proc/cpuinfo` and `lspci`, emits `hardware_profile.toml` to `state_dir`. The file feeds kconfig automation (kernel stage) and is shown in the reconfigure config review. **Detections and kconfig output:** diff --git a/docs/design/18-reconverge.md b/docs/design/18-reconverge.md index 89916957..860f4706 100644 --- a/docs/design/18-reconverge.md +++ b/docs/design/18-reconverge.md @@ -12,7 +12,7 @@ **Config defaults.** All three drift-rebuild flags (`--rebuild-on-drift`, `--rebuild-on-toolchain-drift`, `--rebuild-on-flag-drift`) fall back to a sysforge.toml `[update]` default (`rebuild_on_drift`, `rebuild_on_toolchain_drift`, `rebuild_on_flag_drift` respectively) via `_resolve_drift_axes`, which resolves each axis through `config.resolve_flag_default`. The CLI flag always wins when explicitly passed; the config value only supplies the default when it isn't. -**PGO toolchain packages** (`build_mode = "pgo_llvm_toolchain"`) are handled specially during update. `makepkg_wrapper.run()` reads `toolchain.toml → pgo_store`, checks for a saved `clang.profdata` and its `clang.profdata.version` sidecar, and compares the sidecar's LLVM major version against the PKGBUILD's `pkgver` major. If they match, `-fprofile-use=` is injected and the build proceeds as a PGO-optimised build. If profdata is absent or version-mismatched (e.g. after a major LLVM bump), the user is prompted: **[p]lain build or [s]kip (default: skip)**. In non-interactive mode the build is skipped automatically. Skipped packages are counted separately in the update summary and do not count as failures. To rebuild profdata after a major version bump, run `sysforge run toolchain`. The toolchain stage itself also reuses compatible profdata — see the **Profdata reuse** section under stage 6. +**PGO toolchain packages** (`build_mode = "pgo_llvm_toolchain"`) are handled specially during update. `makepkg_wrapper.run()` reads `toolchain.toml → pgo_store`, checks for a saved `clang.profdata` and its `clang.profdata.version` sidecar, and compares the sidecar's LLVM major version against the PKGBUILD's `pkgver` major. If they match, `-fprofile-use=` is injected and the build proceeds as a PGO-optimised build. If profdata is absent or version-mismatched (e.g. after a major LLVM bump), the user is prompted: **[p]lain build or [s]kip (default: skip)**. In non-interactive mode the build is skipped automatically. Skipped packages are counted separately in the update summary and do not count as failures. To rebuild profdata after a major version bump, run `sysforge run toolchain`. The toolchain stage itself also reuses compatible profdata — see the **Profdata reuse** section under stage 5. **Stale-profraw post-build check.** After every non-PGO-managed build, `makepkg_wrapper.run()` globs `pgo_store` for `*.profraw` files. Any file with `mtime >= build_start - 1s` is treated as **fresh** — it was written by the build just completed, which means an instrumented LLVM is still installed on the system and the build was leaking profile data. The wrapper fatals, telling the user to reinstall `llvm`/`llvm-libs` or run `sysforge run toolchain`. Files strictly older than `build_start` are **orphans** left behind by a prior failed or partial toolchain run whose instrumented binaries the user has since cleaned up; these are unlinked in place and an info line is logged. The split makes the safety net self-healing: once the system is clean, the next build purges the residue automatically instead of requiring manual cleanup of `pgo_store`. **Neither verdict applies while a toolchain PGO run is in progress** (`3.3.0-B8`): if `build_lock.is_held(makepkg_pgo.resolve_pgo_lock_path(tcfg))` reports the `sysforge-pgo.lock` held, the store is that run's live Pass-3 training data, so a fresh file is not a leak and an "orphan" is data its merge monitor has not read yet. The check logs that it is deferring and touches nothing. Before this, a `sysforge build` started alongside `run toolchain` either fatally aborted on the training data or deleted whatever the monitor had not yet merged. `is_held` is a read-only probe: it never creates the lock file or overwrites the holder's PID. diff --git a/docs/release-notes/unreleased.md b/docs/release-notes/unreleased.md index be873f9e..37202461 100644 --- a/docs/release-notes/unreleased.md +++ b/docs/release-notes/unreleased.md @@ -273,6 +273,10 @@ https://keepachangelog.com/en/1.1.0/ --- +- **`3.5.0-DOC15` — the shipped config files explain themselves accurately.** The comments in the files under `/etc/sysforge` are the reference for their keys, and several were wrong or misleading. `profiles.toml` explained why the `optimized` profile has no `-fuse-ld=` flag, which made it look as if it chose its linker differently. No profile names a compiler or linker: both come from `[defaults] toolchain`, and the file now says so once, above the profiles. Its `LDFLAGS` reference no longer suggests adding `-fuse-ld=lld` yourself. `kernel.toml` said the kconfig fragment is applied with `scripts/config` (sysforge merges it in the PKGBUILD's `prepare()`), said there is "no kernel PGO" just above its own AutoFDO/Propeller `[fdo]` section, and listed optional keys under "Required fields". `toolchain.toml` now says how the compiler it registers reaches later builds, and how `profile_store` and `SYSFORGE_PROFILE_STORE` combine. `bootstrap.toml` named install stages that do not exist (`partition`, `base_install`). `packages.toml` gave its last example a heading about recommended dependencies that matched neither the example nor any key. `sysforge.toml` described a per-user override file that sysforge never reads, and listed only `run reconfigure` for `[ui] editor`. Pointers to `DESIGN.md` sections, which the package does not install, were removed. + +--- + - **`3.5.0-F2` — `run toolchain` builds the LLVM toolchain from upstream's single `llvm` package.** The repos now build clang, lld, polly, compiler-rt and openmp, along with lldb, libclc and others, as part of the `llvm` package. With PGO on, the whole set now goes through the profiling build together, which also means clang's own code (`libclang-cpp`) is optimized with the profile for the first time; before, only libLLVM was. Building them separately would have meant building the entire llvm source tree two extra times. When it installs, the toolchain stage now also replaces the other packages that come from the same build and that you already have installed (for example lldb and libclc), so they match the new libLLVM, and the rollback covers them too. Packages from that build you never installed, such as the WebAssembly runtimes, are built but not installed. Your `[packages]` lists in `toolchain.toml` keep working unchanged. --- diff --git a/etc/sysforge/bootstrap.toml b/etc/sysforge/bootstrap.toml index 1cfbde9f..e0ff55fa 100644 --- a/etc/sysforge/bootstrap.toml +++ b/etc/sysforge/bootstrap.toml @@ -4,8 +4,8 @@ # Everything down to line 25 ("END OF HEADER") is documentation; # the settings themselves start below it. # -# Used by the bootstrap pipeline stages (partition, base_install, hardware, -# configure). Edit this file before running sysforge run pipeline from a live +# Used by the bootstrap pipeline stages (install, hardware, configure). +# Edit this file before running sysforge run pipeline from a live # Arch install environment. Inert outside `sysforge run pipeline` — on an # already-installed system this file is never read. # @@ -20,8 +20,8 @@ # # text prose. Explanation only; nothing to uncomment. # #key = value a real setting, shown at its default or with an example # value. Delete the single leading `#` to activate it. -# Section headers are always live — only their keys are commented out, so -# activating a setting is a one-line edit. +# Section headers are never commented out, even where every key under one +# is, so activating a setting is a one-line edit. # ============================== END OF HEADER ================================ # Mount point for the new system during install. @@ -61,10 +61,10 @@ root_fs = "ext4" # pacman ParallelDownloads (default: 5) #parallel_downloads = 5 -# Account created by the base_install stage. Defaults to "builder". +# Account created by the install stage. Defaults to "builder". #username = "builder" -# Passwords applied during base_install. Leave unset to skip (run `passwd` +# Passwords applied by the install stage. Leave unset to skip (run `passwd` # manually inside the chroot or after first boot). Values are not retained # on disk after a successful pipeline run. #user_password = "" diff --git a/etc/sysforge/kernel.toml b/etc/sysforge/kernel.toml index 12c6568d..bc7e6d29 100644 --- a/etc/sysforge/kernel.toml +++ b/etc/sysforge/kernel.toml @@ -1,7 +1,7 @@ # ============================================================================= # kernel.toml — custom kernel build (stage 7 / sysforge run kernel) # -# Everything down to line 22 ("END OF HEADER") is documentation; +# Everything down to line 23 ("END OF HEADER") is documentation; # the settings themselves start below it. # # This stage is opt-in. Building a custom kernel is a deliberate choice; @@ -17,14 +17,15 @@ # # text prose. Explanation only; nothing to uncomment. # #key = value a real setting, shown at its default or with an example # value. Delete the single leading `#` to activate it. -# Section headers are always live — only their keys are commented out, so -# activating a setting is a one-line edit. +# Section headers are never commented out, so activating a setting is a +# one-line edit. The [[kconfig]] examples at the bottom are the exception: +# each is a whole entry, so uncomment all of one block or none of it. # ============================== END OF HEADER ================================ enabled = false # --------------------------------------------------------------------------- -# Required fields +# Which kernel to build — set pkgname, upstream_pkgname, or both # --------------------------------------------------------------------------- # pkgname — LOCAL package name to build/install as. For a pure-local tree it @@ -90,8 +91,9 @@ pkgname = "linux-sysforge" # unset, the kernel inherits whatever the toolchain stage registered (or # profile defaults when toolchain is disabled). # NOTE: compiler = "llvm" builds the kernel WITH clang; it does NOT apply -# PGO profile data. The profdata trains the clang binary itself, not the -# linux kernel target — there is no kernel PGO here. +# the toolchain's PGO profile data, which trains the clang binary itself, +# not the kernel. The kernel's own profile-guided path is AutoFDO/Propeller +# (see [fdo] below). #compiler = "gcc" # pkgbuild_src_dir — optional per-kernel override of the global [paths] @@ -140,7 +142,6 @@ pkgname = "linux-sysforge" # controller, or core boot infrastructure), and a post-install boot-readiness # check (vmlinuz + initramfs present and referenced by a boot entry; DKMS # modules rebuilt). Brick-class findings hard-fail; everything else warns. -# See DESIGN.md §Kernel stage boot-safety. # --------------------------------------------------------------------------- # require_fallback_kernel — default: true — refuse to install a custom @@ -276,10 +277,10 @@ pkgname = "linux-sysforge" # --------------------------------------------------------------------------- # kconfig — kernel config fragment # -# Entries are written to a sysforge.config fragment and applied via -# scripts/config before the build. Precedence (highest first): manual entries -# below > hardware_profile [kconfig] > hardware_profile [kconfig_devices]; -# manual overrides warn on conflict. +# Entries are written to a sysforge.config fragment, which sysforge merges in +# the PKGBUILD's prepare() (merge_config.sh). Precedence (highest first): +# manual entries below > hardware_profile [kconfig] > hardware_profile +# [kconfig_devices]; manual overrides warn on conflict. # # Each override is one [[kconfig]] array-of-tables entry with an 'option' and a # 'value'. Note this is NOT the same shape as hardware_profile.toml's [kconfig] diff --git a/etc/sysforge/packages.toml b/etc/sysforge/packages.toml index 2ba3d2ba..a88b1ace 100644 --- a/etc/sysforge/packages.toml +++ b/etc/sysforge/packages.toml @@ -4,7 +4,7 @@ # Everything down to line 51 ("END OF HEADER") is documentation; # the settings themselves start below it. # -# Roles (see DESIGN.md §Package Manifest): +# Roles: # - Pipeline `run packages` (bootstrap): every entry is installed. # - `sysforge update` / `sysforge build` (steady-state): entries are # build-rule OVERRIDES applied to the live install set. @@ -51,7 +51,8 @@ # ============================== END OF HEADER ================================ [build] -# pkgbuild_src_dir — override the path from profiles.toml for this manifest +# pkgbuild_src_dir — used by `sysforge update` and `run packages` in place of +# profiles.toml [paths] pkgbuild_src_dir (a differing value warns once) #pkgbuild_src_dir = "~/src" # # repo_mode — "pacman" (default) | "build_from_source" — bootstrap mode for @@ -149,7 +150,8 @@ # for group members. # # Curated desktop-environment groups can be written without hand-editing TOML: -# sysforge packages add-group gnome # or: kde +# sysforge packages add-group gnome # or kde, xfce, mate, cinnamon, +# # lxqt, budgie, cosmic # The bootstrap configure stage offers the same selection (or reads it from # bootstrap.toml [desktop] environment for unattended installs). # ============================================================================= @@ -159,13 +161,10 @@ # ============================================================================= -# SysForge recommended dependencies +# Packages — one [[package]] table per entry (fields in the header above) # -# ccache/sccache are used automatically when a profile enables them in -# profiles.toml — the "ccache" token in BUILDENV, plus RUSTC_WRAPPER / -# CCACHE_DIR / SCCACHE_DIR in the profile's env keys. Set `cache = false` on a -# [[package]] entry above to opt a single package out (required for PGO). -# reflector is used by the configure stage to set up /etc/pacman.d/mirrorlist. +# Example: install sysforge's own development package from the AUR during +# bootstrap. # ============================================================================= #[[package]] diff --git a/etc/sysforge/profiles.toml b/etc/sysforge/profiles.toml index ef112156..c1be22f5 100644 --- a/etc/sysforge/profiles.toml +++ b/etc/sysforge/profiles.toml @@ -2,7 +2,7 @@ # profiles.toml — compiler flag profiles, package matching rules, and # flag-resolution reference data (conflict groups + consumes inference) # -# Everything down to line 225 ("END OF HEADER") is documentation; +# Everything down to line 226 ("END OF HEADER") is documentation; # the settings themselves start below it. # # SysForge resolves a profile for each package by evaluating [[rules]] in @@ -84,13 +84,14 @@ # Arch's makepkg.conf sets _FORTIFY_SOURCE in CFLAGS (-Wp,…=3, kept by # [preserved_system_tokens]); defining it here again defines it twice. # -# LDFLAGS — use -fuse-ld=lld to select lld. Flag guards in -# emit_makepkg_conf handle incompatibilities automatically: lld-only -# flags (--icf=*) stripped when lld is not the linker; RUSTFLAGS linker -# reconciled to match LDFLAGS; -flto=thin rewritten to -flto for GCC; -# LTO disabled entirely when CC=gcc + linker=lld (GCC LTO bitcode is -# incompatible with lld). -# LDFLAGS = "-fuse-ld=lld -Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now,--icf=all" +# LDFLAGS — linker flags. The linker itself comes from `toolchain` (llvm +# adds -fuse-ld=lld); name a -fuse-ld= token here only to pick another +# one (e.g. mold). Flag guards in emit_makepkg_conf then reconcile it: +# lld-only flags (--icf=*) stripped when lld is not the linker; +# RUSTFLAGS linker reconciled to match LDFLAGS; -flto=thin rewritten to +# -flto for GCC; LTO disabled entirely when CC=gcc + linker=lld (GCC LTO +# bitcode is incompatible with lld). +# LDFLAGS = "-Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now,--icf=all" # DEBUG_CFLAGS = "-g" # DEBUG_CXXFLAGS = "-g" # DEBUG_LDFLAGS = "" @@ -242,6 +243,10 @@ toolchain = "gcc" # global package-compiler default: "gcc" | "llvm" (see heade # ============================================================================= # What each flag below changes and costs (-O2/-O3, -march=native, LTO, the # linker options, the hardening flags): docs/concepts/compiler-optimization.md +# +# No profile names a compiler or linker: both come from [defaults] `toolchain` +# ("gcc" → gcc/g++ + system ld; "llvm" → clang/clang++ + llvm-* + lld). Set +# `toolchain` in a profile to switch just that profile. [profiles.bare] # Fallback — no flags injected @@ -249,10 +254,6 @@ BUILDDIR = "$HOME/builds" [profiles.standard] extends = "bare" -# Compiler comes from the `toolchain` field (defaults to "gcc" via [defaults]). -# That expands to CC=gcc/CXX=g++ + system binutils, or CC=clang/CXX=clang++ + -# llvm-* binutils + -fuse-ld=lld for "llvm". Set `toolchain = "llvm"` here or in -# a rule to switch, or override individual CC/CXX/AR/… keys to win over it. CFLAGS = "-march=native -O2 -pipe" CXXFLAGS = "$CFLAGS" LDFLAGS = "-Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now" @@ -263,9 +264,8 @@ makepkg_flags = ["--noconfirm"] [profiles.optimized] extends = "standard" CFLAGS = "-march=native -O3 -pipe -fno-plt" -# No -fuse-ld= here: the llvm toolchain adds lld itself, and under gcc an lld -# link would cost LTO (GCC LTO bitcode is incompatible with lld). --icf=all is -# lld-only and is stripped when another linker is in use. +# --icf=all (fold identical functions) is an lld option, so it only takes +# effect under toolchain = "llvm"; under gcc's linker it is stripped at build. LDFLAGS = "-Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now,--icf=all" makepkg_flags = ["--noconfirm", "--cleanbuild"] diff --git a/etc/sysforge/sysforge.toml b/etc/sysforge/sysforge.toml index 9db31820..1318895e 100644 --- a/etc/sysforge/sysforge.toml +++ b/etc/sysforge/sysforge.toml @@ -5,14 +5,14 @@ # the settings themselves start below it. # # System default: /etc/sysforge/sysforge.toml -# Override individual values by creating this file or editing it in place. +# sysforge reads only this one copy (there is no per-user file): edit it here. # # COMMENT CONVENTION — two kinds of `#` lines appear below: # # text prose. Explanation only; nothing to uncomment. # #key = value a real setting, shown at its default or with an example # value. Delete the single leading `#` to activate it. -# Section headers are always live — only their keys are commented out, so -# activating a setting is a one-line edit. +# Section headers are never commented out, even where every key under one +# is, so activating a setting is a one-line edit. # ============================== END OF HEADER ================================ [ui] @@ -20,7 +20,8 @@ # NO_COLOR / FORCE_COLOR; "always" forces colour on (e.g. piping into a pager); # "never" disables it. The --color CLI flag overrides this. color = "auto" -#editor = "vim" # Override $EDITOR for sysforge run reconfigure +#editor = "vim" # Override $EDITOR wherever sysforge opens an editor: +# # run reconfigure, artifact edit, build-failure menu #merge = "vimdiff" # Diff/merge tool for `sysforge config merge` (.sfnew # # adoption). Resolution: SYSFORGE_MERGE > this > $DIFFPROG # # > vimdiff. Accepts args, e.g. "nvim -d" or "meld". @@ -47,8 +48,8 @@ clone_timeout = 60 [aur] # Politeness / rate-limit throttling for AUR (and gitlab repo) source fetches, -# applied by the source-sync scheduler. Both have safe defaults; uncomment to -# tune for a slow or rate-limit-sensitive connection. +# applied by the source-sync scheduler. All three keys have safe defaults; +# uncomment to tune for a slow or rate-limit-sensitive connection. # # enabled: false = never contact the AUR. No clones, no RPC lookups, no # package-list download. Packages tracked with source = "aur" are reported as @@ -144,10 +145,10 @@ clone_timeout = 60 #pre_build_snapshot = false [failure_handling] -# Per-scenario behaviour for build-failure auto-repair (DESIGN.md §1242). +# Per-scenario behaviour for build-failure auto-repair. # Values: "auto_repair" | "auto_repair_with_warning" | "prompt_user" | # "abort" | "warn_and_fallback" | "fallback" | "error". -# Defaults below match what auto_repair.py uses when a key is absent: +# Defaults below match what sysforge uses when a key is absent: # silent fixes for the safe scenarios, explicit user consent for # checksum_mismatch — silent auto-fix would mask supply-chain compromise, # DO NOT relax this without an equivalent compensating control. diff --git a/etc/sysforge/toolchain.toml b/etc/sysforge/toolchain.toml index f896a7f2..5cd1c00f 100644 --- a/etc/sysforge/toolchain.toml +++ b/etc/sysforge/toolchain.toml @@ -1,7 +1,7 @@ # ============================================================================= # toolchain.toml — custom LLVM or GCC toolchain build (stage 5 / sysforge run toolchain) # -# Everything down to line 30 ("END OF HEADER") is documentation; +# Everything down to line 28 ("END OF HEADER") is documentation; # the settings themselves start below it. # # This stage is opt-in. Building a custom toolchain is a deliberate choice; @@ -14,19 +14,17 @@ # paths into pipeline state and returns. Stock gcc-libs from pacman's # base-devel provides the runtime; sysforge never builds GCC from source. # -# "llvm" is opt-in only and enables the 4-pass PGO bootstrap, which is -# fragile across LLVM major bumps. Leave on "gcc" unless you specifically -# want PGO LLVM. -# -# The built compiler is automatically propagated to subsequent stages via -# pipeline state: packages and kernel will use CC/CXX from this result. +# The compiler this stage registers carries over: the packages and kernel +# stages of the same pipeline build with it (kernel.toml `compiler` can +# override), and profiles.toml's [defaults] toolchain is rewritten to match, +# so later `sysforge build` / `update` runs use it too. # # COMMENT CONVENTION — two kinds of `#` lines appear below: # # text prose. Explanation only; nothing to uncomment. # #key = value a real setting, shown at its default or with an example # value. Delete the single leading `#` to activate it. -# Section headers are always live — only their keys are commented out, so -# activating a setting is a one-line edit. +# Section headers are never commented out, even where every key under one +# is, so activating a setting is a one-line edit. # ============================== END OF HEADER ================================ enabled = false @@ -84,15 +82,15 @@ drift_detect = "fingerprint" #pgo_store = "/var/cache/sysforge/llvm-pgo" # # profile_store — root directory of the other profile stores: build --pgo's -# pgo/, the kernel's autofdo/ and propeller/, and bolt/. Overridden -# by SYSFORGE_PROFILE_STORE when unset here. +# pgo/, the kernel's autofdo/ and propeller/, and bolt/. When unset, +# SYSFORGE_PROFILE_STORE is used if set, else the default shown. #profile_store = "/var/cache/sysforge" # --------------------------------------------------------------------------- # Build-safety gates (LLVM path only — the GCC path is register-only and skips # all gates). Gate 1 runs cheap pre-build checks and hard-aborts before any # build time is spent on a condition that will definitely fail or break the -# live toolchain. See DESIGN.md §Toolchain stage boot-safety. +# live toolchain. # --------------------------------------------------------------------------- # min_build_free_gb — minimum free space (GiB) required on each filesystem @@ -199,6 +197,9 @@ enabled = false # member of those PKGBUILDs that is already installed (e.g. lldb); members you # never installed are built but left out. # +# The [packages] section only applies to compiler = "llvm" (the GCC path is +# register-only — no build, no package list). +# [packages] #pgo = ["llvm", "llvm-libs"] #non_pgo = ["clang", "lld", "polly", "compiler-rt", "openmp", "spirv-llvm-translator"] @@ -215,16 +216,13 @@ enabled = false # non-fatal (the run continues with LLVM-only data). Default ["llvm"] # (self-build only). LLVM/PGO path only. #training_corpus = ["llvm", "mesa"] - -# The [packages] section only applies to compiler = "llvm" (the GCC path is -# register-only — no build, no package list). # --------------------------------------------------------------------------- # --------------------------------------------------------------------------- # LLVM_TARGETS_TO_BUILD filtering # # When sysforge builds an LLVM PKGBUILD (llvm, clang, compiler-rt, lld, …), -# pkgbuild_patcher injects -DLLVM_TARGETS_TO_BUILD="" into the cmake +# sysforge injects -DLLVM_TARGETS_TO_BUILD="" into the cmake # invocation so the built toolchain only carries the backends this host # actually uses. # diff --git a/sysforge/CLAUDE.md b/sysforge/CLAUDE.md index 7ce1bff7..15575a91 100644 --- a/sysforge/CLAUDE.md +++ b/sysforge/CLAUDE.md @@ -262,10 +262,10 @@ Mechanism lives in the cited §DESIGN section. Gate 2 outside `sentinel_scope`, install→Gate-3→rollback inside. **GCC path never builds GCC** (register-only). LLVM `pgo=false` + repo_mode=`pacman` → install stock from repos; **PGO always builds from source**. `repo_mode` read via `config.resolve_repo_mode` only. -- **Pass-3 builds non-pgo against the libLLVM it ships** (`CMAKE_PREFIX_PATH` **and** forced +- **Pass-4 builds non-pgo against the libLLVM it ships** (`CMAKE_PREFIX_PATH` **and** forced `-DLLVM_DIR` via `patch_llvm_dir`; staging3 needs both `llvm-libs`+`llvm`). Build reuse opt-in via `build_fingerprint.py` (`--reuse-built`, fail-safe to rebuild; bump `_SCHEMA` on new inputs). -- **Pass-2 training corpus**: `resolve_training_corpus`; extras (mesa) compile with the instrumented +- **Pass-3 training corpus**: `resolve_training_corpus`; extras (mesa) compile with the instrumented stage1 clang so profraw merges into the one `clang.profdata` — never installed, never `-fprofile-use` (`pgo_reuse=False`), no profile-runtime force-load. Best-effort, but `BuildAborted` (user abort) re-raises. PGO path only. From 56e5afacbcdc665c51d4693d92ae93a93561cb8e Mon Sep 17 00:00:00 2001 From: Keith Raghubar Date: Sat, 24 Oct 2026 11:04:11 -0400 Subject: [PATCH 02/11] fix(build): honour a package entry's cache = false (3.5.0-B139) packages add --no-cache recorded cache = false but no build read it. makepkg_wrapper.run now looks the build up in packages.toml (config.cache_disabled_for, entries and group defaults) and adds CCACHE_DISABLE=1 and an empty RUSTC_WRAPPER to its injected env, which win over BUILDENV, a ccache PATH directory and a shell-exported wrapper. Drops the "required for PGO" claim from the help, completion, man page, shipped config and design doc. Co-Authored-By: Claude Opus 5.5 --- DESIGN.md | 2 +- ROADMAP.md | 18 ------ completions/_sysforge | 2 +- docs/design/05-package-manifest.md | 2 +- docs/release-notes/unreleased.md | 4 ++ etc/sysforge/packages.toml | 5 +- man/sysforge.1 | 4 +- sysforge/build/makepkg_wrapper.py | 22 ++++++- sysforge/packages_cmd.py | 2 +- sysforge/primitives/config.py | 21 +++++++ tests/data/etc/sysforge/packages.toml | 3 +- tests/test_package_cache_opt_out.py | 90 +++++++++++++++++++++++++++ 12 files changed, 147 insertions(+), 28 deletions(-) create mode 100644 tests/test_package_cache_opt_out.py diff --git a/DESIGN.md b/DESIGN.md index 1878893b..763a3a2c 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -394,7 +394,7 @@ The orthogonality of the roles means: Each entry overrides at most these fields (all optional except `name`): - `source` — `repo` (pacman) vs `aur`/`git`/`local`. A **routing hint**: it tells the bootstrap/build paths how to obtain the package, falling through to pacman / AUR RPC inference if omitted. It does **not** by itself put a package under steady-state tracking — tracking comes from sysforge having built the package (build_state), not from a manifest entry. So an entry with only `name` + `source` has no override effect at steady-state, and `sysforge packages add` rejects it (pair it with a behavior-changing field — `enable_build_from_source`, `cache`, `reason` — for the entry to override anything). When sysforge builds a package, it records the resolved `source` in build_state, so the registry is self-describing without re-inferring origin later. - `enable_build_from_source` *(bool)* — if `true`, this **repo** package is built from source (via `pkgctl repo clone` + makepkg with sysforge flag profiles) instead of installed as the binary via pacman. It also opts the package into `sysforge build` without a confirmation prompt (see §`build`). The boolean replaced the misleadingly-named `pkgbuild_patch` (which never patched anything — real PKGBUILD patching is gated by separate predicates); as of 3.0.0 the legacy `pkgbuild_patch` key is no longer honored on read (`config.normalize_package_entry` is now an identity function) — a manifest that still spells it no longer opts the package into a source build. `packages_cmd` still rewrites the key to the current name on the next `packages.toml` write (`add`/`remove`), so a file touched since the rename self-cleans; a never-rewritten file keeps the stale key inert. -- `cache` *(bool)* — `false` disables ccache/sccache for this package (required for PGO stages). +- `cache` *(bool)* — `false` builds this package with ccache and sccache off: `CCACHE_DISABLE=1` and an empty `RUSTC_WRAPPER` in its makepkg environment (`config.cache_disabled_for`, applied in `makepkg_wrapper.run`). ```toml [build] diff --git a/ROADMAP.md b/ROADMAP.md index 90b2f5a6..ff450958 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -112,7 +112,6 @@ canonical ordering. | ID | Item | Priority | Effort | Bump | |----|------|----------|--------|------| -| `3.5.0-B139` | a package entry's cache = false is recorded but never applied | med | small | patch | | `3.5.0-B140` | sysforge.toml [failure_handling] is never read | med | medium | patch | | `3.5.0-DEV23` | widen the end-to-end tier beyond build, update and setup | med | large | patch | @@ -121,23 +120,6 @@ canonical ordering. ### Bugs -- **`3.5.0-B139` — a package entry's `cache = false` is recorded but never applied.** - `sysforge packages add --no-cache` writes `cache = false` to the `[[package]]` entry (a - `[group.*]` table accepts it as a member default too), `packages list` shows it, and - `verbs.shared.OVERRIDE_FIELDS` counts it as a behaviour-changing override, which silences - `update`'s inert-entry warning. No build path reads it: `build_core`, `makepkg_wrapper` and - `makepkg_invoke` never consult the entry, so ccache and sccache stay on for that package. The - shipped `packages.toml` and the `--no-cache` help both call it "required for PGO", which is - doubtful as well: neither cache sits between an instrumented binary and the profraw it writes, - and the toolchain stage already sets `CCACHE_DISABLE`/`SCCACHE_DISABLE` itself where it needs - to. Fix by honouring it (the same two env vars, plus dropping `RUSTC_WRAPPER`, for that - package's makepkg env) and correcting the "required for PGO" wording in `packages.toml`, the - help text and the man page. Retiring the key instead would be a `major` removal through - `tools/retired_names.toml` and `config_schema.RETIRED_KEYS`. Found in the shipped-config audit - of 2026-10-24 (`3.5.0-DOC15`). - *Priority: med · Effort: small · Bump: patch* — a documented switch that silently does - nothing; anyone relying on it gets cached builds. - - **`3.5.0-B140` — `sysforge.toml [failure_handling]` is never read.** The shipped `sysforge.toml` documents `[failure_handling]` and `config_schema.KNOWN_SECTIONS` lists it there, but both readers take it from the `profiles.toml` dict: diff --git a/completions/_sysforge b/completions/_sysforge index ff9193ef..6e985929 100644 --- a/completions/_sysforge +++ b/completions/_sysforge @@ -435,7 +435,7 @@ _sysforge_packages_add() { '--packages[path to packages.toml]:file:_files' \ '--source[pin routing (metadata only)]:source:(repo aur local)' \ '--enable-build-from-source[build from source]' \ - '--no-cache[disable ccache/sccache (required for PGO)]' \ + '--no-cache[build this package with ccache and sccache off]' \ '--reason[free-form note attached to the entry]:text' \ '1: :_sysforge_pkg_names' } diff --git a/docs/design/05-package-manifest.md b/docs/design/05-package-manifest.md index 73adb45e..bf87fc7c 100644 --- a/docs/design/05-package-manifest.md +++ b/docs/design/05-package-manifest.md @@ -49,7 +49,7 @@ The orthogonality of the roles means: Each entry overrides at most these fields (all optional except `name`): - `source` — `repo` (pacman) vs `aur`/`git`/`local`. A **routing hint**: it tells the bootstrap/build paths how to obtain the package, falling through to pacman / AUR RPC inference if omitted. It does **not** by itself put a package under steady-state tracking — tracking comes from sysforge having built the package (build_state), not from a manifest entry. So an entry with only `name` + `source` has no override effect at steady-state, and `sysforge packages add` rejects it (pair it with a behavior-changing field — `enable_build_from_source`, `cache`, `reason` — for the entry to override anything). When sysforge builds a package, it records the resolved `source` in build_state, so the registry is self-describing without re-inferring origin later. - `enable_build_from_source` *(bool)* — if `true`, this **repo** package is built from source (via `pkgctl repo clone` + makepkg with sysforge flag profiles) instead of installed as the binary via pacman. It also opts the package into `sysforge build` without a confirmation prompt (see §`build`). The boolean replaced the misleadingly-named `pkgbuild_patch` (which never patched anything — real PKGBUILD patching is gated by separate predicates); as of 3.0.0 the legacy `pkgbuild_patch` key is no longer honored on read (`config.normalize_package_entry` is now an identity function) — a manifest that still spells it no longer opts the package into a source build. `packages_cmd` still rewrites the key to the current name on the next `packages.toml` write (`add`/`remove`), so a file touched since the rename self-cleans; a never-rewritten file keeps the stale key inert. -- `cache` *(bool)* — `false` disables ccache/sccache for this package (required for PGO stages). +- `cache` *(bool)* — `false` builds this package with ccache and sccache off: `CCACHE_DISABLE=1` and an empty `RUSTC_WRAPPER` in its makepkg environment (`config.cache_disabled_for`, applied in `makepkg_wrapper.run`). ```toml [build] diff --git a/docs/release-notes/unreleased.md b/docs/release-notes/unreleased.md index 37202461..169e4d51 100644 --- a/docs/release-notes/unreleased.md +++ b/docs/release-notes/unreleased.md @@ -860,6 +860,10 @@ https://keepachangelog.com/en/1.1.0/ - **`3.5.0-B138` — a command that fails before changing anything no longer blocks the next run.** `build`, `update`, `uninstall`, `revert-to-stock` and the other commands that change the system write a record when they start, so that if one is interrupted mid-install the next run stops and offers to restore the system. The record was written before the command's own checks, so a harmless early failure, such as `sysforge build` on a system without `pkgctl`, left it behind: the next run then refused to start ("Stale stage-in-progress sentinel present"), asking for a recovery at a terminal and simply failing under a timer or cron. The record is now written at the first change to the system, the first package install, removal or boot update. Downloads, the build sandbox's own setup and config-file writes do not count, since an interruption cannot leave them half done. A second run is still refused while the first is running, and a Ctrl-C before anything changed says so. +--- + +- **`3.5.0-B139` — `cache = false` on a package entry now turns the compiler caches off.** `sysforge packages add --no-cache` recorded the setting and `packages list` showed it, but no build read it, so ccache and sccache kept running for that package. Builds now honour it, from a `[[package]]` entry or a `[group.*]` default: ccache is off however it is reached (makepkg's `BUILDENV` or a ccache directory first on `PATH`), and so is a `RUSTC_WRAPPER` such as sccache, even one your shell exports. The `--no-cache` help, the man page and `packages.toml` no longer say the setting is required for PGO; it is not. + ## Security - **`3.5.0-B37` — the root pacman hook no longer follows links in the state directory.** The hook helper runs as root in every pacman transaction and wrote into `/var/lib/sysforge/sentinels`, whose parent any `sysforge` group member can write to. A member could swap `sentinels` for a symlink to another directory, which the next transaction made group-writable, or symlink one of its files so the hook appended to any file as root. The helper now enters the directory and acts only if it is the real `sentinels`, owned by root. It appends only to a regular file, opened so that a symlink is refused and a planted FIFO cannot stall the transaction. As before, it never fails a transaction. diff --git a/etc/sysforge/packages.toml b/etc/sysforge/packages.toml index a88b1ace..d2499548 100644 --- a/etc/sysforge/packages.toml +++ b/etc/sysforge/packages.toml @@ -1,7 +1,7 @@ # ============================================================================= # packages.toml — SysForge package manifest # -# Everything down to line 51 ("END OF HEADER") is documentation; +# Everything down to line 52 ("END OF HEADER") is documentation; # the settings themselves start below it. # # Roles: @@ -26,7 +26,8 @@ # enable_build_from_source # optional true = build this repo package from source # instead of installing the binary via pacman -# cache optional false = disable ccache/sccache (required for PGO) +# cache optional false = build this package with ccache and +# sccache off # # An entry with only `name` (or only `name` + `source`) has no override # effect at steady-state. `sysforge packages add` rejects such calls, and diff --git a/man/sysforge.1 b/man/sysforge.1 index ea9b1eeb..fa2e8f46 100644 --- a/man/sysforge.1 +++ b/man/sysforge.1 @@ -5,7 +5,7 @@ .nh .ad l .\" Begin generated content: -.TH "sysforge" "1" "2026\-10\-22" +.TH "sysforge" "1" "2026\-10\-24" .PP .PP .SH NAME @@ -1187,7 +1187,7 @@ Build this repo package from source instead of installing the binary via pacman. .RE \fB\-\-no\-cache\fR .RS 4 -Disable ccache/sccache for this package (required for PGO).\& +Build this package with ccache and sccache off.\& .PP .RE \fB\-\-reason\fR \fITEXT\fR diff --git a/sysforge/build/makepkg_wrapper.py b/sysforge/build/makepkg_wrapper.py index 43e174db..a0018e06 100644 --- a/sysforge/build/makepkg_wrapper.py +++ b/sysforge/build/makepkg_wrapper.py @@ -51,6 +51,7 @@ ) from sysforge.primitives.config import ( _active_profiles_path, + cache_disabled_for, find_pkgbuild, load_config, load_conflict_groups, @@ -589,6 +590,24 @@ def _pkgname_from_meta(pkgmeta: dict | None) -> str: return name or "unknown" +# A packages.toml `cache = false` entry (3.5.0-B139). CCACHE_DISABLE reaches +# ccache however it was invoked, BUILDENV's or a ccache directory first on PATH; +# the empty RUSTC_WRAPPER wins over one the shell or the profile exports. +_CACHE_OFF_ENV = {"CCACHE_DISABLE": "1", "RUSTC_WRAPPER": ""} + + +def _with_cache_opt_out(env: dict | None, config: dict, pkgmeta: dict | None, + pkgbuild_path: Path) -> dict | None: + """``env`` plus the cache-off keys when packages.toml opts this build out.""" + globals_ = (pkgmeta or {}).get("globals", {}) + names = [globals_.get("pkgbase"), pkgbuild_path.parent.name, + *((pkgmeta or {}).get("pkgnames") or [])] + if not cache_disabled_for(config, names): + return env + _build_log.info("packages.toml cache = false: ccache and sccache off for this build") + return {**(env or {}), **_CACHE_OFF_ENV} + + def _apply_staged_llvm_patches(pkgbuild_path, cmake_llvm_dir: str | None) -> bool: """Steer a staged toolchain pass (1b/3b/3c) at the staged libLLVM prefix. @@ -1892,7 +1911,8 @@ def run(pkgbuild_path, options: BuildOptions | None = None) -> Path | None: compiler_flags_extra=effective_flags_extra, linker_flags_extra=options.linker_flags_extra, strip_full_lto=options.strip_full_lto, - injected_env=options.extra_env, + injected_env=_with_cache_opt_out( + options.extra_env, config, pkgmeta, pkgbuild_path), strip_flags=effective_strip_flags, pkgbuild_has_hardcoded_gcc=pkgbuild_has_hardcoded_gcc, state_dir=options.state_dir, diff --git a/sysforge/packages_cmd.py b/sysforge/packages_cmd.py index 9877f4b1..3a8c9fee 100644 --- a/sysforge/packages_cmd.py +++ b/sysforge/packages_cmd.py @@ -279,7 +279,7 @@ def add_parser(cls, sub): help="Build this repo package from source instead of installing the " "binary via pacman.") p.add_argument("--no-cache", action="store_true", dest="no_cache", - help="Disable ccache/sccache for this package (required for PGO).") + help="Build this package with ccache and sccache off.") p.add_argument("--reason", metavar="TEXT", dest="reason", help="Free-form note attached to the entry.") p.add_argument("--packages", metavar="FILE", dest="packages", diff --git a/sysforge/primitives/config.py b/sysforge/primitives/config.py index 77a3665d..0766e04a 100644 --- a/sysforge/primitives/config.py +++ b/sysforge/primitives/config.py @@ -17,6 +17,7 @@ load_sysforge_toml() -> dict find_pkgbuild(pkg, config=None) -> Path (AUR clone on miss if pkgbuild_src_dir set) resolve_pkgbuild_src_dir(config, build_cfg=None) -> str | None + cache_disabled_for(config, names) -> bool (packages.toml `cache = false`) """ import json import pprint @@ -407,6 +408,26 @@ def expand_package_groups(data: dict) -> list[dict]: return entries +def cache_disabled_for(config: dict, names) -> bool: + """True when packages.toml opts any of ``names`` out of the compiler caches. + + ``names`` are one build's identities (pkgbase, every pkgname, the checkout + directory), since an entry may name any of them. The opt-out is an entry's + or group default's ``cache = false``, written by ``packages add --no-cache`` + (3.5.0-B139). A missing or unreadable manifest opts nothing out: the caches + only make a build faster, so the lookup never fails one. + """ + from sysforge.primitives.paths import resolve_packages_path + try: + with resolve_packages_path(config).open("rb") as f: + data = tomllib.load(f) + except (OSError, tomllib.TOMLDecodeError): + return False + wanted = {n for n in names if n} + return any(e.get("name") in wanted and e.get("cache") is False + for e in expand_package_groups(data)) + + def load_config(config_paths=None) -> dict: """ Load profiles.toml from CONFIG_PATHS (user, then system). diff --git a/tests/data/etc/sysforge/packages.toml b/tests/data/etc/sysforge/packages.toml index d51b4233..0cba4475 100644 --- a/tests/data/etc/sysforge/packages.toml +++ b/tests/data/etc/sysforge/packages.toml @@ -12,7 +12,8 @@ # classification is ambiguous or you want to force routing # enable_build_from_source optional true = build this repo package from source # instead of installing the binary via pacman -# cache optional false = disable ccache/sccache (required for PGO) +# cache optional false = build this package with ccache and +# sccache off # # An entry with only `name` (or only `name` + `source`) has no override # effect at steady-state — `source` is metadata only. `sysforge packages add` diff --git a/tests/test_package_cache_opt_out.py b/tests/test_package_cache_opt_out.py new file mode 100644 index 00000000..7e25c5e2 --- /dev/null +++ b/tests/test_package_cache_opt_out.py @@ -0,0 +1,90 @@ +# SPDX-FileCopyrightText: 2026 Keith Raghubar +# +# SPDX-License-Identifier: MIT + +"""A packages.toml entry's `cache = false` turns the compiler caches off (3.5.0-B139). + +`packages add --no-cache` wrote the key and `packages list` showed it, but no +build read it, so ccache and sccache stayed on. The opt-out reaches makepkg's +environment: CCACHE_DISABLE for ccache (BUILDENV's or a ccache directory first +on PATH alike) and an empty RUSTC_WRAPPER, which wins over one exported by the +shell or a profile. +""" +from unittest.mock import patch + +import pytest + +from sysforge.build import makepkg_wrapper as mw +from sysforge.build.makepkg_wrapper import BuildOptions +from sysforge.primitives import config as cfgmod +from sysforge.primitives import paths + + +@pytest.fixture +def manifest(tmp_path, monkeypatch): + path = tmp_path / "packages.toml" + monkeypatch.setattr(paths, "PACKAGES_PATH", path) + return path + + +@pytest.mark.parametrize("text, names, expected", [ + ('[[package]]\nname = "foo"\ncache = false\n', ["foo"], True), + ('[[package]]\nname = "foo"\ncache = false\n', ["foo-git", "foo"], True), + ('[[package]]\nname = "foo"\ncache = true\n', ["foo"], False), + ('[[package]]\nname = "foo"\nenable_build_from_source = true\n', ["foo"], False), + ('[[package]]\nname = "bar"\ncache = false\n', ["foo"], False), + ('[group.g]\npackages = ["foo"]\ncache = false\n', ["foo"], True), + # An explicit entry wins over a group default outright. + ('[[package]]\nname = "foo"\n\n[group.g]\npackages = ["foo"]\ncache = false\n', + ["foo"], False), +], ids=["entry", "any-name", "cache-true", "no-key", "other-pkg", "group", "entry-over-group"]) +def test_cache_disabled_for(manifest, text, names, expected): + manifest.write_text(text) + assert cfgmod.cache_disabled_for({}, names) is expected + + +def test_no_manifest_disables_nothing(manifest): + assert cfgmod.cache_disabled_for({}, ["foo"]) is False + + +def test_an_unreadable_manifest_disables_nothing(manifest): + manifest.write_text("[[package]\n") + assert cfgmod.cache_disabled_for({}, ["foo"]) is False + + +@pytest.fixture +def checkout(tmp_path): + d = tmp_path / "foo" + d.mkdir() + (d / "PKGBUILD").write_text("pkgname=foo\npkgver=1\npkgrel=1\n") + return d + + +def _injected_env(pkgbuild, extra_env=None): + seen = {} + with patch.object(mw, "_run_build", side_effect=lambda *a, **k: seen.update(k)): + mw.run(pkgbuild, options=BuildOptions( + update=False, pkg_log=False, profile_override="standard", + extra_env=extra_env)) + return seen["injected_env"] or {} + + +def test_opted_out_build_turns_both_caches_off(manifest, checkout): + manifest.write_text('[[package]]\nname = "foo"\ncache = false\n') + env = _injected_env(checkout / "PKGBUILD") + assert env["CCACHE_DISABLE"] == "1" + assert env["RUSTC_WRAPPER"] == "" + + +def test_opt_out_keeps_the_callers_env(manifest, checkout): + manifest.write_text('[[package]]\nname = "foo"\ncache = false\n') + env = _injected_env(checkout / "PKGBUILD", extra_env={"LLVM_PROFILE_FILE": "x"}) + assert env["LLVM_PROFILE_FILE"] == "x" + assert env["CCACHE_DISABLE"] == "1" + + +def test_a_build_without_the_opt_out_leaves_the_caches_alone(manifest, checkout): + manifest.write_text('[[package]]\nname = "foo"\nenable_build_from_source = true\n') + env = _injected_env(checkout / "PKGBUILD") + assert "CCACHE_DISABLE" not in env + assert "RUSTC_WRAPPER" not in env From c66816ce6e48f399dd8fb39d531e1e093cfd3a4c Mon Sep 17 00:00:00 2001 From: Keith Raghubar Date: Sat, 24 Oct 2026 11:10:32 -0400 Subject: [PATCH 03/11] fix(build): read [failure_handling] from sysforge.toml (3.5.0-B140) makepkg_wrapper.run passed every reader the profiles.toml dict, so the documented sysforge.toml section did nothing. failure.SCENARIOS is now the one home for each key's accepted values and default; resolve_failure_handling validates the sysforge.toml table and run() folds it into the config the readers already take. checksum_mismatch accepts only prompt_user or abort, and an unexpected auto-repair value aborts instead of repairing. Drops the five scenarios nothing raises and documents the three live ones in the shipped file. Co-Authored-By: Claude Opus 5.5 --- DESIGN.md | 29 +++---- ROADMAP.md | 36 -------- docs/design/09-primitives-layer.md | 2 +- docs/design/11-makepkg-wrapper.md | 27 +++--- docs/release-notes/unreleased.md | 4 + etc/sysforge/sysforge.toml | 31 +++++-- sysforge/build/makepkg_wrapper.py | 15 ++-- sysforge/primitives/auto_repair.py | 10 +-- sysforge/primitives/failure.py | 83 ++++++++++++------ sysforge/primitives/profile.py | 2 + tests/data/etc/sysforge/sysforge.toml | 5 +- tests/data/surface.json | 3 + tests/test_failure.py | 97 ++++++--------------- tests/test_failure_handling_config.py | 118 ++++++++++++++++++++++++++ 14 files changed, 277 insertions(+), 185 deletions(-) create mode 100644 tests/test_failure_handling_config.py diff --git a/DESIGN.md b/DESIGN.md index 763a3a2c..8fab2cc2 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -2366,7 +2366,7 @@ Log tag: `[PROV]`. The `suggest_*` / `files_db_present` queries are pure read-on Cross-cutting failure scenario handler. Imported by `makepkg_wrapper` and `dep_analysis` to avoid circular imports. -`handle_failure(scenario, message, config, fallback=None)` dispatches to `abort`, `error`, `warn_and_fallback`, or `fallback` based on `[failure_handling]` config. `profile_missing` and `tempfile_write_failed` always abort regardless of config. +`handle_failure(scenario, message, config, fallback=None)` dispatches to `abort`, `error`, `warn_and_fallback`, or `fallback` based on `sysforge.toml [failure_handling]`, validated per key by `resolve_failure_handling` against `SCENARIOS` (the one home for every key's values and default). `tempfile_write_failed` always aborts regardless of config, as does a scenario not in `SCENARIOS`. ### `run.py` @@ -3427,23 +3427,18 @@ Preflight refuses rather than downgrades — missing `devtools`, a missing `/src`, and `makepkg_env._logdest_tail(pkgbuild_path)` reads the tail of the newest `$LOGDEST/-*.log` (makepkg's captured stdout when `OPTIONS+=log`). Both `BUILDDIR` and `LOGDEST` are resolved through `pacman.get_builddir()` / `pacman.get_logdest()` — env first, then the layered system `makepkg.conf` — so a user who configures these only in `/etc/makepkg.conf` still gets a diagnosis. Don't read `os.environ["BUILDDIR"]` or assume `~/builds` directly here. -Configuration extends `[failure_handling]` with four scenario keys: +Auto-repair has four keys of its own in the same table: ```toml -vendored_deps_missing = "auto_repair" +vendored_deps_missing = "auto_repair" # auto_repair | auto_repair_with_warning | abort pgp_key_missing = "auto_repair" -srcinfo_drift = "auto_repair_with_warning" -checksum_mismatch = "prompt_user" +srcinfo_drift = "auto_repair_with_warning" # … | warn_and_fallback (preflight only) +checksum_mismatch = "prompt_user" # prompt_user | abort, nothing else ``` -Three new behaviour values join the existing `abort` / `warn_and_fallback` / `fallback` / `error` set: `auto_repair` (repair silently, retry, log at info), `auto_repair_with_warning` (repair, retry, log at `[WARN]`), `prompt_user` (surface the diff and require explicit consent before repairing). In **batch mode** (`batch=true` on the resolved profile) the `prompt_user` behaviour is short-circuited to `aborted` — auto-repair never runs unattended for security-sensitive scenarios. This is the load-bearing rule for `checksum_mismatch`. +`auto_repair` repairs, retries and logs at info; `auto_repair_with_warning` logs the same at `[WARN]`; `prompt_user` surfaces the failing files and requires explicit consent before repairing. `checksum_mismatch` accepts only `prompt_user` or `abort`, so neither an edit nor a typo can select an unattended `updpkgsums`, which would mask a tampered source. In **batch mode** (`batch=true` on the resolved profile) `prompt_user` is short-circuited to `aborted`, so the repair never runs unattended. `auto_repair._normalise_behaviour` maps anything unexpected that still reaches it to `abort`, never to a repair. `.SRCINFO` drift is the one scenario that is **not** in `REGISTRY` — it has a different lifecycle. `_run_build` calls `auto_repair.preflight_srcinfo(pkgbuild_dir, behaviour)` before the build starts, regenerating `.SRCINFO` (with a `[WARN]` by default) when `makepkg --printsrcinfo` differs from the committed file. The other three scenarios fire on retry from a captured failure. diff --git a/ROADMAP.md b/ROADMAP.md index ff450958..a6cd5550 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -112,7 +112,6 @@ canonical ordering. | ID | Item | Priority | Effort | Bump | |----|------|----------|--------|------| -| `3.5.0-B140` | sysforge.toml [failure_handling] is never read | med | medium | patch | | `3.5.0-DEV23` | widen the end-to-end tier beyond build, update and setup | med | large | patch | @@ -120,41 +119,6 @@ canonical ordering. ### Bugs -- **`3.5.0-B140` — `sysforge.toml [failure_handling]` is never read.** - The shipped `sysforge.toml` documents `[failure_handling]` and `config_schema.KNOWN_SECTIONS` - lists it there, but both readers take it from the `profiles.toml` dict: - `makepkg_wrapper.run` calls `load_config()` (the profiles loader) and passes that `config` to - the auto-repair driver (`_failure_cfg`, the `srcinfo_drift` preflight and `apply_first_match`), - to `failure.handle_failure` for `pkgbuild_unparseable` and `tempfile_write_failed`, and to - `dep_analysis.run_dep_analysis` for `abi_mismatch` and `makedep_probe_failed`. Editing the - documented section therefore changes nothing; the same table in `profiles.toml` would be - honoured, but the schema calls it an unknown section there. Today's behaviour is the hardcoded - defaults, which happen to be the safe ones. Three further faults sit in the same seam: - (1) **two vocabularies, two fallbacks.** The four auto-repair keys take - `auto_repair`/`auto_repair_with_warning`/`prompt_user`/… and an unknown value falls back to - `auto_repair` (`_normalise_behaviour`), so a typo in `checksum_mismatch` selects the - silent-repair behaviour the shipped comment says must never be relaxed. The `failure.py` - scenarios take only `abort`/`error`/`warn_and_fallback`/`fallback`, and an unknown value - aborts. The shipped comment lists all seven values for every key. (2) **the - `checksum_mismatch` guard is not where the config says.** It is safe only because - `_repair_checksum_mismatch` always prompts, whatever the behaviour. Under `auto_repair` the - `batch` short-circuit (which only checks for `prompt_user`) is skipped, so a `batch = true` - profile run at a terminal still stops to ask. (3) **undocumented and dead scenarios.** - `_FAILURE_DEFAULTS` has nine keys; four have callers (`pkgbuild_unparseable`, `abi_mismatch`, - `makedep_probe_failed`, and `tempfile_write_failed`, which always aborts), and none of the four - is in the shipped file. `no_rule_matched`, `profile_missing`, `profile_cycle`, `env_conflict` - and `dep_unsatisfied` are never raised. `failure_handling` is also still in - `profile.SYSFORGE_KEYS`, left from when it was a per-profile key. Fix: one resolver in - `primitives/config.py` reads the table from `load_sysforge_toml()` and validates each key against - its own vocabulary (an unknown value takes that scenario's safe default, never `auto_repair`); - pass it to all three readers; make `checksum_mismatch` refuse anything but `prompt_user` or - `abort`; document the four live `failure.py` keys and each key's values in `sysforge.toml`; drop - the dead defaults and the `SYSFORGE_KEYS` entry. Found while auditing the shipped configs on - 2026-10-24 (`3.5.0-DOC15`). - *Priority: med · Effort: medium · Bump: patch* — a documented, security-flagged setting that - silently does nothing; the defaults keep it safe today, but any edit is ignored and a typo in the - right file would weaken the checksum guard. - ### Developer tooling - **`3.5.0-DEV23` — widen the end-to-end tier beyond `build`, `update` and setup.** diff --git a/docs/design/09-primitives-layer.md b/docs/design/09-primitives-layer.md index 26d4b623..3e5c00c2 100644 --- a/docs/design/09-primitives-layer.md +++ b/docs/design/09-primitives-layer.md @@ -478,7 +478,7 @@ Log tag: `[PROV]`. The `suggest_*` / `files_db_present` queries are pure read-on Cross-cutting failure scenario handler. Imported by `makepkg_wrapper` and `dep_analysis` to avoid circular imports. -`handle_failure(scenario, message, config, fallback=None)` dispatches to `abort`, `error`, `warn_and_fallback`, or `fallback` based on `[failure_handling]` config. `profile_missing` and `tempfile_write_failed` always abort regardless of config. +`handle_failure(scenario, message, config, fallback=None)` dispatches to `abort`, `error`, `warn_and_fallback`, or `fallback` based on `sysforge.toml [failure_handling]`, validated per key by `resolve_failure_handling` against `SCENARIOS` (the one home for every key's values and default). `tempfile_write_failed` always aborts regardless of config, as does a scenario not in `SCENARIOS`. ### `run.py` diff --git a/docs/design/11-makepkg-wrapper.md b/docs/design/11-makepkg-wrapper.md index a009615a..4879570a 100644 --- a/docs/design/11-makepkg-wrapper.md +++ b/docs/design/11-makepkg-wrapper.md @@ -53,23 +53,18 @@ Preflight refuses rather than downgrades — missing `devtools`, a missing `/src`, and `makepkg_env._logdest_tail(pkgbuild_path)` reads the tail of the newest `$LOGDEST/-*.log` (makepkg's captured stdout when `OPTIONS+=log`). Both `BUILDDIR` and `LOGDEST` are resolved through `pacman.get_builddir()` / `pacman.get_logdest()` — env first, then the layered system `makepkg.conf` — so a user who configures these only in `/etc/makepkg.conf` still gets a diagnosis. Don't read `os.environ["BUILDDIR"]` or assume `~/builds` directly here. -Configuration extends `[failure_handling]` with four scenario keys: +Auto-repair has four keys of its own in the same table: ```toml -vendored_deps_missing = "auto_repair" +vendored_deps_missing = "auto_repair" # auto_repair | auto_repair_with_warning | abort pgp_key_missing = "auto_repair" -srcinfo_drift = "auto_repair_with_warning" -checksum_mismatch = "prompt_user" +srcinfo_drift = "auto_repair_with_warning" # … | warn_and_fallback (preflight only) +checksum_mismatch = "prompt_user" # prompt_user | abort, nothing else ``` -Three new behaviour values join the existing `abort` / `warn_and_fallback` / `fallback` / `error` set: `auto_repair` (repair silently, retry, log at info), `auto_repair_with_warning` (repair, retry, log at `[WARN]`), `prompt_user` (surface the diff and require explicit consent before repairing). In **batch mode** (`batch=true` on the resolved profile) the `prompt_user` behaviour is short-circuited to `aborted` — auto-repair never runs unattended for security-sensitive scenarios. This is the load-bearing rule for `checksum_mismatch`. +`auto_repair` repairs, retries and logs at info; `auto_repair_with_warning` logs the same at `[WARN]`; `prompt_user` surfaces the failing files and requires explicit consent before repairing. `checksum_mismatch` accepts only `prompt_user` or `abort`, so neither an edit nor a typo can select an unattended `updpkgsums`, which would mask a tampered source. In **batch mode** (`batch=true` on the resolved profile) `prompt_user` is short-circuited to `aborted`, so the repair never runs unattended. `auto_repair._normalise_behaviour` maps anything unexpected that still reaches it to `abort`, never to a repair. `.SRCINFO` drift is the one scenario that is **not** in `REGISTRY` — it has a different lifecycle. `_run_build` calls `auto_repair.preflight_srcinfo(pkgbuild_dir, behaviour)` before the build starts, regenerating `.SRCINFO` (with a `[WARN]` by default) when `makepkg --printsrcinfo` differs from the committed file. The other three scenarios fire on retry from a captured failure. diff --git a/docs/release-notes/unreleased.md b/docs/release-notes/unreleased.md index 169e4d51..793081fa 100644 --- a/docs/release-notes/unreleased.md +++ b/docs/release-notes/unreleased.md @@ -864,6 +864,10 @@ https://keepachangelog.com/en/1.1.0/ - **`3.5.0-B139` — `cache = false` on a package entry now turns the compiler caches off.** `sysforge packages add --no-cache` recorded the setting and `packages list` showed it, but no build read it, so ccache and sccache kept running for that package. Builds now honour it, from a `[[package]]` entry or a `[group.*]` default: ccache is off however it is reached (makepkg's `BUILDENV` or a ccache directory first on `PATH`), and so is a `RUSTC_WRAPPER` such as sccache, even one your shell exports. The `--no-cache` help, the man page and `packages.toml` no longer say the setting is required for PGO; it is not. +--- + +- **`3.5.0-B140` — `[failure_handling]` in `sysforge.toml` now takes effect.** The section was documented in `sysforge.toml`, but sysforge read it from `profiles.toml`, so an edit to the documented section changed nothing and every build used the defaults. It is now read from `sysforge.toml`; if you had added the table to `profiles.toml` to make it work, move it. Each key now accepts only the values that mean something for it, listed beside it in the shipped file, and any other value falls back to that key's default with a warning, where before some fell back to a silent repair. `checksum_mismatch` accepts only `prompt_user` or `abort`, so neither an edit nor a typo can make sysforge update a failed checksum without asking. Three keys that were configurable but never documented are now in the shipped file: `pkgbuild_unparseable`, `abi_mismatch` and `makedep_probe_failed`. `vendored_deps_missing` and `pgp_key_missing` no longer accept `prompt_user`, which never asked anything. + ## Security - **`3.5.0-B37` — the root pacman hook no longer follows links in the state directory.** The hook helper runs as root in every pacman transaction and wrote into `/var/lib/sysforge/sentinels`, whose parent any `sysforge` group member can write to. A member could swap `sentinels` for a symlink to another directory, which the next transaction made group-writable, or symlink one of its files so the hook appended to any file as root. The helper now enters the directory and acts only if it is the real `sentinels`, owned by root. It appends only to a regular file, opened so that a symlink is refused and a planted FIFO cannot stall the transaction. As before, it never fails a transaction. diff --git a/etc/sysforge/sysforge.toml b/etc/sysforge/sysforge.toml index 1318895e..c5ef9235 100644 --- a/etc/sysforge/sysforge.toml +++ b/etc/sysforge/sysforge.toml @@ -145,17 +145,34 @@ clone_timeout = 60 #pre_build_snapshot = false [failure_handling] -# Per-scenario behaviour for build-failure auto-repair. -# Values: "auto_repair" | "auto_repair_with_warning" | "prompt_user" | -# "abort" | "warn_and_fallback" | "fallback" | "error". -# Defaults below match what sysforge uses when a key is absent: -# silent fixes for the safe scenarios, explicit user consent for -# checksum_mismatch — silent auto-fix would mask supply-chain compromise, -# DO NOT relax this without an equivalent compensating control. +# What a build does when it meets each named problem. Every key is shown at its +# default; a value a key does not accept takes that default, with a warning. +# +# Build-failure auto-repair: recognised in a failed build's output, repaired, +# and the build retried. "auto_repair" | "auto_repair_with_warning" | "abort". #vendored_deps_missing = "auto_repair" #pgp_key_missing = "auto_repair" +# +# srcinfo_drift — a .SRCINFO out of step with its PKGBUILD, checked before the +# build: "auto_repair" | "auto_repair_with_warning" (regenerate it), +# "warn_and_fallback" (warn and build anyway) | "abort". #srcinfo_drift = "auto_repair_with_warning" +# +# checksum_mismatch — a source file failed its checksum: "prompt_user" (show +# the files and ask before running updpkgsums; a run with no terminal, or a +# batch profile, aborts) | "abort". Updating the sums unasked would mask a +# tampered source, so no other value is accepted. #checksum_mismatch = "prompt_user" +# +# Problems a build can go on past: "warn_and_fallback" (warn and go on) | +# "fallback" (go on quietly) | "abort" | "error" (both stop the build). +# pkgbuild_unparseable — sysforge could not read the PKGBUILD's metadata +# abi_mismatch — a declared .so dependency is not on this system +# makedep_probe_failed — a makedepends that needs a working runtime (such as +# libguestfs) failed its check +#pkgbuild_unparseable = "warn_and_fallback" +#abi_mismatch = "warn_and_fallback" +#makedep_probe_failed = "warn_and_fallback" [safety] # LLVM source-tree safety pre-flight. Surfaces variant, origin, dirty diff --git a/sysforge/build/makepkg_wrapper.py b/sysforge/build/makepkg_wrapper.py index a0018e06..32f06c3a 100644 --- a/sysforge/build/makepkg_wrapper.py +++ b/sysforge/build/makepkg_wrapper.py @@ -56,10 +56,11 @@ load_config, load_conflict_groups, load_consumes_inference, + load_sysforge_toml, parse_system_makepkg_conf, ) from sysforge.primitives.dep_analysis import run_dep_analysis -from sysforge.primitives.failure import handle_failure +from sysforge.primitives.failure import handle_failure, resolve_failure_handling # Flag-string manipulation lives in makepkg_flags (owns the [FLAG] tag). # Re-exported here so emit_makepkg_conf and the CLI/update call sites @@ -1048,10 +1049,8 @@ def _run_build(pkgbuild_path, resolved_profile, config, groups, # ── Pre-flight: .SRCINFO drift regeneration ───────────────────── from sysforge.primitives import auto_repair as _ar - _failure_cfg = (config or {}).get("failure_handling", {}) - _srcinfo_behaviour = _failure_cfg.get( - "srcinfo_drift", "auto_repair_with_warning" - ) + _failure_cfg = resolve_failure_handling((config or {}).get("failure_handling")) + _srcinfo_behaviour = _failure_cfg["srcinfo_drift"] try: _ar.preflight_srcinfo(pkgbuild_path.parent, _srcinfo_behaviour) except RuntimeError as _e: @@ -1147,7 +1146,7 @@ def _reemit_conf(cc, cxx, ld, _kw: _ConfKwargs = _conf_kwargs): _ar.REGISTRY, _accum, pkgbuild_dir=pkgbuild_path.parent, - behaviour_for=lambda key: _failure_cfg.get(key, "auto_repair"), + behaviour_for=_failure_cfg.get, batch=_batch_mode, already_repaired=_repaired_scenarios, ) @@ -1564,6 +1563,10 @@ def run(pkgbuild_path, options: BuildOptions | None = None) -> Path | None: config_paths = [Path(options.profile_conf)] if options.profile_conf is not None else None config = load_config(config_paths=config_paths) + # [failure_handling] is a sysforge.toml table; every reader below takes it + # from this dict, which is otherwise profiles.toml (3.5.0-B140). + config = {**config, "failure_handling": resolve_failure_handling( + load_sysforge_toml().get("failure_handling"))} # Note: load_config() debug dumps (full flag_profiles etc.) fire here, before the # pkg log is open — they go to the unified log (if open) and terminal only. diff --git a/sysforge/primitives/auto_repair.py b/sysforge/primitives/auto_repair.py index 43b9dda8..b99148d7 100644 --- a/sysforge/primitives/auto_repair.py +++ b/sysforge/primitives/auto_repair.py @@ -420,16 +420,16 @@ def _repair_checksum_mismatch(pkgbuild_dir: Path, info: MatchInfo) -> None: # (`preflight_srcinfo`), not a retry-on-failure scenario. -_ALLOWED_BEHAVIOURS = frozenset({ - "auto_repair", "auto_repair_with_warning", "prompt_user", - "abort", "warn_and_fallback", "fallback", "error", -}) +_ALLOWED_BEHAVIOURS = frozenset({"auto_repair", "auto_repair_with_warning", "prompt_user", "abort"}) def _normalise_behaviour(behaviour: object) -> str: + # failure.resolve_failure_handling has already validated the table; a value + # that still slips through stops the build rather than repairing it unasked, + # which for checksum_mismatch would mask a tampered source (3.5.0-B140). if isinstance(behaviour, str) and behaviour in _ALLOWED_BEHAVIOURS: return behaviour - return "auto_repair" # safe default for the three silent scenarios + return "abort" def apply_first_match( diff --git a/sysforge/primitives/failure.py b/sysforge/primitives/failure.py index a68121be..201258cf 100644 --- a/sysforge/primitives/failure.py +++ b/sysforge/primitives/failure.py @@ -5,30 +5,67 @@ """ failure.py — SysForge failure scenario handling -Cross-cutting concern used by makepkg_wrapper, dep_analysis, and any future -modules that need to handle named failure scenarios from [failure_handling] config. +Cross-cutting concern used by makepkg_wrapper, dep_analysis and auto_repair to +handle the named failure scenarios of sysforge.toml [failure_handling]. Public API: + SCENARIOS every configurable key: (values, default) + resolve_failure_handling(raw) -> dict (validated, defaults filled) handle_failure(scenario, message, config, fallback=None) """ from sysforge import log _log = log.get_logger("FAILURE") -_ALWAYS_ABORT = {"profile_missing", "tempfile_write_failed"} - -_FAILURE_DEFAULTS = { - "pkgbuild_unparseable": "warn_and_fallback", - "no_rule_matched": "fallback", - "profile_missing": "abort", - "profile_cycle": "abort", - "tempfile_write_failed": "abort", - "env_conflict": "warn_and_fallback", - "abi_mismatch": "warn_and_fallback", - "makedep_probe_failed": "warn_and_fallback", - "dep_unsatisfied": "warn_and_fallback", +# The one home for the [failure_handling] keys (3.5.0-B140). Each key accepts +# only the values its reader acts on, so a value from the other family, or a +# typo, takes the default rather than a reader's catch-all. +_REPAIR = ("auto_repair", "auto_repair_with_warning", "abort") +_FALLBACK = ("abort", "error", "warn_and_fallback", "fallback") +SCENARIOS: dict[str, tuple[tuple[str, ...], str]] = { + # auto_repair: matched in a failed build's output, repaired, retried. + "vendored_deps_missing": (_REPAIR, "auto_repair"), + "pgp_key_missing": (_REPAIR, "auto_repair"), + # Updating sums unasked would mask a tampered source: a person decides. + "checksum_mismatch": (("prompt_user", "abort"), "prompt_user"), + # auto_repair preflight: regenerate, warn and go on, or stop. + "srcinfo_drift": (_REPAIR + ("warn_and_fallback",), "auto_repair_with_warning"), + # handle_failure below. + "pkgbuild_unparseable": (_FALLBACK, "warn_and_fallback"), + "abi_mismatch": (_FALLBACK, "warn_and_fallback"), + "makedep_probe_failed": (_FALLBACK, "warn_and_fallback"), } -_VALID_BEHAVIOURS = {"abort", "warn_and_fallback", "fallback", "error"} +# Not configurable: there is no sensible way to go on. +_ALWAYS_ABORT = {"tempfile_write_failed"} + +_warned: set[str] = set() + + +def _warn_once(message: str) -> None: + # A batch resolves the table once per build; say each problem once. + if message not in _warned: + _warned.add(message) + _log.warn(message) + + +def resolve_failure_handling(raw) -> dict[str, str]: + """Validate a ``[failure_handling]`` table: one value for every key. + + An absent key, or a value outside that key's set, takes its default (an + invalid one with a warning); an unknown key is ignored with a warning. + """ + table = raw if isinstance(raw, dict) else {} + for key in sorted(set(table) - set(SCENARIOS)): + _warn_once(f"[failure_handling] {key} is not a known scenario: ignored") + resolved = {} + for key, (values, default) in SCENARIOS.items(): + value = table.get(key, default) + if value not in values: + _warn_once(f"[failure_handling] {key} = {value!r} is not one of " + f"{', '.join(values)}: using {default!r}") + value = default + resolved[key] = value + return resolved def handle_failure(scenario, message, config, fallback=None): @@ -41,19 +78,13 @@ def handle_failure(scenario, message, config, fallback=None): warn_and_fallback — log as warning, return fallback value fallback — return fallback value silently - profile_missing and tempfile_write_failed always abort regardless of config. + tempfile_write_failed, and any scenario not in SCENARIOS, always abort. """ - failure_cfg = config.get("failure_handling", {}) - behaviour = failure_cfg.get(scenario, _FAILURE_DEFAULTS.get(scenario, "abort")) - - if scenario in _ALWAYS_ABORT: - behaviour = "abort" - - if behaviour not in _VALID_BEHAVIOURS: - _log.error( - f"Unknown behaviour {behaviour!r} for scenario {scenario!r}: defaulting to abort" - ) + if scenario in _ALWAYS_ABORT or scenario not in SCENARIOS: behaviour = "abort" + else: + raw = (config.get("failure_handling") or {}).get(scenario) + behaviour = resolve_failure_handling({scenario: raw} if raw is not None else {})[scenario] if behaviour == "abort": _log.error(f"[{scenario}] ABORT: {message}") diff --git a/sysforge/primitives/profile.py b/sysforge/primitives/profile.py index 2800f190..8eef2995 100644 --- a/sysforge/primitives/profile.py +++ b/sysforge/primitives/profile.py @@ -63,6 +63,8 @@ "clean_builddir", "consumes", "cpu_quota", + # Read from sysforge.toml now (3.5.0-B140); kept so a stale per-profile + # table is not passed into the build env or into flag drift. "failure_handling", "ionice", "jobs", diff --git a/tests/data/etc/sysforge/sysforge.toml b/tests/data/etc/sysforge/sysforge.toml index 50e78035..69f8520b 100644 --- a/tests/data/etc/sysforge/sysforge.toml +++ b/tests/data/etc/sysforge/sysforge.toml @@ -75,11 +75,14 @@ clone_timeout = 60 # pre_build_snapshot = false [failure_handling] -# Build-failure auto-repair (DESIGN.md §1242). +# Build-failure handling; see etc/sysforge/sysforge.toml for full docs. vendored_deps_missing = "auto_repair" pgp_key_missing = "auto_repair" srcinfo_drift = "auto_repair_with_warning" checksum_mismatch = "prompt_user" +pkgbuild_unparseable = "warn_and_fallback" +abi_mismatch = "warn_and_fallback" +makedep_probe_failed = "warn_and_fallback" [safety] # LLVM source pre-flight. See etc/sysforge/sysforge.toml for full docs. diff --git a/tests/data/surface.json b/tests/data/surface.json index 582ed372..db1a19f2 100644 --- a/tests/data/surface.json +++ b/tests/data/surface.json @@ -607,8 +607,11 @@ "pkg_cache_warn_gb" ], "failure_handling": [ + "abi_mismatch", "checksum_mismatch", + "makedep_probe_failed", "pgp_key_missing", + "pkgbuild_unparseable", "srcinfo_drift", "vendored_deps_missing" ], diff --git a/tests/test_failure.py b/tests/test_failure.py index 3b672e84..14bf45d8 100644 --- a/tests/test_failure.py +++ b/tests/test_failure.py @@ -3,12 +3,13 @@ Covers: handle_failure behaviours — abort, error, warn_and_fallback, fallback - _ALWAYS_ABORT scenarios — profile_missing and tempfile_write_failed - abort regardless of config override + _ALWAYS_ABORT scenarios — tempfile_write_failed aborts regardless of + config override config override — [failure_handling] table changes behaviour fallback return value — correct value returned on fallback paths - unknown behaviour — invalid config value falls back to abort - all default scenarios — correct default for every named scenario + invalid behaviour — a value outside the scenario's own set takes + that scenario's default (3.5.0-B140) + all default scenarios — correct default for every raised scenario """ import os import sys @@ -40,18 +41,18 @@ def test_abort_raises_runtime_error(): def test_abort_message_contains_message(): with pytest.raises(RuntimeError, match="the detail"): - handle_failure("no_rule_matched", "the detail", cfg(no_rule_matched="abort")) + handle_failure("abi_mismatch", "the detail", cfg(abi_mismatch="abort")) def test_error_raises_runtime_error(): - with pytest.raises(RuntimeError, match="no_rule_matched"): - handle_failure("no_rule_matched", "oops", cfg(no_rule_matched="error")) + with pytest.raises(RuntimeError, match="makedep_probe_failed"): + handle_failure("makedep_probe_failed", "oops", cfg(makedep_probe_failed="error")) def test_abort_and_error_both_raise(): for behaviour in ("abort", "error"): with pytest.raises(RuntimeError): - handle_failure("no_rule_matched", "x", cfg(no_rule_matched=behaviour)) + handle_failure("abi_mismatch", "x", cfg(abi_mismatch=behaviour)) # --------------------------------------------------------------------------- @@ -78,93 +79,64 @@ def test_warn_and_fallback_does_not_raise(): # --------------------------------------------------------------------------- def test_fallback_returns_fallback_value(): - result = handle_failure("no_rule_matched", "no match", - cfg(no_rule_matched="fallback"), fallback=42) + result = handle_failure("abi_mismatch", "no match", + cfg(abi_mismatch="fallback"), fallback=42) assert result == 42 def test_fallback_default_is_none(): - result = handle_failure("no_rule_matched", "no match", cfg(no_rule_matched="fallback")) + result = handle_failure("abi_mismatch", "no match", cfg(abi_mismatch="fallback")) assert result is None -def test_fallback_does_not_raise(): - handle_failure("no_rule_matched", "x", cfg(no_rule_matched="fallback")) - - # --------------------------------------------------------------------------- # _ALWAYS_ABORT — override any config setting # --------------------------------------------------------------------------- -def test_profile_missing_always_aborts_despite_config(): - with pytest.raises(RuntimeError): - handle_failure("profile_missing", "gone", cfg(profile_missing="fallback")) - - def test_tempfile_write_failed_always_aborts_despite_config(): with pytest.raises(RuntimeError): handle_failure("tempfile_write_failed", "disk full", cfg(tempfile_write_failed="warn_and_fallback")) -def test_all_always_abort_scenarios_covered(): - # Verify _ALWAYS_ABORT set is what we expect - assert "profile_missing" in _ALWAYS_ABORT - assert "tempfile_write_failed" in _ALWAYS_ABORT +def test_always_abort_scenarios(): + assert {"tempfile_write_failed"} == _ALWAYS_ABORT # --------------------------------------------------------------------------- -# Unknown behaviour value in config +# A value outside the scenario's own set # --------------------------------------------------------------------------- -def test_unknown_behaviour_defaults_to_abort(): - with pytest.raises(RuntimeError): - handle_failure("no_rule_matched", "x", cfg(no_rule_matched="invalid_value")) +@pytest.mark.parametrize("value", ["invalid_value", "auto_repair", "prompt_user", 3]) +def test_invalid_behaviour_takes_the_scenario_default(value): + # abi_mismatch defaults to warn_and_fallback: the fallback comes back. + result = handle_failure("abi_mismatch", "x", cfg(abi_mismatch=value), fallback="fb") + assert result == "fb" # --------------------------------------------------------------------------- # Config override changes default behaviour # --------------------------------------------------------------------------- -def test_config_override_changes_abort_to_warn(): +def test_config_override_changes_warn_to_abort(): # abi_mismatch defaults to warn_and_fallback; override to abort with pytest.raises(RuntimeError): handle_failure("abi_mismatch", "mismatch", cfg(abi_mismatch="abort")) -def test_config_override_changes_fallback_to_abort(): - with pytest.raises(RuntimeError): - handle_failure("no_rule_matched", "x", cfg(no_rule_matched="abort")) - - def test_empty_config_uses_defaults(): - # no_rule_matched default is "fallback" — should return None without raising - result = handle_failure("no_rule_matched", "x", {}) + result = handle_failure("makedep_probe_failed", "x", {}) assert result is None # --------------------------------------------------------------------------- -# Default behaviour for every named scenario +# Default behaviour for every raised scenario # --------------------------------------------------------------------------- -def test_default_pkgbuild_unparseable_is_warn_and_fallback(): - result = handle_failure("pkgbuild_unparseable", "bad", {}, fallback="fb") - assert result == "fb" - - -def test_default_no_rule_matched_is_fallback(): - result = handle_failure("no_rule_matched", "x", {}, fallback="fb") - assert result == "fb" - - -def test_default_profile_missing_aborts(): - with pytest.raises(RuntimeError): - handle_failure("profile_missing", "x", {}) - - -def test_default_profile_cycle_aborts(): - with pytest.raises(RuntimeError): - handle_failure("profile_cycle", "x", {}) +@pytest.mark.parametrize("scenario", + ["pkgbuild_unparseable", "abi_mismatch", "makedep_probe_failed"]) +def test_default_is_warn_and_fallback(scenario): + assert handle_failure(scenario, "x", {}, fallback="fb") == "fb" def test_default_tempfile_write_failed_aborts(): @@ -172,21 +144,6 @@ def test_default_tempfile_write_failed_aborts(): handle_failure("tempfile_write_failed", "x", {}) -def test_default_env_conflict_is_warn_and_fallback(): - result = handle_failure("env_conflict", "x", {}, fallback="fb") - assert result == "fb" - - -def test_default_abi_mismatch_is_warn_and_fallback(): - result = handle_failure("abi_mismatch", "x", {}, fallback="fb") - assert result == "fb" - - -def test_default_dep_unsatisfied_is_warn_and_fallback(): - result = handle_failure("dep_unsatisfied", "x", {}, fallback="fb") - assert result == "fb" - - # --------------------------------------------------------------------------- # Unknown scenario uses abort as ultimate default # --------------------------------------------------------------------------- diff --git a/tests/test_failure_handling_config.py b/tests/test_failure_handling_config.py new file mode 100644 index 00000000..2f104dce --- /dev/null +++ b/tests/test_failure_handling_config.py @@ -0,0 +1,118 @@ +# SPDX-FileCopyrightText: 2026 Keith Raghubar +# +# SPDX-License-Identifier: MIT + +"""`sysforge.toml [failure_handling]` reaches the builds it governs (3.5.0-B140). + +Every reader took the table from the profiles.toml dict `makepkg_wrapper.run` +loads, so the documented section was never read. Each key now has its own set +of values; a value outside it takes that key's default, and `checksum_mismatch` +accepts only the two values that keep a person in the loop. +""" +from unittest.mock import patch + +import pytest + +from sysforge.build import makepkg_wrapper as mw +from sysforge.build.makepkg_wrapper import BuildOptions +from sysforge.primitives import auto_repair as ar +from sysforge.primitives import config as cfgmod +from sysforge.primitives.failure import SCENARIOS, resolve_failure_handling + +DEFAULTS = { + "vendored_deps_missing": "auto_repair", + "pgp_key_missing": "auto_repair", + "srcinfo_drift": "auto_repair_with_warning", + "checksum_mismatch": "prompt_user", + "pkgbuild_unparseable": "warn_and_fallback", + "abi_mismatch": "warn_and_fallback", + "makedep_probe_failed": "warn_and_fallback", +} + + +def test_every_key_has_its_documented_default(): + assert resolve_failure_handling(None) == DEFAULTS + assert {k: d for k, (_, d) in SCENARIOS.items()} == DEFAULTS + + +@pytest.mark.parametrize("key, value", [ + ("vendored_deps_missing", "abort"), + ("pgp_key_missing", "auto_repair_with_warning"), + ("srcinfo_drift", "warn_and_fallback"), + ("checksum_mismatch", "abort"), + ("pkgbuild_unparseable", "abort"), + ("abi_mismatch", "fallback"), + ("makedep_probe_failed", "error"), +]) +def test_a_valid_value_is_kept(key, value): + assert resolve_failure_handling({key: value})[key] == value + + +@pytest.mark.parametrize("key, value", [ + ("checksum_mismatch", "auto_repair"), # would mask a tampered source + ("checksum_mismatch", "promt_user"), # a typo never relaxes the guard + ("vendored_deps_missing", "prompt_user"), # never prompts; not offered + ("srcinfo_drift", "error"), + ("abi_mismatch", "auto_repair"), # the other family's vocabulary + ("pgp_key_missing", True), +]) +def test_an_invalid_value_takes_the_default(key, value): + assert resolve_failure_handling({key: value})[key] == DEFAULTS[key] + + +def test_an_unknown_key_is_dropped(): + assert resolve_failure_handling({"no_rule_matched": "abort"}) == DEFAULTS + + +def test_a_non_table_is_ignored(): + assert resolve_failure_handling("abort") == DEFAULTS + + +def test_an_unknown_auto_repair_behaviour_aborts(): + # Belt and braces behind the resolver: never the silent repair. + assert ar._normalise_behaviour("bogus") == "abort" + assert ar._normalise_behaviour(None) == "abort" + + +@pytest.fixture +def sysforge_toml(tmp_path, monkeypatch): + path = tmp_path / "sysforge.toml" + monkeypatch.setattr(cfgmod, "SYSFORGE_TOML_PATH", path) + return path + + +@pytest.fixture +def checkout(tmp_path): + d = tmp_path / "foo" + d.mkdir() + (d / "PKGBUILD").write_text("pkgname=foo\npkgver=1\npkgrel=1\n") + return d + + +def _build_config(pkgbuild): + seen = {} + with patch.object(mw, "_run_build", side_effect=lambda *a, **k: seen.update(config=a[2])): + mw.run(pkgbuild, options=BuildOptions( + update=False, pkg_log=False, profile_override="standard")) + return seen["config"] + + +def test_run_reads_the_table_from_sysforge_toml(sysforge_toml, checkout): + sysforge_toml.write_text('[failure_handling]\nabi_mismatch = "abort"\n') + assert _build_config(checkout / "PKGBUILD")["failure_handling"]["abi_mismatch"] == "abort" + + +def test_run_without_the_table_uses_the_defaults(sysforge_toml, checkout): + sysforge_toml.write_text("[ui]\n") + assert _build_config(checkout / "PKGBUILD")["failure_handling"] == DEFAULTS + + +def test_dep_analysis_sees_the_sysforge_toml_value(sysforge_toml, checkout): + sysforge_toml.write_text('[failure_handling]\nabi_mismatch = "abort"\n') + seen = {} + with patch.object(mw, "run_dep_analysis", + side_effect=lambda meta, config: seen.update(config)), \ + patch.object(mw, "_run_build"): + mw.run(checkout / "PKGBUILD", options=BuildOptions( + update=False, pkg_log=False, profile_override="standard")) + assert seen["failure_handling"]["abi_mismatch"] == "abort" From b92563e4a75a696d5349921322521431c54fe59b Mon Sep 17 00:00:00 2001 From: Keith Raghubar Date: Sat, 24 Oct 2026 11:24:44 -0400 Subject: [PATCH 04/11] test(e2e): scenarios for 35 of 42 commands, and a coverage check (3.5.0-DEV23) Six new scenarios (maintain, revert-to-stock, config, inspect, artifacts, source-policy) and a seventh, xfail, previewing run toolchain/kernel with non-default configs. tools/e2e_coverage.py compares surface.json's commands with the scenarios; tests/test_e2e_coverage.py fails on a command that is neither run nor excused with a reason in tools/e2e/uncovered.toml. make e2e-coverage prints both sets. lib.sh gains expect_exit_in, json_report, output_has (both streams, no pipe under pipefail) and assert_not_installed. Files two bugs the scenarios found: 3.5.0-B141 (stage --dry-run skips the stage and prints nothing) and 3.5.0-B142 (artifact registry bypasses resolve_state_dir, so it crashes in a first session). Passes in podman and in CI's shared-system mode. Co-Authored-By: Claude Opus 5.5 --- DESIGN.md | 7 +- Makefile | 8 +- ROADMAP.md | 57 +++++++++----- docs/design/17-release-plan.md | 7 +- docs/release-notes/unreleased.md | 4 + tests/test_e2e_coverage.py | 72 ++++++++++++++++++ tools/e2e/README.md | 21 ++++- tools/e2e/lib.sh | 43 +++++++++++ tools/e2e/scenarios/10-quickstart.sh | 2 +- tools/e2e/scenarios/50-maintain.sh | 53 +++++++++++++ tools/e2e/scenarios/60-revert-to-stock.sh | 26 +++++++ tools/e2e/scenarios/70-config.sh | 41 ++++++++++ tools/e2e/scenarios/80-inspect.sh | 33 ++++++++ tools/e2e/scenarios/85-artifacts.sh | 49 ++++++++++++ tools/e2e/scenarios/90-source-policy.sh | 27 +++++++ tools/e2e/scenarios/95-stage-dry-run.sh | 42 ++++++++++ tools/e2e/uncovered.toml | 15 ++++ tools/e2e_coverage.py | 93 +++++++++++++++++++++++ 18 files changed, 573 insertions(+), 27 deletions(-) create mode 100644 tests/test_e2e_coverage.py create mode 100644 tools/e2e/scenarios/50-maintain.sh create mode 100644 tools/e2e/scenarios/60-revert-to-stock.sh create mode 100644 tools/e2e/scenarios/70-config.sh create mode 100644 tools/e2e/scenarios/80-inspect.sh create mode 100644 tools/e2e/scenarios/85-artifacts.sh create mode 100644 tools/e2e/scenarios/90-source-policy.sh create mode 100644 tools/e2e/scenarios/95-stage-dry-run.sh create mode 100644 tools/e2e/uncovered.toml create mode 100644 tools/e2e_coverage.py diff --git a/DESIGN.md b/DESIGN.md index 8fab2cc2..7a1afeaa 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -4106,11 +4106,14 @@ codes, `pacman -Qi`, the state files and leftover sentinels, with prompts answer pseudo-terminal. It covers what the unit suite stubs: root versus the user, group membership that applies only at the next login, the state one command leaves for the next. It builds sysforge from the checkout itself, so it needs no `vm-pkg-stable`. `pre-release` runs it; the release preflight -does not (`_RELEASE_GATE_EXEMPT`: minutes long, podman, and one networked scenario, so not +does not (`_RELEASE_GATE_EXEMPT`: minutes long, podman, and three networked scenarios, so not deterministic). CI runs the same scenarios in an `e2e` job of its own, directly in the job's container (`make e2e E2E_HERE=1`), listed in the Makefile's `CI_JOBS` beside `GATES`. A scenario marked `xfail: ` fails until that item is fixed and reports `XFAIL`; once it passes it fails -the run, so the marker comes off with the fix. +the run, so the marker comes off with the fix. Every command in `tests/data/surface.json` is run by a scenario or +listed, with its reason, in `tools/e2e/uncovered.toml` (`3.5.0-DEV23`); `tools/e2e_coverage.py` +does the comparison, `tests/test_e2e_coverage.py` fails the suite on a command in neither set or +a listing a scenario has made redundant, and `make e2e-coverage` prints both. **CI** (`.github/workflows/ci.yml`, `3.4.0-DEV1`) runs every `pre-release` gate on each pull request (a newer push to it cancels the superseded run) and on each push to `main`, in a fresh `archlinux:base-devel` container, as an unprivileged user, with only the diff --git a/Makefile b/Makefile index 088841a8..5b026aed 100644 --- a/Makefile +++ b/Makefile @@ -7,7 +7,7 @@ preflight release-major release-minor release-patch release-resume \ vm-deps vm-image vm-boot vm-boot-gui vm-snapshot vm-loadvm vm-iso vm-monitor vm-console vm-savevm vm-snapshots vm-ssh vm-ssh-builder vm-stop vm-clean \ vm-pkg-stable vm-pkg-git vm-pkg-all vm-install-stable vm-install-git vm-smoke vm-test \ - container-build container-smoke container-smoke-cachyos container-shell container-clean e2e \ + container-build container-smoke container-smoke-cachyos container-shell container-clean e2e e2e-coverage \ ci-local # --------------------------------------------------------------------------- @@ -658,6 +658,12 @@ container-clean: ## Remove the test container and image e2e: ## Run the end-to-end scenarios in fresh Arch containers (SCENARIO=, needs podman) E2E_COMMIT=$(E2E_COMMIT) bash tools/e2e/run.sh $(if $(E2E_HERE),--here) $(SCENARIO) +# Which commands a scenario runs and which tools/e2e/uncovered.toml excuses, +# with its reason (3.5.0-DEV23). tests/test_e2e_coverage.py fails the suite on +# a command in neither set, so this is the readable view of that check. +e2e-coverage: ## List the commands the end-to-end scenarios run, and the excused rest + uv run --no-sync python tools/e2e_coverage.py + # Replay the CI job (.github/workflows/ci.yml, 3.4.0-DEV1) against the working # tree, uncommitted edits included, in the same base image. The tree is mounted # read-only: tools/ci.sh copies it to an unprivileged user before running the diff --git a/ROADMAP.md b/ROADMAP.md index a6cd5550..d459cdd3 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -112,32 +112,49 @@ canonical ordering. | ID | Item | Priority | Effort | Bump | |----|------|----------|--------|------| -| `3.5.0-DEV23` | widen the end-to-end tier beyond build, update and setup | med | large | patch | +| `3.5.0-B142` | an artifact command in your first session crashes on the state directory | med | small | patch | +| `3.5.0-B141` | run kernel/toolchain/packages --dry-run shows nothing and skips the stage | med | medium | patch | ### Features ### Bugs -### Developer tooling +- **`3.5.0-B141` — `run kernel/toolchain/packages --dry-run` shows nothing and skips the stage.** + Each promises "Show what would run without executing anything", but `pipeline.runner` (the + standalone-stage path) short-circuits on `options.dry_run` before `stage.run`, logging only + `[dry-run] would run stage: ` at INFO, which the default verbosity hides. So the command + prints nothing at all, and the dry-run handling the stages carry themselves is unreachable: the + kernel stage's "Kernel build plan" resolution summary (compiler, variant, bootloader, source, + kconfig counts; §Kernel stage calls it "the readable core of `--dry-run`"), its boot-entry prune + preview, `--autofdo=capture --dry-run`, and the pre-build snapshot's dry-run branch. None of + `toolchain.toml` or `kernel.toml` past `enabled` is read, so a misconfiguration surfaces only + when the real run starts. Found writing the `3.5.0-DEV23` end-to-end scenarios on 2026-10-24 + (`tools/e2e/scenarios/95-stage-dry-run.sh` is `xfail` against it). Fix: pass the dry run into + `stage.run` and have each stage stop at its first mutation (the toolchain stage before any + build, the kernel stage after its resolution summary and gates), printing its plan through + `ui()`; `run pipeline --dry-run` shares the runner and needs the same check. + *Priority: med · Effort: medium · Bump: patch* — a documented preview flag that previews + nothing; the stage runs that most need one take hours. + +- **`3.5.0-B142` — an `artifact` command in your first session crashes on the state directory.** + `artifacts.ArtifactRegistry` and `artifacts.IgnoreList` resolve the state directory themselves + (argument, then `SYSFORGE_STATE_DIR`, then a private `/var/lib/sysforge` constant) instead of + through `paths.resolve_state_dir`, the one home for it and the place `3.5.0-B134` provisions + the `sysforge` group and the per-user ACL that opens the directory before the group applies at + the next login. So when `artifact adopt` (or any registry write) is the first thing a user runs + after installing, before a `build` has provisioned the directory, it ends in a Python + traceback: `PermissionError: … /var/lib/sysforge/.artifacts.toml.*.tmp`. Found by the + `3.5.0-DEV23` end-to-end scenario `85-artifacts` in its own container; the scenario now builds + first, because in CI's shared system an earlier scenario has always provisioned the directory, + so the tier cannot reproduce a first-session bug past its first scenario and the fix needs a + unit test (a registry write as a user the directory is closed to). Fix: resolve + through `paths.resolve_state_dir` (provisioning on write), and check the other private copies of + the default path for the same gap: `build_sandbox.py` (local repo), `init_notice.py` and + `stage_sentinel.py`, keeping any that are deliberately read-only. + *Priority: med · Effort: small · Bump: patch* — a traceback on a documented command in the + first session, the one `3.5.0-B134` fixed for `build` and `update`. -- **`3.5.0-DEV23` — widen the end-to-end tier beyond `build`, `update` and setup.** - The tier (`3.5.0-DEV21`, `tools/e2e/`) has four scenarios that call 4 of the 41 commands in - `tests/data/surface.json` (`build`, `update`, `run reconfigure`, `run hardware`), all on the - shipped config except one `profiles.toml` edit and the `packages.toml` opt-in write. Add - scenarios, local `pkgs/` packages and no network wherever possible, in this order: (1) the - mutating commands that cross the user/root seam the tier exists for: `uninstall`, - `revert-to-stock`, `state forget`, `packages add`/`remove`/`add-group`, `config reset`/`clean`, - `artifact adopt`/`deploy`; (2) read-only commands against the state the earlier scenarios - leave: `state list`/`failed`/`orphans`, `doctor system`/`pkg`, `config check`, `resolve`, `log`; - (3) `run toolchain` and `run kernel` as `--dry-run` passes with non-default `toolchain.toml` - and `kernel.toml` (a real run needs hours or a reboot), plus `sysforge.toml` keys that change a - path a scenario takes (`[security] freeze_sources`, `[aur] enabled = false`). Pair it with a - coverage check that lists the commands no scenario calls, so a new verb shows up as uncovered - instead of silently absent. Separately, decide whether `e2e` becomes a required status check - beside `gates` on the `main` ruleset; today a red run does not block a merge, and - `10-quickstart` is networked, so requiring it puts AUR and mirror outages on the merge path. - *Priority: med · Effort: large · Bump: patch* — most commands have no test of their real - root/user, terminal and cross-command behaviour, the seams where the 2026-10-23 bugs lived. +### Developer tooling ### Open questions diff --git a/docs/design/17-release-plan.md b/docs/design/17-release-plan.md index 0bf74fd7..d136d0ba 100644 --- a/docs/design/17-release-plan.md +++ b/docs/design/17-release-plan.md @@ -43,11 +43,14 @@ codes, `pacman -Qi`, the state files and leftover sentinels, with prompts answer pseudo-terminal. It covers what the unit suite stubs: root versus the user, group membership that applies only at the next login, the state one command leaves for the next. It builds sysforge from the checkout itself, so it needs no `vm-pkg-stable`. `pre-release` runs it; the release preflight -does not (`_RELEASE_GATE_EXEMPT`: minutes long, podman, and one networked scenario, so not +does not (`_RELEASE_GATE_EXEMPT`: minutes long, podman, and three networked scenarios, so not deterministic). CI runs the same scenarios in an `e2e` job of its own, directly in the job's container (`make e2e E2E_HERE=1`), listed in the Makefile's `CI_JOBS` beside `GATES`. A scenario marked `xfail: ` fails until that item is fixed and reports `XFAIL`; once it passes it fails -the run, so the marker comes off with the fix. +the run, so the marker comes off with the fix. Every command in `tests/data/surface.json` is run by a scenario or +listed, with its reason, in `tools/e2e/uncovered.toml` (`3.5.0-DEV23`); `tools/e2e_coverage.py` +does the comparison, `tests/test_e2e_coverage.py` fails the suite on a command in neither set or +a listing a scenario has made redundant, and `make e2e-coverage` prints both. **CI** (`.github/workflows/ci.yml`, `3.4.0-DEV1`) runs every `pre-release` gate on each pull request (a newer push to it cancels the superseded run) and on each push to `main`, in a fresh `archlinux:base-devel` container, as an unprivileged user, with only the diff --git a/docs/release-notes/unreleased.md b/docs/release-notes/unreleased.md index 793081fa..758612ec 100644 --- a/docs/release-notes/unreleased.md +++ b/docs/release-notes/unreleased.md @@ -215,6 +215,10 @@ https://keepachangelog.com/en/1.1.0/ - **`3.5.0-DEV22` — the progress bar is tested on a model terminal.** `3.5.0-B132` and `B133` drew output over earlier lines in xterm and VS Code's terminal, which the progress bar's tests could not see: they check the escape sequences it writes, not what those do to a screen. A small terminal model under `tests/` now replays the bar's real output, and `update`'s end-of-run reset, and checks the screen: the command line survives, output follows it in order, the bar leaves nothing behind and the next prompt lands after the last line. Every check runs under both rules terminals follow for a saved cursor when the screen scrolls, xterm's and xterm.js's. Replayed through it, the sequences each fix replaced show their bug. The model refuses a sequence it has not been taught, so a new one cannot be misread silently. + +--- + +- **`3.5.0-DEV23` — the end-to-end tier runs 35 of the 42 commands, and a new one cannot be missed.** Six scenarios join the four under `tools/e2e/`: maintaining a package and letting it go (`state list`, `resolve`, `log`, the `packages` commands, `state forget`, `uninstall`), `revert-to-stock`, keeping the config healthy (`config check`, `clean` and `reset`, and a `setup --preset` undone by `config reset`), the read-only reports changing nothing (`state`, `doctor`, `env`, `fetch`), a user script through the `artifact` commands, and `[aur] enabled = false` and `freeze_sources` refusing before any checkout, with `search` still answering from the repos. A seventh previews `run toolchain` and `run kernel` with non-default `toolchain.toml` and `kernel.toml`; it is marked to fail until their dry run shows something, a bug it found. `make e2e-coverage` lists which scenario runs each command, and `tests/test_e2e_coverage.py` fails the suite on a command that no scenario runs and `tools/e2e/uncovered.toml` does not excuse with a reason; seven are excused, such as `run pipeline`, which installs Arch from the live ISO. The `artifact` scenario also found that those commands crash in a user's first session when nothing has prepared the state directory yet; both bugs are on the roadmap. --- - **`3.5.0-DOC1` — the package description says what sysforge optimizes.** The `pkgdesc` shown by `pacman -Si` and on the AUR now names what people search for in one short line: "AUR helper that builds and tracks packages tuned to your CPU with PGO, BOLT and LTO", instead of a generic "build and maintenance suite" line. The README's opening paragraph says the same. diff --git a/tests/test_e2e_coverage.py b/tests/test_e2e_coverage.py new file mode 100644 index 00000000..ea0fb402 --- /dev/null +++ b/tests/test_e2e_coverage.py @@ -0,0 +1,72 @@ +# SPDX-FileCopyrightText: 2026 Keith Raghubar +# +# SPDX-License-Identifier: MIT + +"""Every command is run by an end-to-end scenario or listed with a reason (3.5.0-DEV23). + +A new command fails here until a scenario under tools/e2e/scenarios/ runs it or +tools/e2e/uncovered.toml says why none can; a listing a scenario has since +made redundant fails too. +""" +import importlib.util +from pathlib import Path + +REPO = Path(__file__).resolve().parent.parent + + +def _tool(): + spec = importlib.util.spec_from_file_location( + "e2e_coverage", REPO / "tools" / "e2e_coverage.py") + assert spec and spec.loader + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + return mod + + +cov = _tool() + + +def test_every_command_is_run_or_listed(): + known = cov.commands() + assert cov.problems(known, cov.covered(known), cov.uncovered()) == [] + + +def _scenario(tmp_path, text): + d = tmp_path / "scenarios" + d.mkdir() + (d / "10-x.sh").write_text(text) + return d + + +def test_a_two_word_command_wins_over_its_first_word(tmp_path): + d = _scenario(tmp_path, "sysforge state list --json `, also inside `answer`, `expect_exit` or `output_has`), however +little it checks: the check finds a missing scenario, not a weak one. + ## Writing a scenario A scenario is a bash fragment in `scenarios/`, sourced with `set -euo pipefail` after `lib.sh`, diff --git a/tools/e2e/lib.sh b/tools/e2e/lib.sh index ff566d3b..0c849a64 100755 --- a/tools/e2e/lib.sh +++ b/tools/e2e/lib.sh @@ -30,6 +30,49 @@ expect_exit() { [[ $got == "$want" ]] || fail "\`$*\` exited $got, expected $want" } +# expect_exit_in "CODES" COMMAND...: run COMMAND, fail unless its exit code is +# one of CODES (space-separated). For a report whose findings depend on the +# host, where any answer is fine but a crash is not. +expect_exit_in() { + local want=" $1 " got=0 + shift + "$@" || got=$? + [[ $want == *" $got "* ]] || fail "\`$*\` exited $got, expected one of:$want" +} + +# json_report "CODES" COMMAND...: run COMMAND (a --json report), fail unless it +# exits one of CODES and prints valid JSON. Not a pipe: under pipefail a +# report's exit 1 for findings would fail the pipeline. +json_report() { + local want=" $1 " got=0 out + shift + out=$("$@") || got=$? + [[ $want == *" $got "* ]] || fail "\`$*\` exited $got, expected one of:$want" + assert_json <<<"$out" +} + +# output_has PATTERN COMMAND...: run COMMAND, then succeed if its output +# matches PATTERN. Both streams: a command's answer goes through ui(), which +# writes to stderr. Captured first: `COMMAND | grep -q` can SIGPIPE COMMAND +# under pipefail once grep has its match. +output_has() { + local pattern=$1 out + shift + out=$("$@" 2>&1) || fail "\`$*\` failed: $out" + grep -q -- "$pattern" <<<"$out" +} + +# assert_json: stdin parses as JSON. +assert_json() { + python3 -c 'import json, sys; json.load(sys.stdin)' || fail "not valid JSON" +} + +assert_not_installed() { + if pacman -Qi "$1" >/dev/null 2>&1; then + fail "$1 is still installed" + fi +} + assert_installed() { pacman -Qi "$1" >/dev/null 2>&1 || fail "$1 is not installed" } diff --git a/tools/e2e/scenarios/10-quickstart.sh b/tools/e2e/scenarios/10-quickstart.sh index 0e7e3bce..e81f49b1 100644 --- a/tools/e2e/scenarios/10-quickstart.sh +++ b/tools/e2e/scenarios/10-quickstart.sh @@ -3,7 +3,7 @@ # the one-time setup, an AUR build (the review gate accepted), then `update`. # The setup adds the user to the sysforge group, which only applies at the next # login, so every step after it runs without that membership (3.5.0-B134). -# The tier's one networked scenario: the AUR, and pacman for build deps. +# Networked: the AUR, and pacman for build deps. # start: installed step "one-time setup" diff --git a/tools/e2e/scenarios/50-maintain.sh b/tools/e2e/scenarios/50-maintain.sh new file mode 100644 index 00000000..3b3ed97f --- /dev/null +++ b/tools/e2e/scenarios/50-maintain.sh @@ -0,0 +1,53 @@ +# shellcheck shell=bash +# Maintaining a package and letting it go: inspect a source-built package, +# give it a packages.toml override and take it away, stop maintaining it while +# it stays installed, then uninstall it. +# start: ready + +dir=$(stage_pkg e2e-hello) +manifest=/etc/sysforge/packages.toml + +step "build it" +sysforge build "$dir" -si --rebuild /dev/null +output_has e2e-hello sysforge log --verb build --no-pager /dev/null +sysforge fetch "$dir" --no-update "$other" +chmod +x "$live" "$other" + +step "the unmanaged script is discovered" +output_has e2e-tool sysforge artifact list --unmanaged >"$1"\n' >"$editor" +chmod +x "$editor" +EDITOR=$editor sysforge artifact edit e2e-tool &1) && fail "the build succeeded: $out" +grep -q 'enabled = false' <<<"$out" || fail "the refusal did not name [aur] enabled: $out" +[[ ! -e ~/src/e2e-not-in-any-repo ]] || fail "a checkout was created" +output_has tree sysforge search tree ~/src/linux-sysforge/PKGBUILD + +profiles=$(sha256sum /etc/sysforge/profiles.toml) +state=$(ls "$STATE_DIR") + +step "the toolchain dry run changes nothing" +sysforge run toolchain --dry-run --no-update &1) || fail "run kernel --dry-run failed: $out" +echo "$out" +assert_not_installed linux-sysforge +[[ $(ls "$STATE_DIR") == "$state" ]] || fail "a dry run added state: $(ls "$STATE_DIR")" +assert_no_sentinel + +step "the kernel dry run shows the plan it resolved" +grep -q 'Kernel build plan' <<<"$out" || fail "run kernel --dry-run showed no plan" +grep -q 'none' <<<"$out" || fail "the plan does not show bootloader = none" diff --git a/tools/e2e/uncovered.toml b/tools/e2e/uncovered.toml new file mode 100644 index 00000000..2a9ac89b --- /dev/null +++ b/tools/e2e/uncovered.toml @@ -0,0 +1,15 @@ +# Commands no end-to-end scenario runs, each with the reason (3.5.0-DEV23). +# +# tools/e2e_coverage.py compares the commands in tests/data/surface.json with +# the `sysforge ` lines in tools/e2e/scenarios/. A command must be run +# by a scenario or listed here; a listed command a scenario now runs fails too, +# so this file only ever names real gaps. `make e2e-coverage` prints both sets. + +[uncovered] +"" = "the bare command prints usage" +help = "prints usage" +"run pipeline" = "partitions a disk and installs Arch; it runs from the live ISO, not on an installed system" +"run packages" = "installs every packages.toml entry, a fresh-install step; the scenarios share one manifest in CI" +"bench toolchain" = "compares the stock clang with one `run toolchain` built, a build of several hours" +"state boot-entries" = "lists systemd-boot entries for booted kernels; the container has no bootloader" +"config merge" = "needs a .sfnew that a package upgrade left and an interactive merge tool" diff --git a/tools/e2e_coverage.py b/tools/e2e_coverage.py new file mode 100644 index 00000000..8bf10224 --- /dev/null +++ b/tools/e2e_coverage.py @@ -0,0 +1,93 @@ +# SPDX-FileCopyrightText: 2026 Keith Raghubar +# +# SPDX-License-Identifier: MIT + +"""e2e_coverage.py — which commands the end-to-end tier runs (3.5.0-DEV23). + +Reads the commands from tests/data/surface.json and the `sysforge ` +lines of tools/e2e/scenarios/*.sh. Every command must be run by a scenario or +listed, with its reason, in tools/e2e/uncovered.toml; a listed command that a +scenario now runs is an error too, so the list names only real gaps. A new +command therefore shows up here instead of going untested unnoticed. + +Usage: + python tools/e2e_coverage.py # report; exit 1 on an unlisted gap or a stale listing +""" +from __future__ import annotations + +import json +import re +import sys +import tomllib +from pathlib import Path + +REPO = Path(__file__).resolve().parent.parent +SCENARIOS = REPO / "tools" / "e2e" / "scenarios" +UNCOVERED = REPO / "tools" / "e2e" / "uncovered.toml" +SURFACE = REPO / "tests" / "data" / "surface.json" + +# `sysforge` as a command word, not a path component (/etc/sysforge/...). +_INVOCATION = re.compile(r"(? set[str]: + return set(json.loads(SURFACE.read_text())["commands"]) + + +def uncovered() -> dict[str, str]: + with UNCOVERED.open("rb") as f: + return tomllib.load(f)["uncovered"] + + +def covered(known: set[str], scenarios: Path = SCENARIOS) -> dict[str, list[str]]: + """Each command a scenario runs, with the scenarios that run it.""" + found: dict[str, list[str]] = {} + for path in sorted(scenarios.glob("*.sh")): + for line in path.read_text().splitlines(): + if line.lstrip().startswith("#"): + continue + for m in _INVOCATION.finditer(line): + words = [w for w in m.group(1).split() if not w.startswith("-")] + for n in (2, 1): + cmd = " ".join(words[:n]) + if len(words) >= n and cmd in known: + found.setdefault(cmd, []) + if path.stem not in found[cmd]: + found[cmd].append(path.stem) + break + return found + + +def problems(known: set[str], runs: dict[str, list[str]], listed: dict[str, str]) -> list[str]: + out = [] + for cmd in sorted(known - set(runs) - set(listed)): + out.append(f"`sysforge {cmd}` is run by no scenario and not listed in {UNCOVERED.name}") + for cmd in sorted(set(listed) & set(runs)): + by = ", ".join(runs[cmd]) + out.append(f"`sysforge {cmd}` is listed in {UNCOVERED.name} but {by} runs it") + for cmd in sorted(set(listed) - known): + out.append(f"{UNCOVERED.name} lists `{cmd}`, which is not a command") + for cmd, reason in sorted(listed.items()): + if not str(reason).strip(): + out.append(f"{UNCOVERED.name} gives no reason for `{cmd}`") + return out + + +def main() -> int: + known, listed = commands(), uncovered() + runs = covered(known) + width = max(len(c) for c in known) + 2 + print(f"Run by a scenario ({len(runs)} of {len(known)}):") + for cmd in sorted(runs): + print(f" {cmd:<{width}}{', '.join(runs[cmd])}") + print(f"Not run, listed in {UNCOVERED.name} ({len(listed)}):") + for cmd in sorted(listed): + print(f" {cmd or '(bare)':<{width}}{listed[cmd]}") + errors = problems(known, runs, listed) + for e in errors: + print(f"ERROR {e}", file=sys.stderr) + return 1 if errors else 0 + + +if __name__ == "__main__": + sys.exit(main()) From 5840d3d2e259233446fbd54a0ce1d55d6ec993d0 Mon Sep 17 00:00:00 2001 From: Keith Raghubar Date: Sat, 24 Oct 2026 11:45:51 -0400 Subject: [PATCH 05/11] docs(roadmap): file 3.5.0-B143, uninstall without a terminal Co-Authored-By: Claude Opus 5.5 --- ROADMAP.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/ROADMAP.md b/ROADMAP.md index d459cdd3..2ea12981 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -114,6 +114,7 @@ canonical ordering. |----|------|----------|--------|------| | `3.5.0-B142` | an artifact command in your first session crashes on the state directory | med | small | patch | | `3.5.0-B141` | run kernel/toolchain/packages --dry-run shows nothing and skips the stage | med | medium | patch | +| `3.5.0-B143` | sysforge uninstall cannot run without a terminal | low | small | minor | ### Features @@ -155,6 +156,18 @@ canonical ordering. *Priority: med · Effort: small · Bump: patch* — a traceback on a documented command in the first session, the one `3.5.0-B134` fixed for `build` and `update`. +- **`3.5.0-B143` — `sysforge uninstall` cannot run without a terminal.** + The verb hands the confirmation to `pacman -Rnsu` (`pacman.uninstall_pkgs`) and has no `--yes`, + unlike `revert-to-stock`. Run from a script, a timer or with stdin closed, pacman reads EOF at + its `[Y/n]`, aborts, and sysforge reports `pacman removal failed (Command '['sudo', 'pacman', + '-Rnsu', '--', …]' returned non-zero exit status 1.)`: a raw command line instead of a reason, + after printing the whole transaction. Found by the `3.5.0-DEV23` end-to-end scenario + `50-maintain`, which answers the prompt on a pseudo-terminal to get past it. Fix: add `--yes` + (passes `--noconfirm`), and without it and without a terminal refuse before anything runs, + naming `--yes`, as `revert-to-stock` does; completions and man page in step. + *Priority: low · Effort: small · Bump: minor* — a new flag; until then a scripted removal + needs `pacman -R` plus `state forget` by hand. + ### Developer tooling ### Open questions From cede5e51728448133923e58ed1ea1b4698ea3af4 Mon Sep 17 00:00:00 2001 From: Keith Raghubar Date: Sat, 24 Oct 2026 11:48:00 -0400 Subject: [PATCH 06/11] fix(artifacts): resolve the state dir through paths.resolve_state_dir (3.5.0-B142) ArtifactRegistry and IgnoreList resolved /var/lib/sysforge themselves, so they never provisioned it: a first-session artifact write crashed with PermissionError. Reads now resolve with provision=False and the first save provisions, as a build's run does. build_sandbox's source-built lookup uses the same read-only resolution; stage_sentinel and init_notice keep the fixed default deliberately (lock and marker live there). 85-artifacts drops its provisioning build and becomes the regression scenario. Co-Authored-By: Claude Opus 5.5 --- ROADMAP.md | 19 -------- docs/release-notes/unreleased.md | 4 ++ sysforge/primitives/artifacts.py | 42 +++++++++++------ sysforge/primitives/build_sandbox.py | 14 ++---- tests/test_artifact_state_dir.py | 68 ++++++++++++++++++++++++++++ tools/e2e/scenarios/85-artifacts.sh | 9 ++-- 6 files changed, 109 insertions(+), 47 deletions(-) create mode 100644 tests/test_artifact_state_dir.py diff --git a/ROADMAP.md b/ROADMAP.md index 2ea12981..eed51ead 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -112,7 +112,6 @@ canonical ordering. | ID | Item | Priority | Effort | Bump | |----|------|----------|--------|------| -| `3.5.0-B142` | an artifact command in your first session crashes on the state directory | med | small | patch | | `3.5.0-B141` | run kernel/toolchain/packages --dry-run shows nothing and skips the stage | med | medium | patch | | `3.5.0-B143` | sysforge uninstall cannot run without a terminal | low | small | minor | @@ -138,24 +137,6 @@ canonical ordering. *Priority: med · Effort: medium · Bump: patch* — a documented preview flag that previews nothing; the stage runs that most need one take hours. -- **`3.5.0-B142` — an `artifact` command in your first session crashes on the state directory.** - `artifacts.ArtifactRegistry` and `artifacts.IgnoreList` resolve the state directory themselves - (argument, then `SYSFORGE_STATE_DIR`, then a private `/var/lib/sysforge` constant) instead of - through `paths.resolve_state_dir`, the one home for it and the place `3.5.0-B134` provisions - the `sysforge` group and the per-user ACL that opens the directory before the group applies at - the next login. So when `artifact adopt` (or any registry write) is the first thing a user runs - after installing, before a `build` has provisioned the directory, it ends in a Python - traceback: `PermissionError: … /var/lib/sysforge/.artifacts.toml.*.tmp`. Found by the - `3.5.0-DEV23` end-to-end scenario `85-artifacts` in its own container; the scenario now builds - first, because in CI's shared system an earlier scenario has always provisioned the directory, - so the tier cannot reproduce a first-session bug past its first scenario and the fix needs a - unit test (a registry write as a user the directory is closed to). Fix: resolve - through `paths.resolve_state_dir` (provisioning on write), and check the other private copies of - the default path for the same gap: `build_sandbox.py` (local repo), `init_notice.py` and - `stage_sentinel.py`, keeping any that are deliberately read-only. - *Priority: med · Effort: small · Bump: patch* — a traceback on a documented command in the - first session, the one `3.5.0-B134` fixed for `build` and `update`. - - **`3.5.0-B143` — `sysforge uninstall` cannot run without a terminal.** The verb hands the confirmation to `pacman -Rnsu` (`pacman.uninstall_pkgs`) and has no `--yes`, unlike `revert-to-stock`. Run from a script, a timer or with stdin closed, pacman reads EOF at diff --git a/docs/release-notes/unreleased.md b/docs/release-notes/unreleased.md index 758612ec..e1769356 100644 --- a/docs/release-notes/unreleased.md +++ b/docs/release-notes/unreleased.md @@ -872,6 +872,10 @@ https://keepachangelog.com/en/1.1.0/ - **`3.5.0-B140` — `[failure_handling]` in `sysforge.toml` now takes effect.** The section was documented in `sysforge.toml`, but sysforge read it from `profiles.toml`, so an edit to the documented section changed nothing and every build used the defaults. It is now read from `sysforge.toml`; if you had added the table to `profiles.toml` to make it work, move it. Each key now accepts only the values that mean something for it, listed beside it in the shipped file, and any other value falls back to that key's default with a warning, where before some fell back to a silent repair. `checksum_mismatch` accepts only `prompt_user` or `abort`, so neither an edit nor a typo can make sysforge update a failed checksum without asking. Three keys that were configurable but never documented are now in the shipped file: `pkgbuild_unparseable`, `abi_mismatch` and `makedep_probe_failed`. `vendored_deps_missing` and `pgp_key_missing` no longer accept `prompt_user`, which never asked anything. +--- + +- **`3.5.0-B142` — the `artifact` commands work in your first session.** Run before anything else after installing, `sysforge artifact adopt` (or any command that saves the artifact list) ended in a Python traceback, `PermissionError` on `/var/lib/sysforge`: the artifact commands found SysForge's state directory on their own and never opened it to you the way `build` does until the `sysforge` group applies at your next login. They now go through the same step, and the read-only ones still never ask for `sudo`. + ## Security - **`3.5.0-B37` — the root pacman hook no longer follows links in the state directory.** The hook helper runs as root in every pacman transaction and wrote into `/var/lib/sysforge/sentinels`, whose parent any `sysforge` group member can write to. A member could swap `sentinels` for a symlink to another directory, which the next transaction made group-writable, or symlink one of its files so the hook appended to any file as root. The helper now enters the directory and acts only if it is the real `sentinels`, owned by root. It appends only to a regular file, opened so that a symlink is refused and a planted FIFO cannot stall the transaction. As before, it never fails a transaction. diff --git a/sysforge/primitives/artifacts.py b/sysforge/primitives/artifacts.py index 38868aa9..f52e252f 100644 --- a/sysforge/primitives/artifacts.py +++ b/sysforge/primitives/artifacts.py @@ -50,7 +50,31 @@ _REGISTRY_NAME = "artifacts.toml" _IGNORE_NAME = "artifacts-ignored.toml" -_DEFAULT_STATE_DIR = Path("/var/lib/sysforge") + + +def _state_dir_for_read(state_dir) -> tuple[Path, bool]: + """The state directory through the one resolver, without provisioning it. + + Reading never provisions (that can ask for sudo); see + :func:`_state_dir_for_write`. Returns the directory and whether it was + given explicitly. + """ + if state_dir is not None: + return Path(state_dir), True + return paths.resolve_state_dir(None, provision=False)[0], False + + +def _state_dir_for_write(current: Path, explicit: bool) -> Path: + """The directory a write goes to, provisioned for this user if need be. + + The registry resolved its directory itself and so never provisioned it: in + a first session, before the ``sysforge`` group applies, an ``artifact`` + write ended in a PermissionError (3.5.0-B142). The resolver opens the + default with the same group and ACL a build's run does. + """ + if explicit: + return current + return paths.resolve_state_dir(None, provision=True)[0] class ArtifactError(Exception): @@ -142,12 +166,7 @@ class ArtifactRegistry: """Reads/writes ``artifacts.toml`` and locates authoritative content.""" def __init__(self, state_dir=None, data_dir=None): - if state_dir is not None: - base = Path(state_dir) - else: - env = os.environ.get("SYSFORGE_STATE_DIR") - base = Path(env) if env else _DEFAULT_STATE_DIR - self._state_dir = base + self._state_dir, self._explicit = _state_dir_for_read(state_dir) self._data_dir = Path(data_dir) if data_dir is not None else paths.USER_DATA_DIR @property @@ -206,6 +225,7 @@ def load(self) -> dict[str, Artifact]: def save(self, entries: dict[str, Artifact]) -> None: """Write the registry atomically (temp file + replace).""" + self._state_dir = _state_dir_for_write(self._state_dir, self._explicit) self.path.parent.mkdir(parents=True, exist_ok=True) lines: list[str] = [ "# sysforge artifact registry: metadata only, never content.", @@ -252,12 +272,7 @@ class IgnoreList: """ def __init__(self, state_dir=None): - if state_dir is not None: - base = Path(state_dir) - else: - env = os.environ.get("SYSFORGE_STATE_DIR") - base = Path(env) if env else _DEFAULT_STATE_DIR - self._state_dir = base + self._state_dir, self._explicit = _state_dir_for_read(state_dir) @property def path(self) -> Path: @@ -304,6 +319,7 @@ def load(self) -> dict[Path, str]: def save(self, entries: dict[Path, str]) -> None: """Write the ignore-list atomically (temp file + replace).""" + self._state_dir = _state_dir_for_write(self._state_dir, self._explicit) self.path.parent.mkdir(parents=True, exist_ok=True) lines: list[str] = [ "# sysforge artifact ignore-list: declined discovery candidates.", diff --git a/sysforge/primitives/build_sandbox.py b/sysforge/primitives/build_sandbox.py index 0ee69cfa..d345ffce 100644 --- a/sysforge/primitives/build_sandbox.py +++ b/sysforge/primitives/build_sandbox.py @@ -100,7 +100,7 @@ from pathlib import Path from sysforge import log -from sysforge.primitives import run +from sysforge.primitives import paths, run from sysforge.primitives.build_throttle import ( CGROUP_PROPERTY_DEFAULTS, BuildThrottle, @@ -1158,17 +1158,13 @@ def _source_built_packages(state_dir) -> set: *files*, one per pkgname. Unreadable state degrades to "nothing is source-built", which costs fidelity rather than correctness. """ - import os - from sysforge.primitives.build_state import BUILD_MODE_PACMAN, BuildState if state_dir is None: - # Same resolution ArtifactRegistry uses, and for the same reason: this - # is a primitive, so it cannot reach pipeline.resolve_state_dir, and a - # caller several layers up that forgets to thread the value would - # silently degrade injection to "nothing is source-built". - env = os.environ.get("SYSFORGE_STATE_DIR") - state_dir = Path(env) if env else Path("/var/lib/sysforge") + # Resolved here so a caller several layers up that forgets to thread the + # value does not silently degrade injection to "nothing is + # source-built". Read-only, so never provisioned (3.5.0-B142). + state_dir = paths.resolve_state_dir(None, provision=False)[0] try: packages = BuildState(state_dir).all_packages() diff --git a/tests/test_artifact_state_dir.py b/tests/test_artifact_state_dir.py new file mode 100644 index 00000000..33beacc9 --- /dev/null +++ b/tests/test_artifact_state_dir.py @@ -0,0 +1,68 @@ +# SPDX-FileCopyrightText: 2026 Keith Raghubar +# +# SPDX-License-Identifier: MIT + +"""The artifact registry finds its state directory the way the rest of sysforge does (3.5.0-B142). + +It resolved the directory itself, so it never provisioned it: in a user's first +session, before the `sysforge` group applies, an `artifact` write as the first +command ended in a PermissionError traceback. Reads resolve without +provisioning (that can ask for sudo); the first write provisions, as `build`'s +does. +""" +import pytest + +from sysforge.primitives import artifacts, build_sandbox, paths + + +@pytest.fixture +def resolver(tmp_path, monkeypatch): + calls = [] + provisioned = tmp_path / "provisioned" + + def fake(cli_override=None, *, provision=True): + calls.append((cli_override, provision)) + if provision: + provisioned.mkdir(exist_ok=True) + return provisioned, "default" + return tmp_path / "unprovisioned", "default" + + monkeypatch.setattr(paths, "resolve_state_dir", fake) + return calls, provisioned + + +@pytest.mark.parametrize("cls", [artifacts.ArtifactRegistry, artifacts.IgnoreList]) +def test_reading_does_not_provision(resolver, cls): + calls, _ = resolver + cls() + assert calls == [(None, False)] + + +def test_the_first_write_provisions_and_lands_there(resolver): + calls, provisioned = resolver + reg = artifacts.ArtifactRegistry() + reg.save({}) + assert calls[-1] == (None, True) + assert reg.path == provisioned / "artifacts.toml" + assert reg.path.is_file() + + +def test_ignore_list_write_provisions(resolver): + calls, provisioned = resolver + ign = artifacts.IgnoreList() + ign.save({}) + assert calls[-1] == (None, True) + assert ign.path.parent == provisioned + + +def test_an_explicit_dir_is_used_as_given(tmp_path, monkeypatch): + monkeypatch.setenv("SYSFORGE_STATE_DIR", str(tmp_path / "env")) + reg = artifacts.ArtifactRegistry(state_dir=tmp_path / "mine") + reg.save({}) + assert reg.path == tmp_path / "mine" / "artifacts.toml" + + +def test_source_built_packages_reads_without_provisioning(resolver): + calls, _ = resolver + assert build_sandbox.source_built_packages() == set() + assert calls == [(None, False)] diff --git a/tools/e2e/scenarios/85-artifacts.sh b/tools/e2e/scenarios/85-artifacts.sh index a54eaf9f..912b2f8b 100644 --- a/tools/e2e/scenarios/85-artifacts.sh +++ b/tools/e2e/scenarios/85-artifacts.sh @@ -1,14 +1,11 @@ # shellcheck shell=bash # User-authored scripts under management: discovered, adopted (by name, and # by review), edited in the managed copy, deployed back to the live file, then -# removed. A build runs first: the registry does not provision the state -# directory itself (3.5.0-B142), and in CI's shared system an earlier scenario -# always has, so starting provisioned keeps both modes alike. +# removed. In its own container it is the user's first session with nothing +# built, so the first registry write provisions the state directory itself +# (3.5.0-B142). # start: ready -step "a build provisions the state directory, as after the Quick start" -sysforge build "$(stage_pkg e2e-hello)" -si --rebuild Date: Sat, 24 Oct 2026 11:51:46 -0400 Subject: [PATCH 07/11] fix(uninstall): --yes, and refuse without a terminal (3.5.0-B143) uninstall left the confirmation to pacman -Rnsu with no --yes, so a scripted run read EOF at [Y/n] and failed with a raw command line. --yes passes --noconfirm; without it and without a terminal the verb refuses before pacman runs, as revert-to-stock does. Completions, man page and surface updated; 50-maintain checks the refusal and the interactive path. Tests that built uninstall args as SimpleNamespace now use cli_args. Co-Authored-By: Claude Opus 5.5 --- ROADMAP.md | 13 --------- completions/_sysforge | 1 + completions/sysforge.bash | 2 +- docs/release-notes/unreleased.md | 5 +++- man/sysforge.1 | 5 ++++ sysforge/uninstall_cmd.py | 13 +++++++-- tests/data/surface.json | 3 +- tests/test_opt_out.py | 2 +- tests/test_removed_failures.py | 3 +- tests/test_uninstall_cmd.py | 46 +++++++++++++++++++++++++++--- tools/e2e/scenarios/50-maintain.sh | 4 ++- 11 files changed, 70 insertions(+), 27 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index eed51ead..61aca056 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -113,7 +113,6 @@ canonical ordering. | ID | Item | Priority | Effort | Bump | |----|------|----------|--------|------| | `3.5.0-B141` | run kernel/toolchain/packages --dry-run shows nothing and skips the stage | med | medium | patch | -| `3.5.0-B143` | sysforge uninstall cannot run without a terminal | low | small | minor | ### Features @@ -137,18 +136,6 @@ canonical ordering. *Priority: med · Effort: medium · Bump: patch* — a documented preview flag that previews nothing; the stage runs that most need one take hours. -- **`3.5.0-B143` — `sysforge uninstall` cannot run without a terminal.** - The verb hands the confirmation to `pacman -Rnsu` (`pacman.uninstall_pkgs`) and has no `--yes`, - unlike `revert-to-stock`. Run from a script, a timer or with stdin closed, pacman reads EOF at - its `[Y/n]`, aborts, and sysforge reports `pacman removal failed (Command '['sudo', 'pacman', - '-Rnsu', '--', …]' returned non-zero exit status 1.)`: a raw command line instead of a reason, - after printing the whole transaction. Found by the `3.5.0-DEV23` end-to-end scenario - `50-maintain`, which answers the prompt on a pseudo-terminal to get past it. Fix: add `--yes` - (passes `--noconfirm`), and without it and without a terminal refuse before anything runs, - naming `--yes`, as `revert-to-stock` does; completions and man page in step. - *Priority: low · Effort: small · Bump: minor* — a new flag; until then a scripted removal - needs `pacman -R` plus `state forget` by hand. - ### Developer tooling ### Open questions diff --git a/completions/_sysforge b/completions/_sysforge index 6e985929..7030a7c3 100644 --- a/completions/_sysforge +++ b/completions/_sysforge @@ -582,6 +582,7 @@ _sysforge_search() { _sysforge_uninstall() { _arguments \ + '--yes[remove without pacman'"'"'s confirmation prompt]' \ '--state-dir[override state directory]:dir:_directories' \ '*:installed package:_sysforge_installed_pkg_names' } diff --git a/completions/sysforge.bash b/completions/sysforge.bash index 5fbc562f..8cc6870e 100644 --- a/completions/sysforge.bash +++ b/completions/sysforge.bash @@ -417,7 +417,7 @@ _sysforge_search() { _sysforge_uninstall() { if [[ $cur == -* ]]; then - COMPREPLY=( $(compgen -W "--state-dir" -- "$cur") ) + COMPREPLY=( $(compgen -W "--yes --state-dir" -- "$cur") ) else _sysforge_installed_pkg_names fi diff --git a/docs/release-notes/unreleased.md b/docs/release-notes/unreleased.md index e1769356..c07d4899 100644 --- a/docs/release-notes/unreleased.md +++ b/docs/release-notes/unreleased.md @@ -52,6 +52,10 @@ https://keepachangelog.com/en/1.1.0/ --- +- **`3.5.0-B143` — `sysforge uninstall --yes`, for scripts and timers.** `uninstall` passed its confirmation to pacman and had no `--yes`, so without a terminal pacman printed the whole transaction, read nothing at its `[Y/n]` and gave up, and sysforge reported the raw command that failed. `--yes` now removes without asking, and without it and without a terminal `uninstall` refuses before doing anything and says to pass `--yes`, as `revert-to-stock` does. At a terminal nothing changes: pacman still asks. + +--- + - **`3.5.0-F1` — `bench toolchain` gets past an out-of-date sync database.** The bench downloads the stock `clang` and `llvm-libs` that your sync database names. When the repo rebuilt them after your last `pacman -Sy`, every mirror had already dropped those files, so each download failed with HTTP 404 and the bench stopped. It now notices that every mirror failed, downloads a fresh copy of the sync database into its own temporary directory, and compares against the build the repo carries now. It does this as your user with `fakeroot` (from `base-devel`), never with sudo, and your system's sync database is left alone, because syncing it without upgrading would leave the system partially upgraded. The report shows the stock version it used, so a newer stock build is flagged as a version difference, as before. If the refresh fails too, the error says so and suggests `pacman -Syu`. `--offline` never refreshes. The PGO guide describes the fallback. --- @@ -90,7 +94,6 @@ https://keepachangelog.com/en/1.1.0/ - **`3.5.0-F12` — a per-package compiler swap switches the whole toolchain.** When a build failed and you picked gcc (or clang) from the build-failure menu, sysforge retried and remembered the choice in `profiles.toml`'s `[package_compiler_overrides]`, but only the C and C++ compiler and the linker changed: a gcc swap on an LLVM profile kept `llvm-ar`, `llvm-nm`, `llvm-ranlib`, `llvm-strip` and lld. A row can now say `toolchain = "gcc"` or `"llvm"`, which switches the whole set the way a `[[rules]]` profile's `toolchain` does, and the menu writes that form and retries with the whole set. At its linker question, leaving the answer blank now means the chosen toolchain's own linker. Rows that name `cc`, `cxx` and `ld` directly keep working and remain the way to name a compiler outside the two sets. When `sysforge build --cc`, `--cxx` or `--ld` overrides a row for a package, `-vv` says so. The comment above the table in `profiles.toml` lists which setting wins; `sysforge config merge` picks it up. - --- - **`3.5.0-F13` — `sysforge log --verb` pages the log a verb left.** `log` could page only the unified log and per-package logs, although `build`, `update`, `doctor`, `fetch`, `setup`, `uninstall`, `revert-to-stock`, `config merge`, `state repair`, `state forget` and each `run ` leave their own `sysforge-.log`, which the end of the run named once. `sysforge log --verb update` (or `build`, `state-repair`, `run-toolchain` and so on; Tab lists them) pages it, and `--log-dir` finds a log written with the same `--log-dir`, for verb, unified and per-package logs alike. diff --git a/man/sysforge.1 b/man/sysforge.1 index fa2e8f46..4a36f465 100644 --- a/man/sysforge.1 +++ b/man/sysforge.1 @@ -1510,6 +1510,11 @@ Remove package(s); demote any sysforge\-tracked ones out of build state.\& Package name(s) to remove (stock or \-sysforge name).\& .PP .RE +\fB\-\-yes\fR +.RS 4 +Remove without pacman'\&s confirmation prompt (needed without a terminal).\& +.PP +.RE \fBShared options:\fR \fB\-\-state\-dir\fR (see SHARED OPTIONS) .PP .SH FILES diff --git a/sysforge/uninstall_cmd.py b/sysforge/uninstall_cmd.py index 375e0e98..7f87ddd9 100644 --- a/sysforge/uninstall_cmd.py +++ b/sysforge/uninstall_cmd.py @@ -19,7 +19,7 @@ from sysforge import log from sysforge.pipeline.state import resolve_state_dir -from sysforge.primitives import install_reconcile, journal, pacman +from sysforge.primitives import install_reconcile, journal, pacman, prompt from sysforge.primitives.build_state import BuildState from sysforge.verbs.shared import ( emit_orphaned_profiles, @@ -63,6 +63,8 @@ def add_parser(cls, sub): help="Remove package(s); demote any sysforge-tracked ones out of build state.") p.add_argument("packages", nargs="+", metavar="PKG", help="Package name(s) to remove (stock or -sysforge name).") + p.add_argument("--yes", action="store_true", dest="yes", + help="Remove without pacman's confirmation prompt (needed without a terminal).") p.add_argument("--state-dir", metavar="DIR", dest="state_dir", help="Override state directory.") p.set_defaults(verb_cls=cls) @@ -86,11 +88,16 @@ def execute(self, args, pre: PreCheckResult) -> ExecResult: tag = "sysforge-tracked" if it.tracked else "repo/untracked" _log.ui(f"[uninstall] {it.target}{renamed}: {tag}") + # pacman asks at a terminal; with none it reads EOF at its [Y/n] and + # aborts after printing the transaction, so refuse first (3.5.0-B143). + if not args.yes and not prompt.is_interactive(): + _log.error("[uninstall] refusing without a terminal; pass --yes") + return ExecResult(exit_code=2) + names = [it.installed_name for it in items] store_names = profile_store_names(pre.ctx["state_dir"], names) try: - # Interactive: pacman prints its own transaction + confirmation. - pacman.uninstall_pkgs(names, extra_flags=list(getattr(args, "pacman_flags", []) or [])) + pacman.uninstall_pkgs(names, extra_flags=["--noconfirm"] if args.yes else []) except subprocess.CalledProcessError as exc: _log.error(f"[uninstall] pacman removal failed ({exc}); nothing demoted") return ExecResult(exit_code=1) diff --git a/tests/data/surface.json b/tests/data/surface.json index db1a19f2..cb9ea0dd 100644 --- a/tests/data/surface.json +++ b/tests/data/surface.json @@ -516,7 +516,8 @@ }, "uninstall": { "flags": { - "--state-dir": null + "--state-dir": null, + "--yes": null }, "positionals": { "PKG": null diff --git a/tests/test_opt_out.py b/tests/test_opt_out.py index b98f1f82..023bbbf6 100644 --- a/tests/test_opt_out.py +++ b/tests/test_opt_out.py @@ -130,7 +130,7 @@ def test_uninstall_writes_no_record(tmp_path, monkeypatch): monkeypatch.setattr(uninstall_cmd.install_reconcile, "consume_external_installs", lambda bs: uninstall_cmd.install_reconcile.Reconciled([], [])) bs = BuildState(tmp_path) - args = SimpleNamespace(packages=["htop"], pacman_flags=[], state_dir=str(tmp_path)) + args = cli_args("uninstall", "htop", "--yes", "--state-dir", str(tmp_path)) pre = PreCheckResult(ctx={"items": uninstall_cmd.plan_uninstall(bs, ["htop"]), "state_dir": str(tmp_path)}) assert uninstall_cmd.UninstallVerb().execute(args, pre).exit_code == 0 diff --git a/tests/test_removed_failures.py b/tests/test_removed_failures.py index e8f102c5..3d1591d8 100644 --- a/tests/test_removed_failures.py +++ b/tests/test_removed_failures.py @@ -7,7 +7,6 @@ (3.5.0-B120), whoever removed it, while a never-installed failure and a split pkgbase with a member still installed keep theirs. """ -from types import SimpleNamespace import pytest @@ -159,7 +158,7 @@ def _uninstall(tmp_path, monkeypatch, targets): monkeypatch.setattr(uninstall_cmd.pacman, "uninstall_pkgs", lambda names, extra_flags=None: None) bs = BuildState(tmp_path) - args = SimpleNamespace(packages=targets, pacman_flags=[], state_dir=str(tmp_path)) + args = cli_args("uninstall", *targets, "--yes", "--state-dir", str(tmp_path)) pre = PreCheckResult(ctx={"items": uninstall_cmd.plan_uninstall(bs, targets), "state_dir": str(tmp_path)}) assert uninstall_cmd.UninstallVerb().execute(args, pre).exit_code == 0 diff --git a/tests/test_uninstall_cmd.py b/tests/test_uninstall_cmd.py index bc0d2130..c716e6b9 100644 --- a/tests/test_uninstall_cmd.py +++ b/tests/test_uninstall_cmd.py @@ -3,11 +3,10 @@ # SPDX-License-Identifier: MIT """Tests for the uninstall verb.""" -from types import SimpleNamespace - from sysforge.primitives.build_state import BuildState from sysforge import uninstall_cmd from sysforge.verbs.base import PreCheckResult +from tests.cli_args import cli_args def _bs(tmp_path, entries): @@ -54,7 +53,8 @@ def test_execute_removes_then_forgets_and_reconciles(tmp_path, monkeypatch): lambda bs: uninstall_cmd.install_reconcile.Reconciled([], [])) verb = uninstall_cmd.UninstallVerb() - args = SimpleNamespace(packages=["mesa"], pacman_flags=[], state_dir=str(tmp_path)) + monkeypatch.setattr(uninstall_cmd.prompt, "is_interactive", lambda: True) + args = cli_args("uninstall", "mesa", "--state-dir", str(tmp_path)) pre = PreCheckResult(ctx={"items": uninstall_cmd.plan_uninstall(bs, ["mesa"]), "state_dir": str(tmp_path)}) res = verb.execute(args, pre) @@ -82,9 +82,47 @@ def test_execute_names_a_profile_store_the_uninstall_leaves_behind( monkeypatch.setattr(state_cmd, "orphaned_profile_lines", lambda names: seen.append(set(names)) or ["profile store kept: /s"]) verb = uninstall_cmd.UninstallVerb() - args = SimpleNamespace(packages=["mesa"], pacman_flags=[], state_dir=str(tmp_path)) + monkeypatch.setattr(uninstall_cmd.prompt, "is_interactive", lambda: True) + args = cli_args("uninstall", "mesa", "--state-dir", str(tmp_path)) pre = PreCheckResult(ctx={"items": uninstall_cmd.plan_uninstall(bs, ["mesa"]), "state_dir": str(tmp_path)}) verb.execute(args, pre) assert seen and "mesa" in seen[0] assert "profile store kept: /s" in "".join(capsys.readouterr()) + + +# 3.5.0-B143: without a terminal pacman's own [Y/n] read EOF and aborted, after +# printing the transaction; --yes confirms up front, and without it the verb +# refuses before anything runs. + +def _run(tmp_path, monkeypatch, *, yes, tty): + calls = [] + monkeypatch.setattr(uninstall_cmd.pacman, "uninstall_pkgs", + lambda names, extra_flags=None: calls.append(extra_flags)) + monkeypatch.setattr(uninstall_cmd.prompt, "is_interactive", lambda: tty) + monkeypatch.setattr(uninstall_cmd.install_reconcile, "consume_external_installs", + lambda bs: uninstall_cmd.install_reconcile.Reconciled([], [])) + argv = ["uninstall", "nano", "--state-dir", str(tmp_path)] + (["--yes"] if yes else []) + args = cli_args(*argv) + verb = uninstall_cmd.UninstallVerb() + res = verb.execute(args, verb.pre_check(args)) + return res, calls + + +def test_yes_removes_without_asking(tmp_path, monkeypatch): + res, calls = _run(tmp_path, monkeypatch, yes=True, tty=False) + assert res.exit_code == 0 + assert calls == [["--noconfirm"]] + + +def test_without_yes_or_a_terminal_it_refuses_before_pacman(tmp_path, monkeypatch, capsys): + res, calls = _run(tmp_path, monkeypatch, yes=False, tty=False) + assert res.exit_code == 2 + assert calls == [] + assert "--yes" in capsys.readouterr().err + + +def test_at_a_terminal_pacman_asks(tmp_path, monkeypatch): + res, calls = _run(tmp_path, monkeypatch, yes=False, tty=True) + assert res.exit_code == 0 + assert calls == [[]] diff --git a/tools/e2e/scenarios/50-maintain.sh b/tools/e2e/scenarios/50-maintain.sh index 3b3ed97f..8ba64eb7 100644 --- a/tools/e2e/scenarios/50-maintain.sh +++ b/tools/e2e/scenarios/50-maintain.sh @@ -46,7 +46,9 @@ assert_installed e2e-hello step "uninstall removes the package and its record" sysforge build "$dir" -si --rebuild Date: Sat, 24 Oct 2026 11:57:12 -0400 Subject: [PATCH 08/11] fix(pipeline): a standalone stage dry run runs the stage (3.5.0-B141) run_stage_standalone returned before stage.run on --dry-run, logging one INFO line, so `run kernel --dry-run` printed nothing and never reached the dry-run handling every stage already carries. It now runs the stage after a ui() `[dry-run] ` line. PipelineState gains read_only, set for every dry run, so a stage's mid-run checkpoints never reach the file; the kernel install window and the packages loop take no sentinel on a dry run. run pipeline --dry-run keeps its stage list (the bootstrap stages are not dry-run safe). 95-stage-dry-run drops its xfail and checks the records, not the state-dir listing. Co-Authored-By: Claude Opus 5.5 --- DESIGN.md | 2 +- ROADMAP.md | 18 ------------ docs/design/07-pipeline-layer.md | 2 +- docs/release-notes/unreleased.md | 4 +++ sysforge/pipeline/runner.py | 10 +++++-- sysforge/pipeline/stages/kernel/stage.py | 7 +++-- sysforge/pipeline/stages/packages.py | 6 ++-- sysforge/primitives/pipeline_state.py | 8 +++++- tests/test_pipeline_runner.py | 36 ++++++++++++++++++++++++ tools/e2e/README.md | 2 +- tools/e2e/scenarios/85-artifacts.sh | 1 + tools/e2e/scenarios/95-stage-dry-run.sh | 17 +++++------ 12 files changed, 75 insertions(+), 38 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index 7a1afeaa..1cb22503 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -780,7 +780,7 @@ The hardware stage (stage 2) needs no config — it auto-detects and writes `har - On `NotImplementedError`: prints `--start-from` guidance and exits - On `BootstrapRebootRequired`: saves state, prints reboot + resume instructions, exits 0 (clean stop, not failure) - On `RuntimeError`: saves state and exits with resume instructions -- `--dry-run`: logs what would run without calling `stage.run()` +- `--dry-run`: `run pipeline` lists the stages it would run without calling `stage.run()` (the bootstrap stages are not dry-run safe). A standalone `run --dry-run` calls `stage.run()` with `options.dry_run` set, after a `[dry-run] ` line through `ui()`: each stage stops short of its own changes and says what it would do, so the preview reads the real config (the kernel's "Kernel build plan", the toolchain's install set). The runner opens `PipelineState(read_only=True)`, so a stage's mid-run checkpoints never reach the file, and the kernel and packages stages take no sentinel (`3.5.0-B141`; before it the runner returned before `stage.run()` and logged one INFO line, so the preview showed nothing). Guard against accidental state clobber: if a state file exists and neither `--resume` nor `--start-from` is passed, the runner exits with instructions rather than overwriting. Both flags are supported on `sysforge run pipeline`. diff --git a/ROADMAP.md b/ROADMAP.md index 61aca056..fdacb539 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -112,30 +112,12 @@ canonical ordering. | ID | Item | Priority | Effort | Bump | |----|------|----------|--------|------| -| `3.5.0-B141` | run kernel/toolchain/packages --dry-run shows nothing and skips the stage | med | medium | patch | ### Features ### Bugs -- **`3.5.0-B141` — `run kernel/toolchain/packages --dry-run` shows nothing and skips the stage.** - Each promises "Show what would run without executing anything", but `pipeline.runner` (the - standalone-stage path) short-circuits on `options.dry_run` before `stage.run`, logging only - `[dry-run] would run stage: ` at INFO, which the default verbosity hides. So the command - prints nothing at all, and the dry-run handling the stages carry themselves is unreachable: the - kernel stage's "Kernel build plan" resolution summary (compiler, variant, bootloader, source, - kconfig counts; §Kernel stage calls it "the readable core of `--dry-run`"), its boot-entry prune - preview, `--autofdo=capture --dry-run`, and the pre-build snapshot's dry-run branch. None of - `toolchain.toml` or `kernel.toml` past `enabled` is read, so a misconfiguration surfaces only - when the real run starts. Found writing the `3.5.0-DEV23` end-to-end scenarios on 2026-10-24 - (`tools/e2e/scenarios/95-stage-dry-run.sh` is `xfail` against it). Fix: pass the dry run into - `stage.run` and have each stage stop at its first mutation (the toolchain stage before any - build, the kernel stage after its resolution summary and gates), printing its plan through - `ui()`; `run pipeline --dry-run` shares the runner and needs the same check. - *Priority: med · Effort: medium · Bump: patch* — a documented preview flag that previews - nothing; the stage runs that most need one take hours. - ### Developer tooling ### Open questions diff --git a/docs/design/07-pipeline-layer.md b/docs/design/07-pipeline-layer.md index ce9cdc55..ce2f9899 100644 --- a/docs/design/07-pipeline-layer.md +++ b/docs/design/07-pipeline-layer.md @@ -156,7 +156,7 @@ The hardware stage (stage 2) needs no config — it auto-detects and writes `har - On `NotImplementedError`: prints `--start-from` guidance and exits - On `BootstrapRebootRequired`: saves state, prints reboot + resume instructions, exits 0 (clean stop, not failure) - On `RuntimeError`: saves state and exits with resume instructions -- `--dry-run`: logs what would run without calling `stage.run()` +- `--dry-run`: `run pipeline` lists the stages it would run without calling `stage.run()` (the bootstrap stages are not dry-run safe). A standalone `run --dry-run` calls `stage.run()` with `options.dry_run` set, after a `[dry-run] ` line through `ui()`: each stage stops short of its own changes and says what it would do, so the preview reads the real config (the kernel's "Kernel build plan", the toolchain's install set). The runner opens `PipelineState(read_only=True)`, so a stage's mid-run checkpoints never reach the file, and the kernel and packages stages take no sentinel (`3.5.0-B141`; before it the runner returned before `stage.run()` and logged one INFO line, so the preview showed nothing). Guard against accidental state clobber: if a state file exists and neither `--resume` nor `--start-from` is passed, the runner exits with instructions rather than overwriting. Both flags are supported on `sysforge run pipeline`. diff --git a/docs/release-notes/unreleased.md b/docs/release-notes/unreleased.md index c07d4899..925f986d 100644 --- a/docs/release-notes/unreleased.md +++ b/docs/release-notes/unreleased.md @@ -877,6 +877,10 @@ https://keepachangelog.com/en/1.1.0/ --- +- **`3.5.0-B141` — `run kernel`, `run toolchain` and the other stage commands preview with `--dry-run`.** `--dry-run` promised to show what would run, but the stage never ran: sysforge logged one line that the default verbosity hid, so the command printed nothing and read nothing from `kernel.toml` or `toolchain.toml` past `enabled`. A dry run now goes through the stage up to the point where it would change something, and says what it would do instead: `run kernel --dry-run` prints its "Kernel build plan" (compiler and where it came from, bootloader, source, kconfig counts, the safety gates) and the build and install it would run, so a mistake in the config shows before a build of an hour or more. The run still installs, builds, records and locks nothing. `run pipeline --dry-run` keeps listing the stages it would run. + +--- + - **`3.5.0-B142` — the `artifact` commands work in your first session.** Run before anything else after installing, `sysforge artifact adopt` (or any command that saves the artifact list) ended in a Python traceback, `PermissionError` on `/var/lib/sysforge`: the artifact commands found SysForge's state directory on their own and never opened it to you the way `build` does until the `sysforge` group applies at your next login. They now go through the same step, and the read-only ones still never ask for `sudo`. ## Security diff --git a/sysforge/pipeline/runner.py b/sysforge/pipeline/runner.py index 57ebd05c..14085d21 100644 --- a/sysforge/pipeline/runner.py +++ b/sysforge/pipeline/runner.py @@ -179,7 +179,7 @@ def run_stage_standalone(stage, config, options): _log = log.get_logger(stage.name.upper()) state_dir, _ = resolve_state_dir(cli_override=options.state_dir) - state = PipelineState(state_dir) + state = PipelineState(state_dir, read_only=options.dry_run) reset_session() emit_system_probes() @@ -214,7 +214,11 @@ def _checkpoint(): success = False try: if options.dry_run: - _log.info(f"[dry-run] would run stage: {stage.name}; {stage.description}") + # The stage runs: each one stops short of its own changes and says + # what it would do, so the preview reads the real config. Only the + # bookkeeping around it is skipped (3.5.0-B141). + _log.ui(f"[dry-run] {stage.name}: {stage.description}; nothing will be changed") + stage.run(config, state, options) else: _log.info(f"── Stage: {stage.name} ── {stage.description}") # 3.1.0-F5: point a fresh install at the TOML that decides what @@ -269,7 +273,7 @@ def run_pipeline(config, options, stages=None): state_dir, state_source = resolve_state_dir( cli_override=options.state_dir, ) - state = PipelineState(state_dir) + state = PipelineState(state_dir, read_only=options.dry_run) stage_names = [s.name for s in stages] diff --git a/sysforge/pipeline/stages/kernel/stage.py b/sysforge/pipeline/stages/kernel/stage.py index 5f0d565a..7d9b35c5 100644 --- a/sysforge/pipeline/stages/kernel/stage.py +++ b/sysforge/pipeline/stages/kernel/stage.py @@ -714,8 +714,9 @@ def _kernel_build_options(extra_flags=None): # sentinel so an interrupted install / mkinitcpio / bootloader regen # blocks the next run with a recovery command (the failure mode that # leaves the system kernel-installed but initramfs-missing → - # unbootable). - with sentinel_scope( + # unbootable). A dry run installs nothing, so it takes no sentinel + # or lock (3.5.0-B141). + with (contextlib.nullcontext() if options.dry_run else sentinel_scope( options.state_dir, "kernel", recovery=constants.kernel_recovery(), @@ -723,7 +724,7 @@ def _kernel_build_options(extra_flags=None): pkgname=fdo_eff_pkgname, bootloader=bootloader, compiler=compiler or "default", - ): + )): if options.dry_run: _log.ui(f"[dry-run] would install {fdo_eff_pkgname} and wire it into boot") else: diff --git a/sysforge/pipeline/stages/packages.py b/sysforge/pipeline/stages/packages.py index 89808a18..a024f3c8 100644 --- a/sysforge/pipeline/stages/packages.py +++ b/sysforge/pipeline/stages/packages.py @@ -35,6 +35,7 @@ 2. profiles.toml [paths] pkgbuild_src_dir//PKGBUILD (if set) 3. AUR clone into pkgbuild_src_dir if package is found on AUR """ +import contextlib import subprocess import tomllib from sysforge import log @@ -396,12 +397,13 @@ def run(self, config, state, options): 1 for n in all_names if n not in built and n not in skipped and n not in skip_set ) - with sentinel_scope( + # A dry run changes nothing, so it takes no sentinel or lock (3.5.0-B141). + with (contextlib.nullcontext() if options.dry_run else sentinel_scope( options.state_dir, "packages", retry_cmd="sysforge run packages", remaining=n_remaining, - ): + )): if aur_names and not options.dry_run: all_aur_deps = [] for name in aur_names: diff --git a/sysforge/primitives/pipeline_state.py b/sysforge/primitives/pipeline_state.py index a31ccbe5..f55f998d 100644 --- a/sysforge/primitives/pipeline_state.py +++ b/sysforge/primitives/pipeline_state.py @@ -75,7 +75,11 @@ class PipelineState: when a stage fails and needs intervention before resuming. """ - def __init__(self, state_dir): + def __init__(self, state_dir, *, read_only: bool = False): + # read_only: a dry run's state. Stages checkpoint mid-run (a toolchain + # result, package progress); under --dry-run none of it may reach the + # file, and this is the one place that holds for every stage (3.5.0-B141). + self._read_only = read_only self._dir = Path(state_dir) self.path = self._dir / "pipeline_state.toml" self._data = self._load() @@ -92,6 +96,8 @@ def _load(self): def save(self): """Write current state to disk atomically (write + rename).""" + if self._read_only: + return atomic_write.write_state(self.path, self._serialize()) def _now(self): diff --git a/tests/test_pipeline_runner.py b/tests/test_pipeline_runner.py index 1f9eeb0a..bc21412b 100644 --- a/tests/test_pipeline_runner.py +++ b/tests/test_pipeline_runner.py @@ -735,3 +735,39 @@ def run(self, config, state, options): with pytest.raises(SystemExit): _standalone(OsFail("toolchain"), tmp_path, monkeypatch) assert PipelineState(tmp_path).stage_status("toolchain") == "failed" + + +# --------------------------------------------------------------------------- +# A standalone dry run runs the stage (3.5.0-B141) +# --------------------------------------------------------------------------- +# The runner used to return before stage.run, logging one line at INFO, so +# `run kernel --dry-run` printed nothing and never showed the plan the stage +# resolves. The stages carry their own dry-run handling; the runner's job is to +# reach it and to keep the state file untouched. + +def test_standalone_dry_run_runs_the_stage(tmp_path, monkeypatch): + seen = {} + + class Recording(OkStage): + def run(self, config, state, options): + seen["dry_run"] = options.dry_run + + _standalone(Recording("kernel"), tmp_path, monkeypatch, dry_run=True) + assert seen == {"dry_run": True} + + +def test_standalone_dry_run_says_so_at_default_verbosity(tmp_path, monkeypatch, capsys): + _standalone(OkStage("kernel"), tmp_path, monkeypatch, dry_run=True) + assert "[dry-run] kernel" in capsys.readouterr().err + + +def test_standalone_dry_run_drops_the_stages_own_saves(tmp_path, monkeypatch): + _standalone(ResultStage("toolchain"), tmp_path, monkeypatch, dry_run=True) + assert not (tmp_path / "pipeline_state.toml").exists() + + +def test_a_read_only_state_never_writes(tmp_path): + state = PipelineState(tmp_path, read_only=True) + state.set_stage_result("toolchain", {"cc": "x"}) + state.save() + assert not (tmp_path / "pipeline_state.toml").exists() diff --git a/tools/e2e/README.md b/tools/e2e/README.md index ed73917c..fa99bedc 100644 --- a/tools/e2e/README.md +++ b/tools/e2e/README.md @@ -48,7 +48,7 @@ sentinels (`lib.sh`), not on rendered output. Prompts are answered on a pseudo-t | `80-inspect` | the read-only commands (`state` reports, `doctor`, `env`, `fetch`) change nothing | | `85-artifacts` | a user script discovered, adopted, reviewed, edited, deployed and removed | | `90-source-policy` | `[aur] enabled = false` and `freeze_sources` refuse before any checkout; `search` with the AUR off | -| `95-stage-dry-run` | `run toolchain`/`run kernel --dry-run` with non-default configs change nothing; `xfail` until the kernel plan shows (`3.5.0-B141`) | +| `95-stage-dry-run` | `run toolchain`/`run kernel --dry-run` with non-default configs change nothing, and the kernel's shows its plan | Packages come from `pkgs/`, local PKGBUILDs with no network sources, so an outage can't flake the tier. Three scenarios need the network, because it is the point: `10-quickstart` (the AUR, and diff --git a/tools/e2e/scenarios/85-artifacts.sh b/tools/e2e/scenarios/85-artifacts.sh index 912b2f8b..c327f433 100644 --- a/tools/e2e/scenarios/85-artifacts.sh +++ b/tools/e2e/scenarios/85-artifacts.sh @@ -27,6 +27,7 @@ output_has e2e-other sysforge artifact list >"$1"\n' >"$editor" chmod +x "$editor" EDITOR=$editor sysforge artifact edit e2e-tool ~/src/linux-sysforge/PKGBUILD -profiles=$(sha256sum /etc/sysforge/profiles.toml) -state=$(ls "$STATE_DIR") +# The records a run changes. Caches (source_meta.toml) may fill in. +records() { + sha256sum /etc/sysforge/profiles.toml + sha256sum "$STATE_DIR"/pipeline_state.toml "$STATE_DIR"/build_state.toml 2>/dev/null || true +} +before=$(records) step "the toolchain dry run changes nothing" sysforge run toolchain --dry-run --no-update &1) || fail "run kernel --dry-run failed: $out" echo "$out" assert_not_installed linux-sysforge -[[ $(ls "$STATE_DIR") == "$state" ]] || fail "a dry run added state: $(ls "$STATE_DIR")" +[[ $(records) == "$before" ]] || fail "a kernel dry run changed a record" assert_no_sentinel step "the kernel dry run shows the plan it resolved" From 8456e640744958d88728952a8e2d40c809732f71 Mon Sep 17 00:00:00 2001 From: Keith Raghubar Date: Sat, 24 Oct 2026 12:07:34 -0400 Subject: [PATCH 09/11] feat: sysforge self-uninstall reverts to stock, then removes sysforge (3.5.0-F19) Plans by reading only: every source build goes through plan_revert, reverted when the sync DB (new pacman.get_sync_pkgnames, one -Slq) carries its stock names, kept installed otherwise. Then drops sf-build from IgnoreGroup (setup_cmd._unpatch_conf_text, the inverse of _patch_conf_text) and the chroot's local repo block, and removes the package, paths.PACKAGE_DIRS and the group in one sudo sh -c, last, since the process may import nothing after pacman deletes it. Reverts run through revert_cmd.apply_reverts, now extracted as the one home both verbs call, under the verb's own sentinel. --dry-run, --yes, --keep-data. The guide's manual removal list (and its wrong .pacsave claim) is replaced; 99-self-uninstall runs it on a live system. Co-Authored-By: Claude Opus 5.5 --- DESIGN.md | 15 +- completions/_sysforge | 10 + completions/sysforge.bash | 8 +- docs/design/08-cli-verb-framework.md | 13 +- docs/design/17-release-plan.md | 2 +- docs/guides/commands.md | 1 + docs/guides/what-sysforge-touches.md | 26 ++- docs/release-notes/unreleased.md | 4 + man/sysforge.1 | 25 +- sysforge/primitives/local_repo.py | 8 + sysforge/primitives/pacman.py | 21 ++ sysforge/primitives/paths.py | 5 + sysforge/revert_cmd.py | 138 ++++++----- sysforge/self_uninstall_cmd.py | 284 +++++++++++++++++++++++ sysforge/setup_cmd.py | 35 +++ sysforge/verbs/registry.py | 5 +- tests/data/surface.json | 9 + tests/test_self_uninstall.py | 177 ++++++++++++++ tools/e2e/README.md | 8 +- tools/e2e/scenarios/99-self-uninstall.sh | 50 ++++ 20 files changed, 761 insertions(+), 83 deletions(-) create mode 100644 sysforge/self_uninstall_cmd.py create mode 100644 tests/test_self_uninstall.py create mode 100644 tools/e2e/scenarios/99-self-uninstall.sh diff --git a/DESIGN.md b/DESIGN.md index 1cb22503..8f3e3d56 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -1421,7 +1421,8 @@ spelling per meaning, across every verb: | `revert-to-stock` | resolve state dir + `plan_revert` over `BuildState` (pure, no mutation) | prompt/`--yes`/`--dry-run` gate, then per-target reinstall / atomic replace / derename-then-reinstall via `pacman` | (state written inline) | yes | | `bench toolchain` | null | resolve preconditions → workload → unprivileged stock fetch + extract → interleaved timing → report → `bench.toml` | null | no | | `search` | null | print installed → repo → AUR sections (fixed order, empty omitted) for one term, with in-line installed / stand-in (`-sysforge`, `-git` drop-in) markers | null | no | -| `uninstall` | resolve state dir + `plan_uninstall` over `BuildState` (pure) | `pacman -Rnsu`, then demote any tracked target via `state forget` + reconcile | (state written inline) | yes | +| `uninstall` | resolve state dir + `plan_uninstall` over `BuildState` (pure) | `--yes`/terminal gate, `pacman -Rnsu`, then demote any tracked target via `state forget` + reconcile | (state written inline) | yes | +| `self-uninstall` | `plan_self_uninstall` over `BuildState`, the sync DB, pacman.conf and the package dirs (reads only) | `--dry-run`/`--yes` gate; `apply_reverts` under its own sentinel; drop `IgnoreGroup` and the chroot repo block; one `sudo sh -c` removing the package, then the data and group | null | own scope, reverts only | | `run …` namespace | build `RunOptions` | delegate to `pipeline.run_pipeline` / `run_stage_standalone` | pipeline framework | (pipeline owns it) | | `artifact list` | null | `primitives/artifacts.unified_rows()` + PATH check + optional `scan()` for `--unmanaged` | null | no | | `artifact review` | `--all`, `--include-unknown` | interactively offer discovered candidates for adoption via `primitives/artifacts.iter_offerable()`; off-TTY lists candidates + adopt hint; `--all` bulk-adopts the same offer set without prompting | null | no | @@ -1432,7 +1433,7 @@ spelling per meaning, across every verb: ### `revert-to-stock` -Undoes a source-built or optimized package back to its official repo version. `pre_check` resolves the state dir and calls `plan_revert(bs, targets)` (pure — no mutation) to classify each target against `BuildState`, including a reverse lookup via the shared `install_reconcile.resolve_installed_name(bs, name)` helper (iterated in sorted key order so a stock base always resolves to the same pkgname deterministically) so naming the *stock* base of a renamed build (e.g. `mesa` when the tracked entry is `mesa-sysforge`) still resolves, as does a renamed split member's own stock name (`vulkan-radeon` → `vulkan-radeon-sysforge`, whose `origin_pkgbase` is `mesa`, `3.5.0-B62`). That helper is the single home for the reverse lookup — `uninstall` uses it too. Four actions, distinguished by `profile.is_optimized_build_mode(mode)` and — for optimized builds — `profile.rename_mode_for_build_mode(mode)`: +Undoes a source-built or optimized package back to its official repo version. `pre_check` resolves the state dir and calls `plan_revert(bs, targets)` (pure — no mutation) to classify each target against `BuildState`, including a reverse lookup via the shared `install_reconcile.resolve_installed_name(bs, name)` helper (iterated in sorted key order so a stock base always resolves to the same pkgname deterministically) so naming the *stock* base of a renamed build (e.g. `mesa` when the tracked entry is `mesa-sysforge`) still resolves, as does a renamed split member's own stock name (`vulkan-radeon` → `vulkan-radeon-sysforge`, whose `origin_pkgbase` is `mesa`, `3.5.0-B62`). That helper is the single home for the reverse lookup — `uninstall` uses it too. Carrying out the plans is `apply_reverts(actionable, state_dir)`, shared with `self-uninstall`. Four actions, distinguished by `profile.is_optimized_build_mode(mode)` and — for optimized builds — `profile.rename_mode_for_build_mode(mode)`: - **`skip`** — untracked or already `build_mode = "pacman"`; nothing to do. - **`reinstall`** — plain `source_built` (or `bolt_llvm`, a BOLTed toolchain member, which also keeps its stock name), installed under the stock name: just reinstall the repo package (`pacman -S `). The name is the member itself, never its pkgbase (`pacman -S mesa` left a source-built `vulkan-radeon` in place); naming the pkgbase of a split set reinstalls every installed member, matching what `forget_packages` drops (`3.5.0-B62`). @@ -1465,7 +1466,13 @@ Read-only (`3.4.0-F6`, `BenchToolchainVerb` under the `bench` `VerbGroup`, Inspe ### `uninstall` -Mutating lifecycle verb (`requires_sentinel = True`). `pre_check` resolves each name through the shared `resolve_installed_name` (so naming a stock base reaches its `-sysforge` build) and builds a pure `plan_uninstall` classifying each target's installed name + whether it's tracked. `execute` narrates the plan, removes via `pacman.uninstall_pkgs` (`pacman -Rnsu`, interactive — pacman prints its own confirmation; a `CalledProcessError` returns exit 1 and demotes nothing), clears the removed targets' recorded build failures through `install_reconcile.clear_removed_failures` (tracked or not, before the forget whose deleted records map a split member to its pkgbase; `3.5.0-B120`), then for any tracked target demotes it out of the build-state authority via `cmd_state_forget` (the same `state forget` code) followed by an `install_reconcile.consume_external_installs(bs)` pass. This is the exact demotion composition `revert-to-stock` uses — no parallel path — so `update` stops rebuilding an uninstalled package. +Mutating lifecycle verb (`requires_sentinel = True`). `pre_check` resolves each name through the shared `resolve_installed_name` (so naming a stock base reaches its `-sysforge` build) and builds a pure `plan_uninstall` classifying each target's installed name + whether it's tracked. `execute` narrates the plan, removes via `pacman.uninstall_pkgs` (`pacman -Rnsu`, interactive — pacman prints its own confirmation; a `CalledProcessError` returns exit 1 and demotes nothing), clears the removed targets' recorded build failures through `install_reconcile.clear_removed_failures` (tracked or not, before the forget whose deleted records map a split member to its pkgbase; `3.5.0-B120`), then for any tracked target demotes it out of the build-state authority via `cmd_state_forget` (the same `state forget` code) followed by an `install_reconcile.consume_external_installs(bs)` pass. This is the exact demotion composition `revert-to-stock` uses — no parallel path — so `update` stops rebuilding an uninstalled package. Without a terminal it refuses before pacman runs unless `--yes` is given, which passes `--noconfirm`: pacman's own `[Y/n]` read EOF and aborted after printing the transaction (`3.5.0-B143`). + +### `self-uninstall` + +Removes SysForge in one command (`3.5.0-F19`), replacing the manual list the "What SysForge touches" guide gave. `pre_check` builds a `SelfUninstallPlan` by reading only: every `build_state` entry not built by pacman goes through `plan_revert`, a split set planned once; a plan whose stock names (`pacman.get_sync_pkgnames`, one `pacman -Slq`) a repo carries is reverted, any other (an AUR or local build, a custom kernel, an FDO kernel whose plain kernel is itself local) is kept installed with the reason. It also records whether `/etc/pacman.conf` has `sf-build` in `IgnoreGroup` (`setup_cmd._check_ignore_group`), whether the sandbox chroot lists the local repo (`local_repo.has_chroot_section`), the package that owns `sysforge/__init__.py` (none from a checkout), which of `paths.PACKAGE_DIRS` and the XDG user dirs exist, and whether the `sysforge` group exists. `PACKAGE_DIRS` is fixed (`/etc/sysforge`, `/var/lib/sysforge`, `/var/cache/sysforge`), never `SYSFORGE_CONFIG_DIR`/`SYSFORGE_STATE_DIR`, which can name a developer's own tree. + +`execute` prints the plan, stops on `--dry-run`, and otherwise needs `--yes` or a terminal's `y` and an authorised `sudo`. Order is what makes a failure safe: (1) the reverts through `revert_cmd.apply_reverts`, the one home `revert-to-stock` also calls, inside the verb's own `sentinel_scope` (recovery: reinstall the stock packages) — the verb sets `requires_sentinel = False` and `wants_run_log = False` because the runner's scope and log live in the state dir it is about to delete; a failed revert stops the run with sysforge still installed; (2) `setup_cmd.remove_ignore_group` (`_unpatch_conf_text`, the inverse of `_patch_conf_text`) and `local_repo.remove_chroot_section`; (3) `removal_script`, one `sudo sh -c` running `pacman -Rns --noconfirm `, then `rm -rf` of the package dirs and `groupdel sysforge`. Step 3 is last and single because once pacman has deleted the package this process must import nothing more from it; after it only `shutil.rmtree` of the user dirs and a `print` run. The package's pacman hooks are gone before pacman runs post-transaction hooks, so none writes into the state dir being deleted. `--keep-data` uses `pacman -Rs` (edited config kept as `.pacsave`) and skips the data, group and user dirs. `tools/e2e/scenarios/99-self-uninstall.sh`, the tier's last scenario, runs the whole thing on a live system. ### `artifact list` @@ -4106,7 +4113,7 @@ codes, `pacman -Qi`, the state files and leftover sentinels, with prompts answer pseudo-terminal. It covers what the unit suite stubs: root versus the user, group membership that applies only at the next login, the state one command leaves for the next. It builds sysforge from the checkout itself, so it needs no `vm-pkg-stable`. `pre-release` runs it; the release preflight -does not (`_RELEASE_GATE_EXEMPT`: minutes long, podman, and three networked scenarios, so not +does not (`_RELEASE_GATE_EXEMPT`: minutes long, podman, and four networked scenarios, so not deterministic). CI runs the same scenarios in an `e2e` job of its own, directly in the job's container (`make e2e E2E_HERE=1`), listed in the Makefile's `CI_JOBS` beside `GATES`. A scenario marked `xfail: ` fails until that item is fixed and reports `XFAIL`; once it passes it fails diff --git a/completions/_sysforge b/completions/_sysforge index 7030a7c3..d433018b 100644 --- a/completions/_sysforge +++ b/completions/_sysforge @@ -40,6 +40,7 @@ _sysforge() { search) _sysforge_search ;; setup) _sysforge_setup ;; uninstall) _sysforge_uninstall ;; + self-uninstall) _sysforge_self_uninstall ;; env) _sysforge_env ;; help) _sysforge_help ;; log) _sysforge_log ;; @@ -95,6 +96,7 @@ _sysforge_commands() { 'run:build toolchain, kernel or packages; or bootstrap' 'revert-to-stock:restore a source-built package to the repo version' 'uninstall:remove package(s); demote tracked ones from build state' + 'self-uninstall:revert sysforge'"'"'s builds to stock, then remove sysforge' ) _describe 'command' commands } @@ -587,6 +589,14 @@ _sysforge_uninstall() { '*:installed package:_sysforge_installed_pkg_names' } +_sysforge_self_uninstall() { + _arguments \ + '--keep-data[remove only the package; keep config, state and profiles]' \ + '--dry-run[show the plan without changing anything]' \ + '--yes[do it without asking]' \ + '--state-dir[override state directory]:dir:_directories' +} + _sysforge_run() { local context state state_descr line typeset -A opt_args diff --git a/completions/sysforge.bash b/completions/sysforge.bash index 8cc6870e..ef997e2b 100644 --- a/completions/sysforge.bash +++ b/completions/sysforge.bash @@ -26,7 +26,7 @@ _sysforge() { # Ordered by usage tier to mirror `sysforge --help` (2.5.0-F1): # Everyday, then Inspect, then Maintain. - local commands="build update fetch search help doctor resolve env log state bench artifact setup config packages run revert-to-stock uninstall" + local commands="build update fetch search help doctor resolve env log state bench artifact setup config packages run revert-to-stock uninstall self-uninstall" # Locate the top-level verb (first non-flag arg after `sysforge`) and an # optional subverb (first non-flag arg after the verb). @@ -80,6 +80,7 @@ _sysforge() { search) _sysforge_search ;; setup) _sysforge_setup ;; uninstall) _sysforge_uninstall ;; + self-uninstall) _sysforge_self_uninstall ;; env) _sysforge_env ;; log) _sysforge_log ;; help) _sysforge_help ;; @@ -423,6 +424,11 @@ _sysforge_uninstall() { fi } +_sysforge_self_uninstall() { + _sysforge_flag_arg && return + [[ $cur == -* ]] && COMPREPLY=( $(compgen -W "--keep-data --dry-run --yes --state-dir" -- "$cur") ) +} + _sysforge_run() { _sysforge_flag_arg && return if [[ -z $subverb ]]; then diff --git a/docs/design/08-cli-verb-framework.md b/docs/design/08-cli-verb-framework.md index 5b0957ec..3cf9a0b6 100644 --- a/docs/design/08-cli-verb-framework.md +++ b/docs/design/08-cli-verb-framework.md @@ -108,7 +108,8 @@ spelling per meaning, across every verb: | `revert-to-stock` | resolve state dir + `plan_revert` over `BuildState` (pure, no mutation) | prompt/`--yes`/`--dry-run` gate, then per-target reinstall / atomic replace / derename-then-reinstall via `pacman` | (state written inline) | yes | | `bench toolchain` | null | resolve preconditions → workload → unprivileged stock fetch + extract → interleaved timing → report → `bench.toml` | null | no | | `search` | null | print installed → repo → AUR sections (fixed order, empty omitted) for one term, with in-line installed / stand-in (`-sysforge`, `-git` drop-in) markers | null | no | -| `uninstall` | resolve state dir + `plan_uninstall` over `BuildState` (pure) | `pacman -Rnsu`, then demote any tracked target via `state forget` + reconcile | (state written inline) | yes | +| `uninstall` | resolve state dir + `plan_uninstall` over `BuildState` (pure) | `--yes`/terminal gate, `pacman -Rnsu`, then demote any tracked target via `state forget` + reconcile | (state written inline) | yes | +| `self-uninstall` | `plan_self_uninstall` over `BuildState`, the sync DB, pacman.conf and the package dirs (reads only) | `--dry-run`/`--yes` gate; `apply_reverts` under its own sentinel; drop `IgnoreGroup` and the chroot repo block; one `sudo sh -c` removing the package, then the data and group | null | own scope, reverts only | | `run …` namespace | build `RunOptions` | delegate to `pipeline.run_pipeline` / `run_stage_standalone` | pipeline framework | (pipeline owns it) | | `artifact list` | null | `primitives/artifacts.unified_rows()` + PATH check + optional `scan()` for `--unmanaged` | null | no | | `artifact review` | `--all`, `--include-unknown` | interactively offer discovered candidates for adoption via `primitives/artifacts.iter_offerable()`; off-TTY lists candidates + adopt hint; `--all` bulk-adopts the same offer set without prompting | null | no | @@ -119,7 +120,7 @@ spelling per meaning, across every verb: ### `revert-to-stock` -Undoes a source-built or optimized package back to its official repo version. `pre_check` resolves the state dir and calls `plan_revert(bs, targets)` (pure — no mutation) to classify each target against `BuildState`, including a reverse lookup via the shared `install_reconcile.resolve_installed_name(bs, name)` helper (iterated in sorted key order so a stock base always resolves to the same pkgname deterministically) so naming the *stock* base of a renamed build (e.g. `mesa` when the tracked entry is `mesa-sysforge`) still resolves, as does a renamed split member's own stock name (`vulkan-radeon` → `vulkan-radeon-sysforge`, whose `origin_pkgbase` is `mesa`, `3.5.0-B62`). That helper is the single home for the reverse lookup — `uninstall` uses it too. Four actions, distinguished by `profile.is_optimized_build_mode(mode)` and — for optimized builds — `profile.rename_mode_for_build_mode(mode)`: +Undoes a source-built or optimized package back to its official repo version. `pre_check` resolves the state dir and calls `plan_revert(bs, targets)` (pure — no mutation) to classify each target against `BuildState`, including a reverse lookup via the shared `install_reconcile.resolve_installed_name(bs, name)` helper (iterated in sorted key order so a stock base always resolves to the same pkgname deterministically) so naming the *stock* base of a renamed build (e.g. `mesa` when the tracked entry is `mesa-sysforge`) still resolves, as does a renamed split member's own stock name (`vulkan-radeon` → `vulkan-radeon-sysforge`, whose `origin_pkgbase` is `mesa`, `3.5.0-B62`). That helper is the single home for the reverse lookup — `uninstall` uses it too. Carrying out the plans is `apply_reverts(actionable, state_dir)`, shared with `self-uninstall`. Four actions, distinguished by `profile.is_optimized_build_mode(mode)` and — for optimized builds — `profile.rename_mode_for_build_mode(mode)`: - **`skip`** — untracked or already `build_mode = "pacman"`; nothing to do. - **`reinstall`** — plain `source_built` (or `bolt_llvm`, a BOLTed toolchain member, which also keeps its stock name), installed under the stock name: just reinstall the repo package (`pacman -S `). The name is the member itself, never its pkgbase (`pacman -S mesa` left a source-built `vulkan-radeon` in place); naming the pkgbase of a split set reinstalls every installed member, matching what `forget_packages` drops (`3.5.0-B62`). @@ -152,7 +153,13 @@ Read-only (`3.4.0-F6`, `BenchToolchainVerb` under the `bench` `VerbGroup`, Inspe ### `uninstall` -Mutating lifecycle verb (`requires_sentinel = True`). `pre_check` resolves each name through the shared `resolve_installed_name` (so naming a stock base reaches its `-sysforge` build) and builds a pure `plan_uninstall` classifying each target's installed name + whether it's tracked. `execute` narrates the plan, removes via `pacman.uninstall_pkgs` (`pacman -Rnsu`, interactive — pacman prints its own confirmation; a `CalledProcessError` returns exit 1 and demotes nothing), clears the removed targets' recorded build failures through `install_reconcile.clear_removed_failures` (tracked or not, before the forget whose deleted records map a split member to its pkgbase; `3.5.0-B120`), then for any tracked target demotes it out of the build-state authority via `cmd_state_forget` (the same `state forget` code) followed by an `install_reconcile.consume_external_installs(bs)` pass. This is the exact demotion composition `revert-to-stock` uses — no parallel path — so `update` stops rebuilding an uninstalled package. +Mutating lifecycle verb (`requires_sentinel = True`). `pre_check` resolves each name through the shared `resolve_installed_name` (so naming a stock base reaches its `-sysforge` build) and builds a pure `plan_uninstall` classifying each target's installed name + whether it's tracked. `execute` narrates the plan, removes via `pacman.uninstall_pkgs` (`pacman -Rnsu`, interactive — pacman prints its own confirmation; a `CalledProcessError` returns exit 1 and demotes nothing), clears the removed targets' recorded build failures through `install_reconcile.clear_removed_failures` (tracked or not, before the forget whose deleted records map a split member to its pkgbase; `3.5.0-B120`), then for any tracked target demotes it out of the build-state authority via `cmd_state_forget` (the same `state forget` code) followed by an `install_reconcile.consume_external_installs(bs)` pass. This is the exact demotion composition `revert-to-stock` uses — no parallel path — so `update` stops rebuilding an uninstalled package. Without a terminal it refuses before pacman runs unless `--yes` is given, which passes `--noconfirm`: pacman's own `[Y/n]` read EOF and aborted after printing the transaction (`3.5.0-B143`). + +### `self-uninstall` + +Removes SysForge in one command (`3.5.0-F19`), replacing the manual list the "What SysForge touches" guide gave. `pre_check` builds a `SelfUninstallPlan` by reading only: every `build_state` entry not built by pacman goes through `plan_revert`, a split set planned once; a plan whose stock names (`pacman.get_sync_pkgnames`, one `pacman -Slq`) a repo carries is reverted, any other (an AUR or local build, a custom kernel, an FDO kernel whose plain kernel is itself local) is kept installed with the reason. It also records whether `/etc/pacman.conf` has `sf-build` in `IgnoreGroup` (`setup_cmd._check_ignore_group`), whether the sandbox chroot lists the local repo (`local_repo.has_chroot_section`), the package that owns `sysforge/__init__.py` (none from a checkout), which of `paths.PACKAGE_DIRS` and the XDG user dirs exist, and whether the `sysforge` group exists. `PACKAGE_DIRS` is fixed (`/etc/sysforge`, `/var/lib/sysforge`, `/var/cache/sysforge`), never `SYSFORGE_CONFIG_DIR`/`SYSFORGE_STATE_DIR`, which can name a developer's own tree. + +`execute` prints the plan, stops on `--dry-run`, and otherwise needs `--yes` or a terminal's `y` and an authorised `sudo`. Order is what makes a failure safe: (1) the reverts through `revert_cmd.apply_reverts`, the one home `revert-to-stock` also calls, inside the verb's own `sentinel_scope` (recovery: reinstall the stock packages) — the verb sets `requires_sentinel = False` and `wants_run_log = False` because the runner's scope and log live in the state dir it is about to delete; a failed revert stops the run with sysforge still installed; (2) `setup_cmd.remove_ignore_group` (`_unpatch_conf_text`, the inverse of `_patch_conf_text`) and `local_repo.remove_chroot_section`; (3) `removal_script`, one `sudo sh -c` running `pacman -Rns --noconfirm `, then `rm -rf` of the package dirs and `groupdel sysforge`. Step 3 is last and single because once pacman has deleted the package this process must import nothing more from it; after it only `shutil.rmtree` of the user dirs and a `print` run. The package's pacman hooks are gone before pacman runs post-transaction hooks, so none writes into the state dir being deleted. `--keep-data` uses `pacman -Rs` (edited config kept as `.pacsave`) and skips the data, group and user dirs. `tools/e2e/scenarios/99-self-uninstall.sh`, the tier's last scenario, runs the whole thing on a live system. ### `artifact list` diff --git a/docs/design/17-release-plan.md b/docs/design/17-release-plan.md index d136d0ba..bffd4e0f 100644 --- a/docs/design/17-release-plan.md +++ b/docs/design/17-release-plan.md @@ -43,7 +43,7 @@ codes, `pacman -Qi`, the state files and leftover sentinels, with prompts answer pseudo-terminal. It covers what the unit suite stubs: root versus the user, group membership that applies only at the next login, the state one command leaves for the next. It builds sysforge from the checkout itself, so it needs no `vm-pkg-stable`. `pre-release` runs it; the release preflight -does not (`_RELEASE_GATE_EXEMPT`: minutes long, podman, and three networked scenarios, so not +does not (`_RELEASE_GATE_EXEMPT`: minutes long, podman, and four networked scenarios, so not deterministic). CI runs the same scenarios in an `e2e` job of its own, directly in the job's container (`make e2e E2E_HERE=1`), listed in the Makefile's `CI_JOBS` beside `GATES`. A scenario marked `xfail: ` fails until that item is fixed and reports `XFAIL`; once it passes it fails diff --git a/docs/guides/commands.md b/docs/guides/commands.md index d8165811..ceace11c 100644 --- a/docs/guides/commands.md +++ b/docs/guides/commands.md @@ -34,6 +34,7 @@ sysforge resolve PKG # show which profile applies to PKG, and why sysforge search cosmic # search installed, repo, and AUR for a term sysforge uninstall mesa # remove a package (demotes it out of build state) sysforge revert-to-stock mesa # undo a source-built package back to the repo version +sysforge self-uninstall # revert everything to stock, then remove SysForge itself ``` ## Manifest and build state diff --git a/docs/guides/what-sysforge-touches.md b/docs/guides/what-sysforge-touches.md index f78282bf..23f196cd 100644 --- a/docs/guides/what-sysforge-touches.md +++ b/docs/guides/what-sysforge-touches.md @@ -57,9 +57,23 @@ partitions disks and installs a whole system. For one package, `sysforge revert-to-stock ` reinstalls the repo build, and `sysforge state forget ` stops maintaining it but keeps your build installed. -To remove SysForge entirely, revert the packages you want back to stock, then -`sudo pacman -Rns sysforge`. Pacman removes the hooks and package files and keeps any config you -edited as `/etc/sysforge/*.pacsave`. Delete `/var/lib/sysforge`, `/var/cache/sysforge` and the -`sysforge` directories under `~/.cache`, `~/.local/share` and `~/.config` yourself, remove -`sf-build` from `IgnoreGroup` in `/etc/pacman.conf` if `setup` added it, and -`sudo groupdel sysforge`. Packages it built stay installed and keep working. +To remove SysForge entirely, run: + +```bash +sysforge self-uninstall --dry-run # see what it will do +sysforge self-uninstall +``` + +It puts back the repo build of every package SysForge built that a repo carries. A build no repo +carries, such as an AUR package or your own kernel, stays installed and keeps working, since +there is nothing to go back to. Then it takes `sf-build` out of `IgnoreGroup` in +`/etc/pacman.conf` and the build sandbox's local repo out of the chroot's `pacman.conf`, removes +the package, and deletes `/etc/sysforge`, `/var/lib/sysforge`, `/var/cache/sysforge` (with any +optimization profiles), the `sysforge` directories under `~/.cache`, `~/.local/share`, +`~/.config` and `~/.local/state`, and the `sysforge` group. If a revert fails, it stops there, +with SysForge still installed, so you can fix it and run the command again. + +`--keep-data` removes only the package: config you edited stays as `/etc/sysforge/*.pacsave`, +and the state, profiles, group and your own directories stay too. Scripts you adopted with +`sysforge artifact` stay where you deployed them either way; only SysForge's copies go. A +`sysforge run reconfigure` setting in `/etc/environment` is yours to remove. diff --git a/docs/release-notes/unreleased.md b/docs/release-notes/unreleased.md index 925f986d..30bd801e 100644 --- a/docs/release-notes/unreleased.md +++ b/docs/release-notes/unreleased.md @@ -106,6 +106,10 @@ https://keepachangelog.com/en/1.1.0/ - **`3.5.0-F18` — find and fix retired or misspelt settings in your config files.** An upgrade never rewrites your config files, so a setting sysforge stopped reading stayed there, silently ignored, and a misspelt key was never reported. `sysforge config check` lists both, with the line (`--json` prints them as one JSON document for scripts): settings sysforge renamed or retired (such as `[git] pull_timeout`, the per-package `pkgbuild_patch`, `repo_mode = "profiled"` and `build_mode = "patched_pkgbuild"`) and keys it does not know. `sysforge config clean` gives retired settings their current name or value, keeping your comments, and with `--unknown` removes the unknown ones. `sysforge config reset FILE [KEY ...]` puts keys back to the shipped default, or the whole file, keeping the old one as `FILE.bak-