Skip to content

Commit ea99cfb

Browse files
committed
fix(snapshot): verify revert deletions against a full tree listing
The per-file 'ls-tree <hash> -- <pathspec>' check behaved differently across platforms for unusual filenames, so revert either kept files it should delete or vice versa depending on OS. List the snapshot tree once per patch with core.quotepath=false (the same mechanism patch() uses) and check membership in code: deterministic on every platform, one git call per snapshot instead of per file, and still fails safe — if the listing errors, files are kept rather than deleted on an unverified miss.
1 parent 19cf52b commit ea99cfb

1 file changed

Lines changed: 30 additions & 15 deletions

File tree

backend/cli/src/snapshot/index.ts

Lines changed: 30 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,33 @@ export namespace Snapshot {
158158
const files = new Set<string>()
159159
const git = gitdir()
160160
for (const item of patches) {
161+
// One full listing per snapshot instead of a per-file ls-tree pathspec:
162+
// membership is checked in code on quotepath=false output, the same
163+
// mechanism patch() uses, so unusual filenames behave identically on
164+
// every platform. If the listing itself fails we know nothing about the
165+
// snapshot, and deleting on an unverified miss would be destructive —
166+
// keep files in that case.
167+
const listing = await $`git -c core.quotepath=false --git-dir ${git} --work-tree ${Instance.worktree} ls-tree -r --name-only ${item.hash}`
168+
.quiet()
169+
.cwd(Instance.worktree)
170+
.nothrow()
171+
const snapshotFiles =
172+
listing.exitCode === 0
173+
? new Set(
174+
listing
175+
.text()
176+
.split("\n")
177+
.map((x) => x.trim())
178+
.filter(Boolean)
179+
.map((x) => path.join(Instance.worktree, x)),
180+
)
181+
: undefined
182+
if (!snapshotFiles)
183+
log.warn("could not list snapshot tree", {
184+
hash: item.hash,
185+
exitCode: listing.exitCode,
186+
stderr: listing.stderr.toString(),
187+
})
161188
for (const file of item.files) {
162189
if (files.has(file)) continue
163190
log.info("reverting", { file, hash: item.hash })
@@ -166,21 +193,9 @@ export namespace Snapshot {
166193
.cwd(Instance.worktree)
167194
.nothrow()
168195
if (result.exitCode !== 0) {
169-
const relativePath = path.relative(Instance.worktree, file)
170-
const checkTree =
171-
await $`git --git-dir ${git} --work-tree ${Instance.worktree} ls-tree ${item.hash} -- ${relativePath}`
172-
.quiet()
173-
.cwd(Instance.worktree)
174-
.nothrow()
175-
if (checkTree.exitCode !== 0) {
176-
// Deleting on an unverified miss is destructive: if ls-tree itself
177-
// failed we know nothing about the snapshot, so keep the file.
178-
log.warn("could not verify file against snapshot, keeping", {
179-
file,
180-
exitCode: checkTree.exitCode,
181-
stderr: checkTree.stderr.toString(),
182-
})
183-
} else if (checkTree.text().trim()) {
196+
if (!snapshotFiles) {
197+
log.warn("could not verify file against snapshot, keeping", { file })
198+
} else if (snapshotFiles.has(file)) {
184199
log.info("file existed in snapshot but checkout failed, keeping", {
185200
file,
186201
})

0 commit comments

Comments
 (0)