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

@@ -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 <executable> <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)
+		defer remove(debugname)
-		if err != nil {
+		processArgs := append([]string{debugname}, targetArgs...)
-			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...)
 		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)
+		defer remove(debugname)
-		processArgs := append([]string{"./" + testdebugname}, targetArgs...)
+		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 {
+func gotestbuild(debugname, pkg string) error {
-	args := []string{"-gcflags", "-N -l", "-c", "-o", testdebugname}
+	args := []string{"-gcflags", "-N -l", "-c", "-o", debugname}
 	if BuildFlags != "" {
 		args = append(args, config.SplitQuotedFields(BuildFlags, '\'')...)
 	}

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 getDlvBin(t *testing.T) (string, string) {
-func TestIssue398(t *testing.T) {
+	tmpdir, err := ioutil.TempDir("", "TestDlv")
-	tmpdir, err := ioutil.TempDir("", "TestIssue398")
 	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!"
+// TestOutput verifies that the debug executable is created in the correct path
-	stdout, _ := testIssue398(t, dlvbin, []string{"continue", "exit"})
+// and removed after exit.
-	if !strings.Contains(string(stdout), hello) {
+func TestOutput(t *testing.T) {
-		t.Errorf("stdout %q should contain %q", stdout, hello)
+	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)
+		}
 	}
 }