From c5cec6710e927457c3c29d6c156415e8539a5111 Mon Sep 17 00:00:00 2001 From: Heschi Kreinick Date: Mon, 24 Feb 2020 16:46:08 -0500 Subject: [PATCH] all: consolidate invokeGo implementations Over time we have accumulated a disturbing number of ways to run the Go command, each of which supported slightly different features and workarounds. Combine all of them. This unavoidably introduces some changes in debug logging behavior; I think we should try to fix them incrementally and consistently. Updates golang/go#37368. Change-Id: I664ca8685bf247a226be3cb807789c2fcddf233d Reviewed-on: https://go-review.googlesource.com/c/tools/+/220737 Run-TryBot: Heschi Kreinick Reviewed-by: Rebecca Stambler --- go/internal/packagesdriver/sizes.go | 102 +++++++--------------------- go/packages/golist.go | 48 ++++--------- go/packages/packagestest/modules.go | 28 +++----- internal/gocommand/invoke.go | 92 +++++++++++++++++++++++++ internal/imports/fix.go | 45 +++--------- internal/imports/mod.go | 4 +- internal/imports/mod_112_test.go | 3 +- internal/imports/mod_test.go | 10 +-- internal/lsp/cache/mod.go | 23 +++++-- internal/lsp/cache/view.go | 19 +++++- internal/lsp/command.go | 17 ++++- internal/lsp/source/errors.go | 40 ----------- 12 files changed, 206 insertions(+), 225 deletions(-) create mode 100644 internal/gocommand/invoke.go delete mode 100644 internal/lsp/source/errors.go diff --git a/go/internal/packagesdriver/sizes.go b/go/internal/packagesdriver/sizes.go index db0c9a7ea61..5ee692d3833 100644 --- a/go/internal/packagesdriver/sizes.go +++ b/go/internal/packagesdriver/sizes.go @@ -11,11 +11,10 @@ import ( "encoding/json" "fmt" "go/types" - "log" - "os" "os/exec" "strings" - "time" + + "golang.org/x/tools/internal/gocommand" ) var debug = false @@ -78,97 +77,42 @@ func GetSizes(ctx context.Context, buildFlags, env []string, dir string, usesExp } func GetSizesGolist(ctx context.Context, buildFlags, env []string, dir string, usesExportData bool) (types.Sizes, error) { - args := []string{"list", "-f", "{{context.GOARCH}} {{context.Compiler}}"} - args = append(args, buildFlags...) - args = append(args, "--", "unsafe") - stdout, stderr, err := invokeGo(ctx, env, dir, usesExportData, args...) + inv := gocommand.Invocation{ + Verb: "list", + Args: []string{"-f", "{{context.GOARCH}} {{context.Compiler}}", "--", "unsafe"}, + Env: env, + BuildFlags: buildFlags, + WorkingDir: dir, + } + stdout, stderr, friendlyErr, rawErr := inv.RunRaw(ctx) var goarch, compiler string - if err != nil { - if strings.Contains(err.Error(), "cannot find main module") { + if rawErr != nil { + if strings.Contains(rawErr.Error(), "cannot find main module") { // User's running outside of a module. All bets are off. Get GOARCH and guess compiler is gc. // TODO(matloob): Is this a problem in practice? - envout, _, enverr := invokeGo(ctx, env, dir, usesExportData, "env", "GOARCH") + inv := gocommand.Invocation{ + Verb: "env", + Args: []string{"GOARCH"}, + Env: env, + WorkingDir: dir, + } + envout, enverr := inv.Run(ctx) if enverr != nil { - return nil, err + return nil, enverr } goarch = strings.TrimSpace(envout.String()) compiler = "gc" } else { - return nil, err + return nil, friendlyErr } } else { fields := strings.Fields(stdout.String()) if len(fields) < 2 { - return nil, fmt.Errorf("could not parse GOARCH and Go compiler in format \" \" from stdout of go command:\n%s\ndir: %s\nstdout: <<%s>>\nstderr: <<%s>>", - cmdDebugStr(env, args...), dir, stdout.String(), stderr.String()) + return nil, fmt.Errorf("could not parse GOARCH and Go compiler in format \" \":\nstdout: <<%s>>\nstderr: <<%s>>", + stdout.String(), stderr.String()) } goarch = fields[0] compiler = fields[1] } return types.SizesFor(compiler, goarch), nil } - -// invokeGo returns the stdout and stderr of a go command invocation. -func invokeGo(ctx context.Context, env []string, dir string, usesExportData bool, args ...string) (*bytes.Buffer, *bytes.Buffer, error) { - if debug { - defer func(start time.Time) { log.Printf("%s for %v", time.Since(start), cmdDebugStr(env, args...)) }(time.Now()) - } - stdout := new(bytes.Buffer) - stderr := new(bytes.Buffer) - cmd := exec.CommandContext(ctx, "go", args...) - // On darwin the cwd gets resolved to the real path, which breaks anything that - // expects the working directory to keep the original path, including the - // go command when dealing with modules. - // The Go stdlib has a special feature where if the cwd and the PWD are the - // same node then it trusts the PWD, so by setting it in the env for the child - // process we fix up all the paths returned by the go command. - cmd.Env = append(append([]string{}, env...), "PWD="+dir) - cmd.Dir = dir - cmd.Stdout = stdout - cmd.Stderr = stderr - if err := cmd.Run(); err != nil { - exitErr, ok := err.(*exec.ExitError) - if !ok { - // Catastrophic error: - // - executable not found - // - context cancellation - return nil, nil, fmt.Errorf("couldn't exec 'go %v': %s %T", args, err, err) - } - - // Export mode entails a build. - // If that build fails, errors appear on stderr - // (despite the -e flag) and the Export field is blank. - // Do not fail in that case. - if !usesExportData { - return nil, nil, fmt.Errorf("go %v: %s: %s", args, exitErr, stderr) - } - } - - // As of writing, go list -export prints some non-fatal compilation - // errors to stderr, even with -e set. We would prefer that it put - // them in the Package.Error JSON (see https://golang.org/issue/26319). - // In the meantime, there's nowhere good to put them, but they can - // be useful for debugging. Print them if $GOPACKAGESPRINTGOLISTERRORS - // is set. - if len(stderr.Bytes()) != 0 && os.Getenv("GOPACKAGESPRINTGOLISTERRORS") != "" { - fmt.Fprintf(os.Stderr, "%s stderr: <<%s>>\n", cmdDebugStr(env, args...), stderr) - } - - // debugging - if false { - fmt.Fprintf(os.Stderr, "%s stdout: <<%s>>\n", cmdDebugStr(env, args...), stdout) - } - - return stdout, stderr, nil -} - -func cmdDebugStr(envlist []string, args ...string) string { - env := make(map[string]string) - for _, kv := range envlist { - split := strings.Split(kv, "=") - k, v := split[0], split[1] - env[k] = v - } - - return fmt.Sprintf("GOROOT=%v GOPATH=%v GO111MODULE=%v PWD=%v go %v", env["GOROOT"], env["GOPATH"], env["GO111MODULE"], env["PWD"], args) -} diff --git a/go/packages/golist.go b/go/packages/golist.go index 459f4addfee..b4a13ef4545 100644 --- a/go/packages/golist.go +++ b/go/packages/golist.go @@ -20,10 +20,10 @@ import ( "strconv" "strings" "sync" - "time" "unicode" "golang.org/x/tools/go/internal/packagesdriver" + "golang.org/x/tools/internal/gocommand" "golang.org/x/tools/internal/packagesinternal" ) @@ -707,29 +707,17 @@ func golistargs(cfg *Config, words []string) []string { func (state *golistState) invokeGo(verb string, args ...string) (*bytes.Buffer, error) { cfg := state.cfg - stdout := new(bytes.Buffer) - stderr := new(bytes.Buffer) - goArgs := []string{verb} - if verb != "env" { - goArgs = append(goArgs, cfg.BuildFlags...) - } - goArgs = append(goArgs, args...) - cmd := exec.CommandContext(state.ctx, "go", goArgs...) - // On darwin the cwd gets resolved to the real path, which breaks anything that - // expects the working directory to keep the original path, including the - // go command when dealing with modules. - // The Go stdlib has a special feature where if the cwd and the PWD are the - // same node then it trusts the PWD, so by setting it in the env for the child - // process we fix up all the paths returned by the go command. - cmd.Env = append(append([]string{}, cfg.Env...), "PWD="+cfg.Dir) - cmd.Dir = cfg.Dir - cmd.Stdout = stdout - cmd.Stderr = stderr - defer func(start time.Time) { - cfg.Logf("%s for %v, stderr: <<%s>> stdout: <<%s>>\n", time.Since(start), cmdDebugStr(cmd, goArgs...), stderr, stdout) - }(time.Now()) - - if err := cmd.Run(); err != nil { + inv := &gocommand.Invocation{ + Verb: verb, + Args: args, + BuildFlags: cfg.BuildFlags, + Env: cfg.Env, + Logf: cfg.Logf, + WorkingDir: cfg.Dir, + } + + stdout, stderr, _, err := inv.RunRaw(cfg.Context) + if err != nil { // Check for 'go' executable not being found. if ee, ok := err.(*exec.Error); ok && ee.Err == exec.ErrNotFound { return nil, fmt.Errorf("'go list' driver requires 'go', but %s", exec.ErrNotFound) @@ -739,7 +727,7 @@ func (state *golistState) invokeGo(verb string, args ...string) (*bytes.Buffer, if !ok { // Catastrophic error: // - context cancellation - return nil, fmt.Errorf("couldn't exec 'go %v': %s %T", args, err, err) + return nil, fmt.Errorf("couldn't run 'go': %v", err) } // Old go version? @@ -860,16 +848,6 @@ func (state *golistState) invokeGo(verb string, args ...string) (*bytes.Buffer, return nil, fmt.Errorf("go %v: %s: %s", args, exitErr, stderr) } } - - // As of writing, go list -export prints some non-fatal compilation - // errors to stderr, even with -e set. We would prefer that it put - // them in the Package.Error JSON (see https://golang.org/issue/26319). - // In the meantime, there's nowhere good to put them, but they can - // be useful for debugging. Print them if $GOPACKAGESPRINTGOLISTERRORS - // is set. - if len(stderr.Bytes()) != 0 && os.Getenv("GOPACKAGESPRINTGOLISTERRORS") != "" { - fmt.Fprintf(os.Stderr, "%s stderr: <<%s>>\n", cmdDebugStr(cmd, args...), stderr) - } return stdout, nil } diff --git a/go/packages/packagestest/modules.go b/go/packages/packagestest/modules.go index e393cdca095..8bf830ad4f9 100644 --- a/go/packages/packagestest/modules.go +++ b/go/packages/packagestest/modules.go @@ -6,17 +6,16 @@ package packagestest import ( "archive/zip" - "bytes" + "context" "fmt" "io/ioutil" "os" - "os/exec" "path" "path/filepath" "regexp" "strings" - "golang.org/x/tools/go/packages" + "golang.org/x/tools/internal/gocommand" ) // Modules is the exporter that produces module layouts. @@ -170,7 +169,14 @@ func (modules) Finalize(exported *Exported) error { // Run go mod download to recreate the mod cache dir with all the extra // stuff in cache. All the files created by Export should be recreated. - if err := invokeGo(exported.Config, "mod", "download"); err != nil { + inv := gocommand.Invocation{ + Verb: "mod", + Args: []string{"download"}, + Env: exported.Config.Env, + BuildFlags: exported.Config.BuildFlags, + WorkingDir: exported.Config.Dir, + } + if _, err := inv.Run(context.Background()); err != nil { return err } return nil @@ -237,20 +243,6 @@ func writeModuleProxy(dir, module, ver string, files map[string]string) error { return nil } -func invokeGo(cfg *packages.Config, args ...string) error { - stdout := new(bytes.Buffer) - stderr := new(bytes.Buffer) - cmd := exec.Command("go", args...) - cmd.Env = append(append([]string{}, cfg.Env...), "PWD="+cfg.Dir) - cmd.Dir = cfg.Dir - cmd.Stdout = stdout - cmd.Stderr = stderr - if err := cmd.Run(); err != nil { - return fmt.Errorf("go %v: %s: %s", args, err, stderr) - } - return nil -} - func modCache(exported *Exported) string { return filepath.Join(exported.temp, "modcache/pkg/mod") } diff --git a/internal/gocommand/invoke.go b/internal/gocommand/invoke.go new file mode 100644 index 00000000000..26cf7b18e35 --- /dev/null +++ b/internal/gocommand/invoke.go @@ -0,0 +1,92 @@ +// Package gocommand is a helper for calling the go command. +package gocommand + +import ( + "bytes" + "context" + "fmt" + "os/exec" + "strings" + "time" +) + +// An Invocation represents a call to the go command. +type Invocation struct { + Verb string + Args []string + BuildFlags []string + Env []string + WorkingDir string + Logf func(format string, args ...interface{}) +} + +// Run runs the invocation, returning its stdout and an error suitable for +// human consumption, including stderr. +func (i *Invocation) Run(ctx context.Context) (*bytes.Buffer, error) { + stdout, _, friendly, _ := i.RunRaw(ctx) + return stdout, friendly +} + +// RunRaw is like Run, but also returns the raw stderr and error for callers +// that want to do low-level error handling/recovery. +func (i *Invocation) RunRaw(ctx context.Context) (stdout *bytes.Buffer, stderr *bytes.Buffer, friendlyError error, rawError error) { + log := i.Logf + if log == nil { + log = func(string, ...interface{}) {} + } + + goArgs := []string{i.Verb} + switch i.Verb { + case "mod": + // mod needs the sub-verb before build flags. + goArgs = append(goArgs, i.Args[0]) + goArgs = append(goArgs, i.BuildFlags...) + goArgs = append(goArgs, i.Args[1:]...) + case "env": + // env doesn't take build flags. + goArgs = append(goArgs, i.Args...) + default: + goArgs = append(goArgs, i.BuildFlags...) + goArgs = append(goArgs, i.Args...) + } + cmd := exec.CommandContext(ctx, "go", goArgs...) + stdout = &bytes.Buffer{} + stderr = &bytes.Buffer{} + cmd.Stdout = stdout + cmd.Stderr = stderr + // On darwin the cwd gets resolved to the real path, which breaks anything that + // expects the working directory to keep the original path, including the + // go command when dealing with modules. + // The Go stdlib has a special feature where if the cwd and the PWD are the + // same node then it trusts the PWD, so by setting it in the env for the child + // process we fix up all the paths returned by the go command. + cmd.Env = append(append([]string{}, i.Env...), "PWD="+i.WorkingDir) + cmd.Dir = i.WorkingDir + + defer func(start time.Time) { log("%s for %v", time.Since(start), cmdDebugStr(cmd)) }(time.Now()) + + rawError = cmd.Run() + friendlyError = rawError + if rawError != nil { + // Check for 'go' executable not being found. + if ee, ok := rawError.(*exec.Error); ok && ee.Err == exec.ErrNotFound { + friendlyError = fmt.Errorf("go command required, not found: %v", ee) + } + if ctx.Err() != nil { + friendlyError = ctx.Err() + } + friendlyError = fmt.Errorf("err: %v: stderr: %s", rawError, stderr) + } + return +} + +func cmdDebugStr(cmd *exec.Cmd) string { + env := make(map[string]string) + for _, kv := range cmd.Env { + split := strings.Split(kv, "=") + k, v := split[0], split[1] + env[k] = v + } + + return fmt.Sprintf("GOROOT=%v GOPATH=%v GO111MODULE=%v GOPROXY=%v PWD=%v go %v", env["GOROOT"], env["GOPATH"], env["GO111MODULE"], env["GOPROXY"], env["PWD"], cmd.Args) +} diff --git a/internal/imports/fix.go b/internal/imports/fix.go index ee01d34b1bd..5e0c9dff036 100644 --- a/internal/imports/fix.go +++ b/internal/imports/fix.go @@ -14,7 +14,6 @@ import ( "go/token" "io/ioutil" "os" - "os/exec" "path" "path/filepath" "reflect" @@ -22,11 +21,11 @@ import ( "strconv" "strings" "sync" - "time" "unicode" "unicode/utf8" "golang.org/x/tools/go/ast/astutil" + "golang.org/x/tools/internal/gocommand" "golang.org/x/tools/internal/gopathwalk" ) @@ -792,7 +791,7 @@ func (e *ProcessEnv) GetResolver() Resolver { if e.resolver != nil { return e.resolver } - out, err := e.invokeGo("env", "GOMOD") + out, err := e.invokeGo(context.TODO(), "env", "GOMOD") if err != nil || len(bytes.TrimSpace(out.Bytes())) == 0 { e.resolver = newGopathResolver(e) return e.resolver @@ -823,38 +822,16 @@ func (e *ProcessEnv) buildContext() *build.Context { return &ctx } -func (e *ProcessEnv) invokeGo(verb string, args ...string) (*bytes.Buffer, error) { - goArgs := []string{verb} - if verb != "env" { - goArgs = append(goArgs, e.BuildFlags...) +func (e *ProcessEnv) invokeGo(ctx context.Context, verb string, args ...string) (*bytes.Buffer, error) { + inv := gocommand.Invocation{ + Verb: verb, + Args: args, + BuildFlags: e.BuildFlags, + Env: e.env(), + Logf: e.Logf, + WorkingDir: e.WorkingDir, } - goArgs = append(goArgs, args...) - cmd := exec.Command("go", goArgs...) - stdout := &bytes.Buffer{} - stderr := &bytes.Buffer{} - cmd.Stdout = stdout - cmd.Stderr = stderr - cmd.Env = e.env() - cmd.Dir = e.WorkingDir - - if e.Debug { - defer func(start time.Time) { e.Logf("%s for %v", time.Since(start), cmdDebugStr(cmd)) }(time.Now()) - } - if err := cmd.Run(); err != nil { - return nil, fmt.Errorf("running go: %v (stderr:\n%s)", err, stderr) - } - return stdout, nil -} - -func cmdDebugStr(cmd *exec.Cmd) string { - env := make(map[string]string) - for _, kv := range cmd.Env { - split := strings.Split(kv, "=") - k, v := split[0], split[1] - env[k] = v - } - - return fmt.Sprintf("GOROOT=%v GOPATH=%v GO111MODULE=%v GOPROXY=%v PWD=%v go %v", env["GOROOT"], env["GOPATH"], env["GO111MODULE"], env["GOPROXY"], env["PWD"], cmd.Args) + return inv.Run(ctx) } func addStdlibCandidates(pass *pass, refs references) { diff --git a/internal/imports/mod.go b/internal/imports/mod.go index c0310e6fb31..28d4b1ff334 100644 --- a/internal/imports/mod.go +++ b/internal/imports/mod.go @@ -146,7 +146,7 @@ func (r *ModuleResolver) init() error { } func (r *ModuleResolver) initAllMods() error { - stdout, err := r.env.invokeGo("list", "-m", "-json", "...") + stdout, err := r.env.invokeGo(context.TODO(), "list", "-m", "-json", "...") if err != nil { return err } @@ -699,7 +699,7 @@ func getMainModuleAnd114(env *ProcessEnv) (*ModuleJSON, bool, error) { {{.GoVersion}} {{range context.ReleaseTags}}{{if eq . "go1.14"}}{{.}}{{end}}{{end}} ` - stdout, err := env.invokeGo("list", "-m", "-f", format) + stdout, err := env.invokeGo(context.TODO(), "list", "-m", "-f", format) if err != nil { return nil, false, nil } diff --git a/internal/imports/mod_112_test.go b/internal/imports/mod_112_test.go index 0305f489f8a..106db7c610c 100644 --- a/internal/imports/mod_112_test.go +++ b/internal/imports/mod_112_test.go @@ -3,6 +3,7 @@ package imports import ( + "context" "testing" ) @@ -13,7 +14,7 @@ func TestNoMainModule(t *testing.T) { package x `, "") defer mt.cleanup() - if _, err := mt.env.invokeGo("mod", "download", "rsc.io/quote@v1.5.1"); err != nil { + if _, err := mt.env.invokeGo(context.Background(), "mod", "download", "rsc.io/quote@v1.5.1"); err != nil { t.Fatal(err) } diff --git a/internal/imports/mod_test.go b/internal/imports/mod_test.go index 4df9c021247..8df024fd0af 100644 --- a/internal/imports/mod_test.go +++ b/internal/imports/mod_test.go @@ -231,10 +231,10 @@ import _ "rsc.io/sampler" mt.assertModuleFoundInDir("rsc.io/sampler", "sampler", `pkg.*mod.*/sampler@.*$`) // Populate vendor/ and clear out the mod cache so we can't cheat. - if _, err := mt.env.invokeGo("mod", "vendor"); err != nil { + if _, err := mt.env.invokeGo(context.Background(), "mod", "vendor"); err != nil { t.Fatal(err) } - if _, err := mt.env.invokeGo("clean", "-modcache"); err != nil { + if _, err := mt.env.invokeGo(context.Background(), "clean", "-modcache"); err != nil { t.Fatal(err) } @@ -259,7 +259,7 @@ import _ "rsc.io/sampler" defer mt.cleanup() // Populate vendor/. - if _, err := mt.env.invokeGo("mod", "vendor"); err != nil { + if _, err := mt.env.invokeGo(context.Background(), "mod", "vendor"); err != nil { t.Fatal(err) } @@ -686,7 +686,7 @@ func setup(t *testing.T, main, wd string) *modTest { t.Fatalf("checking if go.mod exists: %v", err) } if err == nil { - if _, err := env.invokeGo("mod", "download"); err != nil { + if _, err := env.invokeGo(context.Background(), "mod", "download"); err != nil { t.Fatal(err) } } @@ -868,7 +868,7 @@ import _ "rsc.io/quote" `, "") defer mt.cleanup() - if _, err := mt.env.invokeGo("mod", "download", "rsc.io/quote/v2@v2.0.1"); err != nil { + if _, err := mt.env.invokeGo(context.Background(), "mod", "download", "rsc.io/quote/v2@v2.0.1"); err != nil { t.Fatal(err) } diff --git a/internal/lsp/cache/mod.go b/internal/lsp/cache/mod.go index 1241c8b68c8..54a0749b231 100644 --- a/internal/lsp/cache/mod.go +++ b/internal/lsp/cache/mod.go @@ -15,6 +15,7 @@ import ( "golang.org/x/mod/modfile" "golang.org/x/tools/go/packages" + "golang.org/x/tools/internal/gocommand" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/source" "golang.org/x/tools/internal/lsp/telemetry" @@ -169,10 +170,14 @@ func dependencyUpgrades(ctx context.Context, cfg *packages.Config, folder string return nil, nil } // Run "go list -u -m all" to be able to see which deps can be upgraded. - args := []string{"list"} - args = append(args, cfg.BuildFlags...) - args = append(args, []string{"-u", "-m", "all"}...) - stdout, err := source.InvokeGo(ctx, folder, cfg.Env, args...) + inv := gocommand.Invocation{ + Verb: "list", + Args: []string{"-u", "-m", "all"}, + BuildFlags: cfg.BuildFlags, + Env: cfg.Env, + WorkingDir: folder, + } + stdout, err := inv.Run(ctx) if err != nil { return nil, err } @@ -273,8 +278,14 @@ func (s *snapshot) ModTidyHandle(ctx context.Context, realfh source.FileHandle) } // We want to run "go mod tidy" to be able to diff between the real and the temp files. - args := append([]string{"mod", "tidy"}, cfg.BuildFlags...) - if _, err := source.InvokeGo(ctx, folder, cfg.Env, args...); err != nil { + inv := gocommand.Invocation{ + Verb: "mod", + Args: []string{"tidy"}, + BuildFlags: cfg.BuildFlags, + Env: cfg.Env, + WorkingDir: folder, + } + if _, err := inv.Run(ctx); err != nil { // Ignore concurrency errors here. if !modConcurrencyError.MatchString(err.Error()) { data.err = err diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index 739f7c44885..2922460b107 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -21,6 +21,7 @@ import ( "time" "golang.org/x/tools/go/packages" + "golang.org/x/tools/internal/gocommand" "golang.org/x/tools/internal/imports" "golang.org/x/tools/internal/lsp/source" "golang.org/x/tools/internal/lsp/telemetry" @@ -685,12 +686,18 @@ func (v *view) getGoEnv(ctx context.Context, env []string) (string, error) { gopackagesdriver = true } } - b, err := source.InvokeGo(ctx, v.folder.Filename(), env, "env", "-json") + inv := gocommand.Invocation{ + Verb: "env", + Args: []string{"-json"}, + Env: env, + WorkingDir: v.Folder().Filename(), + } + stdout, err := inv.Run(ctx) if err != nil { return "", err } envMap := make(map[string]string) - decoder := json.NewDecoder(b) + decoder := json.NewDecoder(stdout) if err := decoder.Decode(&envMap); err != nil { return "", err } @@ -769,7 +776,13 @@ func (v *view) modfileFlagExists(ctx context.Context, env []string) (bool, error // Borrowed from internal/imports/mod.go:620. const format = `{{range context.ReleaseTags}}{{if eq . "go1.14"}}{{.}}{{end}}{{end}}` folder := v.folder.Filename() - stdout, err := source.InvokeGo(ctx, folder, append(env, "GO111MODULE=off"), "list", "-e", "-f", format) + inv := gocommand.Invocation{ + Verb: "list", + Args: []string{"-e", "-f", format}, + Env: append(env, "GO111MODULE=off"), + WorkingDir: v.Folder().Filename(), + } + stdout, err := inv.Run(ctx) if err != nil { return false, err } diff --git a/internal/lsp/command.go b/internal/lsp/command.go index a7907e67f24..4c0ad878d2e 100644 --- a/internal/lsp/command.go +++ b/internal/lsp/command.go @@ -3,6 +3,7 @@ package lsp import ( "context" + "golang.org/x/tools/internal/gocommand" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/source" errors "golang.org/x/xerrors" @@ -20,7 +21,13 @@ func (s *Server) executeCommand(ctx context.Context, params *protocol.ExecuteCom return nil, err } // Run go.mod tidy on the view. - if _, err := source.InvokeGo(ctx, snapshot.View().Folder().Filename(), snapshot.Config(ctx).Env, "mod", "tidy"); err != nil { + inv := gocommand.Invocation{ + Verb: "mod", + Args: []string{"tidy"}, + Env: snapshot.Config(ctx).Env, + WorkingDir: snapshot.View().Folder().Filename(), + } + if _, err := inv.Run(ctx); err != nil { return nil, err } case "upgrade.dependency": @@ -34,7 +41,13 @@ func (s *Server) executeCommand(ctx context.Context, params *protocol.ExecuteCom } dep := params.Arguments[1].(string) // Run "go get" on the dependency to upgrade it to the latest version. - if _, err := source.InvokeGo(ctx, snapshot.View().Folder().Filename(), snapshot.Config(ctx).Env, "get", dep); err != nil { + inv := gocommand.Invocation{ + Verb: "get", + Args: []string{dep}, + Env: snapshot.Config(ctx).Env, + WorkingDir: snapshot.View().Folder().Filename(), + } + if _, err := inv.Run(ctx); err != nil { return nil, err } } diff --git a/internal/lsp/source/errors.go b/internal/lsp/source/errors.go deleted file mode 100644 index 58586035788..00000000000 --- a/internal/lsp/source/errors.go +++ /dev/null @@ -1,40 +0,0 @@ -package source - -import ( - "bytes" - "context" - "fmt" - "os/exec" - - errors "golang.org/x/xerrors" -) - -// InvokeGo returns the output of a go command invocation. -// It does not try to recover from errors. -func InvokeGo(ctx context.Context, dir string, env []string, args ...string) (*bytes.Buffer, error) { - stdout := new(bytes.Buffer) - stderr := new(bytes.Buffer) - cmd := exec.CommandContext(ctx, "go", args...) - // On darwin the cwd gets resolved to the real path, which breaks anything that - // expects the working directory to keep the original path, including the - // go command when dealing with modules. - // The Go stdlib has a special feature where if the cwd and the PWD are the - // same node then it trusts the PWD, so by setting it in the env for the child - // process we fix up all the paths returned by the go command. - cmd.Env = append(append([]string{}, env...), "PWD="+dir) - cmd.Dir = dir - cmd.Stdout = stdout - cmd.Stderr = stderr - - if err := cmd.Run(); err != nil { - // Check for 'go' executable not being found. - if ee, ok := err.(*exec.Error); ok && ee.Err == exec.ErrNotFound { - return nil, fmt.Errorf("'gopls requires 'go', but %s", exec.ErrNotFound) - } - if ctx.Err() != nil { - return nil, ctx.Err() - } - return stdout, errors.Errorf("err: %v: stderr: %s", err, stderr) - } - return stdout, nil -}