refactor(packages): one manifest parser, so a name means one thing to both tools
install.sh and pkg-inventory.sh carried byte-identical copies of read_manifest. That is the duplicated-code half of the problem; the worse half is untested — neither script had any test for the grammar that decides what gets installed. - scripts/lib/manifest.sh is the parser; both scripts source it - tests/manifest-parser.bats writes the contract first (comments, CRLF, first field only, unreadable file fails loudly instead of yielding "nothing desired") - an unreadable manifest now fails inside the helper rather than depending on whichever `set` options the caller happens to run under - install.sh check mode verified end to end against a stubbed pacman: a CRLF manifest with trailing comments resolves to bare names, installed names drop Refs #60
This commit is contained in:
1 parent
40b9725433
commit
06f45e83c1
4 files changed
+86
-8
No files matched your search
@@ -25,9 +25,13 @@ if both directions are checked.
|
|||||||
## Manifests
|
## Manifests
|
||||||
|
|
||||||
`packages/common.pkgs` (every host) + `packages/<host>.pkgs` (this host) + `packages/aur.pkgs`
|
`packages/common.pkgs` (every host) + `packages/<host>.pkgs` (this host) + `packages/aur.pkgs`
|
||||||
(AUR-only, installed through `yay`). The parser is `sed 's/#.*//'` then the first field, so `#`
|
(AUR-only, installed through `yay`). **One parser reads them for both tools**:
|
||||||
comments trail freely and one token per line is the whole grammar. An unknown `--host` gets the
|
`scripts/lib/manifest.sh` — `sed 's/#.*//'` then the first field, so `#` comments trail freely and
|
||||||
common manifest only: a wrong-host run is a no-op, not a hazard.
|
one token per line is the whole grammar; an unreadable manifest is a loud failure rather than an
|
||||||
|
empty desired list. `tests/manifest-parser.bats` owns that contract. The two scripts carried
|
||||||
|
verbatim copies of the body until 2026-10-21, which is exactly how a manifest line can come to mean
|
||||||
|
two things to the two halves of the same check. An unknown `--host` gets the common manifest only:
|
||||||
|
a wrong-host run is a no-op, not a hazard.
|
||||||
|
|
||||||
## Rebuild a workstation
|
## Rebuild a workstation
|
||||||
|
|
||||||
@@ -110,5 +114,6 @@ never teach a fresh box to install.
|
|||||||
|
|
||||||
- [tools/index](index.md) · [dotfiles-deploy](dotfiles-deploy.md) (the config tier that follows) ·
|
- [tools/index](index.md) · [dotfiles-deploy](dotfiles-deploy.md) (the config tier that follows) ·
|
||||||
[live-sync](live-sync.md) (the heartbeat that commits machine state)
|
[live-sync](live-sync.md) (the heartbeat that commits machine state)
|
||||||
- Tests: `tests/pkg-inventory.bats` (fixtures + stubbed `pacman`, no real package database touched).
|
- Tests: `tests/pkg-inventory.bats` (fixtures + stubbed `pacman`, no real package database touched)
|
||||||
|
and `tests/manifest-parser.bats` (the grammar above, observed as names in / names out).
|
||||||
- Refs #51.
|
- Refs #51.
|
||||||
+3
-4
@@ -17,7 +17,8 @@
|
|||||||
# otherwise sudo prompts normally. The value is never stored in this repo.
|
# otherwise sudo prompts normally. The value is never stored in this repo.
|
||||||
set -euo pipefail
|
set -euo pipefail
|
||||||
|
|
||||||
REPO="$(git -C "$(dirname "$(realpath "$0")")" rev-parse --show-toplevel 2>/dev/null || true)"
|
HERE="$(dirname "$(realpath "$0")")"
|
||||||
|
REPO="$(git -C "$HERE" rev-parse --show-toplevel 2>/dev/null || true)"
|
||||||
REPO="${INSTALL_REPO:-$REPO}"
|
REPO="${INSTALL_REPO:-$REPO}"
|
||||||
[ -n "$REPO" ] && [ -d "$REPO/packages" ] || { echo "install.sh: no packages/ under ${REPO:-<repo>}" >&2; exit 1; }
|
[ -n "$REPO" ] && [ -d "$REPO/packages" ] || { echo "install.sh: no packages/ under ${REPO:-<repo>}" >&2; exit 1; }
|
||||||
|
|
||||||
@@ -36,9 +37,7 @@ SUDO() {
|
|||||||
else sudo "$@"; fi
|
else sudo "$@"; fi
|
||||||
}
|
}
|
||||||
|
|
||||||
read_manifest() { # $1 = manifest path; emits package names, strips comments/blanks
|
source "$HERE/lib/manifest.sh" # the same parser pkg-inventory.sh reads with
|
||||||
sed 's/#.*//' "$1" | tr -d '\r' | awk '{if ($1) print $1}'
|
|
||||||
}
|
|
||||||
# ---- desired-minus-installed delta ------------------------------------------
|
# ---- desired-minus-installed delta ------------------------------------------
|
||||||
PKGS=()
|
PKGS=()
|
||||||
for m in common.pkgs "$HOST.pkgs"; do
|
for m in common.pkgs "$HOST.pkgs"; do
|
||||||
|
|||||||
Executable
+23
@@ -0,0 +1,23 @@
|
|||||||
|
#!/usr/bin/env bash
|
||||||
|
# scripts/lib/manifest.sh — the one parser for packages/*.pkgs
|
||||||
|
#
|
||||||
|
# install.sh walks desired → installed; pkg-inventory.sh walks installed → declared. Both read the
|
||||||
|
# same hand-written manifests, so both must agree on what a line MEANS: if one forgives an inline
|
||||||
|
# comment and the other does not, the same name is simultaneously "desired" and "drift", and the
|
||||||
|
# pair stops being a check on the machine. Source it — do not paste the body into a caller again
|
||||||
|
# (Refs #60: it was pasted twice, in install.sh and in pkg-inventory.sh).
|
||||||
|
#
|
||||||
|
# Contract, and tests/manifest-parser.bats is that contract:
|
||||||
|
# name first field of a line, emitted as one bare token — callers hand it to pacman
|
||||||
|
# comment `#` and everything after it is dropped, whole-line or trailing
|
||||||
|
# CRLF stripped (the manifests are edited on more than one box)
|
||||||
|
# blank empty and whitespace-only rows vanish
|
||||||
|
# unreadable non-zero exit, no names: a silent empty list reads as "nothing desired", and a
|
||||||
|
# rebuild would then install nothing while reporting success
|
||||||
|
#
|
||||||
|
# Sourced, never executed; callers own `set -euo pipefail`.
|
||||||
|
|
||||||
|
read_manifest() { # $1 = manifest path; prints one package name per line
|
||||||
|
[[ -r "$1" ]] || { printf 'read_manifest: cannot read %s\n' "$1" >&2; return 2; }
|
||||||
|
sed 's/#.*//' "$1" | tr -d '\r' | awk '{if ($1) print $1}'
|
||||||
|
}
|
||||||
Executable
+51
@@ -0,0 +1,51 @@
|
|||||||
|
#!/usr/bin/env bats
|
||||||
|
# tests/manifest-parser.bats — the one manifest parser, shared by install.sh and pkg-inventory.sh
|
||||||
|
#
|
||||||
|
# `packages/*.pkgs` is hand-written, so the parser's whole job is to forgive the ways a hand-edited
|
||||||
|
# list is wrong. The same function decides what install.sh tries to install and what pkg-inventory
|
||||||
|
# calls drift, so a manifest line must mean the same thing to both — which is why it lives in one
|
||||||
|
# file instead of being pasted into each caller (Refs #60: it was pasted twice).
|
||||||
|
#
|
||||||
|
# Observed through the function's exit status and stdout only:
|
||||||
|
# names first field of a line, one bare token; `#` and everything after it is a comment
|
||||||
|
# CRLF tolerated (the manifests get edited on more than one box)
|
||||||
|
# blank lines vanish
|
||||||
|
# unreadable non-zero with no names — an empty desired list must never be a silent success,
|
||||||
|
# because "nothing desired" is exactly what a rebuild would install
|
||||||
|
|
||||||
|
setup() {
|
||||||
|
TEST_TMP="$(mktemp -d)"
|
||||||
|
LIB="$BATS_TEST_DIRNAME/../scripts/lib/manifest.sh"
|
||||||
|
}
|
||||||
|
|
||||||
|
teardown() { rm -rf "$TEST_TMP"; }
|
||||||
|
|
||||||
|
parse() { # $1 = manifest contents, verbatim
|
||||||
|
printf '%s' "$1" > "$TEST_TMP/m.pkgs"
|
||||||
|
run bash -c 'source "$1"; read_manifest "$2"' _ "$LIB" "$TEST_TMP/m.pkgs"
|
||||||
|
}
|
||||||
|
|
||||||
|
@test "inline notes, full-line comments, blank rows and CRLF come out as bare names" {
|
||||||
|
parse $'# desired state\r\nghostty # terminal\r\n\r\n \r\nzoxide\r\n'
|
||||||
|
[ "$status" -eq 0 ]
|
||||||
|
[ "$(printf '%s\n' "$output" | tr '\n' ' ')" = "ghostty zoxide " ]
|
||||||
|
}
|
||||||
|
|
||||||
|
@test "only the first field of a line is a package" {
|
||||||
|
# install.sh hands every emitted token straight to pacman, so a stray second token on a line
|
||||||
|
# must never become an install target; whatever it is, the manifest did not declare it.
|
||||||
|
parse $'neovim extra tokens here\n'
|
||||||
|
[ "$output" = "neovim" ]
|
||||||
|
}
|
||||||
|
|
||||||
|
@test "a comment that names a package is not a package" {
|
||||||
|
parse $'# vlc\n#typora\nbun\n'
|
||||||
|
[ "$output" = "bun" ]
|
||||||
|
}
|
||||||
|
|
||||||
|
@test "an unreadable manifest fails loudly instead of yielding an empty desired list" {
|
||||||
|
run --separate-stderr bash -c 'source "$1"; read_manifest "$2"' _ "$LIB" "$TEST_TMP/nope.pkgs"
|
||||||
|
[ "$status" -ne 0 ]
|
||||||
|
[ -z "$output" ]
|
||||||
|
[[ "$stderr" == *nope.pkgs* ]]
|
||||||
|
}
|
||||||
Reference in new issue
Block a user