From 6fe97fa75be05bbf991aed8d02f2ed313aa99618 Mon Sep 17 00:00:00 2001 From: Martin Tournoij Date: Tue, 28 Nov 2017 18:51:30 +0000 Subject: [PATCH] Support --output for debug, trace, and test commands (#1028) * Support --output for debug, trace, and test commands With the `--output` parameter you can configure the output binary. For example: dlv debug --output /tmp/xxx Will build the binary to `/tmp/xxx`, instead of always putting it as `debug` in the current directory. This ensures that the command always works (even if there is already a file or directory named `debug`) and doesn't write to the source directory. Especially for things like Delve/Vim integration this is a good thing to have, I think. * Address PR feedback and add a test - We don't need to use `filepath.IsAbs()` on startup; I added that because it previously did `"./" + debugname` everywhere, but I don't think that's needed at all, since `pathname` without a leading `./` implies the current directory. - Repurpose the existing `TestIssue398` to also test the `--output` flag. Also fix an issue where tests wouldn't work if `GOPATH` has multiple entries (e..g `GOPATH=$HOME/go:$HOME/mygocode`). - Print an error if we can't remove the debug binary on exit instead of failing silently. Not strictly related to this PR, but a good change to add I think. * Also warn when delve can't remove the binary in test/trace I only added that to debug, but good to issue this warning consistently. --- cmd/dlv/cmds/commands.go | 69 +++++++++++++++++++++-------------- cmd/dlv/dlv_test.go | 78 ++++++++++++++++++++++++++++++---------- 2 files changed, 101 insertions(+), 46 deletions(-) diff --git a/cmd/dlv/cmds/commands.go b/cmd/dlv/cmds/commands.go index 1f928b43..38b77ded 100644 --- a/cmd/dlv/cmds/commands.go +++ b/cmd/dlv/cmds/commands.go @@ -53,11 +53,6 @@ var ( conf *config.Config ) -const ( - debugname = "debug" - testdebugname = "debug.test" -) - const dlvCommandLongDesc = `Delve is a source level debugger for Go programs. Delve enables you to interact with your program by controlling the execution of the process, @@ -151,6 +146,7 @@ package name and Delve will compile that package instead, and begin a new debug session.`, Run: debugCmd, } + debugCommand.Flags().String("output", "debug", "Output path for the binary.") RootCommand.AddCommand(debugCommand) // 'exec' subcommand. @@ -198,6 +194,7 @@ Alternatively you can specify a package name, and Delve will debug the tests in that package instead.`, Run: testCmd, } + testCommand.Flags().String("output", "debug.test", "Output path for the binary.") RootCommand.AddCommand(testCommand) // 'trace' subcommand. @@ -214,13 +211,14 @@ to know what functions your process is executing.`, } traceCommand.Flags().IntVarP(&traceAttachPid, "pid", "p", 0, "Pid to attach to.") traceCommand.Flags().IntVarP(&traceStackDepth, "stack", "s", 0, "Show stack trace with given depth.") + traceCommand.Flags().String("output", "debug", "Output path for the binary.") RootCommand.AddCommand(traceCommand) coreCommand := &cobra.Command{ Use: "core ", Short: "Examine a core dump.", Long: `Examine a core dump. - + The core command will open the specified core file and the associated executable and let you examine the state of the process when the core dump was taken.`, @@ -249,7 +247,7 @@ core dump was taken.`, Use: "replay [trace directory]", Short: "Replays a rr trace.", Long: `Replays a rr trace. - + The replay command will open a trace generated by mozilla rr. Mozilla rr must be installed: https://github.com/mozilla/rr `, @@ -270,31 +268,35 @@ https://github.com/mozilla/rr return RootCommand } +// Remove the file at path and issue a warning to stderr if this fails. +func remove(path string) { + err := os.Remove(path) + if err != nil { + fmt.Fprintf(os.Stderr, "could not remove %v: %v\n", path, err) + } +} + func debugCmd(cmd *cobra.Command, args []string) { status := func() int { + debugname, err := filepath.Abs(cmd.Flag("output").Value.String()) + if err != nil { + fmt.Fprintf(os.Stderr, "%v\n", err) + return 1 + } + var pkg string dlvArgs, targetArgs := splitArgs(cmd, args) if len(dlvArgs) > 0 { pkg = args[0] } - err := gobuild(debugname, pkg) + err = gobuild(debugname, pkg) if err != nil { fmt.Fprintf(os.Stderr, "%v\n", err) return 1 } - fp, err := filepath.Abs("./" + debugname) - if err != nil { - fmt.Fprintf(os.Stderr, "%v\n", err) - return 1 - } - defer os.Remove(fp) - abs, err := filepath.Abs(debugname) - if err != nil { - fmt.Fprintf(os.Stderr, "%v\n", err) - return 1 - } - processArgs := append([]string{abs}, targetArgs...) + defer remove(debugname) + processArgs := append([]string{debugname}, targetArgs...) return execute(0, processArgs, conf, "", executingGeneratedFile) }() os.Exit(status) @@ -302,6 +304,13 @@ func debugCmd(cmd *cobra.Command, args []string) { func traceCmd(cmd *cobra.Command, args []string) { status := func() int { + + debugname, err := filepath.Abs(cmd.Flag("output").Value.String()) + if err != nil { + fmt.Fprintf(os.Stderr, "%v\n", err) + return 1 + } + var regexp string var processArgs []string @@ -319,9 +328,9 @@ func traceCmd(cmd *cobra.Command, args []string) { if err := gobuild(debugname, pkg); err != nil { return 1 } - defer os.Remove("./" + debugname) + defer remove(debugname) - processArgs = append([]string{"./" + debugname}, targetArgs...) + processArgs = append([]string{debugname}, targetArgs...) } // Make a TCP listener listener, err := net.Listen("tcp", Addr) @@ -372,18 +381,24 @@ func traceCmd(cmd *cobra.Command, args []string) { func testCmd(cmd *cobra.Command, args []string) { status := func() int { + debugname, err := filepath.Abs(cmd.Flag("output").Value.String()) + if err != nil { + fmt.Fprintf(os.Stderr, "%v\n", err) + return 1 + } + var pkg string dlvArgs, targetArgs := splitArgs(cmd, args) if len(dlvArgs) > 0 { pkg = args[0] } - err := gotestbuild(pkg) + err = gotestbuild(debugname, pkg) if err != nil { return 1 } - defer os.Remove("./" + testdebugname) - processArgs := append([]string{"./" + testdebugname}, targetArgs...) + defer remove(debugname) + processArgs := append([]string{debugname}, targetArgs...) return execute(0, processArgs, conf, "", executingGeneratedTest) }() @@ -542,8 +557,8 @@ func gobuild(debugname, pkg string) error { return gocommand("build", args...) } -func gotestbuild(pkg string) error { - args := []string{"-gcflags", "-N -l", "-c", "-o", testdebugname} +func gotestbuild(debugname, pkg string) error { + args := []string{"-gcflags", "-N -l", "-c", "-o", debugname} if BuildFlags != "" { args = append(args, config.SplitQuotedFields(BuildFlags, '\'')...) } diff --git a/cmd/dlv/dlv_test.go b/cmd/dlv/dlv_test.go index 98d02abc..6fc7bafd 100644 --- a/cmd/dlv/dlv_test.go +++ b/cmd/dlv/dlv_test.go @@ -12,6 +12,7 @@ import ( "runtime" "strings" "testing" + "time" protest "github.com/derekparker/delve/pkg/proc/test" "github.com/derekparker/delve/service/rpc2" @@ -39,21 +40,23 @@ func assertNoError(err error, t testing.TB, s string) { } } -func goEnv(name string) string { +func goPath(name string) string { if val := os.Getenv(name); val != "" { - return val + // Use first GOPATH entry if there are multiple. + return filepath.SplitList(val)[0] } + val, err := exec.Command("go", "env", name).Output() if err != nil { panic(err) // the Go tool was tested to work earlier } - return strings.TrimSpace(string(val)) + return filepath.SplitList(strings.TrimSpace(string(val)))[0] } func TestBuild(t *testing.T) { const listenAddr = "localhost:40573" var err error - makedir := filepath.Join(goEnv("GOPATH"), "src", "github.com", "derekparker", "delve") + makedir := filepath.Join(goPath("GOPATH"), "src", "github.com", "derekparker", "delve") for _, makeProgram := range []string{"make", "mingw32-make"} { var out []byte cmd := exec.Command(makeProgram, "build") @@ -100,10 +103,21 @@ func TestBuild(t *testing.T) { cmd.Wait() } -func testIssue398(t *testing.T, dlvbin string, cmds []string) (stdout, stderr []byte) { +func testOutput(t *testing.T, dlvbin, output string, delveCmds []string) (stdout, stderr []byte) { var stdoutBuf, stderrBuf bytes.Buffer buildtestdir := filepath.Join(protest.FindFixturesDir(), "buildtest") - cmd := exec.Command(dlvbin, "debug") + + c := []string{dlvbin, "debug"} + debugbin := filepath.Join(buildtestdir, "debug") + if output != "" { + c = append(c, "--output", output) + if filepath.IsAbs(output) { + debugbin = output + } else { + debugbin = filepath.Join(buildtestdir, output) + } + } + cmd := exec.Command(c[0], c[1:]...) cmd.Dir = buildtestdir stdin, err := cmd.StdinPipe() if err != nil { @@ -111,37 +125,52 @@ func testIssue398(t *testing.T, dlvbin string, cmds []string) (stdout, stderr [] } cmd.Stdout = &stdoutBuf cmd.Stderr = &stderrBuf + if err := cmd.Start(); err != nil { t.Fatal(err) } - for _, c := range cmds { + + // Give delve some time to compile and write the binary. + foundIt := false + for wait := 0; wait < 30; wait++ { + _, err = os.Stat(debugbin) + if err == nil { + foundIt = true + break + } + + time.Sleep(1 * time.Second) + } + if !foundIt { + t.Errorf("running %q: file not created: %v", delveCmds, err) + } + + for _, c := range delveCmds { fmt.Fprintf(stdin, "%s\n", c) } + // ignore "dlv debug" command error, it returns // errors even after successful debug session. cmd.Wait() stdout, stderr = stdoutBuf.Bytes(), stderrBuf.Bytes() - debugbin := filepath.Join(buildtestdir, "debug") _, err = os.Stat(debugbin) if err == nil { - t.Errorf("running %q: file %v was not deleted\nstdout is %q, stderr is %q", cmds, debugbin, stdout, stderr) + t.Errorf("running %q: file %v was not deleted\nstdout is %q, stderr is %q", delveCmds, debugbin, stdout, stderr) return } if !os.IsNotExist(err) { - t.Errorf("running %q: %v\nstdout is %q, stderr is %q", cmds, err, stdout, stderr) + t.Errorf("running %q: %v\nstdout is %q, stderr is %q", delveCmds, err, stdout, stderr) return } return } -// TestIssue398 verifies that the debug executable is removed after exit. -func TestIssue398(t *testing.T) { - tmpdir, err := ioutil.TempDir("", "TestIssue398") +func getDlvBin(t *testing.T) (string, string) { + tmpdir, err := ioutil.TempDir("", "TestDlv") if err != nil { t.Fatal(err) } - defer os.RemoveAll(tmpdir) dlvbin := filepath.Join(tmpdir, "dlv.exe") out, err := exec.Command("go", "build", "-o", dlvbin, "github.com/derekparker/delve/cmd/dlv").CombinedOutput() @@ -149,11 +178,22 @@ func TestIssue398(t *testing.T) { t.Fatalf("go build -o %v github.com/derekparker/delve/cmd/dlv: %v\n%s", dlvbin, err, string(out)) } - testIssue398(t, dlvbin, []string{"exit"}) + return dlvbin, tmpdir +} - const hello = "hello world!" - stdout, _ := testIssue398(t, dlvbin, []string{"continue", "exit"}) - if !strings.Contains(string(stdout), hello) { - t.Errorf("stdout %q should contain %q", stdout, hello) +// TestOutput verifies that the debug executable is created in the correct path +// and removed after exit. +func TestOutput(t *testing.T) { + dlvbin, tmpdir := getDlvBin(t) + defer os.RemoveAll(tmpdir) + + for _, output := range []string{"", "myownname", filepath.Join(tmpdir, "absolute.path")} { + testOutput(t, dlvbin, output, []string{"exit"}) + + const hello = "hello world!" + stdout, _ := testOutput(t, dlvbin, output, []string{"continue", "exit"}) + if !strings.Contains(string(stdout), hello) { + t.Errorf("stdout %q should contain %q", stdout, hello) + } } }