internal/lsp: use gocommand.Invocation more
We pass around verb/args/wd in many places. Bundle them up as an
Invocation instead. goCommandInvocation now updates and returns an input
Invocation.
packages.Config is conceptually an extra layer of parsing and
type-checking on top of a go list invocation. It doesn't make sense for
us to construct the latter using the former. A later CL will construct
the Config in terms of the Invocation; for now duplicate a bit of logic.
Use the environment in the cache key for various module operations
rather than the Config hash. I'm not sure either is fully correct but I
think the environment captures everything that's likely to matter. This
way we don't need Configs in those functions at all.
Change-Id: Iebee2705e63638ab365b3ee18b23f8c3e8ca30ce
Reviewed-on: https://go-review.googlesource.com/c/tools/+/265237
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go
index c7bf6ec..74a3ffb 100644
--- a/internal/lsp/cache/check.go
+++ b/internal/lsp/cache/check.go
@@ -189,6 +189,19 @@
return packageHandleKey(hashContents(b.Bytes()))
}
+// hashEnv returns a hash of the snapshot's configuration.
+func hashEnv(s *snapshot) string {
+ s.view.optionsMu.Lock()
+ env := s.view.options.EnvSlice()
+ s.view.optionsMu.Unlock()
+
+ b := &bytes.Buffer{}
+ for _, e := range env {
+ b.WriteString(e)
+ }
+ return hashContents(b.Bytes())
+}
+
// hashConfig returns the hash for the *packages.Config.
func hashConfig(config *packages.Config) string {
b := bytes.NewBuffer(nil)
diff --git a/internal/lsp/cache/mod.go b/internal/lsp/cache/mod.go
index 0c2a96b..736b56b 100644
--- a/internal/lsp/cache/mod.go
+++ b/internal/lsp/cache/mod.go
@@ -19,11 +19,11 @@
"golang.org/x/mod/modfile"
"golang.org/x/mod/module"
"golang.org/x/tools/internal/event"
+ "golang.org/x/tools/internal/gocommand"
"golang.org/x/tools/internal/lsp/debug/tag"
"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"
errors "golang.org/x/xerrors"
)
@@ -165,7 +165,7 @@
// modKey is uniquely identifies cached data for `go mod why` or dependencies
// to upgrade.
type modKey struct {
- sessionID, cfg, view string
+ sessionID, env, view string
mod source.FileIdentity
verb modAction
}
@@ -208,11 +208,9 @@
if handle := s.getModWhyHandle(fh.URI()); handle != nil {
return handle.why(ctx, s)
}
- // Make sure to use the module root as the working directory.
- cfg := s.config(ctx, filepath.Dir(fh.URI().Filename()))
key := modKey{
sessionID: s.view.session.id,
- cfg: hashConfig(cfg),
+ env: hashEnv(s),
mod: fh.FileIdentity(),
view: s.view.rootURI.Filename(),
verb: why,
@@ -232,11 +230,15 @@
return &modWhyData{}
}
// Run `go mod why` on all the dependencies.
- args := []string{"why", "-m"}
- for _, req := range pm.File.Require {
- args = append(args, req.Mod.Path)
+ inv := &gocommand.Invocation{
+ Verb: "mod",
+ Args: []string{"why", "-m"},
+ WorkingDir: filepath.Dir(fh.URI().Filename()),
}
- stdout, err := snapshot.runGoCommandWithConfig(ctx, cfg, "mod", args)
+ for _, req := range pm.File.Require {
+ inv.Args = append(inv.Args, req.Mod.Path)
+ }
+ stdout, err := snapshot.RunGoCommandDirect(ctx, inv)
if err != nil {
return &modWhyData{err: err}
}
@@ -300,11 +302,9 @@
if handle := s.getModUpgradeHandle(fh.URI()); handle != nil {
return handle.upgrades(ctx, s)
}
- // Use the module root as the working directory.
- cfg := s.config(ctx, filepath.Dir(fh.URI().Filename()))
key := modKey{
sessionID: s.view.session.id,
- cfg: hashConfig(cfg),
+ env: hashEnv(s),
mod: fh.FileIdentity(),
view: s.view.rootURI.Filename(),
verb: upgrade,
@@ -326,13 +326,17 @@
}
// Run "go list -mod readonly -u -m all" to be able to see which deps can be
// upgraded without modifying mod file.
- args := []string{"-u", "-m", "-json", "all"}
+ inv := &gocommand.Invocation{
+ Verb: "list",
+ Args: []string{"-u", "-m", "-json", "all"},
+ WorkingDir: filepath.Dir(fh.URI().Filename()),
+ }
if s.workspaceMode()&tempModfile == 0 || containsVendor(fh.URI()) {
// Use -mod=readonly if the module contains a vendor directory
// (see golang/go#38711).
- packagesinternal.SetModFlag(cfg, "readonly")
+ inv.ModFlag = "readonly"
}
- stdout, err := snapshot.runGoCommandWithConfig(ctx, cfg, "list", args)
+ stdout, err := snapshot.RunGoCommandDirect(ctx, inv)
if err != nil {
return &modUpgradeData{err: err}
}
diff --git a/internal/lsp/cache/mod_tidy.go b/internal/lsp/cache/mod_tidy.go
index 67feb17..375fddc 100644
--- a/internal/lsp/cache/mod_tidy.go
+++ b/internal/lsp/cache/mod_tidy.go
@@ -17,6 +17,7 @@
"golang.org/x/mod/modfile"
"golang.org/x/tools/internal/event"
+ "golang.org/x/tools/internal/gocommand"
"golang.org/x/tools/internal/lsp/debug/tag"
"golang.org/x/tools/internal/lsp/diff"
"golang.org/x/tools/internal/lsp/protocol"
@@ -27,7 +28,7 @@
type modTidyKey struct {
sessionID string
- cfg string
+ env string
gomod source.FileIdentity
imports string
unsavedOverlays string
@@ -82,15 +83,13 @@
overlayHash := hashUnsavedOverlays(s.files)
s.mu.Unlock()
- // Make sure to use the module root in the configuration.
- cfg := s.config(ctx, filepath.Dir(fh.URI().Filename()))
key := modTidyKey{
sessionID: s.view.session.id,
view: s.view.folder.Filename(),
imports: importHash,
unsavedOverlays: overlayHash,
gomod: fh.FileIdentity(),
- cfg: hashConfig(cfg),
+ env: hashEnv(s),
}
h := s.generation.Bind(key, func(ctx context.Context, arg memoize.Arg) interface{} {
ctx, done := event.Start(ctx, "cache.ModTidyHandle", tag.URI.Of(fh.URI()))
@@ -114,17 +113,19 @@
err: err,
}
}
- // Get a new config to avoid races, since it may be modified by
- // goCommandInvocation.
- cfg := s.config(ctx, filepath.Dir(fh.URI().Filename()))
- tmpURI, runner, inv, cleanup, err := snapshot.goCommandInvocation(ctx, cfg, true, "mod", []string{"tidy"})
+ inv := &gocommand.Invocation{
+ Verb: "mod",
+ Args: []string{"tidy"},
+ WorkingDir: filepath.Dir(fh.URI().Filename()),
+ }
+ tmpURI, inv, cleanup, err := snapshot.goCommandInvocation(ctx, true, inv)
if err != nil {
return &modTidyData{err: err}
}
// Keep the temporary go.mod file around long enough to parse it.
defer cleanup()
- if _, err := runner.Run(ctx, *inv); err != nil {
+ if _, err := s.view.session.gocmdRunner.Run(ctx, *inv); err != nil {
return &modTidyData{err: err}
}
// Go directly to disk to get the temporary mod file, since it is
diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go
index 021c534..34d78a1 100644
--- a/internal/lsp/cache/snapshot.go
+++ b/internal/lsp/cache/snapshot.go
@@ -184,7 +184,8 @@
// module.
func (s *snapshot) config(ctx context.Context, dir string) *packages.Config {
s.view.optionsMu.Lock()
- env, buildFlags := s.view.envLocked()
+ env := s.view.options.EnvSlice()
+ buildFlags := append([]string{}, s.view.options.BuildFlags...)
verboseOutput := s.view.options.VerboseOutput
s.view.optionsMu.Unlock()
@@ -192,7 +193,7 @@
Context: ctx,
Dir: dir,
Env: append(append([]string{}, env...), "GO111MODULE="+s.view.go111module),
- BuildFlags: append([]string{}, buildFlags...),
+ BuildFlags: buildFlags,
Mode: packages.NeedName |
packages.NeedFiles |
packages.NeedCompiledGoFiles |
@@ -220,63 +221,48 @@
return cfg
}
-func (s *snapshot) RunGoCommandDirect(ctx context.Context, wd, verb string, args []string) error {
- cfg := s.config(ctx, wd)
- _, runner, inv, cleanup, err := s.goCommandInvocation(ctx, cfg, false, verb, args)
- if err != nil {
- return err
- }
- defer cleanup()
-
- _, err = runner.Run(ctx, *inv)
- return err
-}
-
-func (s *snapshot) runGoCommandWithConfig(ctx context.Context, cfg *packages.Config, verb string, args []string) (*bytes.Buffer, error) {
- _, runner, inv, cleanup, err := s.goCommandInvocation(ctx, cfg, true, verb, args)
+func (s *snapshot) RunGoCommandDirect(ctx context.Context, inv *gocommand.Invocation) (*bytes.Buffer, error) {
+ _, inv, cleanup, err := s.goCommandInvocation(ctx, false, inv)
if err != nil {
return nil, err
}
defer cleanup()
- return runner.Run(ctx, *inv)
+ return s.view.session.gocmdRunner.Run(ctx, *inv)
}
-func (s *snapshot) RunGoCommandPiped(ctx context.Context, wd, verb string, args []string, stdout, stderr io.Writer) error {
- cfg := s.config(ctx, wd)
- _, runner, inv, cleanup, err := s.goCommandInvocation(ctx, cfg, true, verb, args)
+func (s *snapshot) RunGoCommandPiped(ctx context.Context, inv *gocommand.Invocation, stdout, stderr io.Writer) error {
+ _, inv, cleanup, err := s.goCommandInvocation(ctx, true, inv)
if err != nil {
return err
}
defer cleanup()
- return runner.RunPiped(ctx, *inv, stdout, stderr)
+ return s.view.session.gocmdRunner.RunPiped(ctx, *inv, stdout, stderr)
}
-func (s *snapshot) goCommandInvocation(ctx context.Context, cfg *packages.Config, allowTempModfile bool, verb string, args []string) (tmpURI span.URI, runner *gocommand.Runner, inv *gocommand.Invocation, cleanup func(), err error) {
+func (s *snapshot) goCommandInvocation(ctx context.Context, allowTempModfile bool, inv *gocommand.Invocation) (tmpURI span.URI, updatedInv *gocommand.Invocation, cleanup func(), err error) {
+ s.view.optionsMu.Lock()
+ env := s.view.options.EnvSlice()
+ s.view.optionsMu.Unlock()
+
cleanup = func() {} // fallback
- modURI := s.GoModForFile(ctx, span.URIFromPath(cfg.Dir))
+ inv.Env = append(append(append([]string{}, env...), inv.Env...), "GO111MODULE="+s.view.go111module)
- inv = &gocommand.Invocation{
- Verb: verb,
- Args: args,
- Env: cfg.Env,
- WorkingDir: cfg.Dir,
- }
-
+ modURI := s.GoModForFile(ctx, span.URIFromPath(inv.WorkingDir))
if allowTempModfile && s.workspaceMode()&tempModfile != 0 {
if modURI == "" {
- return "", nil, nil, cleanup, fmt.Errorf("no go.mod file found in %s", cfg.Dir)
+ return "", nil, cleanup, fmt.Errorf("no go.mod file found in %s", inv.WorkingDir)
}
modFH, err := s.GetFile(ctx, modURI)
if err != nil {
- return "", nil, nil, cleanup, err
+ return "", nil, cleanup, err
}
// Use the go.sum if it happens to be available.
sumFH, _ := s.sumFH(ctx, modFH)
tmpURI, cleanup, err = tempModFile(modFH, sumFH)
if err != nil {
- return "", nil, nil, cleanup, err
+ return "", nil, cleanup, err
}
inv.ModFile = tmpURI.Filename()
}
@@ -285,23 +271,22 @@
if modURI != "" {
modFH, err := s.GetFile(ctx, modURI)
if err != nil {
- return "", nil, nil, cleanup, err
+ return "", nil, cleanup, err
}
modContent, err = modFH.Read()
if err != nil {
- return "", nil, nil, nil, err
+ return "", nil, nil, err
}
}
modMod, err := s.needsModEqualsMod(ctx, modURI, modContent)
if err != nil {
- return "", nil, nil, cleanup, err
+ return "", nil, cleanup, err
}
if modMod {
inv.ModFlag = "mod"
}
- runner = packagesinternal.GetGoCmdRunner(cfg)
- return tmpURI, runner, inv, cleanup, nil
+ return tmpURI, inv, cleanup, nil
}
func (s *snapshot) buildOverlay() map[string][]byte {
diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go
index b20182a..42017a8 100644
--- a/internal/lsp/cache/view.go
+++ b/internal/lsp/cache/view.go
@@ -285,7 +285,8 @@
func (s *snapshot) WriteEnv(ctx context.Context, w io.Writer) error {
s.view.optionsMu.Lock()
- env, buildFlags := s.view.envLocked()
+ env := s.view.options.EnvSlice()
+ buildFlags := append([]string{}, s.view.options.BuildFlags...)
s.view.optionsMu.Unlock()
fullEnv := make(map[string]string)
@@ -330,14 +331,6 @@
return s.view.importsState.runProcessEnvFunc(ctx, s, fn)
}
-// envLocked returns the environment and build flags for the current view.
-// It assumes that the caller is holding the view's optionsMu.
-func (v *View) envLocked() ([]string, []string) {
- env := append(os.Environ(), v.options.EnvSlice()...)
- buildFlags := append([]string{}, v.options.BuildFlags...)
- return env, buildFlags
-}
-
func (v *View) contains(uri span.URI) bool {
return strings.HasPrefix(string(uri), string(v.rootURI))
}
diff --git a/internal/lsp/command.go b/internal/lsp/command.go
index 2603a51..80e762c 100644
--- a/internal/lsp/command.go
+++ b/internal/lsp/command.go
@@ -14,6 +14,7 @@
"path/filepath"
"golang.org/x/tools/internal/event"
+ "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/span"
@@ -262,10 +263,14 @@
if err != nil {
return err
}
- wdir := filepath.Dir(uri.SpanURI().Filename())
snapshot, release := view.Snapshot(ctx)
defer release()
- return snapshot.RunGoCommandDirect(ctx, wdir, verb, args)
+ _, err = snapshot.RunGoCommandDirect(ctx, &gocommand.Invocation{
+ Verb: verb,
+ Args: args,
+ WorkingDir: filepath.Dir(uri.SpanURI().Filename()),
+ })
+ return err
}
func (s *Server) runTests(ctx context.Context, snapshot source.Snapshot, uri protocol.DocumentURI, work *workDone, tests, benchmarks []string) error {
@@ -283,13 +288,15 @@
ew := &eventWriter{ctx: ctx, operation: "test"}
out := io.MultiWriter(ew, workDoneWriter{work}, buf)
- wdir := filepath.Dir(uri.SpanURI().Filename())
-
// Run `go test -run Func` on each test.
var failedTests int
for _, funcName := range tests {
- args := []string{pkgPath, "-v", "-count=1", "-run", fmt.Sprintf("^%s$", funcName)}
- if err := snapshot.RunGoCommandPiped(ctx, wdir, "test", args, out, out); err != nil {
+ inv := &gocommand.Invocation{
+ Verb: "test",
+ Args: []string{pkgPath, "-v", "-count=1", "-run", fmt.Sprintf("^%s$", funcName)},
+ WorkingDir: filepath.Dir(uri.SpanURI().Filename()),
+ }
+ if err := snapshot.RunGoCommandPiped(ctx, inv, out, out); err != nil {
if errors.Is(err, context.Canceled) {
return err
}
@@ -300,8 +307,12 @@
// Run `go test -run=^$ -bench Func` on each test.
var failedBenchmarks int
for _, funcName := range benchmarks {
- args := []string{pkgPath, "-v", "-run=^$", "-bench", fmt.Sprintf("^%s$", funcName)}
- if err := snapshot.RunGoCommandPiped(ctx, wdir, "test", args, out, out); err != nil {
+ inv := &gocommand.Invocation{
+ Verb: "test",
+ Args: []string{pkgPath, "-v", "-run=^$", "-bench", fmt.Sprintf("^%s$", funcName)},
+ WorkingDir: filepath.Dir(uri.SpanURI().Filename()),
+ }
+ if err := snapshot.RunGoCommandPiped(ctx, inv, out, out); err != nil {
if errors.Is(err, context.Canceled) {
return err
}
@@ -344,16 +355,19 @@
defer cancel()
er := &eventWriter{ctx: ctx, operation: "generate"}
- args := []string{"-x"}
+
pattern := "."
if recursive {
pattern = "..."
}
- args = append(args, pattern)
+ inv := &gocommand.Invocation{
+ Verb: "generate",
+ Args: []string{"-x", pattern},
+ WorkingDir: dir.Filename(),
+ }
stderr := io.MultiWriter(er, workDoneWriter{work})
-
- if err := snapshot.RunGoCommandPiped(ctx, dir.Filename(), "generate", args, er, stderr); err != nil {
+ if err := snapshot.RunGoCommandPiped(ctx, inv, er, stderr); err != nil {
return err
}
return nil
diff --git a/internal/lsp/source/gc_annotations.go b/internal/lsp/source/gc_annotations.go
index 6494b6a..0a0d2b5 100644
--- a/internal/lsp/source/gc_annotations.go
+++ b/internal/lsp/source/gc_annotations.go
@@ -14,6 +14,7 @@
"path/filepath"
"strings"
+ "golang.org/x/tools/internal/gocommand"
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/span"
)
@@ -36,12 +37,16 @@
if !strings.HasPrefix(outDir, "/") {
outDirURI = span.URI(strings.Replace(string(outDirURI), "file:///", "file://", 1))
}
- args := []string{
- fmt.Sprintf("-gcflags=-json=0,%s", outDirURI),
- fmt.Sprintf("-o=%s", tmpFile.Name()),
- ".",
+ inv := &gocommand.Invocation{
+ Verb: "build",
+ Args: []string{
+ fmt.Sprintf("-gcflags=-json=0,%s", outDirURI),
+ fmt.Sprintf("-o=%s", tmpFile.Name()),
+ ".",
+ },
+ WorkingDir: pkgDir.Filename(),
}
- err = snapshot.RunGoCommandDirect(ctx, pkgDir.Filename(), "build", args)
+ _, err = snapshot.RunGoCommandDirect(ctx, inv)
if err != nil {
return nil, err
}
diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go
index c84ff06..2f3a212 100644
--- a/internal/lsp/source/view.go
+++ b/internal/lsp/source/view.go
@@ -5,6 +5,7 @@
package source
import (
+ "bytes"
"context"
"fmt"
"go/ast"
@@ -16,6 +17,7 @@
"golang.org/x/mod/modfile"
"golang.org/x/mod/module"
"golang.org/x/tools/go/analysis"
+ "golang.org/x/tools/internal/gocommand"
"golang.org/x/tools/internal/imports"
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/span"
@@ -80,16 +82,13 @@
// Analyze runs the analyses for the given package at this snapshot.
Analyze(ctx context.Context, pkgID string, analyzers ...*analysis.Analyzer) ([]*Error, error)
- // RunGoCommandPiped runs the given `go` command in the view, using the
- // provided stdout and stderr. It will use the -modfile flag, if possible.
- // If the provided working directory is empty, the snapshot's root folder
- // will be used as the working directory.
- RunGoCommandPiped(ctx context.Context, wd, verb string, args []string, stdout, stderr io.Writer) error
+ // RunGoCommandPiped runs the given `go` command, writing its output
+ // to stdout and stderr. Verb, Args, and WorkingDir must be specified.
+ RunGoCommandPiped(ctx context.Context, inv *gocommand.Invocation, stdout, stderr io.Writer) error
- // RunGoCommandDirect runs the given `go` command, never using the
- // -modfile flag. If the provided working directory is empty, the
- // snapshot's root folder will be used as the working directory.
- RunGoCommandDirect(ctx context.Context, wd, verb string, args []string) error
+ // RunGoCommandDirect runs the given `go` command. Verb, Args, and
+ // WorkingDir must be specified.
+ RunGoCommandDirect(ctx context.Context, inv *gocommand.Invocation) (*bytes.Buffer, error)
// RunProcessEnvFunc runs fn with the process env for this snapshot's view.
// Note: the process env contains cached module and filesystem state.
