Skip to content

Commit 4f3de79

Browse files
committed
dap: properly map source paths to client side paths
Properly map the source paths from the metadata in the solve to the client side paths. The source path returns is relative to the context that gets uploaded which is usually a subdirectory. The original code noticed this when mapping the paths but made the invalid assumption that the dockerfile would always be in the context path so it combined the dockerfile name with the context path. It is possible for the dockerfile to be in a subdirectory of the context. In which case, we computed the paths incorrectly. This modifies DAP to instead use the `DockerfileMappingDst` and `DockerfileMappingSrc` which are special included variables to the inputs that get filled in during the build for the purpose of mapping the source path to the client side path. Tests have also been added for this functionality to ensure it doesn't break again. This should work with both absolute and relative paths although absolute paths should probably be preferred for usage just because they're less likely to result in weird things happening. The sources are also normalized to always convert the source filenames to absolute paths and DAP itself will accept relative paths but will only ever communicate in absolute paths. When you set a breakpoint, it will convert it to an absolute path and reference it in that way rather than a relative path. Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
1 parent c423bc7 commit 4f3de79

6 files changed

Lines changed: 183 additions & 77 deletions

File tree

dap/adapter.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"fmt"
88
"io"
99
"path"
10-
"path/filepath"
1110
"slices"
1211
"sync"
1312
"sync/atomic"
@@ -598,22 +597,22 @@ func (b *breakpointMap) Set(fname string, sbps []dap.SourceBreakpoint) (breakpoi
598597
return breakpoints
599598
}
600599

