diff --git a/service/dap/server.go b/service/dap/server.go index e0ba5382..d6a8054a 100644 --- a/service/dap/server.go +++ b/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, -// relative to the current working directory of the server. +// Default output file pathname for the compiled binary in debug or test modes +// 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 - if output == "" { - output = defaultDebugBinary + debugbinary := args.Output + if debugbinary == "" { + debugbinary = s.tempDebugBinary() + } else { + debugbinary = cleanExeName(debugbinary) } - output = cleanExeName(output) - debugbinary, err := filepath.Abs(output) - if err != nil { - s.sendShowUserErrorResponse(request.Request, FailedToLaunch, "Failed to launch", err.Error()) + + if o, err := filepath.Abs(debugbinary); err != nil { + s.sendErrorResponse(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) { diff --git a/service/dap/server_test.go b/service/dap/server_test.go index 393525f8..52d6ec43 100644 --- a/service/dap/server_test.go +++ b/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{}) - client := startDapServerWithClient(t, serverStopped) - defer client.Close() - runDebugSession(t, client, "launch", func() { - fixtures := protest.FindFixturesDir() - testdir, _ := filepath.Abs(filepath.Join(fixtures, "buildtest")) - client.LaunchRequestWithArgs(map[string]interface{}{ - "mode": "test", "program": testdir, "output": "__mytestdir"}) - }) - <-serverStopped + orgWD, _ := os.Getwd() + fixtures := protest.FindFixturesDir() // relative to current working directory. + absoluteFixturesDir, _ := filepath.Abs(fixtures) + absolutePkgDir, _ := filepath.Abs(filepath.Join(fixtures, "buildtest")) + testFile := filepath.Join(absolutePkgDir, "main_test.go") + + for _, tc := range []struct { + name string + dlvWD string + 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 diff --git a/service/dap/types.go b/service/dap/types.go index 832dfb67..0cbf4806 100644 --- a/service/dap/types.go +++ b/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 - // if a non-empty value is specified. If a relative path is provided, - // it will be interpreted as a relative path to the delve's - // working directory. + // Working directory of the program being debugged. + // If a relative path is provided, it will be interpreted as + // a relative path to the delve's working directory. This is + // similar to delve's `--wd` flag. // - // If not specified or empty, currently the built program's directory will - // be used. - // This is similar to delve's `--wd` flag. + // If not specified or empty, delve's working directory is + // used by default. But for `test` mode, delve tries to find + // 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 }