Skip to content

Fix distributed peer build failing on VCS stamping in CI - #967

Merged
evanphx merged 1 commit into
mainfrom
phinze/mir-822-fix-distributed-peer-build-vcs
Jul 23, 2026
Merged

Fix distributed peer build failing on VCS stamping in CI#967
evanphx merged 1 commit into
mainfrom
phinze/mir-822-fix-distributed-peer-build-vcs

Conversation

@phinze

@phinze phinze commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

The test-blackbox-distributed job 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 with make bin/miren, and iso runs that as root. Since /src is owned by the host user, git's dubious-ownership guard kills git status with 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.sh already uses: mark /src a safe directory before building. Both the git config and the build run through peer_exec as the same user, so the entry lands in exactly the config the build reads.

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.

@miren-code-agent miren-code-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🍪 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.

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0f4c9c78-8d29-461a-9940-9ac913d0136c

📥 Commits

Reviewing files that changed from the base of the PR and between cce98bd and 5f2e13d.

📒 Files selected for processing (1)
  • hack/dev-distributed

📝 Walkthrough

Walkthrough

Updated hack/dev-distributed so bootstrap_peer globally marks /src as a Git safe directory inside each peer, allowing Git commands and Go VCS stamping to run when the build executes as root.


Comment @coderabbitai help to get the list of available commands.

@phinze
phinze marked this pull request as ready for review July 23, 2026 22:40
@phinze
phinze requested a review from a team as a code owner July 23, 2026 22:40
@evanphx
evanphx merged commit f661797 into main Jul 23, 2026
22 checks passed
@evanphx
evanphx deleted the phinze/mir-822-fix-distributed-peer-build-vcs branch July 23, 2026 23:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants