Merge pull request #3687 from jsternberg/dap-filter-inputs

dap: defer inputs for a step to prevent overeager evaluation

Author: tonistiigi

dap/thread.go

@@ -2,6 +2,7 @@ package dap
 
 import (
 	"context"
+	"maps"
 	"path/filepath"
 	"slices"
 	"strings"
@@ -138,6 +139,11 @@ type step struct {
 	// breakpoint resolution.
 	dgst digest.Digest
 
+	// deferred holds the inputs that should have its evaluation deferred.
+	// These inputs are still included in the references but will only be
+	// evaluated when needed.
+	deferred map[int]bool
+
 	// in holds the next target when step in is used.
 	in *step
 
@@ -241,10 +247,21 @@ func (t *thread) createBranch(dgst digest.Digest, exitpoint *step) (entrypoint *
 			// If this branch is empty (signified by a nil return value) then
 			// skip it.
 			if head.in == nil {
+				// Always mark this input as deferred since it doesn't have
+				// an associated branch.
+				if entrypoint.deferred == nil {
+					entrypoint.deferred = make(map[int]bool)
+				}
+				entrypoint.deferred[i] = true
 				continue
 			}
-
 			entrypoint.dgst = ""
+
+			// Filter this input from the target so it doesn't get solved
+			// when moving to this step.
+			head.deferred = make(map[int]bool)
+			maps.Copy(head.deferred, entrypoint.deferred)
+			head.deferred[i] = true
 			entrypoint = &head
 		}
 
@@ -256,11 +273,12 @@ func (t *thread) createBranch(dgst digest.Digest, exitpoint *step) (entrypoint *
 
 		// Create a new step that refers to the direct parent.
 		head := &step{
-			dgst:   digest.Digest(op.Inputs[entrypoint.parent].Digest),
-			in:     entrypoint,
-			next:   entrypoint,
-			out:    entrypoint.out,
-			parent: -1,
+			dgst:     digest.Digest(op.Inputs[entrypoint.parent].Digest),
+			deferred: entrypoint.deferred,
+			in:       entrypoint,
+			next:     entrypoint,
+			out:      entrypoint.out,
+			parent:   -1,
 		}
 		head.frame = t.getStackFrame(head.dgst, entrypoint)
 		entrypoint = head
@@ -628,6 +646,12 @@ func (t *thread) solveInputs(ctx context.Context, target *step) (string, map[str
 		if err != nil {
 			return "", nil, err
 		}
+
+		// If we have marked this input to be deferred, wrap it in a reference
+		// that suppresses the evaluate call.
+		if target.deferred[i] {
+			ref = &deferredReference{Reference: ref}
+		}
 		refs[k] = ref
 	}
 	return root, refs, nil
@@ -823,3 +847,11 @@ func (r *mountReference) ReadDir(ctx context.Context, req gateway.ReadDirRequest
 		MountIndex:     r.index,
 	})
 }
+
+type deferredReference struct {
+	gateway.Reference
+}
+
+func (r *deferredReference) Evaluate(ctx context.Context) error {
+	return nil
+}

tests/dap_build.go

@@ -7,6 +7,7 @@ import (
 	"path"
 	"runtime"
 	"slices"
+	"strings"
 	"syscall"
 	"testing"
 	"time"
@@ -85,6 +86,7 @@ var dapBuildTests = []func(t *testing.T, sb integration.Sandbox){
 	testDapBuildStepNext,
 	testDapBuildStepOut,
 	testDapBuildVariables,
+	testDapBuildDeferredEval,
 }
 
 func testDapBuild(t *testing.T, sb integration.Sandbox) {
@@ -857,6 +859,61 @@ func testDapBuildVariables(t *testing.T, sb integration.Sandbox) {
 	}
 }
 
+func testDapBuildDeferredEval(t *testing.T, sb integration.Sandbox) {
+	dir := createTestProject(t)
+	client, done, err := dapBuildCmd(t, sb)
+	require.NoError(t, err)
+
+	// Track when we see this message.
+	seen := make(chan struct{}, 1)
+	client.RegisterEvent("output", func(em dap.EventMessage) {
+		e := em.(*dap.OutputEvent)
+		if strings.Contains(e.Body.Output, "RUN cp /etc/foo /etc/bar") {
+			select {
+			case seen <- struct{}{}:
+			default:
+			}
+		}
+	})
+
+	interruptCh := pollInterruptEvents(client)
+	doLaunch(t, client, commands.LaunchConfig{
+		Dockerfile:  path.Join(dir, "Dockerfile"),
+		ContextPath: dir,
+	},
+		dap.SourceBreakpoint{Line: 7},
+	)
+
+	stopped := waitForInterrupt[*dap.StoppedEvent](t, interruptCh)
+	require.NotNil(t, stopped)
+
+	// The output event is usually immediate but it can sometimes be delayed due to
+	// the multithreading in the printer. Just wait for a little bit.
+	select {
+	case <-seen:
+		// We should not have seen this message since the branch this
+		// message comes from should be deferred because we have
+		// not passed the breakpoint.
+		t.Fatal("step has been invoked before intended")
+	case <-time.After(100 * time.Millisecond):
+	}
+
+	doNext(t, client, stopped.Body.ThreadId)
+
+	stopped = waitForInterrupt[*dap.StoppedEvent](t, interruptCh)
+	require.NotNil(t, stopped)
+
+	select {
+	case <-seen:
+		// Wait up to a second for the input to be seen.
+	case <-time.After(time.Second):
+		t.Fatal("step should have been seen")
+	}
+
+	var exitErr *exec.ExitError
+	require.ErrorAs(t, done(true), &exitErr)
+}
+
 func doLaunch(t *testing.T, client *daptest.Client, config commands.LaunchConfig, bps ...dap.SourceBreakpoint) {
 	t.Helper()