conform to the rewritten format spec
commit ff2c10498921d73548848467a7365177b2cf95b2 · parent e3554c6561
· browse this tree
Ximianth, 2026-10-18 23:27:01 UTC
Three behavior changes, per docs/hacking/FORMAT.md and REPLAYING.md: - The message-no-final-newline declaration is a directive line inside the message flanks: bare, directly after the opening ----, before any content. An unrecognized bare line in that position is an error, as is a bare non-empty line anywhere else in the message block. The old line-2 position now fails loudly too, via a new rule that header lines must contain a space (the spec's key-value shape guarantees it, and continuation lines start with one), rather than being swallowed verbatim into a replayed commit. - tip is optional in STACK. Export still always writes it; a hand-made STACK without one replays fine, verify says it had nothing to cross-check against, and import skips its advisory comparison. - Folder discordances (duplicate series entries, series entries naming no file, orphan patch files) are errors for every reader. The checks live in one shared validation pass, run by branch_dirs for the whole patches dir up front: discordance is a static property of the files, so it's refused before any repo is touched, and in import's case before any ref could be created. With FORMAT.md now in-repo as the single source of truth, the tool's header comment stops restating the byte-level format (that duplication is exactly what had drifted) and points at the docs instead.
diff --git a/bin/patchette b/bin/patchette index a2df97b..5355755 100755 --- a/bin/patchette +++ b/bin/patchette @@ -6,29 +6,12 @@ ## patchette import <target-repo> <patches-dir> ## patchette verify <subject-repo> <patches-dir> ## -## Layout, per branch (folder name = branch name, `/` encoded as `¦` U+00A6): -## STACK `ref`/`base`/`tip` lines, then `series`, then slugs to EOF -## <slug>.patch one file per commit: parents line, headers, framed message, diff +## The on-disk format is specified in docs/hacking/FORMAT.md, and the replay +## algorithm in docs/hacking/REPLAYING.md; those documents are the single +## source of truth, and this tool conforms to them. ## -## The patch file, exactly: -## line 1 `parents <ref>...` (each ref: BASE, or an earlier slug) -## line 2 `message-no-final-newline` (only in that freak case; contains no -## space, so it can never collide with a verbatim `key value` header) -## next lines commit headers verbatim, minus tree/parent -## blank line -## `----` opening flank of the message block -## next lines message, framed per the sigil dialect (see below) -## `----` closing flank -## blank line -## rest diff vs first parent, to EOF (empty for empty commits) -## -## Message framing is exactly invertible and never transforms content bytes. -## Two sigil dialects exist; export emits one (default `quote`), the parser -## accepts both unconditionally (the `----` flanks keep mixed acceptance -## unambiguous: a message line reading `----` is always sigil-framed): -## quote: empty line -> `>`, else `> ` + line -## tab: empty line -> ``, else TAB + line -## A message line in any other shape is a parse error, never a guess. +## Two message-framing sigil dialects exist; export emits one (default `quote`), +## and the parser accepts both unconditionally. ## ## Fidelity and fail-closed behavior outrank convenience: parse errors never ## guess, every refusal says what it refused and why, and a failed import @@ -126,11 +109,37 @@ parse_stack() { *) die "$f: unrecognized line: $line" ;; esac done <"$f" - [[ -n $STACK_REF && -n $STACK_BASE && -n $STACK_TIP && $mode == series ]] || - die "$f: missing ref, base, tip, or series section" + # tip is optional: a hand-made STACK has no expectable value for it + # until the branch has been built. + [[ -n $STACK_REF && -n $STACK_BASE && $mode == series ]] || + die "$f: missing ref, base, or series section" [[ $STACK_REF == refs/heads/* ]] || die "$f: ref is not under refs/heads/: $STACK_REF" } +## Check folder-level concordance between a STACK's series and the *.patch +## files beside it. Any discordance is an error for every reader: +## a series entry naming no file would fail replay anyway, and an orphan +## patch file is silently inert -- nothing would ever read it. +## (Duplicate series entries are caught earlier, in parse_stack.) +## +## Expects parse_stack to have run for this folder. +## +## - @param $1 branch folder path +validate_branch_dir() { + local dir=$1 slug f + declare -A listed=() + for slug in "${STACK_SERIES[@]}"; do + listed[$slug]=1 + [[ -f $dir/$slug.patch ]] || die "$dir/STACK: series lists $slug, but $slug.patch is missing" + done + for f in "$dir"/*.patch; do + [[ -f $f ]] || continue # unmatched glob + slug=${f##*/} + slug=${slug%.patch} + [[ -n ${listed[$slug]:-} ]] || die "$f: orphan patch file, not listed in the STACK series" + done +} + ## Replay one branch folder's series into the current repo's object store. ## ## Expects parse_stack to have run for this folder, cwd to be a git repo @@ -148,7 +157,6 @@ replay_series() { for slug in "${STACK_SERIES[@]}"; do f="$dir/$slug.patch" - [[ -f $f ]] || die "$dir/STACK: series lists $slug, but $slug.patch is missing" # Parse the patch file: a strict little state machine over its prelude. # The diff section is everything after the blank line that follows the @@ -161,27 +169,37 @@ replay_series() { start) [[ $line == parents\ * ]] || die "$f: line 1 is not a parents line" pline=$line - state=flagmaybe - ;; - flagmaybe) - if [[ $line == message-no-final-newline ]]; then - nofinal=1 - state=headers - continue - fi state=headers - ;& + ;; headers) if [[ -z $line ]]; then state=msgopen + elif [[ $line != *' '* ]]; then + # Headers are `key value`, and continuation lines begin with + # a space, so every header line contains one. + die "$f: malformed header line (no space): $line" else headers+=("$line") fi ;; msgopen) [[ $line == '----' ]] || die "$f: expected opening ---- flank, got: $line" - state=message + state=directives ;; + directives) + # Bare directive lines may sit directly after the opening flank, + # before any message content. Anything unrecognized and equally + # bare in this position is an error; anything message-shaped + # means the content has begun. + if [[ $line == message-no-final-newline ]]; then + nofinal=1 + continue + fi + if [[ -n $line && $line != '----' && $line != '>' && $line != '> '* && $line != $'\t'* ]]; then + die "$f: unrecognized directive line: $line" + fi + state=message + ;& message) # Both sigil dialects are accepted here, unconditionally. if [[ $line == '----' ]]; then @@ -241,10 +259,14 @@ replay_series() { done } -## List the branch folders of a patches dir, failing closed on strangers. +## List the branch folders of a patches dir, failing closed on strangers, +## and validate every folder's concordance up front: discordance is a static +## property of the patches dir, so it's refused before any repo is touched +## (and in import's case, before any ref could be created). ## ## - @param $1 patches dir -## - @state BRANCH_DIRS[] -- absolute paths, each guaranteed to hold a STACK +## - @state BRANCH_DIRS[] -- absolute paths, each holding a validated STACK +## - @state STACK_* -- clobbered; callers re-run parse_stack per folder branch_dirs() { local patches=$1 dir BRANCH_DIRS=() @@ -252,6 +274,8 @@ branch_dirs() { dir=${dir%/} [[ -d $dir ]] || continue # unmatched glob [[ -f $dir/STACK ]] || die "$dir has no STACK file; not a patchette branch folder" + parse_stack "$dir/STACK" + validate_branch_dir "$dir" BRANCH_DIRS+=("$dir") done [[ ${#BRANCH_DIRS[@]} -gt 0 ]] || die "no branch folders found in $patches" @@ -344,11 +368,11 @@ cmd_export() { { printf 'parents %s\n' "${parents[*]}" + sed -n '1,/^$/p' "$raw" | grep -vE '^(tree|parent) ' | sed '/^$/d' + printf '\n----\n' if [[ -s $msg && $(tail -c 1 "$msg") != '' ]]; then printf 'message-no-final-newline\n' fi - sed -n '1,/^$/p' "$raw" | grep -vE '^(tree|parent) ' | sed '/^$/d' - printf '\n----\n' while IFS= read -r mline || [[ -n $mline ]]; do if [[ -z $mline ]]; then if [[ $sigil == quote ]]; then printf '>\n'; else printf '\n'; fi @@ -390,7 +414,7 @@ cmd_import() { # Empty old-value makes must-not-exist atomic: no clobbering, even racily. git update-ref "$STACK_REF" "$REPLAY_TIP" "" echo "import: created $STACK_REF at $REPLAY_TIP" - if [[ $REPLAY_TIP != "$STACK_TIP" ]]; then + if [[ -n $STACK_TIP && $REPLAY_TIP != "$STACK_TIP" ]]; then echo "import: note: replayed tip differs from recorded tip $STACK_TIP (patches edited since the snapshot?)" fi done @@ -398,8 +422,7 @@ cmd_import() { cmd_verify() { [[ $# -eq 2 ]] || die "usage: patchette verify <subject-repo> <patches-dir>" - local subject patches dir f slug bad=0 - declare -A listed + local subject patches dir bad=0 subject=$(absdir "$1") patches=$(absdir "$2") branch_dirs "$patches" @@ -415,27 +438,15 @@ cmd_verify() { git cat-file -e "$STACK_BASE^{commit}" 2>/dev/null || die "recorded base $STACK_BASE is not present in $subject" replay_series "$dir" - if [[ $REPLAY_TIP == "$STACK_TIP" ]]; then + if [[ -z $STACK_TIP ]]; then + echo "verify: note: $STACK_REF replays cleanly to $REPLAY_TIP, but records no tip to cross-check against" + elif [[ $REPLAY_TIP == "$STACK_TIP" ]]; then echo "verify: ok: $STACK_REF reproduces recorded tip $STACK_TIP" else echo "verify: MISMATCH: $STACK_REF replays to $REPLAY_TIP, but records tip $STACK_TIP" echo "verify: either the snapshot was unfaithful, or the patches were edited since (patches repo is ahead of its snapshot)" bad=1 fi - - # Orphan patch files are silently inert at replay; nothing else would - # ever read them, so flagging them is specifically verify's job. - listed=() - for slug in "${STACK_SERIES[@]}"; do listed[$slug]=1; done - for f in "$dir"/*.patch; do - [[ -f $f ]] || continue # unmatched glob - slug=${f##*/} - slug=${slug%.patch} - if [[ -z ${listed[$slug]:-} ]]; then - echo "verify: orphan: $f is not listed in STACK" - bad=1 - fi - done done [[ $bad -eq 0 ]] || exit 1 diff --git a/tests/test-failclosed.sh b/tests/test-failclosed.sh index ed27b1a..6c77dd5 100755 --- a/tests/test-failclosed.sh +++ b/tests/test-failclosed.sh @@ -44,6 +44,27 @@ sed -i 's/> Do not ask why./| Do not ask why./' "$TAMPER/$fancy/teach-gizmo-to-h EXPECT refusal "a malformed message line is a parse error" "malformed message line" -- \ "$PATCHETTE" verify "$work/corpus/dev" "$TAMPER" +# An unrecognized bare line in directive position (directly after the +# opening flank) is an error. +clone_layout baddirective +sed -i '0,/^----$/s//----\nmystery-directive/' "$TAMPER/$fancy/note-to-self.patch" +EXPECT refusal "an unknown directive line is an error" "unrecognized directive" -- \ + "$PATCHETTE" verify "$work/corpus/dev" "$TAMPER" + +# A directive is only legal before message content; after content has begun, +# the same bare line is just a malformed message line. +clone_layout latedirective +sed -i 's/^> Note to self$/> Note to self\nmessage-no-final-newline/' "$TAMPER/$fancy/note-to-self.patch" +EXPECT refusal "a directive after message content is a parse error" "malformed message line" -- \ + "$PATCHETTE" verify "$work/corpus/dev" "$TAMPER" + +# The directive's pre-spec position (bare line in the header area) must fail +# loudly, not be swallowed as a verbatim header into the replayed commit. +clone_layout olddirective +sed -i '1a message-no-final-newline' "$TAMPER/$fancy/note-to-self.patch" +EXPECT refusal "a bare line in the header area is a parse error" "malformed header line" -- \ + "$PATCHETTE" verify "$work/corpus/dev" "$TAMPER" + # An unknown parent reference is an error. clone_layout badparent sed -i '1s/.*/parents no-such-slug/' "$TAMPER/$fancy/add-sprocket-docs.patch" @@ -56,11 +77,25 @@ echo "note-to-self" >>"$TAMPER/$fancy/STACK" EXPECT refusal "a duplicate series entry is an error" "duplicate series entry" -- \ "$PATCHETTE" verify "$work/corpus/dev" "$TAMPER" -# An orphan patch file (nothing would ever read it) is flagged by verify. +# Folder discordances are errors for every reader: an orphan patch file +# (nothing would ever read it) and a series entry naming no file must be +# refused by verify and import alike. clone_layout orphan cp "$TAMPER/$fancy/note-to-self.patch" "$TAMPER/$fancy/stray.patch" -EXPECT refusal "an orphan patch file is flagged by verify" "orphan" -- \ +EXPECT refusal "an orphan patch file is refused by verify" "orphan" -- \ + "$PATCHETTE" verify "$work/corpus/dev" "$TAMPER" +git clone -q "$work/corpus/upstream" "$work/orphantarget" +EXPECT refusal "an orphan patch file is refused by import" "orphan" -- \ + "$PATCHETTE" import "$work/orphantarget" "$TAMPER" +EXPECT equal "the refused import created no refs" -- \ + "$(git -C "$work/orphantarget" for-each-ref --format='%(refname)' 'refs/heads/*' --exclude refs/heads/main)" "" + +clone_layout missingfile +rm "$TAMPER/$fancy/note-to-self.patch" +EXPECT refusal "a series entry naming no file is refused by verify" "note-to-self.patch is missing" -- \ "$PATCHETTE" verify "$work/corpus/dev" "$TAMPER" +EXPECT refusal "a series entry naming no file is refused by import" "note-to-self.patch is missing" -- \ + "$PATCHETTE" import "$work/orphantarget" "$TAMPER" # Export never deletes a folder it didn't write. mkdir -p "$work/notours/$fancy" diff --git a/tests/test-optional-tip.sh b/tests/test-optional-tip.sh new file mode 100755 index 0000000..ecbda59 --- /dev/null +++ b/tests/test-optional-tip.sh @@ -0,0 +1,29 @@ +#!/bin/bash +## The tip line in STACK is optional: export always writes it, but a hand-made +## STACK without one must replay fine. Verify then does everything except the +## tip comparison and says so; import simply skips its advisory comparison. +set -euo pipefail +cd "$(dirname "$0")" +source ./lib.sh + +work=$(mktemp -d) +trap 'rm -rf "$work"' EXIT + +make_corpus "$work/corpus" +source "$work/corpus/FACTS" + +"$PATCHETTE" export "$work/corpus/dev" "$work/patches" "$base" topic/fancy >/dev/null +sed -i '/^tip /d' "$work/patches/topic¦fancy/STACK" + +"$PATCHETTE" verify "$work/corpus/dev" "$work/patches" >"$work/verify.out" +EXPECT success "verify passes on a tipless STACK, noting it couldn't cross-check" -- \ + grep -q "records no tip to cross-check" "$work/verify.out" + +git clone -q "$work/corpus/upstream" "$work/fresh" +"$PATCHETTE" import "$work/fresh" "$work/patches" >"$work/import.out" +EXPECT equal "import reproduces the branch from a tipless STACK" -- \ + "$(git -C "$work/fresh" rev-parse refs/heads/topic/fancy)" "$tip_topic_fancy" +EXPECT equal "import stays silent about tips it cannot compare" -- \ + "$(grep -c 'note' "$work/import.out")" "0" + +exit "$RESULT"