Fix distributed peer build failing on VCS stamping in CI - #967
Conversation
The new test-blackbox-distributed job builds miren on each peer via `make bin/miren`, which iso peers runs as root. But /src is owned by the host user, so git's dubious-ownership check aborts `git status` (exit 128) and Go's build VCS stamping fails the whole build. The standalone dev flow (hack/dev.sh) sidesteps this by building as the host user; the peers path never reconciled that and builds as root. Mark /src a safe directory before building, the same fix hack/package-release.sh uses.
There was a problem hiding this comment.
🍪 biscuit: ✅ ready to merge — auto-review, non-blocking
This is a focused, single-purpose fix: the distributed peer build was failing in CI because make bin/miren runs as root inside the peer, but /src is owned by the host user — causing Git's dubious-ownership check to abort git status (exit 128) and break Go's VCS build stamping.
The fix — git config --global --add safe.directory /src — is exactly right and consistent with how hack/package-release.sh handles the same scenario (confirmed at line 9 of that file). The comment is clear and accurate about the root cause and why the standalone dev flow (hack/dev.sh) doesn't hit this.
One thing worth noting for a future PR: the rebuild command (line 229 in the base) runs iso peers exec --all -- make bin/miren directly, bypassing bootstrap_peer, so it would also fail in CI for the same ownership reason. This pre-exists the change and isn't made worse by it, but it's worth tracking. Since this is a draft, I'd flag it as something to consider before graduating — the rebuild path is incomplete without the same safe.directory guard.
Otherwise the change is clean and the reasoning is solid. Ready to graduate to human review once the rebuild path is considered.
🍪 full review note · comment /biscuit review to run biscuit again.
Inline comments
hack/dev-distributed:229
The rebuild path calls iso peers exec --all -- make bin/miren directly, bypassing bootstrap_peer, so it won't set safe.directory and will fail in CI for the same root-cause reason this PR fixes. Consider adding a git config --global --add safe.directory /src invocation here (for all peers) before the make call, or refactoring the safe.directory step out of bootstrap_peer into a shared helper so both paths call it.
🤖 Prompt for AI Agents
In hack/dev-distributed, the `rebuild` case
(around line 229) calls `iso peers exec --all --
make bin/miren` without first configuring git's
safe.directory, so it fails in CI for the same
reason this PR fixes. Before the `iso peers exec
--all -- make bin/miren` line, add a call that
sets safe.directory on all peers, e.g. `iso peers
exec --all -- git config --global --add
safe.directory /src`. Alternatively, extract the
safe.directory step from bootstrap_peer into a
shared helper function and call it in both
bootstrap_peer and the rebuild case.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughUpdated Comment |
The
test-blackbox-distributedjob I just added went green nowhere it mattered: it skips on PRs and only runs on main, so its first real execution was right after merge, where it promptly fell over. The peers bootstrap builds miren on each peer withmake bin/miren, and iso runs that as root. Since/srcis owned by the host user, git's dubious-ownership guard killsgit statuswith exit 128, and Go's build-time VCS stamping turns that into a hard build failure before any test even runs.Funny little divergence at the root of it: the standalone dev flow (
hack/dev.sh) builds as the host user, so ownership lines up and this never bites. The peers path builds as root and never accounted for it. Nothing exercised that combination until this job started running in CI as root.Fix is the same one
hack/package-release.shalready uses: mark/srca safe directory before building. Both thegit configand the build run throughpeer_execas the same user, so the entry lands in exactly the config the build reads.