601-
func (b *breakpointMap) Intersect(ctx Context, src *pb.Source, ws string) map[digest.Digest]int {
600+
func (b *breakpointMap) Intersect(ctx Context, src *pb.Source) map[digest.Digest]int {
602601
b.mu.Lock()
603602
defer b.mu.Unlock()
604603

605604
digests := make(map[digest.Digest]int)
606605

607606
for dgst, locs := range src.Locations {
608-
if id := b.intersect(ctx, src, locs, ws); id > 0 {
607+
if id := b.intersect(ctx, src, locs); id > 0 {
609608
digests[digest.Digest(dgst)] = id
610609
}
611610
}
612611

613612
// Mark unverified breakpoints as failed at this point since we couldn't find an area
614613
// in the source where they applied.
615614
for _, info := range src.Infos {
616-
fname := filepath.Join(ws, info.Filename)
615+
fname := info.Filename
617616

618617
bps := b.byPath[fname]
619618
for _, bp := range bps {
@@ -633,7 +632,7 @@ func (b *breakpointMap) Intersect(ctx Context, src *pb.Source, ws string) map[di
633632
return digests
634633
}
635634

636-
func (b *breakpointMap) intersect(ctx Context, src *pb.Source, locs *pb.Locations, ws string) int {
635+
func (b *breakpointMap) intersect(ctx Context, src *pb.Source, locs *pb.Locations) int {
637636
overlaps := func(r *pb.Range, bp *dap.Breakpoint) bool {
638637
if bp.Line < int(r.Start.Line) || bp.Line > int(r.End.Line) {
639638
return false
@@ -654,7 +653,7 @@ func (b *breakpointMap) intersect(ctx Context, src *pb.Source, locs *pb.Location
654653
r := loc.Ranges[0]
655654

656655
info := src.Infos[loc.SourceIndex]
657-
fname := filepath.Join(ws, info.Filename)
656+
fname := info.Filename
658657

659658
bps := b.byPath[fname]
660659
if len(bps) == 0 {

dap/adapter_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -187,12 +187,12 @@ func TestBreakpointMapIntersectVerified(t *testing.T) {
187187
src := &pb.Source{
188188
Locations: srcLocs,
189189
Infos: []*pb.SourceInfo{
190-
{Filename: filename},
190+
{Filename: fpath},
191191
},
192192
}
193193

194194
ctx := newBreakpointTestContext(t)
195-
digests := bm.Intersect(ctx, src, ws)
195+
digests := bm.Intersect(ctx, src)
196196
wantMatches := 0
197197
for _, bc := range breakpointCases {
198198
if bc.expectVerified {

dap/thread.go

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@ package dap
22

33
import (
44
"context"
5-
"path"
65
"path/filepath"
76
"slices"
7+
"strings"
88
"sync"
99

1010
"github.com/docker/buildx/build"
@@ -31,10 +31,10 @@ type thread struct {
3131
variables *variableReferences
3232

3333
// Inputs to the evaluate call.
34-
c gateway.Client
35-
ref gateway.Reference
36-
meta map[string][]byte
37-
sourcePath string
34+
c gateway.Client
35+
ref gateway.Reference
36+
meta map[string][]byte
37+
sourceInfoMap func(*pb.Source) *pb.Source
3838

3939
// LLB state for the evaluate call.
4040
def *llb.Definition
@@ -107,14 +107,21 @@ func (t *thread) init(ctx Context, c gateway.Client, ref gateway.Reference, meta
107107
t.c = c
108108
t.ref = ref
109109
t.meta = meta
110-
111-
// Combine the dockerfile directory with the context path to find the
112-
// real base path. The frontend will report the base path as the filename.
113-
dir := path.Dir(inputs.DockerfilePath)
114-
if !path.IsAbs(dir) {
115-
dir = path.Join(inputs.ContextPath, dir)
110+
t.sourceInfoMap = func(s *pb.Source) *pb.Source {
111+
s = s.CloneVT()
112+
for _, sinfo := range s.Infos {
113+
// Map the filename from the source info from the frontend location to the
114+
// client location.
115+
fname := strings.Replace(sinfo.Filename, inputs.DockerfileMappingDst, inputs.DockerfileMappingSrc, 1)
116+
117+
// Convert to an absolute path.
118+
if abspath, err := filepath.Abs(fname); err == nil {
119+
fname = abspath
120+
}
121+
sinfo.Filename = fname
122+
}
123+
return s
116124
}
117-
t.sourcePath = dir
118125

119126
if err := t.getLLBState(ctx); err != nil {
120127
return err
@@ -252,7 +259,7 @@ func (t *thread) getStackFrame(dgst digest.Digest, next *step) *frame {
252259
f.setNameFromMeta(meta)
253260
}
254261
if loc, ok := t.def.Source.Locations[string(dgst)]; ok {
255-
f.fillLocation(t.def, loc, t.sourcePath, next)
262+
f.fillLocation(t.def, loc, next)
256263
}
257264
t.frames[int32(f.Id)] = f
258265
return f
@@ -296,7 +303,6 @@ func (t *thread) reset() {
296303
t.c = nil
297304
t.ref = nil
298305
t.meta = nil
299-
t.sourcePath = ""
300306
t.ops = nil
301307
}
302308

@@ -462,9 +468,12 @@ func (t *thread) getLLBState(ctx Context) error {
462468
return err
463469
}
464470

471+
if t.sourceInfoMap != nil {
472+
t.def.Source = t.sourceInfoMap(t.def.Source)
473+
}
474+
465475
for _, src := range t.def.Source.Infos {
466-
fname := filepath.Join(t.sourcePath, src.Filename)
467-
t.sourceMap.Put(ctx, fname, src.Data)
476+
t.sourceMap.Put(ctx, src.Filename, src.Data)
468477
}
469478

470479
t.ops = make(map[digest.Digest]*pb.Op, len(t.def.Def))
@@ -483,7 +492,7 @@ func (t *thread) getLLBState(ctx Context) error {
483492
}
484493

485494
func (t *thread) setBreakpoints(ctx Context) {
486-
t.bps = t.breakpointMap.Intersect(ctx, t.def.Source, t.sourcePath)
495+
t.bps = t.breakpointMap.Intersect(ctx, t.def.Source)
487496
}
488497

489498
func (t *thread) seekNext(ctx Context, from *step, action stepType) (string, *step, map[string]gateway.Reference, error) {

dap/variables.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ func (f *frame) setNameFromMeta(meta llb.OpMetadata) {
3838
// TODO: should we infer the name from somewhere else?
3939
}
4040

41-
func (f *frame) fillLocation(def *llb.Definition, loc *pb.Locations, ws string, next *step) {
41+
func (f *frame) fillLocation(def *llb.Definition, loc *pb.Locations, next *step) {
4242
for _, l := range loc.Locations {
4343
for _, r := range l.Ranges {
4444
if next != nil && f.Line != 0 {
@@ -57,7 +57,7 @@ func (f *frame) fillLocation(def *llb.Definition, loc *pb.Locations, ws string,
5757
info := def.Source.Infos[l.SourceIndex]
5858
f.Source = &dap.Source{
5959
Name: path.Base(info.Filename),
60-
Path: filepath.Join(ws, info.Filename),
60+
Path: info.Filename,
6161
}
6262

6363
// If we do not have a next operation, then we don't have

0 commit comments

Comments
 (0)