Repository: go-delve/delve
Pull request #2007 (2020-04-13 18:15:16)

*: Consolidate service/debugger config
Embed the debugger config object in the service config object to avoid needless duplication of fields.


Patch:
diff --git a/.travis.yml b/.travis.yml
index 03119174..0230bfc5 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -61,6 +61,7 @@ script: >-
       docker run \
       -v $(pwd):/delve \
       --env TRAVIS=true \
+      --env CI=true \
       --privileged i386/centos:7 \
       /bin/bash -c "set -x && \
            cd delve && \
diff --git a/cmd/dlv/cmds/commands.go b/cmd/dlv/cmds/commands.go
index 08db4871..35bae2d0 100644
--- a/cmd/dlv/cmds/commands.go
+++ b/cmd/dlv/cmds/commands.go
@@ -21,6 +21,7 @@ import (
 	"github.com/go-delve/delve/service"
 	"github.com/go-delve/delve/service/api"
 	"github.com/go-delve/delve/service/dap"
+	"github.com/go-delve/delve/service/debugger"
 	"github.com/go-delve/delve/service/rpc2"
 	"github.com/go-delve/delve/service/rpccommon"
 	"github.com/spf13/cobra"
@@ -401,13 +402,15 @@ func dapCmd(cmd *cobra.Command, args []string) {
 		}
 		disconnectChan := make(chan struct{})
 		server := dap.NewServer(&service.Config{
-			Listener:             listener,
-			Backend:              backend,
-			Foreground:           (headless && tty == ""),
-			DebugInfoDirectories: conf.DebugInfoDirectories,
-			CheckGoVersion:       checkGoVersion,
-			DisconnectChan:       disconnectChan,
-			TTY:                  tty,
+			Listener:       listener,
+			DisconnectChan: disconnectChan,
+			Debugger: debugger.Config{
+				Backend:              backend,
+				Foreground:           headless && tty == "",
+				DebugInfoDirectories: conf.DebugInfoDirectories,
+				CheckGoVersion:       checkGoVersion,
+				TTY:                  tty,
+			},
 		})
 		defer server.Stop()
 
@@ -505,13 +508,15 @@ func traceCmd(cmd *cobra.Command, args []string) {
 
 		// Create and start a debug server
 		server := rpccommon.NewServer(&service.Config{
-			Listener:       listener,
-			ProcessArgs:    processArgs,
-			AttachPid:      traceAttachPid,
-			APIVersion:     2,
-			WorkingDir:     workingDir,
-			Backend:        backend,
-			CheckGoVersion: checkGoVersion,
+			Listener:    listener,
+			ProcessArgs: processArgs,
+			APIVersion:  2,
+			Debugger: debugger.Config{
+				AttachPid:      traceAttachPid,
+				WorkingDir:     workingDir,
+				Backend:        backend,
+				CheckGoVersion: checkGoVersion,
+			},
 		})
 		if err := server.Run(); err != nil {
 			fmt.Fprintln(os.Stderr, err)
@@ -739,20 +744,22 @@ func execute(attachPid int, processArgs []string, conf *config.Config, coreFile
 	switch apiVersion {
 	case 1, 2:
 		server = rpccommon.NewServer(&service.Config{
-			Listener:             listener,
-			ProcessArgs:          processArgs,
-			AttachPid:            attachPid,
-			AcceptMulti:          acceptMulti,
-			APIVersion:           apiVersion,
-			WorkingDir:           workingDir,
-			Backend:              backend,
-			CoreFile:             coreFile,
-			Foreground:           (headless && tty == ""),
-			DebugInfoDirectories: conf.DebugInfoDirectories,
-			CheckGoVersion:       checkGoVersion,
-			CheckLocalConnUser:   checkLocalConnUser,
-			DisconnectChan:       disconnectChan,
-			TTY:                  tty,
+			Listener:           listener,
+			ProcessArgs:        processArgs,
+			AcceptMulti:        acceptMulti,
+			APIVersion:         apiVersion,
+			CheckLocalConnUser: checkLocalConnUser,
+			DisconnectChan:     disconnectChan,
+			Debugger: debugger.Config{
+				AttachPid:            attachPid,
+				WorkingDir:           workingDir,
+				Backend:              backend,
+				CoreFile:             coreFile,
+				Foreground:           headless && tty == "",
+				DebugInfoDirectories: conf.DebugInfoDirectories,
+				CheckGoVersion:       checkGoVersion,
+				TTY:                  tty,
+			},
 		})
 	default:
 		fmt.Printf("Unknown API version: %d\n", apiVersion)
diff --git a/service/config.go b/service/config.go
index 7ff4a5a3..822066bb 100644
--- a/service/config.go
+++ b/service/config.go
@@ -1,6 +1,10 @@
 package service
 
-import "net"
+import (
+	"net"
+
+	"github.com/go-delve/delve/service/debugger"
+)
 
 // Config provides the configuration to start a Debugger and expose it with a
 // service.
@@ -9,41 +13,23 @@ import "net"
 // provided, a new process will be launched. Otherwise, the debugger will try
 // to attach to an existing process with AttachPid.
 type Config struct {
+	// Debugger configuration object, used to configure the underlying
+	// debugger used by the server.
+	Debugger debugger.Config
+
 	// Listener is used to serve requests.
 	Listener net.Listener
+
 	// ProcessArgs are the arguments to launch a new process.
 	ProcessArgs []string
-	// WorkingDir is working directory of the new process. This field is used
-	// only when launching a new process.
-	WorkingDir string
 
-	// AttachPid is the PID of an existing process to which the debugger should
-	// attach.
-	AttachPid int
 	// AcceptMulti configures the server to accept multiple connection.
 	// Note that the server API is not reentrant and clients will have to coordinate.
 	AcceptMulti bool
+
 	// APIVersion selects which version of the API to serve (default: 1).
 	APIVersion int
 
-	// CoreFile specifies the path to the core dump to open.
-	CoreFile string
-
-	// DebugInfoDirectories is the list of directories to look for
-	// when resolving external debug info files.
-	DebugInfoDirectories []string
-
-	// Selects server backend.
-	Backend string
-
-	// Foreground lets target process access stdin.
-	Foreground bool
-
-	// CheckGoVersion is true if the debugger should check the version of Go
-	// used to compile the executable and refuse to work on incompatible
-	// versions.
-	CheckGoVersion bool
-
 	// CheckLocalConnUser is true if the debugger should check that local
 	// connections come from the same user that started the headless server
 	CheckLocalConnUser bool
diff --git a/service/dap/server.go b/service/dap/server.go
index fadbee78..3ce27b6c 100644
--- a/service/dap/server.go
+++ b/service/dap/server.go
@@ -89,7 +89,7 @@ func (s *Server) Stop() {
 		s.conn.Close()
 	}
 	if s.debugger != nil {
-		kill := s.config.AttachPid == 0
+		kill := s.config.Debugger.AttachPid == 0
 		if err := s.debugger.Detach(kill); err != nil {
 			s.log.Error(err)
 		}
@@ -354,24 +354,15 @@ func (s *Server) onLaunchRequest(request *dap.LaunchRequest) {
 	}
 
 	stop, ok := request.Arguments["stopOnEntry"]
-	s.stopOnEntry = (ok && stop == true)
+	s.stopOnEntry = ok && stop == true
 
 	// TODO(polina): support target args
 	s.config.ProcessArgs = []string{program}
-	s.config.WorkingDir = filepath.Dir(program)
-
-	config := &debugger.Config{
-		WorkingDir:           s.config.WorkingDir,
-		AttachPid:            0,
-		CoreFile:             "",
-		Backend:              s.config.Backend,
-		Foreground:           s.config.Foreground,
-		DebugInfoDirectories: s.config.DebugInfoDirectories,
-		CheckGoVersion:       s.config.CheckGoVersion,
-		TTY:                  s.config.TTY,
-	}
+	s.config.Debugger.WorkingDir = filepath.Dir(program)
+
+	config := s.config.Debugger
 	var err error
-	if s.debugger, err = debugger.New(config, s.config.ProcessArgs); err != nil {
+	if s.debugger, err = debugger.New(&config, s.config.ProcessArgs); err != nil {
 		s.sendErrorResponse(request.Request,
 			FailedToContinue, "Failed to launch", err.Error())
 		return
@@ -394,7 +385,7 @@ func (s *Server) onDisconnectRequest(request *dap.DisconnectRequest) {
 		if err != nil {
 			s.log.Error(err)
 		}
-		kill := s.config.AttachPid == 0
+		kill := s.config.Debugger.AttachPid == 0
 		err = s.debugger.Detach(kill)
 		if err != nil {
 			s.log.Error(err)
diff --git a/service/debugger/debugger.go b/service/debugger/debugger.go
index cec918a4..61435efe 100644
--- a/service/debugger/debugger.go
+++ b/service/debugger/debugger.go
@@ -77,6 +77,7 @@ type Config struct {
 
 	// CoreFile specifies the path to the core dump to open.
 	CoreFile string
+
 	// Backend specifies the debugger backend.
 	Backend string
 
diff --git a/service/rpc1/server.go b/service/rpc1/server.go
index d2435034..7d6f4f2f 100644
--- a/service/rpc1/server.go
+++ b/service/rpc1/server.go
@@ -43,7 +43,7 @@ func (s *RPCServer) Detach(kill bool, ret *int) error {
 }
 
 func (s *RPCServer) Restart(arg1 interface{}, arg2 *int) error {
-	if s.config.AttachPid != 0 {
+	if s.config.Debugger.AttachPid != 0 {
 		return errors.New("cannot restart process Delve did not create")
 	}
 	_, err := s.debugger.Restart(false, "", false, nil)
@@ -298,7 +298,7 @@ func (s *RPCServer) ListGoroutines(arg interface{}, goroutines *[]*api.Goroutine
 }
 
 func (c *RPCServer) AttachedToExistingProcess(arg interface{}, answer *bool) error {
-	if c.config.AttachPid != 0 {
+	if c.config.Debugger.AttachPid != 0 {
 		*answer = true
 	}
 	return nil
diff --git a/service/rpc2/server.go b/service/rpc2/server.go
index 5f96f6cb..b9a1ad58 100644
--- a/service/rpc2/server.go
+++ b/service/rpc2/server.go
@@ -84,7 +84,7 @@ type RestartOut struct {
 
 // Restart restarts program.
 func (s *RPCServer) Restart(arg RestartIn, cb service.RPCCallback) {
-	if s.config.AttachPid != 0 {
+	if s.config.Debugger.AttachPid != 0 {
 		cb.Return(nil, errors.New("cannot restart process Delve did not create"))
 		return
 	}
@@ -571,7 +571,7 @@ type AttachedToExistingProcessOut struct {
 
 // AttachedToExistingProcess returns whether we attached to a running process or not
 func (c *RPCServer) AttachedToExistingProcess(arg AttachedToExistingProcessIn, out *AttachedToExistingProcessOut) error {
-	if c.config.AttachPid != 0 {
+	if c.config.Debugger.AttachPid != 0 {
 		out.Answer = true
 	}
 	return nil
diff --git a/service/rpccommon/server.go b/service/rpccommon/server.go
index b07ece3e..887540c4 100644
--- a/service/rpccommon/server.go
+++ b/service/rpccommon/server.go
@@ -71,7 +71,7 @@ func NewServer(config *service.Config) *ServerImpl {
 	if config.APIVersion < 2 {
 		logger.Info("Using API v1")
 	}
-	if config.Foreground {
+	if config.Debugger.Foreground {
 		// Print listener address
 		logflags.WriteAPIListeningMessage(config.Listener.Addr().String())
 		logger.Debug("API server pid = ", os.Getpid())
@@ -90,7 +90,7 @@ func (s *ServerImpl) Stop() error {
 		close(s.stopChan)
 		s.listener.Close()
 	}
-	kill := s.config.AttachPid == 0
+	kill := s.config.Debugger.AttachPid == 0
 	return s.debugger.Detach(kill)
 }
 
@@ -108,17 +108,8 @@ func (s *ServerImpl) Run() error {
 	}
 
 	// Create and start the debugger
-	if s.debugger, err = debugger.New(&debugger.Config{
-		AttachPid:            s.config.AttachPid,
-		WorkingDir:           s.config.WorkingDir,
-		CoreFile:             s.config.CoreFile,
-		Backend:              s.config.Backend,
-		Foreground:           s.config.Foreground,
-		DebugInfoDirectories: s.config.DebugInfoDirectories,
-		CheckGoVersion:       s.config.CheckGoVersion,
-		TTY:                  s.config.TTY,
-	},
-		s.config.ProcessArgs); err != nil {
+	config := s.config.Debugger
+	if s.debugger, err = debugger.New(&config, s.config.ProcessArgs); err != nil {
 		return err
 	}
 


Test patch:
diff --git a/pkg/terminal/command_test.go b/pkg/terminal/command_test.go
index 448b8463..6f279773 100644
--- a/pkg/terminal/command_test.go
+++ b/pkg/terminal/command_test.go
@@ -21,6 +21,7 @@ import (
 	"github.com/go-delve/delve/pkg/proc/test"
 	"github.com/go-delve/delve/service"
 	"github.com/go-delve/delve/service/api"
+	"github.com/go-delve/delve/service/debugger"
 	"github.com/go-delve/delve/service/rpc2"
 	"github.com/go-delve/delve/service/rpccommon"
 )
@@ -154,7 +155,9 @@ func withTestTerminalBuildFlags(name string, t testing.TB, buildFlags test.Build
 	server := rpccommon.NewServer(&service.Config{
 		Listener:    listener,
 		ProcessArgs: []string{test.BuildFixture(name, buildFlags).Path},
-		Backend:     testBackend,
+		Debugger: debugger.Config{
+			Backend: testBackend,
+		},
 	})
 	if err := server.Run(); err != nil {
 		t.Fatal(err)
diff --git a/service/dap/server_test.go b/service/dap/server_test.go
index b86705ba..f578676c 100644
--- a/service/dap/server_test.go
+++ b/service/dap/server_test.go
@@ -16,6 +16,7 @@ import (
 	protest "github.com/go-delve/delve/pkg/proc/test"
 	"github.com/go-delve/delve/service"
 	"github.com/go-delve/delve/service/dap/daptest"
+	"github.com/go-delve/delve/service/debugger"
 	"github.com/google/go-dap"
 )
 
@@ -42,8 +43,10 @@ func runTest(t *testing.T, name string, test func(c *daptest.Client, f protest.F
 	disconnectChan := make(chan struct{})
 	server := NewServer(&service.Config{
 		Listener:       listener,
-		Backend:        "default",
 		DisconnectChan: disconnectChan,
+		Debugger: debugger.Config{
+			Backend: "default",
+		},
 	})
 	server.Run()
 	// Give server time to start listening for clients
diff --git a/service/debugger/debugger_unix_test.go b/service/debugger/debugger_unix_test.go
index caa12c96..8592968c 100644
--- a/service/debugger/debugger_unix_test.go
+++ b/service/debugger/debugger_unix_test.go
@@ -54,7 +54,7 @@ func TestDebugger_LaunchNoExecutablePerm(t *testing.T) {
 }
 
 func TestDebugger_LaunchWithTTY(t *testing.T) {
-	if os.Getenv("TRAVIS") == "true" {
+	if os.Getenv("CI") == "true" {
 		if _, err := exec.LookPath("lsof"); err != nil {
 			t.Skip("skipping test in CI, system does not contain lsof")
 		}
diff --git a/service/test/integration1_test.go b/service/test/integration1_test.go
index 46559a8d..f4f20e2b 100644
--- a/service/test/integration1_test.go
+++ b/service/test/integration1_test.go
@@ -12,6 +12,7 @@ import (
 	"time"
 
 	protest "github.com/go-delve/delve/pkg/proc/test"
+	"github.com/go-delve/delve/service/debugger"
 
 	"github.com/go-delve/delve/pkg/goversion"
 	"github.com/go-delve/delve/service"
@@ -43,7 +44,9 @@ func withTestClient1Extended(name string, t *testing.T, fn func(c *rpc1.RPCClien
 	server := rpccommon.NewServer(&service.Config{
 		Listener:    listener,
 		ProcessArgs: []string{fixture.Path},
-		Backend:     testBackend,
+		Debugger: debugger.Config{
+			Backend: testBackend,
+		},
 	})
 	if err := server.Run(); err != nil {
 		t.Fatal(err)
@@ -71,7 +74,9 @@ func Test1RunWithInvalidPath(t *testing.T) {
 	server := rpccommon.NewServer(&service.Config{
 		Listener:    listener,
 		ProcessArgs: []string{"invalid_path"},
-		Backend:     testBackend,
+		Debugger: debugger.Config{
+			Backend: testBackend,
+		},
 	})
 	if err := server.Run(); err == nil {
 		t.Fatal("Expected Run to return error for invalid program path")
diff --git a/service/test/integration2_test.go b/service/test/integration2_test.go
index ee5bff81..e62720ed 100644
--- a/service/test/integration2_test.go
+++ b/service/test/integration2_test.go
@@ -17,6 +17,7 @@ import (
 	"time"
 
 	protest "github.com/go-delve/delve/pkg/proc/test"
+	"github.com/go-delve/delve/service/debugger"
 
 	"github.com/go-delve/delve/pkg/goversion"
 	"github.com/go-delve/delve/pkg/logflags"
@@ -68,10 +69,12 @@ func startServer(name string, buildFlags protest.BuildFlags, t *testing.T) (clie
 	}
 	fixture = protest.BuildFixture(name, buildFlags)
 	server := rpccommon.NewServer(&service.Config{
-		Listener:       listener,
-		ProcessArgs:    []string{fixture.Path},
-		Backend:        testBackend,
-		CheckGoVersion: true,
+		Listener:    listener,
+		ProcessArgs: []string{fixture.Path},
+		Debugger: debugger.Config{
+			Backend:        testBackend,
+			CheckGoVersion: true,
+		},
 	})
 	if err := server.Run(); err != nil {
 		t.Fatal(err)
@@ -105,7 +108,9 @@ func TestRunWithInvalidPath(t *testing.T) {
 		Listener:    listener,
 		ProcessArgs: []string{"invalid_path"},
 		APIVersion:  2,
-		Backend:     testBackend,
+		Debugger: debugger.Config{
+			Backend: testBackend,
+		},
 	})
 	if err := server.Run(); err == nil {
 		t.Fatal("Expected Run to return error for invalid program path")
@@ -1566,9 +1571,11 @@ func TestAcceptMulticlient(t *testing.T) {
 		server := rpccommon.NewServer(&service.Config{
 			Listener:       listener,
 			ProcessArgs:    []string{protest.BuildFixture("testvariables2", 0).Path},
-			Backend:        testBackend,
 			AcceptMulti:    true,
 			DisconnectChan: disconnectChan,
+			Debugger: debugger.Config{
+				Backend: testBackend,
+			},
 		})
 		if err := server.Run(); err != nil {
 			t.Fatal(err)