diff --git a/go.mod b/go.mod index 2d4931137..f983a2de8 100644 --- a/go.mod +++ b/go.mod @@ -183,4 +183,6 @@ require ( kernel.org/pub/linux/libs/security/libcap/psx v1.2.67 // indirect ) -replace github.com/tonistiigi/fsutil => github.com/earthbuild/fsutil v0.0.0-20261002194825-890baaee2294 +// TODO: fsutil points at the head of the unmerged EarthBuild/fsutil#4 (stacked on #3). +// Advance this to the merged commit on EarthBuild/fsutil earthly-main once those land. +replace github.com/tonistiigi/fsutil => github.com/earthbuild/fsutil v0.0.0-20261009204740-e66a226757f7 diff --git a/go.sum b/go.sum index 545c44553..42a0043a5 100644 --- a/go.sum +++ b/go.sum @@ -155,8 +155,8 @@ github.com/docker/go-units v0.5.0 h1:69rxXcBk27SvSaaxTtLh/8llcHD8vYHT7WSdRZ/jvr4 github.com/docker/go-units v0.5.0/go.mod h1:fgPhTUdO+D/Jk86RDLlptpiXQzgHJF7gydDDbaIK4Dk= github.com/docker/libtrust v0.0.0-20160708172513-aabc10ec26b7 h1:UhxFibDNY/bfvqU5CAUmr9zpesgbU6SWc8/B4mflAE4= github.com/docker/libtrust v0.0.0-20160708172513-aabc10ec26b7/go.mod h1:cyGadeNEkKy96OOhEzfZl+yxihPEzKnqJwvfuSUqbZE= -github.com/earthbuild/fsutil v0.0.0-20261002194825-890baaee2294 h1:gFtQodZi786++bc4OPIeVKIwrQWdP0sJLGCHKWenQmo= -github.com/earthbuild/fsutil v0.0.0-20261002194825-890baaee2294/go.mod h1:vbbYqJlnswsbJqWUcJN8fKtBhnEgldDrcagTgnBVKKM= +github.com/earthbuild/fsutil v0.0.0-20261009204740-e66a226757f7 h1:u29QbXHxE6KijCAUwplJyPexWRL4oMcenhBMFaAgQ2Y= +github.com/earthbuild/fsutil v0.0.0-20261009204740-e66a226757f7/go.mod h1:vbbYqJlnswsbJqWUcJN8fKtBhnEgldDrcagTgnBVKKM= github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4= github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4= github.com/envoyproxy/go-control-plane v0.9.4/go.mod h1:6rpuAdCZL397s3pYoYcLgu1mIlRU8Am5FuJP05cCM98= diff --git a/solver/earthly_progress_fanout_test.go b/solver/earthly_progress_fanout_test.go new file mode 100644 index 000000000..35d3e400b --- /dev/null +++ b/solver/earthly_progress_fanout_test.go @@ -0,0 +1,149 @@ +package solver + +import ( + "context" + "sync" + "testing" + + "github.com/moby/buildkit/client" + "github.com/moby/buildkit/identity" + "github.com/moby/buildkit/util/progress" + "github.com/stretchr/testify/require" +) + +// earthly-specific: these tests characterise how a vertex's log output is +// fanned out to concurrent jobs (separate solve requests) that end up sharing +// work in the scheduler. EarthBuild prints every log line it receives from +// every Status stream, so a log line that is delivered to more than one job is +// printed more than once. + +// runFanoutJobs builds e0 in job j0, then (with j0 still alive) builds e1 in +// job j1, and returns how many times logLine was delivered to each job's +// Status stream. +func runFanoutJobs(t *testing.T, e0, e1 Edge, logLine string) (j0Count, j1Count int) { + t.Helper() + ctx := context.TODO() + + l := NewSolver(SolverOpt{ + ResolveOpFunc: testOpResolver, + DefaultCache: NewInMemoryCacheManager(), + }) + defer l.Close() + + j0, err := l.NewJob("j0") + require.NoError(t, err) + j1, err := l.NewJob("j1") + require.NoError(t, err) + + count := func(j *Job, out *int, wg *sync.WaitGroup) { + defer wg.Done() + ch := make(chan *client.SolveStatus) + go func() { + _ = j.Status(ctx, false, ch) + }() + for ss := range ch { + for _, lg := range ss.Logs { + if string(lg.Data) == logLine { + *out++ + } + } + } + } + + var wg sync.WaitGroup + wg.Add(2) + go count(j0, &j0Count, &wg) + go count(j1, &j1Count, &wg) + + _, err = j0.Build(ctx, e0) + require.NoError(t, err) + + // j0 is intentionally not discarded yet, so its active state (and edge + // index entry) is still live while j1 solves. + _, err = j1.Build(ctx, e1) + require.NoError(t, err) + + j0.CloseProgress() + j1.CloseProgress() + wg.Wait() + + require.NoError(t, j0.Discard()) + require.NoError(t, j1.Discard()) + return j0Count, j1Count +} + +// logOnExec returns an execPreFunc that writes logLine to the vertex's +// progress stream, the same way an ExecOp writes RUN output. +func logOnExec(logLine string) func(context.Context) error { + return func(ctx context.Context) error { + pw, _, _ := progress.NewFromContext(ctx) + defer pw.Close() + return pw.Write(identity.NewID(), client.VertexLog{Stream: 1, Data: []byte(logLine)}) + } +} + +// TestEarthlyMergedEdgeLogsReachMergedJob covers two solves whose vertices +// have different digests but the same cache key, so the scheduler merges j1's +// edge into j0's active edge and the operation runs once. +// +// This is a characterisation test of upstream behaviour, not of +// earthly-specific code. On old EarthBuild main (BuildKit v0.12 base) the log +// line of the merged operation was delivered only to the job that ran it. +// Since upstream commit e1da8b7f ("solver: fix printing progress messages +// after merged edges", first released in v0.13), state.setEdge adds the merge +// source's progress writer to the target, and MultiWriter.Add replays the +// target's history, so j1 also receives the log line of j0's vertex. +// +// BuildKit is deliberately not patched to suppress this (see AGENTS.md): +// streaming vertex output to every attached caller is upstream's contract. +// EarthBuild relies on that contract and must dedupe on its side, by not +// issuing redundant concurrent solves for the same work, or by +// deduplicating log output across the Status streams of concurrent solves +// (earthbuild#1004). If this test starts failing after an upstream bump, the +// fan-out contract has changed and EarthBuild's dedupe must be revisited. +func TestEarthlyMergedEdgeLogsReachMergedJob(t *testing.T) { + t.Parallel() + + const logLine = "merged-edge-log\n" + e0 := Edge{Vertex: vtx(vtxOpt{ + name: "v0", + cacheKeySeed: "same-cache-key", + value: "result", + execPreFunc: logOnExec(logLine), + })} + e1 := Edge{Vertex: vtx(vtxOpt{ + name: "v1", // different digest from v0 + cacheKeySeed: "same-cache-key", + value: "result", + execPreFunc: logOnExec(logLine), + })} + + j0Count, j1Count := runFanoutJobs(t, e0, e1, logLine) + + require.Equal(t, 1, j0Count, "job that ran the operation should see its log once") + require.Equal(t, 1, j1Count, "job whose edge was merged into another job's edge should have that job's log replayed (upstream e1da8b7f)") +} + +// TestEarthlySharedVertexLogsDeliveredToEveryJob covers two solves that load +// a vertex with the identical digest, so both jobs attach to the same active +// state. This is a control case that passes on both old EarthBuild main and +// this merge: loadUnlocked has always added every job's progress writer to +// the shared state's MultiWriter, and MultiWriter.Add replays history, so +// every attached job receives the log line. This fan-out is longstanding +// upstream behaviour, not a regression from the v0.14.1 merge. +func TestEarthlySharedVertexLogsDeliveredToEveryJob(t *testing.T) { + t.Parallel() + + const logLine = "shared-vertex-log\n" + v := vtx(vtxOpt{ + name: "shared", + cacheKeySeed: "shared-cache-key", + value: "result", + execPreFunc: logOnExec(logLine), + }) + + j0Count, j1Count := runFanoutJobs(t, Edge{Vertex: v}, Edge{Vertex: v}, logLine) + + require.Equal(t, 1, j0Count, "job that ran the operation should see its log once") + require.Equal(t, 1, j1Count, "job attached to an already-executed shared vertex should have its log replayed") +} diff --git a/solver/llbsolver/earthly_exporters_test.go b/solver/llbsolver/earthly_exporters_test.go new file mode 100644 index 000000000..4e1f9f788 --- /dev/null +++ b/solver/llbsolver/earthly_exporters_test.go @@ -0,0 +1,81 @@ +package llbsolver + +import ( + "context" + "sync/atomic" + "testing" + + "github.com/moby/buildkit/cache" + "github.com/moby/buildkit/client" + "github.com/moby/buildkit/exporter" + "github.com/moby/buildkit/exporter/containerimage/exptypes" + "github.com/moby/buildkit/exporter/verifier" + "github.com/moby/buildkit/solver" + "github.com/moby/buildkit/solver/result" + "github.com/stretchr/testify/require" +) + +// fakeExporterInstance is a minimal exporter.ExporterInstance that records +// whether Export was called. +type fakeExporterInstance struct { + typ string + exported atomic.Bool +} + +func (e *fakeExporterInstance) ID() int { return 0 } +func (e *fakeExporterInstance) Name() string { return "fake " + e.typ } +func (e *fakeExporterInstance) Config() *exporter.Config { return exporter.NewConfig() } +func (e *fakeExporterInstance) Type() string { return e.typ } +func (e *fakeExporterInstance) Attrs() map[string]string { return nil } +func (e *fakeExporterInstance) Export(_ context.Context, _ *exporter.Source, _ exptypes.InlineCache, _ string) (map[string]string, exporter.DescriptorReference, error) { + e.exported.Store(true) + return nil, nil, nil +} + +// earthly-specific: runExporters skips verifier.CheckInvalidPlatforms only +// when every exporter is the EarthBuild exporter. A non-EarthBuild exporter in +// the same solve must keep the upstream v0.14 result validation rather than +// be handed a multi-ref result without a platforms mapping. +func TestRunExportersValidatesNonEarthlyExporterAlongsideEarthly(t *testing.T) { + t.Parallel() + ctx := context.TODO() + + jl := solver.NewSolver(solver.SolverOpt{}) + defer jl.Close() + job, err := jl.NewJob("j0") + require.NoError(t, err) + defer job.Discard() + + // EarthBuild-shaped result: several named refs, no platforms mapping. + inp := &result.Result[cache.ImmutableRef]{ + Refs: map[string]cache.ImmutableRef{ + "output-0": nil, + "output-1": nil, + }, + } + // Solve always records the frontend request options before exporting. + require.NoError(t, verifier.CaptureFrontendOpts(map[string]string{}, inp)) + + s := &Solver{} + + // Control: with only a non-EarthBuild exporter the upstream check runs. + imageOnly := &fakeExporterInstance{typ: client.ExporterImage} + _, _, err = s.runExporters(ctx, []exporter.ExporterInstance{imageOnly}, nil, job, nil, inp) + require.ErrorContains(t, err, "build result contains multiple refs without platforms mapping") + require.False(t, imageOnly.exported.Load()) + + // EarthBuild-only solve: its named-output refs skip the check. + earthlyOnly := &fakeExporterInstance{typ: client.ExporterEarthly} + _, _, err = s.runExporters(ctx, []exporter.ExporterInstance{earthlyOnly}, nil, job, nil, inp) + require.NoError(t, err) + require.True(t, earthlyOnly.exported.Load()) + + // Mixed solve: the image exporter must still be protected by the check. + earthly := &fakeExporterInstance{typ: client.ExporterEarthly} + image := &fakeExporterInstance{typ: client.ExporterImage} + _, _, err = s.runExporters(ctx, []exporter.ExporterInstance{earthly, image}, nil, job, nil, inp) + require.ErrorContains(t, err, "build result contains multiple refs without platforms mapping", + "platform validation was skipped for the image exporter because an EarthBuild exporter was also present") + require.False(t, image.exported.Load(), "image exporter ran on a result that failed platform validation") + require.False(t, earthly.exported.Load()) +} diff --git a/solver/llbsolver/ops/earthly_exec_mounts_test.go b/solver/llbsolver/ops/earthly_exec_mounts_test.go new file mode 100644 index 000000000..2c2d5408d --- /dev/null +++ b/solver/llbsolver/ops/earthly_exec_mounts_test.go @@ -0,0 +1,49 @@ +package ops + +import ( + "context" + "testing" + + "github.com/moby/buildkit/session" + "github.com/moby/buildkit/solver/pb" + "github.com/stretchr/testify/require" +) + +// earthly-specific +// +// TestExecOpEarthlySpecialMountsNotContentHashed pins the mount shape that +// EarthBuild sends for every RUN (decoded from a real Solve request): the +// rootfs, a host-bind debugger binary, a secret mount and two socket mounts +// (earthly_interactive / earthly_save_file). The secret and socket mounts +// carry Input:0 and no selector even though they do not consume the rootfs. +// +// If getMountDeps treats them as real inputs, the "whole source" branch +// marks input 0 as content-hashed and, being last-mount-wins, overrides the +// rootfs mount's own "no content cache on /" decision. Every RUN then gets a +// content-based cache key for its rootfs, so a RUN following a +// `RUN --no-cache` that did not change the filesystem is wrongly reported as +// *cached*. +func TestExecOpEarthlySpecialMountsNotContentHashed(t *testing.T) { + op := &ExecOp{numInputs: 1, op: &pb.ExecOp{ + Meta: &pb.Meta{Args: []string{"/bin/sh", "-c", "echo hi"}}, + Mounts: []*pb.Mount{ + {Input: 0, Dest: "/", Output: 0}, + {Input: pb.Empty, Selector: "/usr/bin/earth_debugger", Dest: "/usr/bin/earth_debugger", Output: 1, MountType: pb.MountType_HOST_BIND}, + {Input: 0, Dest: "/run/secrets/earthly_debugger_settings", Output: 0, MountType: pb.MountType_SECRET, SecretOpt: &pb.SecretOpt{ID: "earthly_debugger_settings"}}, + {Input: 0, Dest: "/var/run/earthly_interactive", Output: 0, MountType: pb.MountType_SOCKET, SockOpt: &pb.SockOpt{ID: "earthly_interactive"}}, + {Input: 0, Dest: "/var/run/earthly_save", Output: 0, MountType: pb.MountType_SOCKET, SockOpt: &pb.SockOpt{ID: "earthly_save_file"}}, + }, + }} + + deps, err := op.getMountDeps() + require.NoError(t, err) + require.Len(t, deps, 1) + require.False(t, deps[0].ContentBasedHash, "rootfs input must not be content-hashed") + require.Equal(t, []string{"/"}, deps[0].Selectors, "only the rootfs mount should select from input 0") + + m, ok, err := op.CacheMap(context.Background(), session.NewGroup(t.Name()), 1) + require.NoError(t, err) + require.True(t, ok) + require.Len(t, m.Deps, 1) + require.Nil(t, m.Deps[0].ComputeDigestFunc, "rootfs dep must not have a content-based (slow) cache key") +} diff --git a/solver/llbsolver/ops/exec.go b/solver/llbsolver/ops/exec.go index 927533584..8b8bdce48 100644 --- a/solver/llbsolver/ops/exec.go +++ b/solver/llbsolver/ops/exec.go @@ -289,6 +289,13 @@ type dep struct { func (e *ExecOp) getMountDeps() ([]dep, error) { deps := make([]dep, e.numInputs) for _, m := range e.op.Mounts { + switch m.MountType { + case pb.MountType_SECRET, pb.MountType_SSH, pb.MountType_TMPFS: + continue + case pb.MountType_SOCKET: // earthly-specific: socket mounts have no input vertex either + continue + } + if m.Input == pb.Empty { continue } diff --git a/solver/llbsolver/solver.go b/solver/llbsolver/solver.go index 17054a6c9..a61a4b053 100644 --- a/solver/llbsolver/solver.go +++ b/solver/llbsolver/solver.go @@ -739,19 +739,25 @@ func runInlineCacheExporter(ctx context.Context, e exporter.ExporterInstance, in } func (s *Solver) runExporters(ctx context.Context, exporters []exporter.ExporterInstance, inlineCacheExporter inlineCacheExporter, job *solver.Job, cached *result.Result[solver.CachedResult], inp *result.Result[cache.ImmutableRef]) (exporterResponse map[string]string, descrefs []exporter.DescriptorReference, err error) { - var isEarthBuild bool - - // EarthBuild uses res.Refs as a dictionary of named build outputs, not target platforms. - // Platform verification is handled per-image inside the Earthly exporter. + // earthly-specific: the EarthBuild frontend uses inp.Refs as a dictionary + // of named build outputs, not target platforms, and sets no platforms + // mapping, so CheckInvalidPlatforms always rejects its result. The + // EarthBuild exporter does not verify platforms itself either; it only + // groups refs into images by their "platform" metadata. Skip the check + // only when every exporter is the EarthBuild exporter. All exporters + // share inp, so a solve that mixes the EarthBuild exporter with another + // exporter is still checked for the other exporter's sake, which rejects + // an EarthBuild-shaped result. EarthBuild never issues such solves. + onlyEarthBuild := len(exporters) > 0 for _, exp := range exporters { - if exp.Type() == client.ExporterEarthly { - isEarthBuild = true + if exp.Type() != client.ExporterEarthly { + onlyEarthBuild = false break } } var warnings []client.VertexWarning - if !isEarthBuild { + if !onlyEarthBuild { warnings, err = verifier.CheckInvalidPlatforms(ctx, inp) if err != nil { return nil, nil, err diff --git a/vendor/github.com/tonistiigi/fsutil/receive.go b/vendor/github.com/tonistiigi/fsutil/receive.go index afb118bb7..dc7335199 100644 --- a/vendor/github.com/tonistiigi/fsutil/receive.go +++ b/vendor/github.com/tonistiigi/fsutil/receive.go @@ -221,10 +221,6 @@ func (r *receiver) run(ctx context.Context) error { } break } - if r.verboseProgressCb != nil { - r.verboseProgressCb(p.Stat.Path, StatusStat, p.Size()) - } - // normalize unix wire-specific paths to platform-specific paths path := filepath.FromSlash(p.Stat.Path) if filepath.ToSlash(path) != p.Stat.Path { @@ -234,6 +230,12 @@ func (r *receiver) run(ctx context.Context) error { p.Stat.Path = path p.Stat.Linkname = filepath.FromSlash(p.Stat.Linkname) + // earthly-specific: report the native path, matching the + // StatusReceiving/StatusReceived paths (r.files keys) and NotifyHashed. + if r.verboseProgressCb != nil { + r.verboseProgressCb(path, StatusStat, p.Size()) + } + if fileCanRequestData(os.FileMode(p.Stat.Mode)) { r.mu.Lock() r.files[p.Stat.Path] = i diff --git a/vendor/github.com/tonistiigi/fsutil/send.go b/vendor/github.com/tonistiigi/fsutil/send.go index 8e6942215..908ba6473 100644 --- a/vendor/github.com/tonistiigi/fsutil/send.go +++ b/vendor/github.com/tonistiigi/fsutil/send.go @@ -114,9 +114,6 @@ func (s *sender) run(ctx context.Context) error { } func (s *sender) updateProgress(size int, last bool) { - s.mu.Lock() - defer s.mu.Unlock() - s.progressCurrent += size if s.progressCb != nil { s.progressCurrentMu.Lock() defer s.progressCurrentMu.Unlock() @@ -149,7 +146,9 @@ func (s *sender) sendFile(h *sendHandle) error { return err } if s.verboseProgressCb != nil { - s.verboseProgressCb(h.path, StatusSent, fs.bytesWritten) + // h.path is the unix wire path from s.files; report the native + // path, matching the StatusStat path reported by walk. + s.verboseProgressCb(filepath.FromSlash(h.path), StatusSent, fs.bytesWritten) } } return s.conn.SendMsg(&types.Packet{ID: h.id, Type: types.PACKET_DATA}) diff --git a/vendor/modules.txt b/vendor/modules.txt index e59863d8f..49ae4bb20 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -821,7 +821,7 @@ github.com/spdx/tools-golang/spdx/v2/v2_3 ## explicit; go 1.20 github.com/stretchr/testify/assert github.com/stretchr/testify/require -# github.com/tonistiigi/fsutil v0.0.0-20240424095704-91a3fc46842c => github.com/earthbuild/fsutil v0.0.0-20261002194825-890baaee2294 +# github.com/tonistiigi/fsutil v0.0.0-20240424095704-91a3fc46842c => github.com/earthbuild/fsutil v0.0.0-20261009204740-e66a226757f7 ## explicit; go 1.20 github.com/tonistiigi/fsutil github.com/tonistiigi/fsutil/copy @@ -1140,4 +1140,4 @@ kernel.org/pub/linux/libs/security/libcap/cap # kernel.org/pub/linux/libs/security/libcap/psx v1.2.67 ## explicit; go 1.11 kernel.org/pub/linux/libs/security/libcap/psx -# github.com/tonistiigi/fsutil => github.com/earthbuild/fsutil v0.0.0-20261002194825-890baaee2294 +# github.com/tonistiigi/fsutil => github.com/earthbuild/fsutil v0.0.0-20261009204740-e66a226757f7