service/dap: accept a string list as launch request's buildFlags (#3496)

This change accepts both string type and []string. dap.BuildFlags
is a union of string and []string.

Fixes #2718
For golang/vscode-go#1831, golang/vscode-go#1027

Author: hyangah

pkg/gobuild/gobuild.go

@@ -41,8 +41,11 @@ func GoBuild(debugname string, pkgs []string, buildflags string) error {
 
 // GoBuildCombinedOutput builds non-test files in 'pkgs' with the specified 'buildflags'
 // and writes the output at 'debugname'.
-func GoBuildCombinedOutput(debugname string, pkgs []string, buildflags string) (string, []byte, error) {
-	args := goBuildArgs(debugname, pkgs, buildflags, false)
+func GoBuildCombinedOutput(debugname string, pkgs []string, buildflags interface{}) (string, []byte, error) {
+	args, err := goBuildArgs2(debugname, pkgs, buildflags, false)
+	if err != nil {
+		return "", nil, err
+	}
 	return gocommandCombinedOutput("build", args...)
 }
 
@@ -55,8 +58,11 @@ func GoTestBuild(debugname string, pkgs []string, buildflags string) error {
 
 // GoTestBuildCombinedOutput builds test files 'pkgs' with the specified 'buildflags'
 // and writes the output at 'debugname'.
-func GoTestBuildCombinedOutput(debugname string, pkgs []string, buildflags string) (string, []byte, error) {
-	args := goBuildArgs(debugname, pkgs, buildflags, true)
+func GoTestBuildCombinedOutput(debugname string, pkgs []string, buildflags interface{}) (string, []byte, error) {
+	args, err := goBuildArgs2(debugname, pkgs, buildflags, true)
+	if err != nil {
+		return "", nil, err
+	}
 	return gocommandCombinedOutput("test", args...)
 }
 
@@ -84,6 +90,27 @@ func goBuildArgs(debugname string, pkgs []string, buildflags string, isTest bool
 	return args
 }
 
+// goBuildArgs2 is like goBuildArgs, but takes either string or []string.
+func goBuildArgs2(debugname string, pkgs []string, buildflags interface{}, isTest bool) ([]string, error) {
+	var args []string
+	switch buildflags := buildflags.(type) {
+	case string:
+		return goBuildArgs(debugname, pkgs, buildflags, isTest), nil
+	case nil:
+	case []string:
+		args = append(args, buildflags...)
+	default:
+		return nil, fmt.Errorf("invalid buildflags type %T", buildflags)
+	}
+
+	args = append(args, "-o", debugname)
+	if isTest {
+		args = append([]string{"-c"}, args...)
+	}
+	args = append(args, "-gcflags", "all=-N -l")
+	return append(args, pkgs...), nil
+}
+
 func gocommandRun(command string, args ...string) error {
 	_, goBuild := gocommandExecCmd(command, args...)
 	goBuild.Stderr = os.Stdout

service/dap/server.go

@@ -982,9 +982,9 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) {
 
 		switch args.Mode {
 		case "debug":
-			cmd, out, err = gobuild.GoBuildCombinedOutput(args.Output, []string{args.Program}, args.BuildFlags)
+			cmd, out, err = gobuild.GoBuildCombinedOutput(args.Output, []string{args.Program}, args.BuildFlags.value)
 		case "test":
-			cmd, out, err = gobuild.GoTestBuildCombinedOutput(args.Output, []string{args.Program}, args.BuildFlags)
+			cmd, out, err = gobuild.GoTestBuildCombinedOutput(args.Output, []string{args.Program}, args.BuildFlags.value)
 		}
 		args.DlvCwd, _ = filepath.Abs(args.DlvCwd)
 		s.config.log.Debugf("building from %q: [%s]", args.DlvCwd, cmd)

service/dap/server_test.go

@@ -5659,6 +5659,18 @@ func TestLaunchRequestWithBuildFlags(t *testing.T) {
 	})
 }
 
+func TestLaunchRequestWithBuildFlags2(t *testing.T) {
+	runTest(t, "buildflagtest", func(client *daptest.Client, fixture protest.Fixture) {
+		runDebugSession(t, client, "launch", func() {
+			// We reuse the harness that builds, but ignore the built binary,
+			// only relying on the source to be built in response to LaunchRequest.
+			client.LaunchRequestWithArgs(map[string]interface{}{
+				"mode": "debug", "program": fixture.Source, "output": "__mybin",
+				"buildFlags": []string{"-ldflags", "-X main.Hello=World"}})
+		})
+	})
+}
+
 func TestLaunchRequestWithEnv(t *testing.T) {
 	// testenv fixture will lookup SOMEVAR with
 	//   x, y := os.Lookup("SOMEVAR")
@@ -6362,7 +6374,7 @@ func TestBadLaunchRequests(t *testing.T) {
 		// Bad "buildFlags"
 		client.LaunchRequestWithArgs(map[string]interface{}{"mode": "debug", "program": fixture.Source, "buildFlags": 123})
 		checkFailedToLaunchWithMessage(client.ExpectVisibleErrorResponse(t),
-			"Failed to launch: invalid debug configuration - cannot unmarshal number into \"buildFlags\" of type string")
+			"Failed to launch: invalid debug configuration - cannot unmarshal number into \"buildFlags\" of type []string or string")
 
 		// Bad "backend"
 		client.LaunchRequestWithArgs(map[string]interface{}{"mode": "debug", "program": fixture.Source, "backend": 123})

service/dap/types.go

@@ -102,9 +102,15 @@ type LaunchConfig struct {
 	// Relative paths used in BuildFlags will be interpreted as paths
 	// relative to Delve's current working directory.
 	//
-	// It is like `dlv --build-flags`. For example,
-	//    "buildFlags": "-tags=integration -mod=vendor -cover -v"
-	BuildFlags string `json:"buildFlags,omitempty"`
+	// It should be a string like `dlv --build-flags`, or
+	// an array of strings that is augmented when invoking the go build or
+	// test command through os/exec.Command API.
+	// For example, both forms are acceptible.
+	//    "buildFlags": "-tags=integration -ldflags='-X main.Hello=World'"
+	// or
+	//    "buildFlags": ["-tags=integration", "-ldflags=-X main.Hello=World"]
+	// Using other types is an error.
+	BuildFlags BuildFlags `json:"buildFlags,omitempty"`
 
 	// Output path for the binary of the debuggee.
 	// Relative path is interpreted as the path relative to
@@ -268,3 +274,30 @@ func prettyPrint(config interface{}) string {
 	}
 	return string(pretty)
 }
+
+// BuildFlags is either string or []string.
+type BuildFlags struct {
+	value interface{}
+}
+
+func (s *BuildFlags) UnmarshalJSON(b []byte) error {
+	if v := string(b); v == "" || v == "null" {
+		s.value = nil
+		return nil
+	}
+	var strs []string
+	if err := json.Unmarshal(b, &strs); err == nil {
+		s.value = strs
+		return nil
+	}
+	var str string
+	if err := json.Unmarshal(b, &str); err != nil {
+		s.value = nil
+		if uerr, ok := err.(*json.UnmarshalTypeError); ok {
+			return fmt.Errorf(`cannot unmarshal %v into "buildFlags" of type []string or string`, uerr.Value)
+		}
+		return err
+	}
+	s.value = str
+	return nil
+}