git-ents.gitmain
⌘K
foforge
commit 98703d7
fix: kill orphaned in-sprite processes before wiping the work directory

sprite exec only tethers the local CLI process, so a worker killed mid-run (a deploy, a restart) leaves its in-sprite build alive; the next run’s wipe then races the orphan’s writes and fails with "Directory not empty". The unpack script now kills any process whose cwd is under the work directory before removing it.

Assisted-by: Claude:claude-haiku-4-5-20251001 Assisted-by: Claude:claude-fable-5

Joseph D. Carpinelli · 1 month ago

Reviews

No reviews of this commit yet — record a verdict below.

Start a review

verdict

crates/git-effect/src/engine.rs @@ -579,10 +579,9 @@ Ok(()) } -/// Stream the pushed tree at `new` into the Sprite's [`WORKDIR`], replacing any -/// previous contents while leaving the rest of the persistent filesystem (build -/// caches and the like) intact. `git archive` emits the tree as a tar that the -/// Sprite unpacks over stdin. +/// Stream the pushed tree at `new` into the Sprite's [`WORKDIR`] via +/// [`unpack_script`]. `git archive` emits the tree as a tar that the Sprite +/// unpacks over stdin. /// /// ## Requirements /// @@ -598,7 +597,7 @@ return Err(format!("git archive failed for {new}")); } - let script = format!("rm -rf {WORKDIR} && mkdir -p {WORKDIR} && tar -x -C {WORKDIR}"); + let script = unpack_script(); let mut child = Command::new("sprite") .args(["exec", "-s", sprite, "--", "sh", "-c", &script]) .stdin(Stdio::piped()) @@ -620,6 +619,27 @@ } } +/// The in-sprite script that replaces [`WORKDIR`]'s contents with the tar +/// streamed over stdin, leaving the rest of the persistent filesystem (build +/// caches and the like) intact. +/// +/// It first kills any process still working under [`WORKDIR`]: a worker +/// killed mid-run (a deploy, a restart) leaves its in-sprite build processes +/// alive, since `sprite exec` only tethers the local CLI process — and an +/// orphaned build still writing under [`WORKDIR`] races the wipe, failing +/// `rm -rf` with "Directory not empty". The final `rm -rf && mkdir && tar` +/// chain is what the exec's exit status reflects, as before. +fn unpack_script() -> String { + format!( + "for cwd in /proc/[0-9]*/cwd; do\n\ + case \"$(readlink \"$cwd\" 2>/dev/null)\" in\n\ + {WORKDIR}|{WORKDIR}/*) kill -9 \"$(basename \"${{cwd%/cwd}}\")\" 2>/dev/null || true ;;\n\ + esac\n\ + done\n\ + rm -rf {WORKDIR} && mkdir -p {WORKDIR} && tar -x -C {WORKDIR}" + ) +} + /// Resolve and extract every distinct toolchain named across `runnable`, /// returning each name's extracted `bin` directory inside the Sprite. A /// failed resolution (the named ref does not exist) is the one place @@ -1170,6 +1190,16 @@ ); } + // @relation(checks.sandbox, role=Verifies) + #[test] + fn unpack_script_kills_stale_processes_before_the_wipe() { + let script = unpack_script(); + let kill = script.find("kill -9").unwrap(); + let wipe = script.find("rm -rf").unwrap(); + assert!(kill < wipe); + assert!(script.ends_with("rm -rf /work && mkdir -p /work && tar -x -C /work")); + } + // @relation(checks.sandbox, role=Verifies) #[test] fn components_key_includes_the_layout() {