fix(installer): make Arch installs work and stop the installer destroying data - #197
Open
Albe841114 wants to merge 4 commits into
Open
Albe841114 wants to merge 4 commits into
Albe841114 wants to merge 4 commits into
Conversation
Four defects reproduced on CachyOS/Arch: - carapace and zsh-theme-powerlevel10k are AUR-only, and pacman is atomic, so one unresolvable name aborted every Arch install - CopyDir handed Unix sockets to os.ReadFile, so one live socket failed the backup of every config - rm -rf ran on a relative path at two sites with no git check, destroying uncommitted work - --dry-run set GENTLEMAN_DRY_RUN and nothing read it Refs Gentleman-Programming#182, Gentleman-Programming#174, Gentleman-Programming#190, Gentleman-Programming#192
A clean working tree proved only that nothing was uncommitted, not that nothing would be lost. Committing local work made a checkout *more* deletable, not less: `git status --porcelain` went empty, the guard classified it as a clean checkout, and removal destroyed commits that existed on no remote. This was observed destroying real work. Add RepoUnpublishedWork. Deletion now requires the working tree to be clean AND the repository's work to be published: no stash entries, and HEAD contained by at least one remote-tracking branch. A repository with no remotes, or one whose publication cannot be determined, fails closed. The guard must not over-block either: a branch with no upstream whose HEAD is already on a remote loses no commits, so it stays deletable. Only the branch name would go, and that is not work. Two earlier tests encoded the old assumption by building fixtures with no remote and expecting deletion; their fixtures now publish to a bare origin, so they still exercise the delete path.
Merge the four delta specs into openspec/specs/ and move the change folder to changes/archive/2026-08-29-fix-arch-installer/. The archive report records what the proposal and design do not: five work units shipped, not four. Unit 5 was added after verification, when the installer destroyed this change's own commits and exposed a flaw in unit 3's guard — a clean working tree proves only that nothing is uncommitted, not that nothing would be lost. Also recorded: three defects found while doing this work that had no upstream issue at the time — backup dying on Unix sockets, the destructive relative-path rm -rf, and the E2E harness masking per-image exit status. They have since been reported upstream as Gentleman-Programming#195, Gentleman-Programming#193 and Gentleman-Programming#194 respectively, alongside Gentleman-Programming#196 for the fish configuration being overwritten without a backup.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Linked issues
Closes #183 —
carapaceis AUR-only, andpacmanis atomic, so the whole install abortsCloses #193 —
rm -rf Gentleman.Dotson a CWD-relative pathCloses #195 — backup aborts when a directory contains a Unix socket
Partially addresses #190: the
--dry-runno-op is fixed here. The Kitty selection point in that issue is not addressed.Not fixed here, listed so the scope is unambiguous: #194 (E2E
pipefail) and #196 (fish configuration overwritten). Both are real and reported, but neither is touched by this branch.Summary
--dry-runactually dry, and lets the backup survive Unix sockets.Why it currently fails on Arch
The shell step runs one atomic
pacman -Sthat includescarapace. That package does not exist incore/extra— it is AUR-only, published ascarapace-bin, which does not declareprovides=carapace. Because apacmantransaction is atomic, one unresolvable name aborts the whole thing and nothing installs.zsh-theme-powerlevel10khas the same problem.Changes
installer/internal/tui/installer.gorm -rfinstaller/internal/system/repostate.goinstaller/internal/system/exec.goCopyFile/CopyDirskip non-regular files (sockets, FIFOs, device nodes) instead of erroring outinstaller/internal/system/dryrun.go--dry-rungate, including theos.RemoveAllpath that theRun/RunWithLogsgate did not coverinstaller/internal/tui/arch_packages_source_test.goinstaller.go, so an AUR-only name cannot come backWhat "safe to delete" means here
This part exists because the guard written earlier in this change destroyed this change's own commits during development.
A clean working tree is not sufficient proof that a checkout can be deleted. A clean checkout can hold local commits that exist on no remote:
git status --porcelainis empty and the work is still unrecoverable. Committing the work made it more deletable, not less.The guard now requires: working tree clean, no untracked files, no stashes, and
HEADreachable from a remote-tracking branch. A branch with no configured upstream is treated as unsafe. It never prompts —--non-interactiveis documented as CI-friendly, so a refusal is a deterministic abort, and callers decide whether that is fatal.Test plan
go test ./...—systemandtuipackages passgo build— cleanarch_packages_source_test.goexists.Notes for review
type:*label is set: this account has read-only permission here and cannot apply labels.openspec/changes/archive/2026-08-29-fix-arch-installer/carries the proposal, design, tasks and verification report. Happy to drop those files if they are not wanted in-tree — the code changes stand on their own.