Skip to content

Commit

Permalink
service/dap: accept a string list as launch request's buildFlags
Browse files Browse the repository at this point in the history
This change accepts both string type and []string. dap.BuildFlags
is a union of string and []string.

Fixes go-delve#2718
For golang/vscode-go#1831, golang/vscode-go#1027
  • Loading branch information
hyangah committed Sep 8, 2023
1 parent f469a0a commit 63f52b1
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 10 deletions.
35 changes: 31 additions & 4 deletions pkg/gobuild/gobuild.go
Original file line number Diff line number Diff line change
Expand Up @@ -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...)
}

Expand All @@ -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...)
}

Expand Down Expand Up @@ -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, false), 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
Expand Down
4 changes: 2 additions & 2 deletions service/dap/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
14 changes: 13 additions & 1 deletion service/dap/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5594,6 +5594,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")
Expand Down Expand Up @@ -6304,7 +6316,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})
Expand Down
39 changes: 36 additions & 3 deletions service/dap/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}

0 comments on commit 63f52b1

Please sign in to comment.