[kmannislands]

Stacking

This PR is stacked on #41 (chore-merge-v0.14.1, itself based on chore-merge-v0.14.1-unresolved). It targets chore-merge-v0.14.1 so the diff shows only the review additions. Merge #41 first, or fold these commits into it.

Commit structure: red, then green

The first two commits add red tests that fail on #41's head (a653104a) and pin down each finding. The next two are green fixes that make those tests pass, so each fix can be reviewed against the test that motivated it.

Commit	Kind	What
2d63eacb	red	test(solver): merged-edge logs now reach every attached job
fd768dc5	red	test(llbsolver): platform check skipped for non-earthly exporters
08c7a4e5	green (finding 1)	test(solver): pin merged-edge log fan-out as upstream behaviour. Test-only, no BuildKit change.
f821ecf1	green (finding 2)	fix(llbsolver): only skip platform check when every exporter is earthly

The T8 regression commits (2cd6cbcc, 4f8b3685, 1f6ea531) follow the same red-then-green pattern. See the T8 section below.

Findings

1. Progress duplication across concurrent solves (earthbuild#1004)

Tests: TestEarthlyMergedEdgeLogsDeliveredOnce (red) and the control TestEarthlySharedVertexLogsDeliveredToEveryJob (passes on old main and here), in solver/earthly_progress_fanout_test.go.

go test ./solver/ -run TestEarthly -count=1 -v

Two jobs build vertices with different digests but the same cache key, so the scheduler merges j1's edge into j0's active edge and the operation runs once. On old EarthBuild main (BuildKit v0.12 base) the log line went only to the job that ran it. Since upstream e1da8b7 ("solver: fix printing progress messages after merged edges", first shipped in v0.13 and so part of this merge), state.setEdge adds the merge source's progress writer to the target, and MultiWriter.Add replays history. j1 therefore also receives j0's log line, and EarthBuild, which prints every log line from every Status stream, prints it twice.

This is upstream behaviour pulled in by the merge, not a conflict-resolution mistake. Per this repo's AGENTS.md, BuildKit must not be patched to dedupe progress logs across concurrent solves; that belongs in EarthBuild (avoid redundant solve requests in earthfile2llb/converter.go, or dedupe on the client side). The green commit for this finding therefore does not change BuildKit. It turns the red test into a characterisation test that asserts the upstream behaviour, so EarthBuild's dedupe has a pinned contract to rely on.

2. Platform validation skipped for every exporter if any is earthly

Test: TestRunExportersValidatesNonEarthlyExporterAlongsideEarthly in solver/llbsolver/earthly_exporters_test.go. It is Linux-only, because llbsolver imports cache, which imports overlayutils:

docker run --rm -v "$PWD":/src -w /src -e GOFLAGS=-mod=vendor golang:1.21 \
  go test ./solver/llbsolver/ -run TestRunExporters -count=1 -v

runExporters skips verifier.CheckInvalidPlatforms for the whole solve as soon as any exporter has Type() == client.ExporterEarthly. A non-EarthBuild exporter in the same solve loses the upstream v0.14 validation and runs on a multi-ref result with no platforms mapping. The accompanying comment says platform verification is "handled per-image inside the Earthly exporter", but it is not: earthlyoutputs only builds a per-image platforms mapping from platform metadata and never compares requested platforms with produced ones.

Old main had no exporter/verifier package and no ExporterInstance.Type(), so this is a divergence from upstream v0.14.1, not a regression from old main.

Resolution of finding 1

TestEarthlyMergedEdgeLogsDeliveredOnce became TestEarthlyMergedEdgeLogsReachMergedJob, which asserts that the merged job does receive the replayed line (j0 = 1, j1 = 1). Its comment cites e1da8b7 and says EarthBuild relies on this contract and must dedupe concurrent solves on its side. If an upstream bump changes the fan-out, this test fails and signals that EarthBuild's dedupe must be revisited.

Resolution of finding 2

Every exporter receives the same inp, so a per-exporter check is not possible. The EarthBuild frontend always returns named Refs and never sets refs.platforms, so CheckInvalidPlatforms always rejects an EarthBuild-shaped result. The fix skips the check only when every exporter is earthly (and at least one exporter exists, so the zero-exporter warnings path still runs as upstream). A mixed solve is checked for the non-earthly exporter's sake, which rejects an EarthBuild-shaped result with the upstream error. EarthBuild never issues mixed solves: every SolveOpt in builder/ has exactly one export entry. The misleading comment is corrected and the block is marked // earthly-specific. The red test's expectations are unchanged. An earthly-only case was added so the skip cannot be removed without the test noticing.

T8: RUN --no-cache no longer invalidates the following RUN

Functional test T8 (earthbuild#1004) found a regression. After a RUN --no-cache echo …, the next RUN in the same target, or in a target that does FROM +common, was reported as *cached* on the second run. On old main it re-executed.

Root cause

Every EarthBuild RUN carries three mounts besides the rootfs: a secret mount (/run/secrets/earthly_debugger_settings) and two socket mounts (/var/run/earthly_interactive and /var/run/earthly_save, MountType_SOCKET). None of them consumes an input, but all three arrive with Input: 0 and no selector.

getMountDeps in solver/llbsolver/ops/exec.go treated them as whole-source mounts of input 0. Its ContentBasedHash flag is last-mount-wins per input, so they overrode the rootfs mount's "no content cache on /" decision. As a result, the rootfs dep of every RUN got a content-based cache key, and the daemon logs showed dep_has_slow_cache=true. An echo-only --no-cache RUN does not change the filesystem, so the next RUN's content key matched its old record.

That content-hash logic came from upstream 98bfcf4 ("exec: refactor content-based cache detection") and ab17c1d ("exec: allow content-cache for root selected mounts"), both pulled in by the v0.13.2 merge. Upstream fixed the same issue for secret, SSH and tmpfs mounts in 3c87ba1 (moby/buildkit#5251, "exec: fix incorrect deps computation for special mounts"). That fix alone is not enough here, because EarthBuild's socket mounts still hit the rootfs branch.

Commit map

Commit	Kind	What
2cd6cbcc	red	test(llbsolver/ops): TestExecOpEarthlySpecialMountsNotContentHashed in solver/llbsolver/ops/earthly_exec_mounts_test.go uses the exact mount shape EarthBuild sends (root + host-bind debugger + secret + 2 sockets) and asserts that the rootfs dep is not content-hashed
4f8b3685	upstream cherry-pick	git cherry-pick -x 3c87ba1b, which skips SECRET/SSH/TMPFS. Applied cleanly. The red test still fails at this commit because of the socket mounts
1f6ea531	green	fix(llbsolver/ops): skip earthly socket mounts in getMountDeps. Adds pb.MountType_SOCKET on its own case line, marked // earthly-specific, so the upstream line stays unchanged

Linux only (ops imports cache, which imports overlayutils):

docker run --rm -v "$PWD":/src -w /src -e GOFLAGS=-mod=vendor golang:1.21 \
  go test ./solver/llbsolver/ops/ -count=1 -v

Verification

• Unit: the red test fails on dd17522a and on 4f8b3685 (rootfs input must not be content-hashed) and passes on 1f6ea531. TestExecOpContentCache, TestDedupePaths and the rest of ./solver/llbsolver/ops/... pass.

• End to end: I built a buildkitd image from this branch with earth ./buildkitd+buildkitd --BUILDKIT_PROJECT=<this tree> (earthbuild pr1004 Earthfile) and ran the PR earth binary against it, twice per target:

  Target	Run 2 on the pre-fix buildkitd-dev-pr-1004 image (BuildKit 97a1abd8; ops/ is unchanged up to dd17522a)	Run 2 on 1f6ea531
  +single (RUN --no-cache echo then RUN echo T8AFTER)	*cached* --> RUN echo T8AFTER-$((3+4))	--> RUN echo T8AFTER-$((3+4)) / T8AFTER-7 (re-executed)
  +child (FROM +common, where +common has the --no-cache RUN)	*cached* --> RUN echo T8CHILD-$((5+5))	--> RUN echo T8CHILD-$((5+5)) / T8CHILD-10 (re-executed)
  +plain (two RUNs, no --no-cache, control)	n/a	*cached* --> RUN echo T8PLAIN-… and *cached* --> RUN echo T8PLAIN2-… (normal cache still works)

Follow-up

dd17522a chore(deps): bump earthbuild/fsutil to e66a226 (EarthBuild/fsutil#4: send progress counted once, native paths in verbose progress). fsutil#4 is unmerged; a TODO in go.mod marks the replace to advance to the merged earthly-main commit once EarthBuild/fsutil#3, #6 and #4 land.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Build logs from shared or merged work are now delivered to each affected job, including when jobs share a cache key.
  • Platform validation now applies correctly when EarthBuild and other exporters are used together.
  • Secret, SSH, temporary filesystem, and socket mounts no longer create content-based cache dependencies. Other mount types retain their existing cache behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: EarthBuild/buildkit/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: d5af0c37-5d8c-49fd-9b11-41bdeaac7e85

---

📥 Commits

Reviewing files that changed from the base of the PR and between a653104 and 1f6ea53.

---

⛔ Files ignored due to path filters (4)

• go.sum is excluded by !**/*.sum
• vendor/github.com/tonistiigi/fsutil/receive.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/tonistiigi/fsutil/send.go is excluded by !vendor/**, !**/vendor/**
• vendor/modules.txt is excluded by !vendor/**, !**/vendor/**

---

📒 Files selected for processing (6)

• go.mod
• solver/earthly_progress_fanout_test.go
• solver/llbsolver/earthly_exporters_test.go
• solver/llbsolver/ops/earthly_exec_mounts_test.go
• solver/llbsolver/ops/exec.go
• solver/llbsolver/solver.go

---

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

---

---

📝 Walkthrough

Walkthrough

The pull request updates the fsutil replacement, changes execution mount dependency handling, refines exporter platform validation, and adds tests for concurrent job log delivery and exporter and mount behavior.

Changes

Progress log fanout

Layer / File(s)	Summary
Concurrent job log delivery solver/earthly_progress_fanout_test.go	Adds tests that check whether logs reach both jobs when their edges have matching cache keys or share a vertex. The helper counts matching lines in each job’s status stream.

Execution mount dependencies

Layer / File(s)	Summary
Special mount dependency handling solver/llbsolver/ops/exec.go, solver/llbsolver/ops/earthly_exec_mounts_test.go	getMountDeps skips SECRET, SSH, TMPFS, and SOCKET mounts. The test checks that the rootfs is the only input-0 selector and that the dependency has no content-digest function.

Exporter platform validation

Layer / File(s)	Summary
Exporter validation and tests solver/llbsolver/solver.go, solver/llbsolver/earthly_exporters_test.go	Platform validation is skipped only when every exporter is Earthly and the exporter list is nonempty. The test covers non-Earthly, Earthly-only, and mixed exporter sets.

fsutil replacement update

Layer / File(s)	Summary
Replacement revision go.mod	Updates the fsutil replacement to version v0.0.0-20261009204740-e66a226757f7. Comments describe it as an unmerged head and note a later update to the merged earthly-main commit.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: janishorsts

---

Merge Risk: ⚪ Minimal · up to 1f6ea

No confirmed issue prevents merging after normal checks.

Pre-merge checks | 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title accurately summarizes the pull request as tests and fixes for the v0.14.1 merge and identifies its stacked relationship to #41.
Docstring Coverage		Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 5 files. (1 skipped: 1 u…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

---

---

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

---

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

---

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

🔧 golangci-lint (2.13.2)

golangci-lint (2.13.2) skipped for .golangci.yml: configuration requires version: "2". Migrate older configurations using golangci-lint migrate and review the result; see https://golangci-lint.run/docs/product/migration-guide/.

---

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

---

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