dap: support delveCwd and use a temporary file as default debug binary (#2660)

service/dap/server.go

@@ -17,6 +17,7 @@ import (
 	"go/constant"
 	"go/parser"
 	"io"
+	"io/ioutil"
 	"net"
 	"os"
 	"os/exec"
@@ -74,7 +75,11 @@ import (
 // error or responding to a (synchronous) DAP disconnect request.
 // Once stop is triggered, the goroutine exits.
 //
-// TODO(polina): add another layer of per-client goroutines to support multiple clients
+// TODO(polina): add another layer of per-client goroutines to support multiple clients.
+// Note that some requests change the process's environment such
+// as working directory (for example, see DelveCwd of launch configuration).
+// So if we want to reuse this process for multiple debugging sessions
+// we need to address that.
 //
 // (3) Per-request goroutine is started for each asynchronous request
 // that resumes execution. We check if target is running already, so
@@ -807,10 +812,29 @@ func (s *Session) setClientCapabilities(args dap.InitializeRequestArguments) {
 	s.clientCapabilities.supportsVariableType = args.SupportsVariableType
 }
 
-// Default output file pathname for the compiled binary in debug or test modes,
+// Default output file pathname for the compiled binary in debug or test modes
-// relative to the current working directory of the server.
+// when temporary debug binary creation fails.
+// This is relative to the current working directory of the server.
 const defaultDebugBinary string = "./__debug_bin"
 
+func (s *Session) tempDebugBinary() string {
+	binaryPattern := "__debug_bin"
+	if runtime.GOOS == "windows" {
+		binaryPattern = "__debug_bin*.exe"
+	}
+	f, err := ioutil.TempFile("", binaryPattern)
+	if err != nil {
+		s.config.log.Errorf("failed to create a temporary binary (%v), falling back to %q", err, defaultDebugBinary)
+		return cleanExeName(defaultDebugBinary)
+	}
+	name := f.Name()
+	if err := f.Close(); err != nil {
+		s.config.log.Errorf("failed to create a temporary binary (%v), falling back to %q", err, defaultDebugBinary)
+		return cleanExeName(defaultDebugBinary)
+	}
+	return name
+}
+
 func cleanExeName(name string) string {
 	if runtime.GOOS == "windows" && filepath.Ext(name) != ".exe" {
 		return name + ".exe"
@@ -832,6 +856,14 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) {
 		return
 	}
 
+	if args.DelveCwd != "" {
+		if err := os.Chdir(args.DelveCwd); err != nil {
+			s.sendShowUserErrorResponse(request.Request,
+				FailedToLaunch, "Failed to launch", fmt.Sprintf("failed to chdir using %q - %v", args.DelveCwd, err))
+			return
+		}
+	}
+
 	mode := args.Mode
 	if mode == "" {
 		mode = "debug"
@@ -887,15 +919,18 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) {
 
 	// Prepare the debug executable filename, build flags and build it
 	if mode == "debug" || mode == "test" {
-		output := args.Output
+		debugbinary := args.Output
-		if output == "" {
+		if debugbinary == "" {
-			output = defaultDebugBinary
+			debugbinary = s.tempDebugBinary()
+		} else {
+			debugbinary = cleanExeName(debugbinary)
 		}
-		output = cleanExeName(output)
+
-		debugbinary, err := filepath.Abs(output)
+		if o, err := filepath.Abs(debugbinary); err != nil {
-		if err != nil {
+			s.sendErrorResponse(request.Request, FailedToLaunch, "Failed to launch", err.Error())
-			s.sendShowUserErrorResponse(request.Request, FailedToLaunch, "Failed to launch", err.Error())
 			return
+		} else {
+			debugbinary = o
 		}
 		buildFlags := args.BuildFlags
 
@@ -903,6 +938,8 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) {
 		var out []byte
 		wd, _ := os.Getwd()
 		s.config.log.Debugf("building program '%s' in '%s' with flags '%v'", program, wd, buildFlags)
+
+		var err error
 		switch mode {
 		case "debug":
 			cmd, out, err = gobuild.GoBuildCombinedOutput(debugbinary, []string{program}, buildFlags)
@@ -934,9 +971,14 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) {
 	}
 
 	s.config.ProcessArgs = append([]string{program}, args.Args...)
-	s.config.Debugger.WorkingDir = filepath.Dir(program)
 	if args.Cwd != "" {
 		s.config.Debugger.WorkingDir = args.Cwd
+	} else if mode == "test" {
+		// In test mode, run the test binary from the package directory
+		// like in `go test` and `dlv test` by default.
+		s.config.Debugger.WorkingDir = s.getPackageDir(args.Program)
+	} else {
+		s.config.Debugger.WorkingDir = "."
 	}
 
 	s.config.log.Debugf("running binary '%s' in '%s'", program, s.config.Debugger.WorkingDir)
@@ -993,6 +1035,16 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) {
 	s.send(&dap.LaunchResponse{Response: *newResponse(request.Request)})
 }
 
+func (s *Session) getPackageDir(pkg string) string {
+	cmd := exec.Command("go", "list", "-f", "{{.Dir}}", pkg)
+	out, err := cmd.Output()
+	if err != nil {
+		s.config.log.Debugf("failed to determin package directory for %v: %v\n%s", pkg, err, out)
+		return "."
+	}
+	return string(bytes.TrimSpace(out))
+}
+
 // newNoDebugProcess is called from onLaunchRequest (run goroutine) and
 // requires holding mu lock. It prepares process exec.Cmd to be started.
 func (s *Session) newNoDebugProcess(program string, targetArgs []string, wd string) (*exec.Cmd, error) {

service/dap/server_test.go

@@ -4504,10 +4504,9 @@ func TestLaunchRequestDefaults(t *testing.T) {
 	})
 	runTest(t, "increment", func(client *daptest.Client, fixture protest.Fixture) {
 		runDebugSession(t, client, "launch", func() {
-			// Use the default output directory.
+			// Use the temporary output binary.
 			client.LaunchRequestWithArgs(map[string]interface{}{
 				"mode": "debug", "program": fixture.Source})
-			// writes to default output dir __debug_bin
 		})
 	})
 }
@@ -4673,16 +4672,95 @@ func TestLaunchRequestWithRelativeExecPath(t *testing.T) {
 }
 
 func TestLaunchTestRequest(t *testing.T) {
-	serverStopped := make(chan struct{})
+	orgWD, _ := os.Getwd()
-	client := startDapServerWithClient(t, serverStopped)
+	fixtures := protest.FindFixturesDir() // relative to current working directory.
-	defer client.Close()
+	absoluteFixturesDir, _ := filepath.Abs(fixtures)
-	runDebugSession(t, client, "launch", func() {
+	absolutePkgDir, _ := filepath.Abs(filepath.Join(fixtures, "buildtest"))
-		fixtures := protest.FindFixturesDir()
+	testFile := filepath.Join(absolutePkgDir, "main_test.go")
-		testdir, _ := filepath.Abs(filepath.Join(fixtures, "buildtest"))
+
-		client.LaunchRequestWithArgs(map[string]interface{}{
+	for _, tc := range []struct {
-			"mode": "test", "program": testdir, "output": "__mytestdir"})
+		name       string
-	})
+		dlvWD      string
-	<-serverStopped
+		launchArgs map[string]interface{}
+		wantWD     string
+	}{{
+		name: "default",
+		launchArgs: map[string]interface{}{
+			"mode": "test", "program": absolutePkgDir,
+		},
+		wantWD: absolutePkgDir,
+	}, {
+		name: "output",
+		launchArgs: map[string]interface{}{
+			"mode": "test", "program": absolutePkgDir, "output": "test.out",
+		},
+		wantWD: absolutePkgDir,
+	}, {
+		name: "delveCWD",
+		launchArgs: map[string]interface{}{
+			"mode": "test", "program": absolutePkgDir, "delveCWD": ".",
+		},
+		wantWD: absolutePkgDir,
+	}, {
+		name: "delveCWD2",
+		launchArgs: map[string]interface{}{
+			"mode": "test", "program": ".", "delveCWD": absolutePkgDir,
+		},
+		wantWD: absolutePkgDir,
+	}, {
+		name: "cwd",
+		launchArgs: map[string]interface{}{
+			"mode": "test", "program": absolutePkgDir, "cwd": fixtures, // fixtures is relative to the current working directory.
+		},
+		wantWD: absoluteFixturesDir,
+	}, {
+		name:  "dlv runs outside of module",
+		dlvWD: os.TempDir(),
+		launchArgs: map[string]interface{}{
+			"mode": "test", "program": absolutePkgDir, "delveCWD": absoluteFixturesDir,
+		},
+		wantWD: absolutePkgDir,
+	}, {
+		name:  "dlv builds in delveCWD but runs in cwd",
+		dlvWD: fixtures,
+		launchArgs: map[string]interface{}{
+			"mode": "test", "program": absolutePkgDir, "delveCWD": absolutePkgDir, "cwd": "..", // relative to delveCWD.
+		},
+		wantWD: absoluteFixturesDir,
+	}} {
+		t.Run(tc.name, func(t *testing.T) {
+			// Some test cases with delveCWD or dlvWD change process working directory.
+			defer os.Chdir(orgWD)
+			if tc.dlvWD != "" {
+				os.Chdir(tc.dlvWD)
+				defer os.Chdir(orgWD)
+			}
+			serverStopped := make(chan struct{})
+			client := startDapServerWithClient(t, serverStopped)
+			defer client.Close()
+
+			runDebugSessionWithBPs(t, client, "launch",
+				func() { // Launch
+					client.LaunchRequestWithArgs(tc.launchArgs)
+				},
+				testFile, []int{14},
+				[]onBreakpoint{{
+					execute: func() {
+						checkStop(t, client, -1, "github.com/go-delve/delve/_fixtures/buildtest.TestCurrentDirectory", 14)
+						client.VariablesRequest(1001) // Locals
+						locals := client.ExpectVariablesResponse(t)
+						checkChildren(t, locals, "Locals", 1)
+						for i := range locals.Body.Variables {
+							switch locals.Body.Variables[i].Name {
+							case "wd": // The test's working directory is the package directory by default.
+								checkVarExact(t, locals, i, "wd", "wd", fmt.Sprintf("%q", tc.wantWD), "string", noChildren)
+							}
+						}
+					}}})
+
+			<-serverStopped
+		})
+	}
 }
 
 // Tests that 'args' from LaunchRequest are parsed and passed to the target

service/dap/types.go

@@ -44,7 +44,6 @@ var (
 	}
 	defaultLaunchConfig = LaunchConfig{
 		Mode:                     "debug",
-		Output:                   defaultDebugBinary,
 		LaunchAttachCommonConfig: defaultLaunchAttachCommonConfig,
 	}
 	defaultAttachConfig = AttachConfig{
@@ -65,28 +64,34 @@ type LaunchConfig struct {
 	// Default is "debug".
 	Mode string `json:"mode,omitempty"`
 
-	// Required when mode is `debug`, `test`, or `exec`.
 	// Path to the program folder (or any go file within that folder)
 	// when in `debug` or `test` mode, and to the pre-built binary file
 	// to debug in `exec` mode.
 	// If it is not an absolute path, it will be interpreted as a path
-	// relative to the working directory of the delve process.
+	// relative to Delve's working directory.
+	// Required when mode is `debug`, `test`, `exec`, and `core`.
 	Program string `json:"program,omitempty"`
 
 	// Command line arguments passed to the debugged program.
+	// Relative paths used in Args will be interpreted as paths relative
+	// to `cwd`.
 	Args []string `json:"args,omitempty"`
 
-	// Working directory of the program being debugged
+	// Working directory of the program being debugged.
-	// if a non-empty value is specified. If a relative path is provided,
+	// If a relative path is provided, it will be interpreted as
-	// it will be interpreted as a relative path to the delve's
+	// a relative path to the delve's working directory. This is
-	// working directory.
+	// similar to delve's `--wd` flag.
 	//
-	// If not specified or empty, currently the built program's directory will
+	// If not specified or empty, delve's working directory is
-	// be used.
+	// used by default. But for `test` mode, delve tries to find
-	// This is similar to delve's `--wd` flag.
+	// the test's package source directory and run tests from there.
+	// This matches the behavior of `dlv test` and `go test`.
 	Cwd string `json:"cwd,omitempty"`
 
 	// Build flags, to be passed to the Go compiler.
+	// Relative paths used in BuildFlags will be interpreted as paths
+	// relative to Delve's current working directory.
+	//
 	// It is like delve's `--build-flags`. For example,
 	//
 	//    "buildFlags": "-tags=integration -mod=vendor -cover -v"
@@ -94,26 +99,38 @@ type LaunchConfig struct {
 
 	// Output path for the binary of the debugee.
 	// Relative path is interpreted as the path relative to
-	// the delve process's working directory.
+	// the delve's current working directory.
 	// This is deleted after the debug session ends.
-	//
-	// FIXIT: the built program's directory is used as the default
-	// working directory of the debugged program, which means
-	// the directory of `output` is used as the default working
-	// directory. This is a bug and needs fix.
 	Output string `json:"output,omitempty"`
 
 	// NoDebug is used to run the program without debugging.
 	NoDebug bool `json:"noDebug,omitempty"`
 
 	// TraceDirPath is the trace directory path for replay mode.
+	// Relative path is interpreted as a path relative to Delve's
+	// current working directory.
 	// This is required for "replay" mode but unused in other modes.
 	TraceDirPath string `json:"traceDirPath,omitempty"`
 
 	// CoreFilePath is the core file path for core mode.
+	//
 	// This is required for "core" mode but unused in other modes.
 	CoreFilePath string `json:"coreFilePath,omitempty"`
 
+	// DelveCwd is the working directory of Delve.
+	// If specified, the delve DAP server will change its working
+	// directory to the specified directory using os.Chdir.
+	// Any relative paths used in most of other attributes are
+	// interpreted as paths relative to Delve's working directory
+	// unless explicitely stated otherwise. When Delve needs to
+	// build the program (in debug/test modes), Delve runs the
+	// go command from the Delve's working directory.
+	//
+	// If a relative path is provided as DelveCwd, it will be
+	// interpreted as a path relative to Delve's current working
+	// directory.
+	DelveCwd string `json:"delveCwd,omitempty"`
+
 	LaunchAttachCommonConfig
 }