internal/lsp/cache: add concurrency error check for go cmds

This change attempts to fix a concurrency error that would cause
textDocument/CodeLens, textDocument/Formatting, textDocument/DocumentLink,
and textDocument/Hover from failing on go.mod files.

The issue was that the go command would return a potential concurrency
error since the ModHandle and the ModTidyHandle are both using the
temporary go.mod file.

Updates golang/go#37824

Change-Id: I6cd63c1f75817c7308e033aec473966536a2a3bd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/224917
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

go/internal/packagesdriver/sizes.go

@@ -19,8 +19,7 @@ import (
 
 var debug = false
 
-// GetSizes returns the sizes used by the underlying driver with the given parameters.
+func GetSizes(ctx context.Context, buildFlags, env []string, gocmdRunner *gocommand.Runner, dir string) (types.Sizes, error) {
-func GetSizes(ctx context.Context, buildFlags, env []string, dir string, usesExportData bool) (types.Sizes, error) {
 	// TODO(matloob): Clean this up. This code is mostly a copy of packages.findExternalDriver.
 	const toolPrefix = "GOPACKAGESDRIVER="
 	tool := ""
@@ -40,7 +39,7 @@ func GetSizes(ctx context.Context, buildFlags, env []string, dir string, usesExp
 	}
 
 	if tool == "off" {
-		return GetSizesGolist(ctx, buildFlags, env, dir, usesExportData)
+		return GetSizesGolist(ctx, buildFlags, env, gocmdRunner, dir)
 	}
 
 	req, err := json.Marshal(struct {
@@ -76,7 +75,7 @@ func GetSizes(ctx context.Context, buildFlags, env []string, dir string, usesExp
 	return response.Sizes, nil
 }
 
-func GetSizesGolist(ctx context.Context, buildFlags, env []string, dir string, usesExportData bool) (types.Sizes, error) {
+func GetSizesGolist(ctx context.Context, buildFlags, env []string, gocmdRunner *gocommand.Runner, dir string) (types.Sizes, error) {
 	inv := gocommand.Invocation{
 		Verb:       "list",
 		Args:       []string{"-f", "{{context.GOARCH}} {{context.Compiler}}", "--", "unsafe"},
@@ -84,7 +83,7 @@ func GetSizesGolist(ctx context.Context, buildFlags, env []string, dir string, u
 		BuildFlags: buildFlags,
 		WorkingDir: dir,
 	}
-	stdout, stderr, friendlyErr, rawErr := inv.RunRaw(ctx)
+	stdout, stderr, friendlyErr, rawErr := gocmdRunner.RunRaw(ctx, inv)
 	var goarch, compiler string
 	if rawErr != nil {
 		if strings.Contains(rawErr.Error(), "cannot find main module") {
@@ -96,7 +95,7 @@ func GetSizesGolist(ctx context.Context, buildFlags, env []string, dir string, u
 				Env:        env,
 				WorkingDir: dir,
 			}
-			envout, enverr := inv.Run(ctx)
+			envout, enverr := gocmdRunner.Run(ctx, inv)
 			if enverr != nil {
 				return nil, enverr
 			}

go/packages/golist.go

@@ -143,7 +143,7 @@ func goListDriver(cfg *Config, patterns ...string) (*driverResponse, error) {
 		sizeswg.Add(1)
 		go func() {
 			var sizes types.Sizes
-			sizes, sizeserr = packagesdriver.GetSizesGolist(ctx, cfg.BuildFlags, cfg.Env, cfg.Dir, usesExportData(cfg))
+			sizes, sizeserr = packagesdriver.GetSizesGolist(ctx, cfg.BuildFlags, cfg.Env, cfg.gocmdRunner, cfg.Dir)
 			// types.SizesFor always returns nil or a *types.StdSizes.
 			response.dr.Sizes, _ = sizes.(*types.StdSizes)
 			sizeswg.Done()
@@ -717,7 +717,7 @@ func golistargs(cfg *Config, words []string) []string {
 func (state *golistState) invokeGo(verb string, args ...string) (*bytes.Buffer, error) {
 	cfg := state.cfg
 
-	inv := &gocommand.Invocation{
+	inv := gocommand.Invocation{
 		Verb:       verb,
 		Args:       args,
 		BuildFlags: cfg.BuildFlags,
@@ -725,8 +725,11 @@ func (state *golistState) invokeGo(verb string, args ...string) (*bytes.Buffer,
 		Logf:       cfg.Logf,
 		WorkingDir: cfg.Dir,
 	}
-
+	gocmdRunner := cfg.gocmdRunner
-	stdout, stderr, _, err := inv.RunRaw(cfg.Context)
+	if gocmdRunner == nil {
+		gocmdRunner = &gocommand.Runner{}
+	}
+	stdout, stderr, _, err := gocmdRunner.RunRaw(cfg.Context, inv)
 	if err != nil {
 		// Check for 'go' executable not being found.
 		if ee, ok := err.(*exec.Error); ok && ee.Err == exec.ErrNotFound {

go/packages/packages.go

@@ -23,6 +23,7 @@ import (
 	"sync"
 
 	"golang.org/x/tools/go/gcexportdata"
+	"golang.org/x/tools/internal/gocommand"
 	"golang.org/x/tools/internal/packagesinternal"
 )
 
@@ -127,6 +128,9 @@ type Config struct {
 	//
 	Env []string
 
+	// gocmdRunner guards go command calls from concurrency errors.
+	gocmdRunner *gocommand.Runner
+
 	// BuildFlags is a list of command-line flags to be passed through to
 	// the build system's query tool.
 	BuildFlags []string
@@ -311,6 +315,12 @@ func init() {
 	packagesinternal.GetModule = func(p interface{}) *packagesinternal.Module {
 		return p.(*Package).module
 	}
+	packagesinternal.GetGoCmdRunner = func(config interface{}) *gocommand.Runner {
+		return config.(*Config).gocmdRunner
+	}
+	packagesinternal.SetGoCmdRunner = func(config interface{}, runner *gocommand.Runner) {
+		config.(*Config).gocmdRunner = runner
+	}
 }
 
 // An Error describes a problem with a package's metadata, syntax, or types.
@@ -473,6 +483,9 @@ func newLoader(cfg *Config) *loader {
 	if ld.Config.Env == nil {
 		ld.Config.Env = os.Environ()
 	}
+	if ld.Config.gocmdRunner == nil {
+		ld.Config.gocmdRunner = &gocommand.Runner{}
+	}
 	if ld.Context == nil {
 		ld.Context = context.Background()
 	}

go/packages/packagestest/modules.go

@@ -16,6 +16,7 @@ import (
 	"strings"
 
 	"golang.org/x/tools/internal/gocommand"
+	"golang.org/x/tools/internal/packagesinternal"
 )
 
 // Modules is the exporter that produces module layouts.
@@ -166,6 +167,8 @@ func (modules) Finalize(exported *Exported) error {
 		"GOPROXY="+proxyDirToURL(proxyDir),
 		"GOSUMDB=off",
 	)
+	gocmdRunner := &gocommand.Runner{}
+	packagesinternal.SetGoCmdRunner(exported.Config, gocmdRunner)
 
 	// 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.
@@ -176,7 +179,7 @@ func (modules) Finalize(exported *Exported) error {
 		BuildFlags: exported.Config.BuildFlags,
 		WorkingDir: exported.Config.Dir,
 	}
-	if _, err := inv.Run(context.Background()); err != nil {
+	if _, err := gocmdRunner.Run(context.Background(), inv); err != nil {
 		return err
 	}
 	return nil

internal/gocommand/invoke.go

@@ -12,10 +12,70 @@ import (
 	"io"
 	"os"
 	"os/exec"
+	"regexp"
 	"strings"
+	"sync"
 	"time"
+
+	"golang.org/x/tools/internal/telemetry/event"
 )
 
+// An Runner will run go command invocations and serialize
+// them if it sees a concurrency error.
+type Runner struct {
+	// LoadMu guards packages.Load calls and associated state.
+	loadMu         sync.Mutex
+	serializeLoads int
+}
+
+// 1.13: go: updates to go.mod needed, but contents have changed
+// 1.14: go: updating go.mod: existing contents have changed since last read
+var modConcurrencyError = regexp.MustCompile(`go:.*go.mod.*contents have changed`)
+
+// Run calls Runner.RunRaw, serializing requests if they fight over
+// go.mod changes.
+func (runner *Runner) Run(ctx context.Context, inv Invocation) (*bytes.Buffer, error) {
+	stdout, _, friendly, _ := runner.RunRaw(ctx, inv)
+	return stdout, friendly
+}
+
+// Run calls Innvocation.RunRaw, serializing requests if they fight over
+// go.mod changes.
+func (runner *Runner) RunRaw(ctx context.Context, inv Invocation) (*bytes.Buffer, *bytes.Buffer, error, error) {
+	// We want to run invocations concurrently as much as possible. However,
+	// if go.mod updates are needed, only one can make them and the others will
+	// fail. We need to retry in those cases, but we don't want to thrash so
+	// badly we never recover. To avoid that, once we've seen one concurrency
+	// error, start serializing everything until the backlog has cleared out.
+	runner.loadMu.Lock()
+	var locked bool // If true, we hold the mutex and have incremented.
+	if runner.serializeLoads == 0 {
+		runner.loadMu.Unlock()
+	} else {
+		locked = true
+		runner.serializeLoads++
+	}
+	defer func() {
+		if locked {
+			runner.serializeLoads--
+			runner.loadMu.Unlock()
+		}
+	}()
+
+	for {
+		stdout, stderr, friendlyErr, err := inv.runRaw(ctx)
+		if friendlyErr == nil || !modConcurrencyError.MatchString(friendlyErr.Error()) {
+			return stdout, stderr, friendlyErr, err
+		}
+		event.Error(ctx, "Load concurrency error, will retry serially", err)
+		if !locked {
+			runner.loadMu.Lock()
+			runner.serializeLoads++
+			locked = true
+		}
+	}
+}
+
 // An Invocation represents a call to the go command.
 type Invocation struct {
 	Verb       string
@@ -26,16 +86,9 @@ type Invocation struct {
 	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 RunPiped, 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) {
+func (i *Invocation) runRaw(ctx context.Context) (stdout *bytes.Buffer, stderr *bytes.Buffer, friendlyError error, rawError error) {
 	stdout = &bytes.Buffer{}
 	stderr = &bytes.Buffer{}
 	rawError = i.RunPiped(ctx, stdout, stderr)

internal/gocommand/invoke_test.go

@@ -15,7 +15,8 @@ func TestGoVersion(t *testing.T) {
 	inv := gocommand.Invocation{
 		Verb: "version",
 	}
-	if _, err := inv.Run(context.Background()); err != nil {
+	gocmdRunner := &gocommand.Runner{}
+	if _, err := gocmdRunner.Run(context.Background(), inv); err != nil {
 		t.Error(err)
 	}
 }

internal/imports/fix.go

@@ -747,6 +747,8 @@ func getPackageExports(ctx context.Context, wrapped func(PackageExport), searchP
 type ProcessEnv struct {
 	LocalPrefix string
 
+	GocmdRunner *gocommand.Runner
+
 	BuildFlags []string
 
 	// If non-empty, these will be used instead of the
@@ -830,7 +832,7 @@ func (e *ProcessEnv) invokeGo(ctx context.Context, verb string, args ...string)
 		Logf:       e.Logf,
 		WorkingDir: e.WorkingDir,
 	}
-	return inv.Run(ctx)
+	return e.GocmdRunner.Run(ctx, inv)
 }
 
 func addStdlibCandidates(pass *pass, refs references) {

internal/imports/fix_test.go

@@ -19,6 +19,7 @@ import (
 	"testing"
 
 	"golang.org/x/tools/go/packages/packagestest"
+	"golang.org/x/tools/internal/gocommand"
 )
 
 var testDebug = flag.Bool("debug", false, "enable debug output")
@@ -1638,6 +1639,7 @@ func (c testConfig) test(t *testing.T, fn func(*goimportTest)) {
 					GO111MODULE: env["GO111MODULE"],
 					GOSUMDB:     env["GOSUMDB"],
 					WorkingDir:  exported.Config.Dir,
+					GocmdRunner: &gocommand.Runner{},
 				},
 				exported: exported,
 			}

internal/imports/imports.go

@@ -27,6 +27,7 @@ import (
 	"strings"
 
 	"golang.org/x/tools/go/ast/astutil"
+	"golang.org/x/tools/internal/gocommand"
 )
 
 // Options is golang.org/x/tools/imports.Options with extra internal-only options.
@@ -154,6 +155,10 @@ func initialize(filename string, src []byte, opt *Options) ([]byte, *Options, er
 			GOSUMDB:     os.Getenv("GOSUMDB"),
 		}
 	}
+	// Set the gocmdRunner if the user has not provided it.
+	if opt.Env.GocmdRunner == nil {
+		opt.Env.GocmdRunner = &gocommand.Runner{}
+	}
 	if src == nil {
 		b, err := ioutil.ReadFile(filename)
 		if err != nil {

internal/imports/imports_test.go

@@ -5,6 +5,7 @@ import (
 	"os"
 	"testing"
 
+	"golang.org/x/tools/internal/gocommand"
 	"golang.org/x/tools/internal/testenv"
 )
 
@@ -31,8 +32,9 @@ func TestNilOpts(t *testing.T) {
 			name: "default",
 			opt: &Options{
 				Env: &ProcessEnv{
-					GOPATH: build.Default.GOPATH,
+					GOPATH:      build.Default.GOPATH,
-					GOROOT: build.Default.GOROOT,
+					GOROOT:      build.Default.GOROOT,
+					GocmdRunner: &gocommand.Runner{},
 				},
 				Comments:  true,
 				TabIndent: true,

internal/imports/mod_test.go

@@ -19,6 +19,7 @@ import (
 	"testing"
 
 	"golang.org/x/mod/module"
+	"golang.org/x/tools/internal/gocommand"
 	"golang.org/x/tools/internal/gopathwalk"
 	"golang.org/x/tools/internal/testenv"
 	"golang.org/x/tools/txtar"
@@ -676,6 +677,7 @@ func setup(t *testing.T, main, wd string) *modTest {
 		GOPROXY:     proxyDirToURL(proxyDir),
 		GOSUMDB:     "off",
 		WorkingDir:  filepath.Join(mainDir, wd),
+		GocmdRunner: &gocommand.Runner{},
 	}
 	if *testDebug {
 		env.Logf = log.Printf
@@ -850,6 +852,7 @@ func TestInvalidModCache(t *testing.T) {
 		GOPATH:      filepath.Join(dir, "gopath"),
 		GO111MODULE: "on",
 		GOSUMDB:     "off",
+		GocmdRunner: &gocommand.Runner{},
 		WorkingDir:  dir,
 	}
 	resolver := newModuleResolver(env)
@@ -918,9 +921,10 @@ import _ "rsc.io/quote"
 
 func BenchmarkScanModCache(b *testing.B) {
 	env := &ProcessEnv{
-		GOPATH: build.Default.GOPATH,
+		GOPATH:      build.Default.GOPATH,
-		GOROOT: build.Default.GOROOT,
+		GOROOT:      build.Default.GOROOT,
-		Logf:   log.Printf,
+		GocmdRunner: &gocommand.Runner{},
+		Logf:        log.Printf,
 	}
 	exclude := []gopathwalk.RootType{gopathwalk.RootGOROOT}
 	scanToSlice(env.GetResolver(), exclude)

internal/lsp/cache/load.go

@@ -84,7 +84,7 @@ func (s *snapshot) load(ctx context.Context, scopes ...interface{}) error {
 	defer done()
 
 	cfg := s.Config(ctx)
-	pkgs, err := s.view.loadPackages(cfg, query...)
+	pkgs, err := packages.Load(cfg, query...)
 
 	// If the context was canceled, return early. Otherwise, we might be
 	// type-checking an incomplete result. Check the context directly,

internal/lsp/cache/mod.go

@@ -20,6 +20,7 @@ import (
 	"golang.org/x/tools/internal/lsp/protocol"
 	"golang.org/x/tools/internal/lsp/source"
 	"golang.org/x/tools/internal/memoize"
+	"golang.org/x/tools/internal/packagesinternal"
 	"golang.org/x/tools/internal/span"
 	"golang.org/x/tools/internal/telemetry/event"
 	errors "golang.org/x/xerrors"
@@ -220,7 +221,7 @@ func goModWhy(ctx context.Context, cfg *packages.Config, folder string, data *mo
 	for _, req := range data.origParsedFile.Require {
 		inv.Args = append(inv.Args, req.Mod.Path)
 	}
-	stdout, err := inv.Run(ctx)
+	stdout, err := packagesinternal.GetGoCmdRunner(cfg).Run(ctx, inv)
 	if err != nil {
 		return err
 	}
@@ -247,7 +248,7 @@ func dependencyUpgrades(ctx context.Context, cfg *packages.Config, folder string
 		Env:        cfg.Env,
 		WorkingDir: folder,
 	}
-	stdout, err := inv.Run(ctx)
+	stdout, err := packagesinternal.GetGoCmdRunner(cfg).Run(ctx, inv)
 	if err != nil {
 		return err
 	}
@@ -288,6 +289,7 @@ func (s *snapshot) ModTidyHandle(ctx context.Context, realfh source.FileHandle)
 	cfg := s.Config(ctx)
 	options := s.View().Options()
 	folder := s.View().Folder().Filename()
+	gocmdRunner := s.view.gocmdRunner
 
 	wsPackages, err := s.WorkspacePackages(ctx)
 	if ctx.Err() != nil {
@@ -358,12 +360,9 @@ func (s *snapshot) ModTidyHandle(ctx context.Context, realfh source.FileHandle)
 			Env:        cfg.Env,
 			WorkingDir: folder,
 		}
-		if _, err := inv.Run(ctx); err != nil {
+		if _, err := gocmdRunner.Run(ctx, inv); err != nil {
-			// Ignore concurrency errors here.
+			return &modData{
-			if !modConcurrencyError.MatchString(err.Error()) {
+				err: err,
-				return &modData{
-					err: err,
-				}
 			}
 		}
 

internal/lsp/cache/session.go

@@ -11,6 +11,7 @@ import (
 	"sync"
 	"sync/atomic"
 
+	"golang.org/x/tools/internal/gocommand"
 	"golang.org/x/tools/internal/lsp/debug"
 	"golang.org/x/tools/internal/lsp/source"
 	"golang.org/x/tools/internal/span"
@@ -133,6 +134,7 @@ func (s *Session) createView(ctx context.Context, name string, folder span.URI,
 			modHandles:        make(map[span.URI]*modHandle),
 		},
 		ignoredURIs: make(map[span.URI]struct{}),
+		gocmdRunner: &gocommand.Runner{},
 	}
 	v.snapshot.view = v
 

internal/lsp/cache/snapshot.go

@@ -19,6 +19,7 @@ import (
 	"golang.org/x/tools/go/packages"
 	"golang.org/x/tools/internal/lsp/debug/tag"
 	"golang.org/x/tools/internal/lsp/source"
+	"golang.org/x/tools/internal/packagesinternal"
 	"golang.org/x/tools/internal/span"
 	"golang.org/x/tools/internal/telemetry/event"
 	errors "golang.org/x/xerrors"
@@ -94,7 +95,7 @@ func (s *snapshot) Config(ctx context.Context) *packages.Config {
 	verboseOutput := s.view.options.VerboseOutput
 	s.view.optionsMu.Unlock()
 
-	return &packages.Config{
+	cfg := &packages.Config{
 		Env:        env,
 		Dir:        s.view.folder.Filename(),
 		Context:    ctx,
@@ -117,6 +118,9 @@ func (s *snapshot) Config(ctx context.Context) *packages.Config {
 		},
 		Tests: true,
 	}
+	packagesinternal.SetGoCmdRunner(cfg, s.view.gocmdRunner)
+
+	return cfg
 }
 
 func (s *snapshot) buildOverlay() map[string][]byte {

internal/lsp/cache/view.go

@@ -15,12 +15,10 @@ import (
 	"os"
 	"path/filepath"
 	"reflect"
-	"regexp"
 	"strings"
 	"sync"
 	"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/debug"
@@ -111,9 +109,8 @@ type view struct {
 	// `go env` variables that need to be tracked.
 	gopath, gocache string
 
-	// LoadMu guards packages.Load calls and associated state.
+	// gocmdRunner guards go command calls from concurrency errors.
-	loadMu         sync.Mutex
+	gocmdRunner *gocommand.Runner
-	serializeLoads int
 }
 
 type builtinPackageHandle struct {
@@ -279,7 +276,7 @@ func (v *view) WriteEnv(ctx context.Context, w io.Writer) error {
 		Env:        env,
 		WorkingDir: v.Folder().Filename(),
 	}
-	stdout, err := inv.Run(ctx)
+	stdout, err := v.gocmdRunner.Run(ctx, inv)
 	if err != nil {
 		return err
 	}
@@ -364,6 +361,7 @@ func (v *view) buildProcessEnv(ctx context.Context) (*imports.ProcessEnv, error)
 		WorkingDir:  v.folder.Filename(),
 		BuildFlags:  buildFlags,
 		LocalPrefix: localPrefix,
+		GocmdRunner: v.gocmdRunner,
 	}
 	if verboseOutput {
 		processEnv.Logf = func(format string, args ...interface{}) {
@@ -734,7 +732,7 @@ func (v *view) getGoEnv(ctx context.Context, env []string) (string, error) {
 		Env:        env,
 		WorkingDir: v.Folder().Filename(),
 	}
-	stdout, err := inv.Run(ctx)
+	stdout, err := v.gocmdRunner.Run(ctx, inv)
 	if err != nil {
 		return "", err
 	}
@@ -767,50 +765,6 @@ func (v *view) getGoEnv(ctx context.Context, env []string) (string, error) {
 	return "", nil
 }
 
-// 1.13: go: updates to go.mod needed, but contents have changed
-// 1.14: go: updating go.mod: existing contents have changed since last read
-var modConcurrencyError = regexp.MustCompile(`go:.*go.mod.*contents have changed`)
-
-// LoadPackages calls packages.Load, serializing requests if they fight over
-// go.mod changes.
-func (v *view) loadPackages(cfg *packages.Config, patterns ...string) ([]*packages.Package, error) {
-	// We want to run go list calls concurrently as much as possible. However,
-	// if go.mod updates are needed, only one can make them and the others will
-	// fail. We need to retry in those cases, but we don't want to thrash so
-	// badly we never recover. To avoid that, once we've seen one concurrency
-	// error, start serializing everything until the backlog has cleared out.
-	// This could all be avoided on 1.14 by using multiple -modfiles.
-
-	v.loadMu.Lock()
-	var locked bool // If true, we hold the mutex and have incremented.
-	if v.serializeLoads == 0 {
-		v.loadMu.Unlock()
-	} else {
-		locked = true
-		v.serializeLoads++
-	}
-	defer func() {
-		if locked {
-			v.serializeLoads--
-			v.loadMu.Unlock()
-		}
-	}()
-
-	for {
-		pkgs, err := packages.Load(cfg, patterns...)
-		if err == nil || !modConcurrencyError.MatchString(err.Error()) {
-			return pkgs, err
-		}
-
-		event.Error(cfg.Context, "Load concurrency error, will retry serially", err)
-		if !locked {
-			v.loadMu.Lock()
-			v.serializeLoads++
-			locked = true
-		}
-	}
-}
-
 // This function will return the main go.mod file for this folder if it exists and whether the -modfile
 // flag exists for this version of go.
 func (v *view) modfileFlagExists(ctx context.Context, env []string) (bool, error) {
@@ -824,7 +778,7 @@ func (v *view) modfileFlagExists(ctx context.Context, env []string) (bool, error
 		Env:        append(env, "GO111MODULE=off"),
 		WorkingDir: v.Folder().Filename(),
 	}
-	stdout, err := inv.Run(ctx)
+	stdout, err := v.gocmdRunner.Run(ctx, inv)
 	if err != nil {
 		return false, err
 	}

internal/lsp/command.go

@@ -11,6 +11,7 @@ import (
 	"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/packagesinternal"
 	"golang.org/x/tools/internal/xcontext"
 	errors "golang.org/x/xerrors"
 )
@@ -32,14 +33,16 @@ func (s *Server) executeCommand(ctx context.Context, params *protocol.ExecuteCom
 		if !ok {
 			return nil, err
 		}
+		cfg := snapshot.Config(ctx)
 		// Run go.mod tidy on the view.
 		inv := gocommand.Invocation{
 			Verb:       "mod",
 			Args:       []string{"tidy"},
-			Env:        snapshot.Config(ctx).Env,
+			Env:        cfg.Env,
 			WorkingDir: snapshot.View().Folder().Filename(),
 		}
-		if _, err := inv.Run(ctx); err != nil {
+		gocmdRunner := packagesinternal.GetGoCmdRunner(cfg)
+		if _, err := gocmdRunner.Run(ctx, inv); err != nil {
 			return nil, err
 		}
 	case "upgrade.dependency":
@@ -52,14 +55,16 @@ func (s *Server) executeCommand(ctx context.Context, params *protocol.ExecuteCom
 		if !ok {
 			return nil, err
 		}
+		cfg := snapshot.Config(ctx)
 		// Run "go get" on the dependency to upgrade it to the latest version.
 		inv := gocommand.Invocation{
 			Verb:       "get",
 			Args:       strings.Split(deps, " "),
-			Env:        snapshot.Config(ctx).Env,
+			Env:        cfg.Env,
 			WorkingDir: snapshot.View().Folder().Filename(),
 		}
-		if _, err := inv.Run(ctx); err != nil {
+		gocmdRunner := packagesinternal.GetGoCmdRunner(cfg)
+		if _, err := gocmdRunner.Run(ctx, inv); err != nil {
 			return nil, err
 		}
 	}

internal/lsp/debug/info.go

@@ -50,10 +50,10 @@ func PrintVersionInfo(ctx context.Context, w io.Writer, verbose bool, mode Print
 	})
 	fmt.Fprint(w, "\n")
 	section(w, mode, "Go info", func() {
-		i := &gocommand.Invocation{
+		gocmdRunner := &gocommand.Runner{}
+		version, err := gocmdRunner.Run(ctx, gocommand.Invocation{
 			Verb: "version",
-		}
+		})
-		version, err := i.Run(ctx)
 		if err != nil {
 			panic(err)
 		}

internal/lsp/fake/workspace.go

@@ -157,7 +157,8 @@ func (w *Workspace) RunGoCommand(ctx context.Context, verb string, args ...strin
 		Args:       args,
 		WorkingDir: w.workdir,
 	}
-	_, stderr, _, err := inv.RunRaw(ctx)
+	gocmdRunner := &gocommand.Runner{}
+	_, stderr, _, err := gocmdRunner.RunRaw(ctx, inv)
 	if err != nil {
 		return err
 	}

internal/packagesinternal/packages.go

@@ -1,7 +1,11 @@
 // Package packagesinternal exposes internal-only fields from go/packages.
 package packagesinternal
 
-import "time"
+import (
+	"time"
+
+	"golang.org/x/tools/internal/gocommand"
+)
 
 // Fields must match go list;
 type Module struct {
@@ -25,3 +29,7 @@ type ModuleError struct {
 var GetForTest = func(p interface{}) string { return "" }
 
 var GetModule = func(p interface{}) *Module { return nil }
+
+var GetGoCmdRunner = func(config interface{}) *gocommand.Runner { return nil }
+
+var SetGoCmdRunner = func(config interface{}, runner *gocommand.Runner) {}