diff --git a/docs/bugs/btrfs-subvolume-not-used-as-root.md b/docs/bugs/btrfs-subvolume-not-used-as-root.md deleted file mode 100644 index 3fc5938..0000000 --- a/docs/bugs/btrfs-subvolume-not-used-as-root.md +++ /dev/null @@ -1,125 +0,0 @@ -# Bug: btrfs install lands on the top-level subvolume; configured subvolumes (`@`) are unused as root - -**Status:** open — found by the automated VM e2e harness (`test/e2e/vm/`), 2026-06-23 -**Area:** `internal/archinstall/archinstall.go` (`btrfsBuilder` / `singleDiskRoot`) — the -reverse-engineered btrfs subvolume JSON shape (the VM-validation-pending item in `CLAUDE.md`) -**Severity:** medium — installs boot and are self-consistent, but the intended `@`-rooted, -snapshot-friendly btrfs layout is **not** what gets built, so snapper/rollback workflows that -assume a `@` root subvolume will not behave as expected. - -## Summary - -For the `btrfs` disk layout, archwright renders the root partition with **both** a -partition-level `mountpoint: "/"` **and** a subvolume `@` whose `mountpoint` is also `"/"`. -A real archinstall 4.3 run resolves that by mounting the **top-level** btrfs subvolume -(subvolid 5) at `/` and installing the whole system there. The configured `@` subvolume is -created but left **empty** and is never used as the root. The conventional Arch btrfs layout -(system installed *inside* `@`, mounted with `subvol=@`) is therefore not produced. - -archwright itself is internally consistent with this — its post-install chroot work -(`rootDevice()` → `PartDev(esp, 2)`, then `mount /mnt`) mounts the bare partition, -i.e. the default/top-level subvolume — so staging and boot agree and the machine boots fine. -The defect is purely that the **named subvolumes are not honored as the root**. - -## Evidence - -### Rendered JSON (the root partition for `disks.layout: btrfs`) - -`./archwright install --only archinstall --yes --dry-run --config ` emits, for -the root partition: - -```json -{ - "fs_type": "btrfs", - "mountpoint": "/", // <-- partition mounted at / (top-level subvol) - "mount_options": ["compress=zstd"], - "btrfs": [ - { "name": "@", "mountpoint": "/" } // <-- @ ALSO claims / ; created but unused - ] -} -``` - -The conflict is the partition having `mountpoint: "/"` while a subvolume also maps to `/`. - -### Observed on a real VM (diagnostic from the e2e harness) - -After a successful `archwright install` of a btrfs config, on the live ISO: - -``` -# btrfs subvolume get-default /mnt -> ID 5 (FS_TREE) # default = top-level, not @ -# mount -o subvol=@ /dev/vda2 /mnt ; ls /mnt/home -> (empty) # @ is empty -# mount -o subvolid=5 /dev/vda2 /mnt-top ; ls /mnt-top - bin boot dev etc home lib ... usr var @ # the whole system is in the TOP-LEVEL subvol, - # with @ present only as an empty subdir/subvol -# btrfs subvolume list /mnt-top - ID 256 gen 9 top level 5 path @ # @ exists, gen 9 (created, ~empty) -# find /mnt-top -maxdepth 5 -name archwright - /mnt-top/home/e2e/archwright # user home + staged files live in top-level -``` - -## Reproduction - -1. Build: `go build -o archwright .` -2. Quick (no VM) — inspect the render: - ```sh - ./archwright install --only archinstall --yes --dry-run \ - --config test/e2e/vm/configs/btrfs-basic.yaml 2>&1 \ - | sed -n '/^{/,/^}/p' | python3 -m json.tool | less - ``` - Confirm the root partition has `"mountpoint": "/"` **and** a `"btrfs"` entry with - `"mountpoint": "/"`. -3. Full (real archinstall) — either: - - `task vm-e2e -- btrfs-basic` and add a diagnostic, **or** - - `sudo bash test/e2e/disks.sh --mode full --layout ...` against a btrfs config, then - `btrfs subvolume get-default` / `btrfs subvolume list` the result. - Observe the default subvolume is `ID 5` (top-level) and `@` is empty. - -`test/e2e/vm/configs/btrfs-basic.yaml` currently uses a single `@` subvolume and the e2e -recipe mounts the bare partition precisely *because* of this bug (see -`test/e2e/vm/README.md` → "Finding: btrfs installs to the top-level subvolume"). A config -with a separate `@home` makes the breakage louder: the user home then lands in `@home`, -which the bare-partition mount doesn't expose. - -## Expected behavior - -The system should be installed **inside** the `@` subvolume and mounted with `subvol=@` at -`/` (the standard Arch/snapper layout), with `@home` at `/home`, etc. `btrfs subvolume -get-default` may remain `5`, but `/` must resolve to `@` (via fstab `subvol=@` and the -bootloader's `rootflags=subvol=@`), and the OS files must live in `@`, not the top-level. - -## Actual behavior - -The system is installed in the **top-level** subvolume (subvolid 5). `@` (and any `@home`, -`@log`) are created but empty and unused as mount roots. - -## Likely root cause & fix direction - -In archinstall's disk model, when a partition carries subvolumes that provide the -mountpoints, the **partition's own `mountpoint` should be `null`** — the subvolume entry -(`{"name": "@", "mountpoint": "/"}`) is what gets mounted at `/`. By emitting the root -partition with `mountpoint: "/"` *and* a subvolume mapping to `/`, archinstall mounts the -partition (top-level subvol) at `/` and the subvolume mapping is effectively ignored for the -root. - -Investigate in `internal/archinstall/archinstall.go`: - -- `btrfsBuilder.build()` → `singleDiskRoot(..., rootSpec{fsType:"btrfs", btrfs: subvols})`. - `singleDiskRoot` sets the root partition `Mountpoint: &root` (`"/"`) unconditionally - (around the `rootFs`/`Mountpoint: &root` assignment). For btrfs-with-subvolumes the - partition `Mountpoint` should be `null` and the per-subvolume mountpoints should drive the - mounts. -- Cross-check against archinstall 4.3 source for how a subvolumed btrfs partition is meant to - be expressed (partition `mountpoint` null vs the `@`/`mountpoint:"/"` subvolume), and how it - writes fstab + `rootflags=subvol=@` for the bootloader. -- This is a schema-shape change, so follow the CLAUDE.md two-commit rule (behavior-preserving - refactor with goldens unchanged, then the shape change regenerating goldens) and the - archinstall-drift gotcha (validate against a real archinstall run, not just the render). - -## Validation after a fix - -- `internal/archinstall` golden snapshots regenerate to show the btrfs root partition - `mountpoint: null` with the `@`/`mountpoint:"/"` subvolume carrying the root. -- `task vm-e2e -- btrfs-basic` with a config that uses `@` + a separate `@home`, and a - `root_mount` recipe of `mount -o subvol=@ /dev/vda2 /mnt` + `mount -o subvol=@home - /dev/vda2 /mnt/home`, boots and passes Phase B (the staged binary is found under `@home`). -- On the booted system, `findmnt /` shows `subvol=/@` and `findmnt /home` shows `subvol=/@home`. diff --git a/docs/bugs/flatpak-system-remote-add-polkit-hang.md b/docs/bugs/flatpak-system-remote-add-polkit-hang.md deleted file mode 100644 index 2f714a2..0000000 --- a/docs/bugs/flatpak-system-remote-add-polkit-hang.md +++ /dev/null @@ -1,84 +0,0 @@ -# Bug: the flatpak stage hangs on a polkit password prompt (system-wide `flatpak remote-add`/install as a non-root user) - -**Status:** open — found by the automated VM e2e harness (`test/e2e/vm/`, descriptor `features-flatpak`), 2026-06-23 -**Area:** `internal/stages/flatpak.go` -**Severity:** high — Phase B `bootstrap` **hangs indefinitely** at the flatpak stage in any -non-graphical session (first-boot TTY, SSH, the e2e harness), because a system-wide flatpak -operation run as a normal user requires polkit authentication and there is no agent to answer it. - -## Summary - -The flatpak stage runs `flatpak remote-add` (and would then run `flatpak install`) as the -**unprivileged user** against the **system** flatpak installation (the default scope). That -needs the polkit action `org.freedesktop.Flatpak.modify-repo`, which prompts for a password. -With no graphical polkit agent (and stdin not a usable tty), the command blocks forever on -`Password:` and `bootstrap` never completes. - -## Evidence (from the `features-flatpak` VM run) - -Phase B serial, at the flatpak stage: - -``` -━━ [4/11] 30 · flatpak ━━━... -→ flatpak remote-add --if-not-exists flathub https://flathub.org/repo/flathub.flatpakrepo -Note that the directories '/var/lib/flatpak/exports/share' ... are not in the search path ... -==== AUTHENTICATING FOR org.freedesktop.Flatpak.modify-repo ==== -Authentication is required to modify a system repository -Authenticating as: e2e -Password: -``` - -…then nothing — the run hit the harness Phase-B timeout (`timed out waiting for -E2E_RESULT`). The `→` prefix shows the command ran **unprivileged** (the runner's `Cmd`, not -`Root`), so it dropped into polkit auth. - -## Reproduction - -1. A config with a flatpak remote + app, e.g. `test/e2e/vm/configs/features-flatpak.yaml` - (flathub + `com.github.tchx84.Flatseal`). -2. Run Phase B `archwright bootstrap` in a **non-graphical** session (TTY/SSH/headless) as the - user — i.e. the normal first-boot situation before a desktop/polkit agent is running. -3. The flatpak stage blocks on the polkit `Password:` prompt for - `org.freedesktop.Flatpak.modify-repo`. - -(`task vm-e2e -- features-flatpak` reproduces it; the stage hangs until the 2700s Phase-B -timeout.) - -## Expected behavior - -The flatpak stage completes unattended: remotes are added and apps installed without an -interactive polkit prompt, in a plain TTY/headless session. - -## Actual behavior - -`flatpak remote-add` (system scope, as the user) blocks on a polkit `Password:` prompt; -`bootstrap` hangs. - -## Fix direction - -Pick one of: - -- **Per-user scope:** run `flatpak --user remote-add …` and `flatpak --user install …`. The - `--user` installation needs no polkit/root. This is usually the right default for a - single-user desktop and matches running Phase B as the user. (Note: app launchers/exports - differ slightly for `--user`.) -- **Privileged scope:** run the system-wide operations via the runner's `Root` (sudo), e.g. - `sudo flatpak remote-add …` / `sudo flatpak install -y …`. Root skips the polkit prompt. -- Either way, pass non-interactive flags so a later `flatpak install` can't prompt: - `--noninteractive` (or `-y/--assumeyes`). - -Decide the intended scope (`--user` vs system) deliberately — it changes where apps land and -how they're exported. Whichever is chosen, the stage must be non-interactive. - -## Validation after a fix - -`task vm-e2e -- features-flatpak` reaches `E2E_RESULT=PASS` with the configured app present -(`flatpak list` shows `com.github.tchx84.Flatseal`). `lib/features.sh`'s `flatpak` token -checks exactly that. - -## Related - -The harness's 2700s Phase-B timeout means a polkit hang wastes the full window. Independent of -this bug, archwright stages that shell out should never be able to block on an interactive -prompt during `bootstrap` — worth auditing other stages (e.g. anything piping to a tool that -might prompt) for the same hazard. diff --git a/docs/bugs/lvm-multivolume-pv-fstype-empty.md b/docs/bugs/lvm-multivolume-pv-fstype-empty.md deleted file mode 100644 index ab1c5ae..0000000 --- a/docs/bugs/lvm-multivolume-pv-fstype-empty.md +++ /dev/null @@ -1,107 +0,0 @@ -# Bug: multi-volume LVM renders the PV partition with an empty `fs_type`; archinstall aborts - -**Status:** open — found by the automated VM e2e harness (`test/e2e/vm/`, descriptor `lvm-volumes`), 2026-06-23 -**Area:** `internal/archinstall/archinstall.go` — `lvmBuilder.build()` (the PV partition `fs_type`) -**Severity:** high — the **multi-volume LVM layout does not install at all**; Phase A archinstall -crashes before partitioning completes. - -## Summary - -For the `lvm` layout in **multi-volume mode** (`disks.lvm.volumes:` set instead of -`lv`+`filesystem`), archwright renders the LVM **PV partition** with `fs_type: ""` (empty -string). A real archinstall 4.3 run rejects that with -`ValueError: File system type is not set` while creating partitions, so the install aborts. - -Single-LV mode (`disks.lvm.lv` + `disks.lvm.filesystem`) works because the PV partition's -`fs_type` is set to the LV filesystem (e.g. `xfs`). - -## Root cause - -`internal/archinstall/archinstall.go`, in `lvmBuilder.build()`: - -```go -// A PV partition carries the LV filesystem as its fs_type purely so parted can -// create it (archinstall 4.x requires a non-null fs_type per partition); the -// filesystem is never written, the partition is pvcreated. -pvFs := b.lvm.Filesystem // <-- empty in multi-volume mode -... -disk1PVPart := Partition{..., FsType: &pvFs, ...} -``` - -`b.lvm.Filesystem` is **only set in single-LV mode**. In multi-volume mode the schema -*requires it to be empty* (`config.go` `lvmVolumeErrors`: "set either lv+filesystem OR -volumes, not both"), and each volume carries its own `filesystem`. So `pvFs == ""`, and every -PV partition (disk-1 PV and any whole-disk PVs, which reuse the same `pvFs`) is emitted with -an empty `fs_type`. - -## Evidence - -### Rendered partitions (`--dry-run`) - -`./archwright install --only archinstall --yes --dry-run --config `: - -| config (mode) | PV partition `fs_type` | -|---------------------------------|------------------------| -| `lvm-single` (single-LV, xfs) | `"xfs"` → installs OK | -| `lvm-volumes` (multi-volume) | `""` → **aborts** | - -The volumes themselves are fine (`root`→xfs, `home`→ext4); only the PV partition is wrong. - -### archinstall traceback (from the VM run) - -``` -Creating partitions: /dev/vda - File ".../archinstall/lib/disk/device_handler.py", line 373, in _setup_partition - fs_value = part_mod.safe_fs_type.parted_value - File ".../archinstall/lib/models/device.py", line 897, in safe_fs_type - raise ValueError('File system type is not set') -ValueError: File system type is not set -``` - -## Reproduction - -1. `go build -o archwright .` -2. Render-only: `./archwright install --only archinstall --yes --dry-run --config - test/e2e/vm/configs/lvm-volumes.yaml 2>&1 | sed -n '/^{/,/^}/p' | python3 -m json.tool` - → the second partition on `/dev/vda` has `"fs_type": ""`. -3. Full: `task vm-e2e -- lvm-volumes` (or `test/e2e/disks.sh` with a multi-volume config) - → archinstall aborts with the traceback above. - -## Expected behavior - -Multi-volume LVM installs successfully, with `root`/`home`/… LVs formatted per their -configured filesystems. - -## Actual behavior - -archinstall aborts in Phase A with `ValueError: File system type is not set`; nothing is -installed. - -## Fix direction - -Give the PV partition a valid non-empty `fs_type` even in multi-volume mode. The comment -already notes the value is cosmetic ("the filesystem is never written, the partition is -pvcreated"), so any valid fs works. Options: - -- Fall back to a volume's filesystem when the top-level one is empty, e.g. - ```go - pvFs := b.lvm.Filesystem - if pvFs == "" && len(b.lvm.Volumes) > 0 { - pvFs = b.lvm.Volumes[0].Filesystem - } - ``` -- Or use a fixed placeholder (e.g. `"ext4"`) for PV partitions regardless of mode (and - consider doing the same in single-LV mode, since `xfs` on a PV partition is equally - cosmetic). - -Add a render golden + a `config_test.go`/`golden_test.go` case for the multi-volume layout so -this is covered, and follow the CLAUDE.md archinstall-drift rule (validate against a real -archinstall run — `task vm-e2e -- lvm-volumes` should reach Phase B and pass). - -## Note for the e2e descriptor - -Once fixed, `lvm-volumes` Phase B will need its `root_mount` to also mount the `home` LV -(the user home lives on a separate LV, so the staged binary/config under `/home/` are -only reachable after `mount /dev//home /mnt/home`) — the same separate-`/home` mount -concern noted for the btrfs `@home` case. Update `test/e2e/vm/matrix/lvm_variants.py` -accordingly when validating the fix. diff --git a/docs/bugs/plymouth-bootctl-update-fails-systemd-boot.md b/docs/bugs/plymouth-bootctl-update-fails-systemd-boot.md deleted file mode 100644 index 13e8d1a..0000000 --- a/docs/bugs/plymouth-bootctl-update-fails-systemd-boot.md +++ /dev/null @@ -1,107 +0,0 @@ -# Bug: Phase B `regenerateBootConfig` runs `bootctl update` on systemd-boot and fails, aborting bootstrap (via the always-on plymouth stage) - -**Status:** resolved 2026-06-30 — Plymouth moved to archinstall (Phase A) on the 4.4 bump. -The Phase B `plymouth` stage, `regenerateBootConfig`/`ensureKernelParam` helpers, and the -`bootctl update --graceful` workaround described below were all removed; archinstall now installs -and configures Plymouth (hook, `quiet splash`, theme) during the install, so no Phase B step ever -runs `bootctl update`. The failure path no longer exists. (Originally found by the automated VM e2e -harness `test/e2e/vm/`, descriptor `sdboot-lvm`, 2026-06-23.) -**Area:** `internal/stages/helpers.go` (`regenerateBootConfig`); surfaced via `internal/stages/plymouth.go` -**Severity:** high — Phase B `bootstrap` **fails on any systemd-boot system** at the plymouth -stage (and any other stage that regenerates boot config), even with a default config. - -## Summary - -On a systemd-boot install, the Phase B `plymouth` stage aborts with -`ERRO stage plymouth: sudo: exit status 1`. The failing command is **`sudo bootctl update`**, -run by `regenerateBootConfig` for the systemd-boot bootloader. Because the plymouth stage -runs **unconditionally** (it defaults the theme to `bgrt` when none is configured), this -breaks `bootstrap` for systemd-boot configs out of the box. The same `regenerateBootConfig` -is also called by the grub-theme stage and the kernel path, so they would hit it too. - -The install itself is fine: the e2e validation that runs afterward passes (root/ESP/LVM, -`bootctl is-installed` → "systemd-boot installed", packages, yay). Only the boot-config -regeneration command fails. - -## Root cause - -`internal/stages/helpers.go`: - -```go -func regenerateBootConfig(ctx *Context) error { - if ctx.Cfg.Bootloader.EffectiveKind() == "systemd-boot" { - return ctx.R.Root("bootctl", "update") // <-- returns exit status 1 here - } - return ctx.R.Root("grub-mkconfig", "-o", "/boot/grub/grub.cfg") -} -``` - -`bootctl update` re-installs the systemd-boot binary into the ESP **only if** the bundled -version is newer than the installed one; when archinstall already installed the current -version it has nothing to do and exits non-zero (rather than a no-op success). archwright -treats that non-zero exit as a stage failure and aborts `bootstrap`. - -`internal/stages/plymouth.go` makes this reachable on every run: - -```go -func (plymouth) Run(ctx *Context) error { - theme := ctx.Cfg.Plymouth.Theme - if theme == "" { - theme = "bgrt" // <-- stage is NOT gated off when unconfigured - } - ... - return regenerateBootConfig(ctx) -} -``` - -So even a config with no `plymouth:` block installs plymouth, edits the cmdline, and runs -`bootctl update`. - -## Evidence (from the `sdboot-lvm` VM run) - -``` -→ [6/11] 50 ⟫ plymouth - → sudo pacman -S --needed --noconfirm plymouth (ok) - → sed ... /etc/mkinitcpio.conf (add plymouth hook) (ok) - → ... /etc/kernel/cmdline (add quiet / splash) (ok) - → sudo plymouth-set-default-theme -R bgrt (ok, rebuilds initramfs) - → sudo bootctl update (FAILS) -ERRO stage plymouth: sudo: exit status 1 -E2E_BOOTSTRAP_RC=1 -``` - -(The post-bootstrap validation still ran and reported `0 failures`, including -`OK: systemd-boot installed` — so the system is healthy; only the stage command failed.) - -## Reproduction - -1. Install any systemd-boot config (e.g. `task vm-e2e -- sdboot-lvm`, or set - `bootloader.kind: systemd-boot` on any layout). -2. Run Phase B `archwright bootstrap`. -3. It aborts at the plymouth stage; the failing command is `sudo bootctl update`. - -## Expected behavior - -`bootstrap` completes on systemd-boot. Regenerating boot config should be a no-op-tolerant -success when there is nothing to update. - -## Actual behavior - -`bootstrap` aborts at the plymouth (or any boot-config-regenerating) stage because -`bootctl update` exits non-zero when the loader is already current. - -## Fix direction - -- In `regenerateBootConfig`, make the systemd-boot path tolerant of the "already current" - case — e.g. `bootctl update --graceful`, or treat the no-update exit code as success - (best-effort via `ctx.R.Try`), or skip `bootctl update` entirely (the loader was just - installed by archinstall; cmdline changes go to `/etc/kernel/cmdline` which systemd-boot - reads directly, so a forced binary update isn't needed for archwright's edits). Confirm the - exact `bootctl update` exit semantics against the installed systemd version. -- Consider whether the **plymouth stage should be gated** when no `plymouth:` config is - present, instead of always defaulting to `bgrt` — running it unconditionally is what makes - every systemd-boot bootstrap hit this path. (Decide intended behavior; if "plymouth on by - default" is desired, keep it but make the boot-config step robust.) -- This sits squarely in the reverse-engineered / VM-validation-pending systemd-boot path - (`CLAUDE.md`): validate the fix with `task vm-e2e -- sdboot-lvm` (and `sdboot-plain`) - reaching `E2E_RESULT=PASS` with `E2E_BOOTSTRAP_RC=0`. diff --git a/docs/vm-validation.md b/docs/vm-validation.md deleted file mode 100644 index ad70615..0000000 --- a/docs/vm-validation.md +++ /dev/null @@ -1,81 +0,0 @@ -# VM validation of the rendered archinstall config - -This was the last open item before trusting archwright on real hardware. The automated harness -below (`test/e2e/vm/`) now closes it: every reverse-engineered shape has been driven through a -real archinstall 4.3 + boot + bootstrap run. Most are confirmed working; three surfaced real -bugs (now tracked in [`docs/bugs/`](bugs/)). See the results table below. - -archinstall's config JSON is **not a stable API** — its schema changes between releases. We -render against the pinned `Version` in `internal/archinstall/archinstall.go` (currently -`4.3`), and the JSON shape (LVM, swap, encryption, bootloader, PV `obj_id` wiring, creds -keys) was **reverse-engineered from archinstall source**. The render tests -(`go test ./internal/archinstall/`, the golden snapshots, and the `e2e` / `e2e-disks` -workflows) prove our output is *stable and self-consistent*, and `e2e-disks` confirms a real -archinstall *parses* it against loopback devices — but none of that proves a real archinstall -*does the right thing with it end-to-end on a booted system*. - -That last mile needs a QEMU run that boots a real systemd live ISO, feeds it the rendered -config, and verifies the machine partitions, installs, and **boots and runs Phase B -end-to-end**. - -## Automated harness — `test/e2e/vm/` - -`test/e2e/vm/e2e.py` (run it with `task vm-e2e -- `, or `task vm-e2e` for the whole -matrix; `task vm-e2e-list` lists them) does this **fully unattended**: it boots the ISO -headless on a serial console, runs `archwright install --yes`, injects a harness-only -Phase-B autorun (a serial autologin + bootstrap+validate trigger that lives **only** in the -test scaffold, never in a real config), reboots from disk, runs `archwright bootstrap`, and -asserts the installed system with a parametrized `lib/validate.sh`. See -`test/e2e/vm/README.md` for the descriptor contract and how to add coverage. The matrix -(`test/e2e/vm/matrix/*.py` + `configs/*.yaml`) covers lvm (single / multi / multi-volume), -btrfs (+ compress / snapper), plain (every swap type), systemd-boot, and the LUKS layouts -(which validate the on-disk encryption on the ISO, since encrypted Phase B staging is not -yet implemented). - -Status (full matrix run, archinstall 4.3): **14 descriptors green**, **3 distinct real bugs -found**. Green end-to-end (install → reboot → bootstrap → validate): all lvm single-LV layouts, -all plain layouts × every swap type, btrfs (basic + snapper), the feature/stage-coverage runs, -and both encryption layouts (validated on the ISO — see the encryption note below). The bugs -are in `docs/bugs/` and the results table maps each to its shape. - -Two things the harness does **not** prove, by design: -- **Graphical desktop rendering.** It validates boot → multi-user → `bootstrap` → assertions, - not that a KDE session visually renders (the trimmed configs mostly use - `desktop.environment: none`; `features-desktop` only checks the plasma tooling installed + - the stage ran). Use `task vm-disk` to watch a real desktop come up by hand. -- **A full encrypted boot.** archwright's Phase B staging is skipped for encrypted installs - (the LUKS remount isn't implemented), so the encryption descriptors assert the on-disk LUKS - shape on the live ISO (container present + passphrase unlocks) rather than booting the - encrypted system and running `bootstrap`. - -The older `test/vm.sh` (`task vm` / `vm-fresh` / `vm-disk`) remains for **interactive** -poking at a VM by hand (including the desktop-render check above). - -> Use `-cpu host` for local VM runs — otherwise the CachyOS repo setup skips and -> `linux-cachyos` fails with "target not found". (The e2e matrix configs use the stock -> `linux` kernel and no CachyOS repo, so they are unaffected; this matters for configs that -> add `linux-cachyos`.) - -## Results — reverse-engineered shapes vs a real archinstall 4.3 run - -Each shape was reverse-engineered; the harness has now exercised them all. - -| Area | Shape | Result | -|------|-------|--------| -| Bootloader | `bootloader_config: {bootloader, uki, removable}` field names/casing | ✅ confirmed — grub (`grub.cfg`, boots) and systemd-boot both install + boot | -| Swap | `partition` (`fs_type: linux-swap`, flag `swap`), zram, swapfile | ✅ confirmed — swapfile (lvm/plain), zram (btrfs, plain-zram), partition (plain-swappart) all active post-boot | -| Encryption | nested `disk_config.disk_encryption` (`encryption_type` + `partitions`); `encryption_password` casing | ✅ confirmed — `enc-lvm` (lvm_on_luks) + `enc-luks-plain` (luks): LUKS container present and the passphrase unlocks (`luksOpen --test-passphrase`). `lvm_on_luks` >2-PV limit not separately exercised; full encrypted Phase B still unimplemented in archwright | -| Snapper | timer-unit + `set-config` key names | ✅ confirmed — `btrfs-snapper` installs snapper + green | -| Btrfs | subvolume JSON `{name, mountpoint}` | ❌ **bug** — shape parses, but archinstall installs to the **top-level** subvolume; the configured `@` is created but unused as root → [`btrfs-subvolume-not-used-as-root.md`](bugs/btrfs-subvolume-not-used-as-root.md) | -| LVM | multi-volume "rest of VG" sizing (fixed root + remainder `/home`) | ❌ **bug** — the PV partition renders with an empty `fs_type`; archinstall aborts Phase A before sizing is reached → [`lvm-multivolume-pv-fstype-empty.md`](bugs/lvm-multivolume-pv-fstype-empty.md) | -| systemd-boot | loader-entry default | ⚠️ install + boot **work**. The Phase-B `bootctl update` refresh path that previously failed is **gone**: the always-on plymouth stage that triggered it was removed when Plymouth moved to archinstall (Phase A) — see [`plymouth-bootctl-update-fails-systemd-boot.md`](bugs/plymouth-bootctl-update-fails-systemd-boot.md) (resolved). | -| Plymouth | `bootloader_config.plymouth` (archinstall 4.4 built-in theme) | ⚠️ **not yet VM-validated** — archinstall installs/configures the splash in Phase A; confirm the `spinner` theme is set and quiet/splash reach the kernel cmdline on a real run. | - -(A fourth bug unrelated to a disk shape — the flatpak stage hangs on a polkit prompt — is in -[`flatpak-system-remote-add-polkit-hang.md`](bugs/flatpak-system-remote-add-polkit-hang.md).) - -## After an archinstall version bump - -Diff the upstream schema and update `internal/archinstall/` **and** the `Version` constant -together, then re-run the VM validation above. Preflight only *warns* on a version mismatch; -it does not block. diff --git a/internal/stages/archinstall.go b/internal/stages/archinstall.go index 695a907..e462ba5 100644 --- a/internal/stages/archinstall.go +++ b/internal/stages/archinstall.go @@ -17,8 +17,9 @@ import ( // archinstallStage is Phase A: render config.yaml into an archinstall config + // credentials file and let archinstall do the partitioning, LVM, pacstrap and // bootloader install. Replaces the old hand-rolled partition/lvm/mount/pacstrap/ -// system/initramfs/grub stages. After archinstall finishes it stages the binary -// + config into the new user's home so Phase B is available post-reboot. +// system/initramfs/grub stages. The post-install customization (swapfile, extra +// locales, user shell, multilib/repos, kernels) is done by the numbered stages +// that follow `mount` (swap@21 … kernels@26), and `finalize` stages the binary. type archinstallStage struct{} func init() { register(archinstallStage{}) } @@ -90,83 +91,7 @@ func (archinstallStage) Run(ctx *Context) error { return err } - if err := postInstall(ctx); err != nil { - return err - } - - ui.OK("archinstall complete; target mounted for Phase A customization") - return nil -} - -// postInstall runs after archinstall, inside the target: configure custom repos -// and install custom kernels (so first boot already has them), then stage the -// binary + config for Phase B. archinstall unmounts the target on finish, so we -// remount the root LV and the ESP (kernels/GRUB live on /boot) for the chroot -// work, then unmount. -func postInstall(ctx *Context) error { - // Encrypted layouts (Issue #2): rootDevice() returns the *plaintext* device - // (the bare partition or /dev/VG/LV), but after archinstall provisions LUKS the - // real root filesystem lives behind a /dev/mapper/* device that requires a LUKS - // remount (cryptsetup open with the passphrase) we have NOT implemented yet — - // encryption is VM-validation-pending. Mounting the still-encrypted device and - // running chroot work against it would either fail or, worse, silently no-op. - // - // Conservative behaviour until the LUKS remount is implemented and VM-validated: - // skip the remount and every chroot-dependent post-install step (swapfile, - // locales, repos, kernels). We also cannot stage the Phase B binary/config, - // because staging copies into /mnt/home/ which requires the (correctly - // mapped+mounted) root — so we warn that Phase B staging must be done manually. - // This keeps the encrypted path coherent and loud rather than silently wrong. - if ctx.Cfg.Disks.Encryption != nil { - ui.Warn("encrypted install: custom repos, kernels, locale extras, and swapfile are NOT applied yet (LUKS remount not implemented)") - return nil - } - - // Remount the target for the chroot work. archinstall unmounts on finish, so we - // rebuild the mount tree exactly as the installed system sees it, and leave it - // mounted: the `mount` stage points the Runner at this chroot for the Phase A - // customization stages, and `finalize` stages the binary + unmounts. - if err := ensureTargetMounted(ctx); err != nil { - return err - } - - if err := setupSwapfile(ctx); err != nil { - return err - } - - if len(ctx.Cfg.System.Locales) > 0 { - if err := configureLocales(ctx, ctx.Cfg.System.Locales); err != nil { - return err - } - } - - if ctx.Cfg.User.Shell != "" { - if err := configureUserShell(ctx, ctx.Cfg.User.Name, ctx.Cfg.User.Shell); err != nil { - return err - } - } - - if ctx.Cfg.Pacman.Multilib { - if err := enableMultilib(ctx); err != nil { - return err - } - } - - if len(ctx.Cfg.Repos) > 0 { - if err := configureRepos(ctx, ctx.Cfg.Repos); err != nil { - return err - } - } - // Run installKernels when there are extra kernel packages to install OR a - // default kernel to pin (Issue #3): kernel.default may name a base kernel with - // no extra packages, and that default must still be written to the bootloader. - if len(ctx.Cfg.Kernel.Packages) > 0 || ctx.Cfg.Kernel.Default != "" { - if err := installKernels(ctx, ctx.Cfg.Kernel); err != nil { - return err - } - } - // The target is left mounted on purpose: the `mount` stage opens the chroot for - // the Phase A customization stages and `finalize` stages the binary + unmounts. + ui.OK("archinstall complete") return nil } @@ -335,8 +260,9 @@ func sortByDepth[T any](s []T, mp func(T) string) { // (its LVM path formats only the boot partition), so swap lives as a file sized // from cfg.Disks.Swap.Size. Written with dd (real zeros) rather than fallocate // so it works on xfs too, where a preallocated file has unwritten extents that -// swapon rejects. The target root is mounted at /mnt, so the in-system path is -// /swapfile. No-op if no swap size is configured. +// swapon rejects. It runs in the `swap` stage after `mount` opens the chroot, so +// the commands use in-chroot paths (/swapfile, /etc/fstab) and the Runner wraps +// them in `arch-chroot /mnt`. No-op if no swap size is configured. func setupSwapfile(ctx *Context) error { // Only the swapfile swap type creates /swapfile here; zram is handled by // archinstall, a swap partition is created by the layout builder, and none @@ -356,17 +282,17 @@ func setupSwapfile(ctx *Context) error { if mib == 0 { return fmt.Errorf("swap size %q is smaller than 1 MiB", size) } - if err := ctx.R.Root("dd", "if=/dev/zero", "of=/mnt/swapfile", + if err := ctx.R.Root("dd", "if=/dev/zero", "of=/swapfile", "bs=1M", fmt.Sprintf("count=%d", mib), "status=none"); err != nil { return err } - if err := ctx.R.Root("chmod", "600", "/mnt/swapfile"); err != nil { + if err := ctx.R.Root("chmod", "600", "/swapfile"); err != nil { return err } - if err := ctx.R.Root("mkswap", "/mnt/swapfile"); err != nil { + if err := ctx.R.Root("mkswap", "/swapfile"); err != nil { return err } - return ctx.R.Shell("echo '/swapfile none swap defaults 0 0' >> /mnt/etc/fstab") + return ctx.R.RootShell("echo '/swapfile none swap defaults 0 0' >> /etc/fstab") } // runReflector refreshes the live ISO's mirrorlist with reflector per the mirrors @@ -399,10 +325,10 @@ func runReflector(ctx *Context) error { } // rootDevice returns the path the freshly-installed root filesystem lives on, so -// postInstall can remount it for chroot work after archinstall unmounts the -// target. It depends on the layout: the LVM root LV, or the root partition on -// disk 1 for plain/btrfs (partition 1 is the ESP; with a swap partition the root -// is partition 3, otherwise partition 2). +// the `mount` stage can remount it for the chroot customization work after +// archinstall unmounts the target. It depends on the layout: the LVM root LV, or +// the root partition on disk 1 for plain/btrfs (partition 1 is the ESP; with a +// swap partition the root is partition 3, otherwise partition 2). func rootDevice(cfg *config.Config) (string, error) { switch cfg.Disks.EffectiveLayout() { case "lvm": @@ -483,7 +409,7 @@ func probeGeometry(devs []string, dryRun bool) (archinstall.Geometry, error) { // stageBinary copies the running binary + config into the freshly installed // user's home so Phase B is available after reboot. Assumes the target is already -// mounted at /mnt (postInstall handles mount/unmount). +// mounted at /mnt (the `finalize` stage handles mount/unmount). func stageBinary(ctx *Context) error { user := ctx.Cfg.User.Name home := "/mnt/home/" + user diff --git a/internal/stages/flatpak.go b/internal/stages/flatpak.go index e1ede48..2c3a321 100644 --- a/internal/stages/flatpak.go +++ b/internal/stages/flatpak.go @@ -7,21 +7,23 @@ import ( ) // flatpak ensures flatpak, registers the declared remotes, then installs each -// configured app from its named remote. It runs in the Phase A chroot: the remote -// + per-user installs run as the configured user (--user, so no polkit/root), and -// ensureTool's pacman install runs as root — both via the Runner's chroot routing. +// configured app from its named remote. It runs in Phase B on the booted system: +// the remote + per-user installs run as the invoking user (--user, so no +// polkit/root) and ensureTool's pacman install runs via sudo. It stays in Phase B +// — not the Phase A chroot — because extra-data flatpaks (Spotify, Zoom, Chrome, +// …) run an `apply_extra` bwrap sandbox at install time that needs an unprivileged +// user namespace, which the arch-chroot on the live ISO can't create. The booted +// system provides a working userns/bwrap, so this joins yay/aur/snapper as the +// "needs a real session" bucket. type flatpak struct{} func init() { register(flatpak{}) } -func (flatpak) Order() int { return 40 } +func (flatpak) Order() int { return 50 } func (flatpak) Name() string { return "flatpak" } -func (flatpak) Phase() Phase { return Install } +func (flatpak) Phase() Phase { return Bootstrap } func (flatpak) Run(ctx *Context) error { - if !requireChroot(ctx, "flatpak") { - return nil - } apps := ctx.Cfg.Flatpaks if len(apps) == 0 { ui.Warn("no flatpaks in config — skipping") diff --git a/internal/stages/flatpak_test.go b/internal/stages/flatpak_test.go index f0b570a..9011e69 100644 --- a/internal/stages/flatpak_test.go +++ b/internal/stages/flatpak_test.go @@ -7,11 +7,11 @@ import ( // The flatpak stage registers exactly the declared remotes (nothing implicit) and // installs each app from its named remote, parsed from the "remote:appid" -// reference. It runs in the Phase A chroot as the user (--user). These assert the -// recorded dry-run plan. +// reference. It runs in Phase B on the booted system as the invoking user +// (--user). These assert the recorded dry-run plan. func TestFlatpak_RemotesAndPerAppInstall(t *testing.T) { - plan := planForCfgChroot(t, "flatpak", ` + plan := planForCfg(t, Bootstrap, "flatpak", ` flatpak_remotes: - name: flathub url: https://flathub.org/repo/flathub.flatpakrepo @@ -24,11 +24,11 @@ flatpaks: mustContain(t, plan, // exactly the declared remotes, added verbatim — per-user scope (--user) // so no polkit/root is needed (org.freedesktop.Flatpak.modify-repo hang). - "arch-chroot /mnt sudo -iu adam -- flatpak --user remote-add --if-not-exists flathub https://flathub.org/repo/flathub.flatpakrepo", - "arch-chroot /mnt sudo -iu adam -- flatpak --user remote-add --if-not-exists flathub-beta https://flathub.org/beta-repo/flathub-beta.flatpakrepo", + "flatpak --user remote-add --if-not-exists flathub https://flathub.org/repo/flathub.flatpakrepo", + "flatpak --user remote-add --if-not-exists flathub-beta https://flathub.org/beta-repo/flathub-beta.flatpakrepo", // each app installed from its own named remote (per-app), per-user + noninteractive - "arch-chroot /mnt sudo -iu adam -- flatpak --user install -y --noninteractive flathub com.spotify.Client", - "arch-chroot /mnt sudo -iu adam -- flatpak --user install -y --noninteractive flathub-beta org.mozilla.firefox", + "flatpak --user install -y --noninteractive flathub com.spotify.Client", + "flatpak --user install -y --noninteractive flathub-beta org.mozilla.firefox", ) joined := strings.Join(plan, "\n") @@ -40,13 +40,14 @@ flatpaks: t.Errorf("firefox should install from flathub-beta, not flathub; plan:\n%s", joined) } - // The flatpak binary must run as the user (sudo -iu), never as root inside the - // chroot: a system-scope flatpak op drops into a polkit Password: prompt and - // hangs forever in a headless session. The legitimately-privileged pacman - // install from ensureTool (`arch-chroot /mnt pacman -S … flatpak`) is fine. + // The flatpak binary must run as the user (--user), never as root via sudo: a + // system-scope flatpak op drops into a polkit Password: prompt and hangs forever + // in a headless session. The legitimately-privileged pacman install from + // ensureTool (`sudo pacman -S … flatpak`) is fine — note the trailing space so + // it doesn't match that line. for _, line := range plan { - if strings.Contains(line, "arch-chroot /mnt flatpak ") { - t.Errorf("flatpak must run as the user (sudo -iu), not as root, to avoid a polkit hang; got: %q", line) + if strings.Contains(line, "sudo flatpak ") { + t.Errorf("flatpak must run as the user (--user), not as root via sudo, to avoid a polkit hang; got: %q", line) } } } @@ -54,7 +55,7 @@ flatpaks: func TestFlatpak_NoRemotesNoImplicitFlathub(t *testing.T) { // With no remotes and no apps the stage skips entirely — and in particular // never adds a built-in flathub remote. - plan := planForCfgChroot(t, "flatpak", "{}\n") + plan := planForCfg(t, Bootstrap, "flatpak", "{}\n") if joined := strings.Join(plan, "\n"); strings.Contains(joined, "remote-add") { t.Errorf("no flatpak remote should be added when none are declared; plan:\n%s", joined) } diff --git a/internal/stages/fromto_test.go b/internal/stages/fromto_test.go index b02d2fc..e8c14c9 100644 --- a/internal/stages/fromto_test.go +++ b/internal/stages/fromto_test.go @@ -28,32 +28,32 @@ func TestWithin(t *testing.T) { { name: "from only by name is inclusive lower bound", from: "packages", to: "", - want: []string{"packages", "flatpak", "grub-theme", "kde", "dotfiles", "setup", "services", "finalize"}, + want: []string{"packages", "grub-theme", "kde", "dotfiles", "setup", "services", "finalize"}, }, { name: "from only by number resolves same as name", from: "30", to: "", - want: []string{"packages", "flatpak", "grub-theme", "kde", "dotfiles", "setup", "services", "finalize"}, + want: []string{"packages", "grub-theme", "kde", "dotfiles", "setup", "services", "finalize"}, }, { name: "to only by name is inclusive upper bound", from: "", to: "packages", - want: []string{"preflight", "archinstall", "mount", "packages"}, + want: []string{"preflight", "archinstall", "mount", "swap", "locale", "user", "repos", "kernels", "packages"}, }, { name: "to only by number resolves same as name", from: "", to: "30", - want: []string{"preflight", "archinstall", "mount", "packages"}, + want: []string{"preflight", "archinstall", "mount", "swap", "locale", "user", "repos", "kernels", "packages"}, }, { name: "both bounds inclusive on each end", from: "packages", to: "kde", - want: []string{"packages", "flatpak", "grub-theme", "kde"}, + want: []string{"packages", "grub-theme", "kde"}, }, { name: "both bounds mixing name and number", from: "30", to: "kde", - want: []string{"packages", "flatpak", "grub-theme", "kde"}, + want: []string{"packages", "grub-theme", "kde"}, }, { name: "single-stage window when from equals to", @@ -106,12 +106,12 @@ func TestWithin(t *testing.T) { // TestWithin_ComposesWithSkip confirms the filter operates on whatever Select // already produced: a skipped stage stays absent even when inside the window. func TestWithin_ComposesWithSkip(t *testing.T) { - in := Select(Install, "", []string{"flatpak"}, nil) + in := Select(Install, "", []string{"grub-theme"}, nil) got, err := Within(in, "packages", "kde") if err != nil { t.Fatalf("unexpected error: %v", err) } - want := []string{"packages", "grub-theme", "kde"} // flatpak removed by --skip + want := []string{"packages", "kde"} // grub-theme removed by --skip if g := names(got); !equalStrings(g, want) { t.Fatalf("Within over skipped input = %v, want %v", g, want) } diff --git a/internal/stages/kernels.go b/internal/stages/kernels.go new file mode 100644 index 0000000..6e40030 --- /dev/null +++ b/internal/stages/kernels.go @@ -0,0 +1,26 @@ +package stages + +// kernelsStage installs the configured kernels in the target, optionally removes +// the stock `linux`, and pins the default kernel in the bootloader — so the +// installed system already boots the right kernel on first boot. It runs at +// Order 26, after `repos`@24 (custom kernels may live in a custom repo) and +// before `packages`@30. Runs when there are extra kernel packages OR a default +// kernel to pin (kernel.default may name a base kernel with no extra packages). +type kernelsStage struct{} + +func init() { register(kernelsStage{}) } + +func (kernelsStage) Order() int { return 26 } +func (kernelsStage) Name() string { return "kernels" } +func (kernelsStage) Phase() Phase { return Install } + +func (kernelsStage) Run(ctx *Context) error { + if !requireChroot(ctx, "kernels") { + return nil + } + k := ctx.Cfg.Kernel + if len(k.Packages) == 0 && k.Default == "" { + return nil + } + return installKernels(ctx, k) +} diff --git a/internal/stages/kernels_test.go b/internal/stages/kernels_test.go new file mode 100644 index 0000000..d0248e6 --- /dev/null +++ b/internal/stages/kernels_test.go @@ -0,0 +1,111 @@ +package stages + +import ( + "strings" + "testing" + + "github.com/AdamJHall/archwright/internal/config" + "github.com/AdamJHall/archwright/internal/run" +) + +// The kernels stage installs the configured kernels in the chroot, optionally +// removes the stock kernel, and pins the default in the bootloader. Self-contained +// YAML keeps these off the shared fixtures. + +const kernelsFullYAML = ` +system: {hostname: arch-k, timezone: Europe/London, locale: en_GB.UTF-8, keymap: uk} +user: {name: adam} +pacstrap: [base-devel, git, zsh, sudo, networkmanager, efibootmgr, intel-ucode] +kernel: {base: [linux], packages: [linux-cachyos, linux-cachyos-headers], default: linux-cachyos, replace_stock: true} +disks: + layout: plain + esp: {device: /dev/nvme0n1, size: 1GiB} + swap: {type: none} + plain: {device: /dev/nvme0n1, filesystem: ext4} +` + +// Custom kernels installed, stock removed, default pinned, GRUB regenerated — all +// in the chroot. +func TestKernelsStage_InstallsRemovesAndPinsDefault(t *testing.T) { + mustContain(t, planForCfgChroot(t, "kernels", kernelsFullYAML), + "arch-chroot /mnt pacman -S --needed --noconfirm linux-cachyos linux-cachyos-headers", + "arch-chroot /mnt pacman -Rns --noconfirm linux", + `GRUB_TOP_LEVEL="/boot/vmlinuz-linux-cachyos"`, + "arch-chroot /mnt grub-mkconfig -o /boot/grub/grub.cfg", + ) +} + +const kernelDefaultOnlyYAML = ` +system: {hostname: arch-kd, timezone: Europe/London, locale: en_GB.UTF-8, keymap: uk} +user: {name: adam} +pacstrap: [base-devel, git, zsh, sudo, networkmanager, efibootmgr, intel-ucode] +kernel: {base: [linux, linux-lts], default: linux-lts} +disks: + layout: plain + esp: {device: /dev/nvme0n1, size: 1GiB} + swap: {type: none} + plain: {device: /dev/nvme0n1, filesystem: ext4} +` + +// kernel.default pointing at a base kernel with no extra packages must still pin +// the default in the bootloader (Issue #3): the stage runs, the pacman -S install +// is skipped (no packages), and GRUB's default is written + regenerated. +func TestKernelsStage_DefaultOnlyPinsBootloader(t *testing.T) { + plan := planForCfgChroot(t, "kernels", kernelDefaultOnlyYAML) + mustContain(t, plan, + `GRUB_TOP_LEVEL="/boot/vmlinuz-linux-lts"`, + "arch-chroot /mnt grub-mkconfig -o /boot/grub/grub.cfg", + ) + // No packages to install -> no `pacman -S` kernel install. + if joined := strings.Join(plan, "\n"); strings.Contains(joined, "pacman -S --needed --noconfirm linux-lts") { + t.Errorf("did not expect a kernel package install for a base-only default.\nplan:\n%s", joined) + } +} + +const noKernelExtrasYAML = ` +system: {hostname: arch-nk, timezone: Europe/London, locale: en_GB.UTF-8, keymap: uk} +user: {name: adam} +pacstrap: [base-devel, git, zsh, sudo, networkmanager, efibootmgr, intel-ucode] +kernel: {base: [linux]} +disks: + layout: plain + esp: {device: /dev/nvme0n1, size: 1GiB} + swap: {type: none} + plain: {device: /dev/nvme0n1, filesystem: ext4} +` + +// No extra kernel packages and no default -> the stage records nothing. +func TestKernelsStage_NoExtrasIsNoOp(t *testing.T) { + plan := planForCfgChroot(t, "kernels", noKernelExtrasYAML) + if len(plan) != 0 { + t.Errorf("no kernel extras should record nothing, got:\n%s", strings.Join(plan, "\n")) + } +} + +// The kernels stage skips cleanly when the chroot isn't open. +func TestKernelsStage_SkipsWhenChrootNotOpen(t *testing.T) { + if plan := planForNoChroot(t, "kernels", kernelsFullYAML); len(plan) != 0 { + t.Errorf("kernels must record nothing without an open chroot, got:\n%s", strings.Join(plan, "\n")) + } +} + +// installKernels with no packages but a default set still pins the bootloader and +// skips the empty `pacman -S` (Issue #3 guard), tested at the function level so it +// is independent of the stage gating above. +func TestInstallKernels_NoPackagesSkipsPacman(t *testing.T) { + r := &run.Runner{DryRun: true} + ctx := &Context{ + Cfg: &config.Config{Kernel: config.KernelConfig{Base: []string{"linux"}, Default: "linux"}}, + R: r, + } + if err := installKernels(ctx, ctx.Cfg.Kernel); err != nil { + t.Fatalf("installKernels: %v", err) + } + joined := strings.Join(r.Plan, "\n") + if strings.Contains(joined, "pacman -S") { + t.Errorf("expected no pacman -S with empty packages.\nplan:\n%s", joined) + } + if !strings.Contains(joined, `GRUB_TOP_LEVEL="/boot/vmlinuz-linux"`) { + t.Errorf("expected GRUB default to be pinned.\nplan:\n%s", joined) + } +} diff --git a/internal/stages/locale.go b/internal/stages/locale.go new file mode 100644 index 0000000..4f35b08 --- /dev/null +++ b/internal/stages/locale.go @@ -0,0 +1,23 @@ +package stages + +// localeStage enables the extra locales (system.locales) in the target's +// locale.gen and regenerates them, so the installed system has them on first +// boot. archinstall already handles the default locale; this adds the extras. +// No-op when no extra locales are configured. +type localeStage struct{} + +func init() { register(localeStage{}) } + +func (localeStage) Order() int { return 22 } +func (localeStage) Name() string { return "locale" } +func (localeStage) Phase() Phase { return Install } + +func (localeStage) Run(ctx *Context) error { + if !requireChroot(ctx, "locale") { + return nil + } + if len(ctx.Cfg.System.Locales) == 0 { + return nil + } + return configureLocales(ctx, ctx.Cfg.System.Locales) +} diff --git a/internal/stages/locale_test.go b/internal/stages/locale_test.go new file mode 100644 index 0000000..afaecd3 --- /dev/null +++ b/internal/stages/locale_test.go @@ -0,0 +1,65 @@ +package stages + +import ( + "strings" + "testing" +) + +// The locale stage enables the extra locales (system.locales) in the target's +// locale.gen and regenerates them, in the chroot. Self-contained YAML. + +const localesYAML = ` +system: + hostname: arch-l + timezone: Europe/London + locale: en_GB.UTF-8 + locales: [en_US.UTF-8, en_AU.UTF-8] + keymap: uk +user: {name: adam} +pacstrap: [base-devel, git, zsh, sudo, networkmanager, efibootmgr, intel-ucode] +kernel: {base: [linux]} +disks: + layout: plain + esp: {device: /dev/nvme0n1, size: 1GiB} + swap: {type: none} + plain: {device: /dev/nvme0n1, filesystem: ext4} +` + +// Extra locales are uncommented in locale.gen (regex dots escaped) and the DB is +// regenerated, all inside the chroot. +func TestLocaleStage_EnablesExtraLocales(t *testing.T) { + mustContain(t, planForCfgChroot(t, "locale", localesYAML), + `sed -i 's/^#\(en_US\.UTF-8\b\)/\1/' /etc/locale.gen`, + `sed -i 's/^#\(en_AU\.UTF-8\b\)/\1/' /etc/locale.gen`, + "arch-chroot /mnt locale-gen", + ) +} + +const noLocalesYAML = ` +system: {hostname: arch-nl, timezone: Europe/London, locale: en_GB.UTF-8, keymap: uk} +user: {name: adam} +pacstrap: [base-devel, git, zsh, sudo, networkmanager, efibootmgr, intel-ucode] +kernel: {base: [linux]} +disks: + layout: plain + esp: {device: /dev/nvme0n1, size: 1GiB} + swap: {type: none} + plain: {device: /dev/nvme0n1, filesystem: ext4} +` + +// No extra locales -> the stage records nothing (archinstall already did the +// default locale). +func TestLocaleStage_NoExtraLocalesIsNoOp(t *testing.T) { + plan := planForCfgChroot(t, "locale", noLocalesYAML) + mustNotContain(t, plan, "locale-gen", "locale.gen") + if len(plan) != 0 { + t.Errorf("no extra locales should record nothing, got:\n%s", strings.Join(plan, "\n")) + } +} + +// The locale stage skips cleanly when the chroot isn't open. +func TestLocaleStage_SkipsWhenChrootNotOpen(t *testing.T) { + if plan := planForNoChroot(t, "locale", localesYAML); len(plan) != 0 { + t.Errorf("locale must record nothing without an open chroot, got:\n%s", strings.Join(plan, "\n")) + } +} diff --git a/internal/stages/postinstall.go b/internal/stages/postinstall.go index e5a4785..0842166 100644 --- a/internal/stages/postinstall.go +++ b/internal/stages/postinstall.go @@ -7,21 +7,14 @@ import ( "github.com/AdamJHall/archwright/internal/config" ) -// This file holds the Phase A post-archinstall steps that run inside -// arch-chroot /mnt: configuring custom repos and installing custom kernels so the -// installed system already has them on first boot. Everything runs as root in the -// chroot (no sudo), and the repo config persists into the target so Phase B -// package installs resolve against it too. - -// chrootCmd runs a command in the target via arch-chroot /mnt. -func chrootCmd(ctx *Context, name string, args ...string) error { - return ctx.R.Chroot("/mnt", append([]string{name}, args...)...) -} - -// chrootShell runs a shell snippet as root in the target. -func chrootShell(ctx *Context, script string) error { - return ctx.R.Cmd("arch-chroot", "/mnt", "bash", "-c", script) -} +// This file holds the Phase A customization steps that run inside the target +// chroot: extra locales, the user login shell, [multilib], custom repos, and +// custom kernels — so the installed system already has them on first boot. They +// are driven by the numbered stages (locale@22, user@23, repos@24, kernels@26), +// which run after the `mount` stage points the Runner at the chroot. Everything +// runs as root in the chroot via ctx.R.Root/RootShell (the Runner wraps those in +// `arch-chroot `), and the repo config persists into the target so +// Phase B package installs resolve against it too. // configureRepos sets up each custom pacman repository in the target: import + // locally sign its key, run its setup script, and append its pacman.conf section @@ -33,25 +26,25 @@ func configureRepos(ctx *Context, repos []config.Repo) error { if r.Keyserver != "" { recv = append(recv, "--keyserver", r.Keyserver) } - if err := chrootCmd(ctx, "pacman-key", recv...); err != nil { + if err := ctx.R.Root("pacman-key", recv...); err != nil { return err } - if err := chrootCmd(ctx, "pacman-key", "--lsign-key", r.Key); err != nil { + if err := ctx.R.Root("pacman-key", "--lsign-key", r.Key); err != nil { return err } } if r.Setup != "" { - if err := chrootShell(ctx, r.Setup); err != nil { + if err := ctx.R.RootShell(r.Setup); err != nil { return err } } if r.Server != "" || r.Include != "" { - if err := chrootShell(ctx, pacmanConfEntry(r)); err != nil { + if err := ctx.R.RootShell(pacmanConfEntry(r)); err != nil { return err } } } - return chrootCmd(ctx, "pacman", "-Sy") + return ctx.R.Root("pacman", "-Sy") } // pacmanConfEntry returns a root shell snippet that appends the repo's section to @@ -76,7 +69,7 @@ func pacmanConfEntry(r config.Repo) string { // binary must be in the pacstrap set (e.g. zsh) so it already exists — and is listed in // /etc/shells — at chsh time. func configureUserShell(ctx *Context, user, shell string) error { - return chrootCmd(ctx, "chsh", "-s", shell, user) + return ctx.R.Root("chsh", "-s", shell, user) } // enableMultilib uncomments the [multilib] repository section in the target's @@ -86,10 +79,10 @@ func configureUserShell(ctx *Context, user, shell string) error { // already-enabled section has no leading '#' on the header, so the range never matches. func enableMultilib(ctx *Context) error { const uncomment = `sed -i '/^#\[multilib\]/,/^#Include/ s/^#//' /etc/pacman.conf` - if err := chrootShell(ctx, uncomment); err != nil { + if err := ctx.R.RootShell(uncomment); err != nil { return err } - return chrootCmd(ctx, "pacman", "-Sy") + return ctx.R.Root("pacman", "-Sy") } // configureLocales enables additional locales in the target's /etc/locale.gen @@ -103,11 +96,11 @@ func configureLocales(ctx *Context, locales []string) error { // charset) so the sed anchor matches literally. re := regexp.QuoteMeta(l) script := fmt.Sprintf(`sed -i 's/^#\(%s\b\)/\1/' /etc/locale.gen`, re) - if err := chrootShell(ctx, script); err != nil { + if err := ctx.R.RootShell(script); err != nil { return err } } - return chrootCmd(ctx, "locale-gen") + return ctx.R.Root("locale-gen") } // installKernels installs the configured kernels in the target, optionally @@ -117,17 +110,17 @@ func configureLocales(ctx *Context, locales []string) error { // bootloader, branched off the configured bootloader (defaulting to grub). func installKernels(ctx *Context, k config.KernelConfig) error { // Skip the pacman install when there are no extra kernel packages (Issue #3): - // postInstall now also calls this purely to pin a base kernel as the default, + // the kernels stage also calls this purely to pin a base kernel as the default, // and `pacman -S` with zero package arguments would error. if len(k.Packages) > 0 { args := append([]string{"-S", "--needed", "--noconfirm"}, k.Packages...) - if err := chrootCmd(ctx, "pacman", args...); err != nil { + if err := ctx.R.Root("pacman", args...); err != nil { return err } } if k.ReplaceStock { // Validated to be safe: at least one replacement kernel is installed above. - if err := chrootCmd(ctx, "pacman", "-Rns", "--noconfirm", "linux"); err != nil { + if err := ctx.R.Root("pacman", "-Rns", "--noconfirm", "linux"); err != nil { return err } } @@ -147,11 +140,11 @@ func installKernelsGrub(ctx *Context, k config.KernelConfig) error { `echo 'GRUB_TOP_LEVEL="/boot/vmlinuz-%s"' >> /etc/default/grub`, k.Default, ) - if err := chrootShell(ctx, set); err != nil { + if err := ctx.R.RootShell(set); err != nil { return err } } - return chrootCmd(ctx, "grub-mkconfig", "-o", "/boot/grub/grub.cfg") + return ctx.R.Root("grub-mkconfig", "-o", "/boot/grub/grub.cfg") } // installKernelsSystemdBoot makes the requested kernel the systemd-boot default. @@ -181,5 +174,5 @@ func installKernelsSystemdBoot(ctx *Context, k config.KernelConfig) error { `else echo "default ${entry}" >> /boot/loader/loader.conf; fi`, k.Default, k.Default, ) - return chrootShell(ctx, set) + return ctx.R.RootShell(set) } diff --git a/internal/stages/postinstall_gating_test.go b/internal/stages/postinstall_gating_test.go index 7a76e88..eaf2b1c 100644 --- a/internal/stages/postinstall_gating_test.go +++ b/internal/stages/postinstall_gating_test.go @@ -9,11 +9,13 @@ import ( "gopkg.in/yaml.v3" ) -// These tests cover the Phase-A postInstall fixes: -// - encrypted layouts skip the chroot/remount/staging steps (Issue #2), -// - the post-install remounts are checked, not best-effort (Issue #2), -// - installKernels runs when only kernel.default is set (Issue #3). -// They use self-contained YAML so they don't touch the shared fixtures. +// These tests cover the encrypted-layout guard (Issue #2): the chroot customization +// steps (swapfile, extra locales, user shell, multilib/repos, kernels) must NOT run +// against a still-encrypted target. Since those steps are now numbered stages guarded +// by requireChroot, the guard is: the mount stage skips the (unimplemented) LUKS +// remount for an encrypted layout, so it never opens the chroot, and every +// downstream stage then no-ops. Self-contained YAML keeps these off the shared +// fixtures. // runArchinstall runs the archinstall stage in dry-run against a caller-supplied // config and returns the recorded plan. Unlike planForCfg it asserts the stage @@ -38,7 +40,7 @@ func runArchinstall(t *testing.T, yamlBody string) []string { const encLVMYAML = ` system: {hostname: arch-luks, timezone: Europe/London, locale: en_GB.UTF-8, keymap: uk} -user: {name: adam} +user: {name: adam, shell: /usr/bin/zsh} pacstrap: [base-devel, git, zsh, sudo, networkmanager, efibootmgr, intel-ucode] kernel: {base: [linux], packages: [linux-cachyos], default: linux-cachyos} repos: @@ -52,69 +54,35 @@ disks: encryption: {type: lvm_on_luks} ` -// Encrypted installs must NOT mount the (still-encrypted) device or run any -// chroot/remount/staging steps: the plan ends at archinstall, and none of the -// post-install side effects appear (Issue #2). archinstall itself still runs. -func TestPostInstall_EncryptedSkipsChrootAndStaging(t *testing.T) { +// The archinstall stage still runs archinstall on an encrypted layout, but it no +// longer performs any post-install side-effects itself (those are separate stages). +func TestArchinstall_EncryptedRunsInstallOnly(t *testing.T) { plan := runArchinstall(t, encLVMYAML) mustContain(t, plan, "archinstall --config") - // No remount of the plaintext device, no swapfile, no repo setup, no kernel - // install, no Phase B staging — all skipped for the encrypted layout. mustNotContain(t, plan, "mount /dev/vg0/root /mnt", - "mount /dev/nvme0n1p1 /mnt/boot", - "of=/mnt/swapfile", - "arch-chroot /mnt pacman -Sy", + "of=/swapfile", + "pacman-key", "linux-cachyos", "/mnt/home/adam", ) } -const kernelDefaultOnlyYAML = ` -system: {hostname: arch-kd, timezone: Europe/London, locale: en_GB.UTF-8, keymap: uk} -user: {name: adam} -pacstrap: [base-devel, git, zsh, sudo, networkmanager, efibootmgr, intel-ucode] -kernel: {base: [linux, linux-lts], default: linux-lts} -disks: - layout: plain - esp: {device: /dev/nvme0n1, size: 1GiB} - swap: {type: none} - plain: {device: /dev/nvme0n1, filesystem: ext4} -` - -// kernel.default pointing at a base kernel with no extra packages must still pin -// the default in the bootloader (Issue #3): installKernels runs, the pacman -S -// install is skipped (no packages), and GRUB's default is written + regenerated. -func TestPostInstall_KernelDefaultOnlyPinsBootloader(t *testing.T) { - plan := runArchinstall(t, kernelDefaultOnlyYAML) - mustContain(t, plan, - `GRUB_TOP_LEVEL="/boot/vmlinuz-linux-lts"`, - "arch-chroot /mnt grub-mkconfig -o /boot/grub/grub.cfg", - ) - // No packages to install -> no `pacman -S` kernel install. - joined := strings.Join(plan, "\n") - if strings.Contains(joined, "pacman -S --needed --noconfirm linux-lts") { - t.Errorf("did not expect a kernel package install for a base-only default.\nplan:\n%s", joined) +// The mount stage skips the LUKS remount for an encrypted layout: it records no +// mount commands and never opens the chroot, so downstream stages no-op. +func TestMount_EncryptedSkipsRemount(t *testing.T) { + plan := planForCfg(t, Install, "mount", encLVMYAML) + if len(plan) != 0 { + t.Errorf("encrypted mount must record nothing, got:\n%s", strings.Join(plan, "\n")) } } -// installKernels with no packages but a default set still pins the bootloader and -// skips the empty `pacman -S` (Issue #3 guard), tested at the function level so it -// is independent of the full archinstall stage plan above. -func TestInstallKernels_NoPackagesSkipsPacman(t *testing.T) { - r := &run.Runner{DryRun: true} - ctx := &Context{ - Cfg: &config.Config{Kernel: config.KernelConfig{Base: []string{"linux"}, Default: "linux"}}, - R: r, - } - if err := installKernels(ctx, ctx.Cfg.Kernel); err != nil { - t.Fatalf("installKernels: %v", err) - } - joined := strings.Join(r.Plan, "\n") - if strings.Contains(joined, "pacman -S") { - t.Errorf("expected no pacman -S with empty packages.\nplan:\n%s", joined) - } - if !strings.Contains(joined, `GRUB_TOP_LEVEL="/boot/vmlinuz-linux"`) { - t.Errorf("expected GRUB default to be pinned.\nplan:\n%s", joined) +// With the chroot never opened (the encrypted path), every post-install stage +// no-ops rather than running its side-effects against the still-encrypted target. +func TestEncrypted_PostInstallStagesSkip(t *testing.T) { + for _, name := range []string{"swap", "locale", "user", "repos", "kernels"} { + if plan := planForNoChroot(t, name, encLVMYAML); len(plan) != 0 { + t.Errorf("stage %q must record nothing on the encrypted path, got:\n%s", name, strings.Join(plan, "\n")) + } } } diff --git a/internal/stages/repos.go b/internal/stages/repos.go new file mode 100644 index 0000000..3f6b525 --- /dev/null +++ b/internal/stages/repos.go @@ -0,0 +1,32 @@ +package stages + +// reposStage prepares pacman in the target chroot before packages/kernels +// install against it: it enables [multilib] (when pacman.multilib is set) and +// configures every custom repo (keys, setup script, pacman.conf section). It +// runs at Order 24 — after `mount` opens the chroot and BEFORE `kernels`@26 and +// `packages`@30, which resolve against the repos it sets up. No-op when neither +// multilib nor any repo is configured. +type reposStage struct{} + +func init() { register(reposStage{}) } + +func (reposStage) Order() int { return 24 } +func (reposStage) Name() string { return "repos" } +func (reposStage) Phase() Phase { return Install } + +func (reposStage) Run(ctx *Context) error { + if !requireChroot(ctx, "repos") { + return nil + } + if ctx.Cfg.Pacman.Multilib { + if err := enableMultilib(ctx); err != nil { + return err + } + } + if len(ctx.Cfg.Repos) > 0 { + if err := configureRepos(ctx, ctx.Cfg.Repos); err != nil { + return err + } + } + return nil +} diff --git a/internal/stages/repos_test.go b/internal/stages/repos_test.go new file mode 100644 index 0000000..249d63b --- /dev/null +++ b/internal/stages/repos_test.go @@ -0,0 +1,76 @@ +package stages + +import ( + "strings" + "testing" +) + +// The repos stage prepares pacman in the chroot before kernels/packages install: +// it enables [multilib] (pacman.multilib) and configures each custom repo (keys, +// setup script, pacman.conf section), then syncs. Self-contained YAML. + +const reposYAML = ` +system: {hostname: arch-r, timezone: Europe/London, locale: en_GB.UTF-8, keymap: uk} +user: {name: adam} +pacstrap: [base-devel, git, zsh, sudo, networkmanager, efibootmgr, intel-ucode] +kernel: {base: [linux]} +pacman: {multilib: true} +repos: + - name: cachyos + setup: | + /tmp/cachyos-repo/cachyos-repo.sh --install + - name: chaotic-aur + key: 3056513887B78AEB + keyserver: keyserver.ubuntu.com + include: /etc/pacman.d/chaotic-mirrorlist +disks: + layout: plain + esp: {device: /dev/nvme0n1, size: 1GiB} + swap: {type: none} + plain: {device: /dev/nvme0n1, filesystem: ext4} +` + +// The repos stage enables multilib, imports+locally-signs the repo key, runs the +// setup script, appends the pacman.conf section, and syncs — all in the chroot. +func TestReposStage_MultilibAndCustomRepos(t *testing.T) { + mustContain(t, planForCfgChroot(t, "repos", reposYAML), + // multilib uncommented + db synced + `sed -i '/^#\[multilib\]/,/^#Include/ s/^#//' /etc/pacman.conf`, + "arch-chroot /mnt pacman -Sy", + // repo key import + local sign + "arch-chroot /mnt pacman-key --recv-keys 3056513887B78AEB --keyserver keyserver.ubuntu.com", + "arch-chroot /mnt pacman-key --lsign-key 3056513887B78AEB", + // setup script runs as root in the chroot + "cachyos-repo.sh --install", + // pacman.conf section appended idempotently (grep guard) + "grep -q '^\\[chaotic-aur\\]' /etc/pacman.conf", + ) +} + +const noReposYAML = ` +system: {hostname: arch-nr, timezone: Europe/London, locale: en_GB.UTF-8, keymap: uk} +user: {name: adam} +pacstrap: [base-devel, git, zsh, sudo, networkmanager, efibootmgr, intel-ucode] +kernel: {base: [linux]} +disks: + layout: plain + esp: {device: /dev/nvme0n1, size: 1GiB} + swap: {type: none} + plain: {device: /dev/nvme0n1, filesystem: ext4} +` + +// No multilib and no repos -> the stage records nothing. +func TestReposStage_UnsetIsNoOp(t *testing.T) { + plan := planForCfgChroot(t, "repos", noReposYAML) + mustNotContain(t, plan, "multilib", "pacman-key", "pacman -Sy") + if len(plan) != 0 { + t.Errorf("no multilib/repos should record nothing, got:\n%s", strings.Join(plan, "\n")) + } +} + +// The repos stage skips cleanly when the chroot isn't open. +func TestReposStage_SkipsWhenChrootNotOpen(t *testing.T) { + if plan := planForNoChroot(t, "repos", reposYAML); len(plan) != 0 { + t.Errorf("repos must record nothing without an open chroot, got:\n%s", strings.Join(plan, "\n")) + } +} diff --git a/internal/stages/selection_test.go b/internal/stages/selection_test.go index 8aae89b..884045f 100644 --- a/internal/stages/selection_test.go +++ b/internal/stages/selection_test.go @@ -49,20 +49,24 @@ func TestSelect_Skip(t *testing.T) { } func TestSelect_DisableByNameAndNumber(t *testing.T) { - // Disable by name excludes both kde and flatpak. - got := Select(Install, "", nil, []string{"kde", "flatpak"}) - if contains(got, "kde") || contains(got, "flatpak") { - t.Errorf("disable=[kde flatpak] still contains one of them: %v", names(got)) + // Disable by name excludes kde (Install) and flatpak (Bootstrap). + got := Select(Install, "", nil, []string{"kde"}) + if contains(got, "kde") { + t.Errorf("disable=[kde] still contains kde: %v", names(got)) + } + gotB := Select(Bootstrap, "", nil, []string{"flatpak"}) + if contains(gotB, "flatpak") { + t.Errorf("disable=[flatpak] still contains flatpak: %v", names(gotB)) } - // Disable by order number: flatpak is order 40. - got = Select(Install, "", nil, []string{"40"}) - if contains(got, "flatpak") { - t.Errorf("disable=[40] still contains flatpak (order 40): %v", names(got)) + // Disable by order number: flatpak is order 50 (Bootstrap). + gotB = Select(Bootstrap, "", nil, []string{"50"}) + if contains(gotB, "flatpak") { + t.Errorf("disable=[50] still contains flatpak (order 50): %v", names(gotB)) } - all := For(Install, "") - if len(got) != len(all)-1 { - t.Fatalf("disable=[40] returned %d stages, want %d", len(got), len(all)-1) + allB := For(Bootstrap, "") + if len(gotB) != len(allB)-1 { + t.Fatalf("disable=[50] returned %d stages, want %d", len(gotB), len(allB)-1) } } diff --git a/internal/stages/selectors_test.go b/internal/stages/selectors_test.go index 74dcdf5..1033921 100644 --- a/internal/stages/selectors_test.go +++ b/internal/stages/selectors_test.go @@ -57,6 +57,27 @@ func planForCfgChroot(t *testing.T, name, yamlBody string) []string { return r.Plan } +// planForNoChroot runs an Install-phase stage in dry-run with NO chroot target +// open (ChrootRoot unset) — the encrypted-layout case, or a stage run without the +// mount stage. A customization stage guarded by requireChroot must no-op here. +func planForNoChroot(t *testing.T, name, yamlBody string) []string { + t.Helper() + var c config.Config + if err := yaml.Unmarshal([]byte(yamlBody), &c); err != nil { + t.Fatalf("unmarshal config: %v", err) + } + ss := For(Install, name) + if len(ss) != 1 { + t.Fatalf("expected exactly one stage %q in Install, got %d", name, len(ss)) + } + r := &run.Runner{DryRun: true} // ChrootRoot unset: chroot not open + ctx := &Context{Cfg: &c, R: r, AssumeYes: true, ConfigPath: "/tmp/config.yaml"} + if err := ss[0].Run(ctx); err != nil { + t.Fatalf("stage %s returned error in dry-run: %v", name, err) + } + return r.Plan +} + func TestSelector_KDEGating(t *testing.T) { // A non-kde environment makes the stage a clean no-op: nothing is written. plan := planForCfgChroot(t, "kde", ` @@ -112,7 +133,7 @@ func TestSelector_AurHelper(t *testing.T) { } func TestSelector_FlatpakRemotes(t *testing.T) { - plan := planForCfgChroot(t, "flatpak", ` + plan := planForCfg(t, Bootstrap, "flatpak", ` flatpak_remotes: - name: flathub url: https://flathub.org/repo/flathub.flatpakrepo @@ -121,10 +142,10 @@ flatpak_remotes: flatpaks: [flathub-beta:com.spotify.Client] `) mustContain(t, plan, - // exactly the declared remotes — no remote is implicit — as the user in the chroot - "arch-chroot /mnt sudo -iu adam -- flatpak --user remote-add --if-not-exists flathub https://flathub.org/repo/flathub.flatpakrepo", - "arch-chroot /mnt sudo -iu adam -- flatpak --user remote-add --if-not-exists flathub-beta https://flathub.org/beta-repo/flathub-beta.flatpakrepo", + // exactly the declared remotes — no remote is implicit — as the user (Phase B) + "flatpak --user remote-add --if-not-exists flathub https://flathub.org/repo/flathub.flatpakrepo", + "flatpak --user remote-add --if-not-exists flathub-beta https://flathub.org/beta-repo/flathub-beta.flatpakrepo", // the app installs from its named remote - "arch-chroot /mnt sudo -iu adam -- flatpak --user install -y --noninteractive flathub-beta com.spotify.Client", + "flatpak --user install -y --noninteractive flathub-beta com.spotify.Client", ) } diff --git a/internal/stages/stages_test.go b/internal/stages/stages_test.go index c7bd25b..0289c13 100644 --- a/internal/stages/stages_test.go +++ b/internal/stages/stages_test.go @@ -142,11 +142,11 @@ func TestRegistry(t *testing.T) { } } check(Install, - []string{"preflight", "archinstall", "mount", "packages", "flatpak", "grub-theme", "kde", "dotfiles", "setup", "services", "finalize"}, - []int{0, 10, 20, 30, 40, 50, 60, 70, 80, 90, 99}) + []string{"preflight", "archinstall", "mount", "swap", "locale", "user", "repos", "kernels", "packages", "grub-theme", "kde", "dotfiles", "setup", "services", "finalize"}, + []int{0, 10, 20, 21, 22, 23, 24, 26, 30, 50, 60, 70, 80, 90, 99}) check(Bootstrap, - []string{"yay", "snapper", "aur"}, - []int{10, 25, 40}) + []string{"yay", "snapper", "aur", "flatpak"}, + []int{10, 25, 40, 50}) } // planForChroot runs a single Phase A customization stage in dry-run with the @@ -174,31 +174,19 @@ func TestPlan_Archinstall(t *testing.T) { "reflector --country AU --latest 20 --sort rate --protocol https --save /etc/pacman.d/mirrorlist", // delegates the install to archinstall with our generated files "archinstall --config "+aiConfigPath+" --creds "+aiCredsPath+" --silent", - // remounts target + ESP for the chroot post-install work - "mount /dev/vg0/root /mnt", - "mount /dev/nvme0n1p1 /mnt/boot", - // swap is a post-install /swapfile (64GiB -> 65536 MiB), not a partition - "dd if=/dev/zero of=/mnt/swapfile bs=1M count=65536 status=none", - "mkswap /mnt/swapfile", - "echo '/swapfile none swap defaults 0 0' >> /mnt/etc/fstab", - // extra locales uncommented in locale.gen (dots escaped) + regenerated - `sed -i 's/^#\(en_US\.UTF-8\b\)/\1/' /etc/locale.gen`, - `sed -i 's/^#\(en_AU\.UTF-8\b\)/\1/' /etc/locale.gen`, - "arch-chroot /mnt locale-gen", - // repos configured in the chroot (persist into the target) - "arch-chroot /mnt pacman-key --recv-keys 3056513887B78AEB --keyserver keyserver.ubuntu.com", - "arch-chroot /mnt pacman-key --lsign-key 3056513887B78AEB", - "cachyos-repo.sh --install", - "arch-chroot /mnt pacman -Sy", - // custom kernel installed, stock removed, default pinned, GRUB regenerated - "arch-chroot /mnt pacman -S --needed --noconfirm linux-cachyos linux-cachyos-headers", - "arch-chroot /mnt pacman -Rns --noconfirm linux", - `GRUB_TOP_LEVEL="/boot/vmlinuz-linux-cachyos"`, - "arch-chroot /mnt grub-mkconfig -o /boot/grub/grub.cfg", ) - // Staging the binary + config and unmounting now belong to the finalize stage, - // so the archinstall stage's own plan must no longer carry them. + // The post-install side-effects (mount, swapfile, extra locales, user shell, + // multilib/repos, kernels) are now their own numbered stages (mount@20, + // swap@21 … kernels@26), and binary staging/unmount belong to finalize@99 — so + // none of them appear in the archinstall stage's own plan anymore. mustNotContain(t, plan, + "mount /dev/vg0/root /mnt", + "of=/swapfile", + "locale-gen", + "pacman-key", + "pacman -Sy", + "linux-cachyos", + "grub-mkconfig", "/mnt/home/adam/archwright", "umount /mnt", ) @@ -262,11 +250,11 @@ func TestPlan_AUR(t *testing.T) { } func TestPlan_Flatpak(t *testing.T) { - // Remotes + per-user installs run as the user inside the chroot (--user, so no - // polkit/root). - mustContain(t, planForChroot(t, "flatpak"), - "arch-chroot /mnt sudo -iu adam -- flatpak --user remote-add --if-not-exists flathub https://flathub.org/repo/flathub.flatpakrepo", - "arch-chroot /mnt sudo -iu adam -- flatpak --user install -y --noninteractive flathub com.spotify.Client", + // Phase B on the booted system: remotes + per-user installs run as the invoking + // user (--user, so no polkit/root), no chroot wrapping. + mustContain(t, planFor(t, Bootstrap, "flatpak"), + "flatpak --user remote-add --if-not-exists flathub https://flathub.org/repo/flathub.flatpakrepo", + "flatpak --user install -y --noninteractive flathub com.spotify.Client", ) } diff --git a/internal/stages/swap.go b/internal/stages/swap.go new file mode 100644 index 0000000..5a00b94 --- /dev/null +++ b/internal/stages/swap.go @@ -0,0 +1,21 @@ +package stages + +// swapStage creates the post-install /swapfile inside the target chroot (see +// setupSwapfile). It runs in the 21-29 window between `mount` (which opens the +// chroot) and `packages`, so it is --only-addressable, headered/timed and +// command-plan testable like the other customization stages. No-op unless the +// swap type is "swapfile" (setupSwapfile applies that gate). +type swapStage struct{} + +func init() { register(swapStage{}) } + +func (swapStage) Order() int { return 21 } +func (swapStage) Name() string { return "swap" } +func (swapStage) Phase() Phase { return Install } + +func (swapStage) Run(ctx *Context) error { + if !requireChroot(ctx, "swap") { + return nil + } + return setupSwapfile(ctx) +} diff --git a/internal/stages/swap_test.go b/internal/stages/swap_test.go index 1ac2a53..f4ea941 100644 --- a/internal/stages/swap_test.go +++ b/internal/stages/swap_test.go @@ -41,12 +41,14 @@ disks: lvm: {vg: vg0, lv: root, filesystem: xfs, pvs: [/dev/nvme0n1p2]} ` -// zram swap must not create a /swapfile (archinstall handles zram); the LVM root -// is still mounted for post-install chroot work. -func TestPlan_Archinstall_ZramSwap(t *testing.T) { - plan := planForCfg(t, Install, "archinstall", lvmZramYAML) - mustContain(t, plan, "mount /dev/vg0/root /mnt") - mustNotContain(t, plan, "of=/mnt/swapfile", "mkswap /mnt/swapfile") +// zram swap must not create a /swapfile (archinstall handles zram): the swap +// stage no-ops. +func TestSwapStage_ZramNoSwapfile(t *testing.T) { + plan := planForCfgChroot(t, "swap", lvmZramYAML) + mustNotContain(t, plan, "of=/swapfile", "mkswap /swapfile") + if len(plan) != 0 { + t.Errorf("zram swap should record nothing, got:\n%s", strings.Join(plan, "\n")) + } } const plainSwapPartYAML = ` @@ -61,12 +63,13 @@ disks: plain: {device: /dev/nvme0n1, filesystem: ext4} ` -// plain layout with a swap partition: no /swapfile, and the root is partition 3 -// (ESP=1, swap=2, root=3) of disk 1. -func TestPlan_Archinstall_PlainSwapPartition(t *testing.T) { - plan := planForCfg(t, Install, "archinstall", plainSwapPartYAML) - mustContain(t, plan, "mount /dev/nvme0n1p3 /mnt") - mustNotContain(t, plan, "of=/mnt/swapfile") +// plain layout with a swap partition: no /swapfile (the partition is the swap). +func TestSwapStage_PartitionNoSwapfile(t *testing.T) { + plan := planForCfgChroot(t, "swap", plainSwapPartYAML) + mustNotContain(t, plan, "of=/swapfile") + if len(plan) != 0 { + t.Errorf("swap partition should record no swapfile setup, got:\n%s", strings.Join(plan, "\n")) + } } const plainNoSwapYAML = ` @@ -81,11 +84,48 @@ disks: plain: {device: /dev/nvme0n1, filesystem: ext4} ` -// plain layout, no swap: root is partition 2, no swapfile. -func TestPlan_Archinstall_PlainNoSwap(t *testing.T) { - plan := planForCfg(t, Install, "archinstall", plainNoSwapYAML) - mustContain(t, plan, "mount /dev/nvme0n1p2 /mnt") - mustNotContain(t, plan, "of=/mnt/swapfile") +// plain layout, no swap: the swap stage no-ops (no swapfile). +func TestSwapStage_NoneNoSwapfile(t *testing.T) { + plan := planForCfgChroot(t, "swap", plainNoSwapYAML) + mustNotContain(t, plan, "of=/swapfile") + if len(plan) != 0 { + t.Errorf("swap type none should record nothing, got:\n%s", strings.Join(plan, "\n")) + } +} + +const swapfileYAML = ` +system: {hostname: arch-swap, timezone: Europe/London, locale: en_GB.UTF-8, keymap: uk} +user: {name: adam} +pacstrap: [base-devel, git, zsh, sudo, networkmanager, efibootmgr, intel-ucode] +kernel: {base: [linux]} +disks: + layout: plain + esp: {device: /dev/nvme0n1, size: 1GiB} + swap: {type: swapfile, size: 4GiB} + plain: {device: /dev/nvme0n1, filesystem: ext4} +` + +// A swapfile swap creates /swapfile inside the chroot: the commands use in-chroot +// paths (/swapfile, /etc/fstab) and the Runner wraps them in arch-chroot /mnt. +// 4GiB -> 4096 MiB. +func TestSwapStage_CreatesSwapfileInChroot(t *testing.T) { + plan := planForCfgChroot(t, "swap", swapfileYAML) + mustContain(t, plan, + "arch-chroot /mnt dd if=/dev/zero of=/swapfile bs=1M count=4096 status=none", + "arch-chroot /mnt chmod 600 /swapfile", + "arch-chroot /mnt mkswap /swapfile", + "arch-chroot /mnt bash -c echo '/swapfile none swap defaults 0 0' >> /etc/fstab", + ) + // The old /mnt-prefixed host paths must be gone (they would double up under + // arch-chroot as /mnt/swapfile inside the target). + mustNotContain(t, plan, "of=/mnt/swapfile", ">> /mnt/etc/fstab") +} + +// swap skips cleanly when the chroot isn't open (encrypted layout / no mount stage). +func TestSwapStage_SkipsWhenChrootNotOpen(t *testing.T) { + if plan := planForNoChroot(t, "swap", swapfileYAML); len(plan) != 0 { + t.Errorf("swap must record nothing without an open chroot, got:\n%s", strings.Join(plan, "\n")) + } } func TestRootDevice(t *testing.T) { diff --git a/internal/stages/user.go b/internal/stages/user.go new file mode 100644 index 0000000..7f60f6f --- /dev/null +++ b/internal/stages/user.go @@ -0,0 +1,22 @@ +package stages + +// userStage applies the configured login shell (user.shell) to the installed +// user via chsh in the target, so first login uses e.g. zsh instead of the +// archinstall default /bin/bash. No-op when user.shell is unset. +type userStage struct{} + +func init() { register(userStage{}) } + +func (userStage) Order() int { return 23 } +func (userStage) Name() string { return "user" } +func (userStage) Phase() Phase { return Install } + +func (userStage) Run(ctx *Context) error { + if !requireChroot(ctx, "user") { + return nil + } + if ctx.Cfg.User.Shell == "" { + return nil + } + return configureUserShell(ctx, ctx.Cfg.User.Name, ctx.Cfg.User.Shell) +} diff --git a/internal/stages/user_test.go b/internal/stages/user_test.go new file mode 100644 index 0000000..81fc9ea --- /dev/null +++ b/internal/stages/user_test.go @@ -0,0 +1,56 @@ +package stages + +import ( + "strings" + "testing" +) + +// The user stage applies user.shell to the installed user via chsh in the chroot +// (it was previously defined-but-unused, so installs defaulted to /bin/bash). +// Self-contained YAML keeps these off the shared fixtures. + +const userShellYAML = ` +system: {hostname: arch-u, timezone: Europe/London, locale: en_GB.UTF-8, keymap: uk} +user: {name: adam, shell: /usr/bin/zsh} +pacstrap: [base-devel, git, zsh, sudo, networkmanager, efibootmgr, intel-ucode] +kernel: {base: [linux]} +disks: + layout: plain + esp: {device: /dev/nvme0n1, size: 1GiB} + swap: {type: none} + plain: {device: /dev/nvme0n1, filesystem: ext4} +` + +func TestUserStage_AppliesLoginShell(t *testing.T) { + mustContain(t, planForCfgChroot(t, "user", userShellYAML), + "arch-chroot /mnt chsh -s /usr/bin/zsh adam", + ) +} + +const noUserShellYAML = ` +system: {hostname: arch-nu, timezone: Europe/London, locale: en_GB.UTF-8, keymap: uk} +user: {name: adam} +pacstrap: [base-devel, git, zsh, sudo, networkmanager, efibootmgr, intel-ucode] +kernel: {base: [linux]} +disks: + layout: plain + esp: {device: /dev/nvme0n1, size: 1GiB} + swap: {type: none} + plain: {device: /dev/nvme0n1, filesystem: ext4} +` + +// Unset user.shell preserves today's behavior: no chsh, nothing recorded. +func TestUserStage_UnsetShellIsNoOp(t *testing.T) { + plan := planForCfgChroot(t, "user", noUserShellYAML) + mustNotContain(t, plan, "chsh") + if len(plan) != 0 { + t.Errorf("unset shell should record nothing, got:\n%s", strings.Join(plan, "\n")) + } +} + +// The user stage skips cleanly when the chroot isn't open. +func TestUserStage_SkipsWhenChrootNotOpen(t *testing.T) { + if plan := planForNoChroot(t, "user", userShellYAML); len(plan) != 0 { + t.Errorf("user must record nothing without an open chroot, got:\n%s", strings.Join(plan, "\n")) + } +} diff --git a/internal/stages/usershell_multilib_test.go b/internal/stages/usershell_multilib_test.go deleted file mode 100644 index 1e29271..0000000 --- a/internal/stages/usershell_multilib_test.go +++ /dev/null @@ -1,53 +0,0 @@ -package stages - -import "testing" - -// These cover the Phase-A postInstall additions: -// - user.shell is applied to the installed user via chsh (it was previously -// defined-but-unused, so installs defaulted to /bin/bash), -// - pacman.multilib uncomments the [multilib] repo and syncs the new db so -// 32-bit packages (steam) resolve in Phase B. -// Self-contained YAML keeps them off the shared fixtures. - -const shellMultilibYAML = ` -system: {hostname: arch-sm, timezone: Europe/London, locale: en_GB.UTF-8, keymap: uk} -user: {name: adam, shell: /usr/bin/zsh} -pacstrap: [base-devel, git, zsh, sudo, networkmanager, efibootmgr, intel-ucode] -kernel: {base: [linux]} -pacman: {multilib: true} -disks: - layout: plain - esp: {device: /dev/nvme0n1, size: 1GiB} - swap: {type: none} - plain: {device: /dev/nvme0n1, filesystem: ext4} -` - -func TestPostInstall_AppliesUserShellAndMultilib(t *testing.T) { - plan := runArchinstall(t, shellMultilibYAML) - mustContain(t, plan, - "arch-chroot /mnt chsh -s /usr/bin/zsh adam", - `sed -i '/^#\[multilib\]/,/^#Include/ s/^#//' /etc/pacman.conf`, - "arch-chroot /mnt pacman -Sy", - ) -} - -const noShellNoMultilibYAML = ` -system: {hostname: arch-ns, timezone: Europe/London, locale: en_GB.UTF-8, keymap: uk} -user: {name: adam} -pacstrap: [base-devel, git, zsh, sudo, networkmanager, efibootmgr, intel-ucode] -kernel: {base: [linux]} -disks: - layout: plain - esp: {device: /dev/nvme0n1, size: 1GiB} - swap: {type: none} - plain: {device: /dev/nvme0n1, filesystem: ext4} -` - -// Unset shell/multilib preserve today's behavior: no chsh, no pacman.conf edit. -func TestPostInstall_UnsetShellAndMultilibAreNoOps(t *testing.T) { - plan := runArchinstall(t, noShellNoMultilibYAML) - mustNotContain(t, plan, - "chsh", - "multilib", - ) -} diff --git a/test/e2e/vm/configs/features-flatpak.yaml b/test/e2e/vm/configs/features-flatpak.yaml index 0736e09..78ce2fe 100644 --- a/test/e2e/vm/configs/features-flatpak.yaml +++ b/test/e2e/vm/configs/features-flatpak.yaml @@ -1,7 +1,9 @@ # e2e feature coverage (heavy): exercises the Phase B flatpak stage — register a -# remote (flathub) and install one small app from it. This pulls a flatpak runtime, -# which is expected (and the reason this lives in its own on-demand descriptor). -# Disk layout is the same minimal single-disk LVM as features-min. +# remote (flathub) and install a plain app (Flatseal) plus an extra-data app +# (Spotify) whose apply_extra bwrap sandbox needs an unprivileged user namespace, +# proving that path works on the booted system. This pulls a flatpak runtime + the +# Spotify binary, which is expected (and the reason this lives in its own on-demand +# descriptor). Disk layout is the same minimal single-disk LVM as features-min. system: hostname: arch-e2e timezone: Etc/UTC @@ -60,6 +62,12 @@ flatpak_remotes: url: https://flathub.org/repo/flathub.flatpakrepo flatpaks: - - flathub:com.github.tchx84.Flatseal + - flathub:com.github.tchx84.Flatseal # plain OSTree app (fast happy-path) + # Spotify is an EXTRA-DATA app: its apply_extra script runs a bwrap sandbox at + # install time that needs an unprivileged user namespace. That can't be created + # in the Phase A arch-chroot, which is why the flatpak stage lives in Phase B — + # install it here so the booted-system userns/bwrap path is actually exercised + # (a regression guard; the validated app below is Spotify, not Flatseal). + - flathub:com.spotify.Client aur_helper: yay diff --git a/test/e2e/vm/lib/validate.sh b/test/e2e/vm/lib/validate.sh index 1d66151..69fa128 100755 --- a/test/e2e/vm/lib/validate.sh +++ b/test/e2e/vm/lib/validate.sh @@ -98,7 +98,11 @@ esac # --- bootloader ------------------------------------------------------------ case "$EXPECT_BOOTLOADER" in grub) - [[ -f /boot/grub/grub.cfg ]] && ok "grub.cfg present" || fail "/boot/grub/grub.cfg missing" + # The ESP is mounted at /boot with dmask=0077 (archinstall's secure default), + # so /boot is 0700 root:root and this unprivileged validation user can't + # traverse it — read grub.cfg via sudo, mirroring the systemd-boot branch's + # `sudo bootctl` and the kernel-zen check in features.sh. + sudo test -f /boot/grub/grub.cfg && ok "grub.cfg present" || fail "/boot/grub/grub.cfg missing" ;; systemd-boot) if sudo bootctl is-installed >/dev/null 2>&1; then ok "systemd-boot installed"; else diff --git a/test/e2e/vm/matrix/features_extra.py b/test/e2e/vm/matrix/features_extra.py index f160a26..ef0e4cf 100644 --- a/test/e2e/vm/matrix/features_extra.py +++ b/test/e2e/vm/matrix/features_extra.py @@ -21,7 +21,10 @@ "validate_script": "lib/features.sh", "expect": { "FEATURES": "flatpak", - "FLATPAK_APP": "com.github.tchx84.Flatseal", + # Validate the extra-data app (Spotify): its install exercises the + # apply_extra/bwrap/userns path that only works now that flatpak runs + # in Phase B on the booted system. + "FLATPAK_APP": "com.spotify.Client", "HOSTNAME": "arch-e2e", "USER": "e2e", }, },