fix: a failed export leaves the patches dir as it found it
commit 8df2a82a3a47ae698e6ccffb61de0c65965e39bb · parent d6aa76ad39
· browse this tree
Ximianth, 2026-10-18 12:57:59 UTC
Export used to clear the branch folder up front and write into it directly, so dying mid-branch (an out-of-range parent, say) destroyed the previous snapshot and left a half-written folder -- one with a STACK file, indistinguishable at a glance from a real snapshot. Each branch now builds in a stage folder beside its destination and is swapped in only once complete. The exposure shrinks from the whole export to the instant between removing the old folder and renaming the stage (same filesystem, so the move is one rename). A death mid-build leaves the stage to the exit trap, so no debris either; both properties were observed failing against the previous code.
diff --git a/bin/patchette b/bin/patchette index 6cfaa2f..a2df97b 100755 --- a/bin/patchette +++ b/bin/patchette @@ -46,7 +46,10 @@ export GIT_ATTR_NOSYSTEM=1 export LC_ALL=C.UTF-8 PTMP=$(mktemp -d) -trap 'rm -rf "$PTMP"' EXIT +# EXPORT_STAGE is a half-built branch folder; it lives next to its final +# destination (not in PTMP), so it also needs cleanup if we die mid-build. +EXPORT_STAGE='' +trap 'rm -rf "$PTMP" ${EXPORT_STAGE:+"$EXPORT_STAGE"}' EXIT ## Report a refusal and quit. ## @@ -295,16 +298,20 @@ cmd_export() { # Re-exporting a tracked branch replaces its folder; that's the # normal snapshot flow. But never delete things we didn't write. [[ -f $dir/STACK ]] || die "$dir exists but has no STACK file; not a patchette branch folder, refusing to replace it" - rm -rf "$dir" fi - mkdir "$dir" + # Build in a stage folder beside the destination, and swap in only a + # finished snapshot: a death mid-export must leave the previous + # snapshot untouched, and no half-written folder posing as a real one. + # (Ref components never start with a dot, so the name can't collide.) + EXPORT_STAGE="$out/.stage.$$" + mkdir "$EXPORT_STAGE" { printf 'ref refs/heads/%s\n' "$branch" printf 'base %s\n' "$base" printf 'tip %s\n' "$tip" printf 'series\n' - } >"$dir/STACK" + } >"$EXPORT_STAGE/STACK" declare -A slug_of=() # commit sha -> assigned slug declare -A taken=() # slug -> 1 @@ -351,12 +358,15 @@ cmd_export() { done <"$msg" printf -- '----\n\n' git diff-tree -r -p --full-index --binary --no-renames --diff-algorithm=patience "$sha^" "$sha" - } >"$dir/$slug.patch" + } >"$EXPORT_STAGE/$slug.patch" - printf '%s\n' "$slug" >>"$dir/STACK" + printf '%s\n' "$slug" >>"$EXPORT_STAGE/STACK" count=$((count + 1)) done < <(git rev-list --reverse --topo-order "$base..$tip") + rm -rf "$dir" + mv "$EXPORT_STAGE" "$dir" + EXPORT_STAGE='' echo "export: wrote $dir ($count patches)" done } diff --git a/tests/test-failclosed.sh b/tests/test-failclosed.sh index e7a5616..ed27b1a 100755 --- a/tests/test-failclosed.sh +++ b/tests/test-failclosed.sh @@ -70,6 +70,25 @@ EXPECT refusal "export into a non-patchette folder is refused" "no STACK file" - EXPECT success "export left the non-patchette folder untouched" -- \ test -f "$work/notours/$fancy/precious.txt" +# A failed export must leave the patches dir as it found it: the previous +# snapshot intact, and no half-written folder that could pass for a real one. +# Trigger: rewrite topic/fancy onto a commit forked from BEFORE the base, +# so its parent is out of range and export dies mid-branch. +cp -a "$work/corpus/dev" "$work/poisoned" +git -C "$work/poisoned" switch -qc side "$base^" +git -C "$work/poisoned" commit -q --allow-empty -m "Out of range" +git -C "$work/poisoned" branch -f topic/fancy side +clone_layout atomic +EXPECT refusal "export dies on an out-of-range parent" "out-of-range" -- \ + "$PATCHETTE" export "$work/poisoned" "$TAMPER" "$base" topic/fancy docs-polish +EXPECT success "the failed re-export left the previous snapshot untouched" -- \ + diff -r "$work/pristine" "$TAMPER" +mkdir "$work/freshout" +EXPECT refusal "export into a fresh dir dies the same way" "out-of-range" -- \ + "$PATCHETTE" export "$work/poisoned" "$work/freshout" "$base" topic/fancy +EXPECT equal "the failed fresh export left no debris behind" -- \ + "$(ls -A "$work/freshout")" "" + # Importing where the recorded base is absent tells the user what to do. git init -q -b main "$work/empty" EXPECT refusal "import without the recorded base points at fetching" "fetch from upstream first" -- \