Merge pull request #3691 from jsternberg/dap-entrypoint-breakpoint

dap: fix skipped breakpoint when the breakpoint and the entrypoint were the same

Author: tonistiigi

dap/thread.go

@@ -71,18 +71,20 @@ func (t *thread) Evaluate(ctx Context, c gateway.Client, headRef gateway.Referen
 	}
 	defer t.reset()
 
+	var next *step
 	action := stepContinue
 	if cfg.StopOnEntry {
-		action = stepNext
+		// If we are stopping on entry, automatically advance to the
+		// entrypoint.
+		action, next = stepNext, t.entrypoint
 	}
 
 	var (
 		k    string
 		refs map[string]gateway.Reference
-		next = t.entrypoint
 		err  error
 	)
-	for next != nil {
+	for {
 		event := t.needsDebug(next, action, err)
 		if event.Reason != "" {
 			select {
@@ -98,7 +100,9 @@ func (t *thread) Evaluate(ctx Context, c gateway.Client, headRef gateway.Referen
 		}
 
 		t.setBreakpoints(ctx)
-		k, next, refs, err = t.seekNext(ctx, next, action)
+		if k, next, refs, err = t.seekNext(ctx, next, action); next == nil {
+			break
+		}
 	}
 	return nil
 }
@@ -509,19 +513,24 @@ func (t *thread) setBreakpoints(ctx Context) {
 }
 
 func (t *thread) seekNext(ctx Context, from *step, action stepType) (string, *step, map[string]gateway.Reference, error) {
-	// If we're at the end, return no digest to signal that
-	// we should conclude debugging.
-	var target *step
+	// Determine how we are going to limit the scan for the next step.
+	var limit func(s *step) *step
 	switch action {
 	case stepNext:
-		target = t.continueDigest(from, from.next)
+		limit = func(s *step) *step {
+			return s.next
+		}
 	case stepIn:
-		target = from.in
+		limit = func(s *step) *step {
+			return s.in
+		}
 	case stepOut:
-		target = t.continueDigest(from, from.out)
-	case stepContinue:
-		target = t.continueDigest(from, nil)
+		limit = func(s *step) *step {
+			return s.out
+		}
 	}
+
+	target := t.continueDigest(from, limit)
 	return t.seek(ctx, target)
 }
 
@@ -547,8 +556,10 @@ func (t *thread) seek(ctx Context, target *step) (k string, result *step, mounts
 	return k, result, refs, nil
 }
 
-func (t *thread) continueDigest(from, until *step) *step {
-	if len(t.bps) == 0 && until == nil {
+func (t *thread) continueDigest(from *step, limit func(*step) *step) *step {
+	// First chance to exit early. If there's no function for limiting
+	// the until step and no breakpoints then just go directly to the end step.
+	if len(t.bps) == 0 && limit == nil {
 		return nil
 	}
 
@@ -561,6 +572,27 @@ func (t *thread) continueDigest(from, until *step) *step {
 		return ok
 	}
 
+	// Special case. When we aren't coming from any step we consider
+	// whether the entrypoint itself is a breakpoint. If it is, we stop
+	// there. Otherwise, we treat the entrypoint as the from location.
+	if from == nil {
+		if isBreakpoint(t.entrypoint.dgst) {
+			return t.entrypoint
+		}
+		from = t.entrypoint
+	}
+
+	var until *step
+	if limit != nil {
+		until = limit(from)
+	}
+
+	// Second chance to exit early. If we've fully resolved from and the
+	// limit function doesn't return an end step, just go directly to the end.
+	if len(t.bps) == 0 && until == nil {
+		return nil
+	}
+
 	next := func(s *step) *step {
 		cur := s.in
 		for cur != nil && cur != until {

tests/dap_build.go

@@ -78,6 +78,7 @@ var dapBuildTests = []func(t *testing.T, sb integration.Sandbox){
 	testDapBuild,
 	testDapBuildStopOnEntry,
 	testDapBuildSetBreakpoints,
+	testDapBuildEntryBreakpoint,
 	testDapBuildVerifiedBreakpoints,
 	testDapBuildLoadedSource,
 	testDapBuildStepIn,
@@ -121,17 +122,19 @@ func testDapBuildStopOnEntry(t *testing.T, sb integration.Sandbox) {
 	})
 
 	stopped := waitForInterrupt[*dap.StoppedEvent](t, interruptCh)
+	require.Equal(t, "step", stopped.Body.Reason)
+
 	threads := doThreads(t, client)
 	require.ElementsMatch(t, []int{stopped.Body.ThreadId}, threads)
 
-	stackTraceResp := <-daptest.DoRequest[*dap.StackTraceResponse](t, client, &dap.StackTraceRequest{
-		Request: dap.Request{Command: "stackTrace"},
-		Arguments: dap.StackTraceArguments{
-			ThreadId: stopped.Body.ThreadId,
+	stackFrames := doStackTrace(t, client, stopped.Body.ThreadId)
+	assertStackTrace(t, stackFrames, []stackFrameMatcher{
+		{
+			SourceName: "Dockerfile",
+			Line:       7,
+			Name:       `^\[stage-1 .*\] COPY`,
 		},
 	})
-	require.True(t, stackTraceResp.Success)
-	require.Len(t, stackTraceResp.Body.StackFrames, 1)
 
 	var exitErr *exec.ExitError
 	require.ErrorAs(t, done(true), &exitErr)
@@ -202,6 +205,39 @@ func testDapBuildSetBreakpoints(t *testing.T, sb integration.Sandbox) {
 	require.NoError(t, done(false))
 }
 
+// testDapBuildEntryBreakpoint checks that the entrypoint is a valid breakpoint.
+func testDapBuildEntryBreakpoint(t *testing.T, sb integration.Sandbox) {
+	dir := createTestProject(t)
+	client, done, err := dapBuildCmd(t, sb, withArgs(dir))
+	require.NoError(t, err)
+
+	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.Equal(t, "breakpoint", stopped.Body.Reason)
+
+	threads := doThreads(t, client)
+	require.ElementsMatch(t, []int{stopped.Body.ThreadId}, threads)
+
+	stackFrames := doStackTrace(t, client, stopped.Body.ThreadId)
+	assertStackTrace(t, stackFrames, []stackFrameMatcher{
+		{
+			SourceName: "Dockerfile",
+			Line:       7,
+			Name:       `^\[stage-1 .*\] COPY`,
+		},
+	})
+
+	var exitErr *exec.ExitError
+	require.ErrorAs(t, done(true), &exitErr)
+}
+
 func testDapBuildVerifiedBreakpoints(t *testing.T, sb integration.Sandbox) {
 	dir := createTestProject(t)
 	client, done, err := dapBuildCmd(t, sb, withArgs(dir))