all: use explicit -mod, -modfile fields for gocommand.Invocation
Build flags, -mod, and -modfile are all accepted by disjoint go command
verbs. Mixing them together had the effect of forcing gocommand users to
figure out which went where themselves, which was annoying and
error-prone. Add ModFlag and ModFile fields to Invocation and update all
uses to use them.
go/packages has a BuildFlags field that's bad for the same reason. Add
private modFlag and modFile fields that forward to the corresponding
Invocation fields.
imports.ProcessEnv gets the same treatment.
Fixes golang/go#41826.
Change-Id: If74d19146e9e62930d7c34f40859c27c25566c4e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/263213
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/go/internal/packagesdriver/sizes.go b/go/internal/packagesdriver/sizes.go
index dc6177c..35bc6a4 100644
--- a/go/internal/packagesdriver/sizes.go
+++ b/go/internal/packagesdriver/sizes.go
@@ -39,7 +39,12 @@
}
if tool == "off" {
- return GetSizesGolist(ctx, buildFlags, env, gocmdRunner, dir)
+ inv := gocommand.Invocation{
+ BuildFlags: buildFlags,
+ Env: env,
+ WorkingDir: dir,
+ }
+ return GetSizesGolist(ctx, inv, gocmdRunner)
}
req, err := json.Marshal(struct {
@@ -75,26 +80,17 @@
return response.Sizes, nil
}
-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"},
- Env: env,
- BuildFlags: buildFlags,
- WorkingDir: dir,
- }
+func GetSizesGolist(ctx context.Context, inv gocommand.Invocation, gocmdRunner *gocommand.Runner) (types.Sizes, error) {
+ inv.Verb = "list"
+ inv.Args = []string{"-f", "{{context.GOARCH}} {{context.Compiler}}", "--", "unsafe"}
stdout, stderr, friendlyErr, rawErr := gocmdRunner.RunRaw(ctx, inv)
var goarch, compiler string
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?
- inv := gocommand.Invocation{
- Verb: "env",
- Args: []string{"GOARCH"},
- Env: env,
- WorkingDir: dir,
- }
+ inv.Verb = "env"
+ inv.Args = []string{"GOARCH"}
envout, enverr := gocmdRunner.Run(ctx, inv)
if enverr != nil {
return nil, enverr
diff --git a/go/packages/golist.go b/go/packages/golist.go
index d7dac20..2696cfb 100644
--- a/go/packages/golist.go
+++ b/go/packages/golist.go
@@ -139,6 +139,12 @@
response := newDeduper()
+ state := &golistState{
+ cfg: cfg,
+ ctx: ctx,
+ vendorDirs: map[string]bool{},
+ }
+
// Fill in response.Sizes asynchronously if necessary.
var sizeserr error
var sizeswg sync.WaitGroup
@@ -146,19 +152,13 @@
sizeswg.Add(1)
go func() {
var sizes types.Sizes
- sizes, sizeserr = packagesdriver.GetSizesGolist(ctx, cfg.BuildFlags, cfg.Env, cfg.gocmdRunner, cfg.Dir)
+ sizes, sizeserr = packagesdriver.GetSizesGolist(ctx, state.cfgInvocation(), cfg.gocmdRunner)
// types.SizesFor always returns nil or a *types.StdSizes.
response.dr.Sizes, _ = sizes.(*types.StdSizes)
sizeswg.Done()
}()
}
- state := &golistState{
- cfg: cfg,
- ctx: ctx,
- vendorDirs: map[string]bool{},
- }
-
// Determine files requested in contains patterns
var containFiles []string
restPatterns := make([]string, 0, len(patterns))
@@ -839,18 +839,26 @@
return fullargs
}
-// invokeGo returns the stdout of a go command invocation.
-func (state *golistState) invokeGo(verb string, args ...string) (*bytes.Buffer, error) {
+// cfgInvocation returns an Invocation that reflects cfg's settings.
+func (state *golistState) cfgInvocation() gocommand.Invocation {
cfg := state.cfg
-
- inv := gocommand.Invocation{
- Verb: verb,
- Args: args,
+ return gocommand.Invocation{
BuildFlags: cfg.BuildFlags,
+ ModFile: cfg.modFile,
+ ModFlag: cfg.modFlag,
Env: cfg.Env,
Logf: cfg.Logf,
WorkingDir: cfg.Dir,
}
+}
+
+// invokeGo returns the stdout of a go command invocation.
+func (state *golistState) invokeGo(verb string, args ...string) (*bytes.Buffer, error) {
+ cfg := state.cfg
+
+ inv := state.cfgInvocation()
+ inv.Verb = verb
+ inv.Args = args
gocmdRunner := cfg.gocmdRunner
if gocmdRunner == nil {
gocmdRunner = &gocommand.Runner{}
diff --git a/go/packages/packages.go b/go/packages/packages.go
index 970c480..38475e8 100644
--- a/go/packages/packages.go
+++ b/go/packages/packages.go
@@ -144,6 +144,12 @@
// the build system's query tool.
BuildFlags []string
+ // modFile will be used for -modfile in go command invocations.
+ modFile string
+
+ // modFlag will be used for -modfile in go command invocations.
+ modFlag string
+
// Fset provides source position information for syntax trees and types.
// If Fset is nil, Load will use a new fileset, but preserve Fset's value.
Fset *token.FileSet
@@ -366,6 +372,12 @@
packagesinternal.SetGoCmdRunner = func(config interface{}, runner *gocommand.Runner) {
config.(*Config).gocmdRunner = runner
}
+ packagesinternal.SetModFile = func(config interface{}, value string) {
+ config.(*Config).modFile = value
+ }
+ packagesinternal.SetModFlag = func(config interface{}, value string) {
+ config.(*Config).modFlag = value
+ }
packagesinternal.TypecheckCgo = int(typecheckCgo)
}
diff --git a/internal/gocommand/invoke.go b/internal/gocommand/invoke.go
index f516e17..b5c061b 100644
--- a/internal/gocommand/invoke.go
+++ b/internal/gocommand/invoke.go
@@ -130,6 +130,8 @@
Verb string
Args []string
BuildFlags []string
+ ModFlag string
+ ModFile string
Env []string
WorkingDir string
Logf func(format string, args ...interface{})
@@ -158,17 +160,35 @@
}
goArgs := []string{i.Verb}
+
+ appendModFile := func() {
+ if i.ModFile != "" {
+ goArgs = append(goArgs, "-modfile="+i.ModFile)
+ }
+ }
+ appendModFlag := func() {
+ if i.ModFlag != "" {
+ goArgs = append(goArgs, "-mod="+i.ModFlag)
+ }
+ }
+
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.
+ case "env", "version":
goArgs = append(goArgs, i.Args...)
- default:
+ case "mod":
+ // mod needs the sub-verb before flags.
+ goArgs = append(goArgs, i.Args[0])
+ appendModFile()
+ goArgs = append(goArgs, i.Args[1:]...)
+ case "get":
goArgs = append(goArgs, i.BuildFlags...)
+ appendModFile()
+ goArgs = append(goArgs, i.Args...)
+
+ default: // notably list and build.
+ goArgs = append(goArgs, i.BuildFlags...)
+ appendModFile()
+ appendModFlag()
goArgs = append(goArgs, i.Args...)
}
cmd := exec.Command("go", goArgs...)
diff --git a/internal/imports/fix.go b/internal/imports/fix.go
index 7a8e93f..d859617 100644
--- a/internal/imports/fix.go
+++ b/internal/imports/fix.go
@@ -804,6 +804,8 @@
GocmdRunner *gocommand.Runner
BuildFlags []string
+ ModFlag string
+ ModFile string
// Env overrides the OS environment, and can be used to specify
// GOPROXY, GO111MODULE, etc. PATH cannot be set here, because
diff --git a/internal/imports/mod.go b/internal/imports/mod.go
index 6982121..8a83613 100644
--- a/internal/imports/mod.go
+++ b/internal/imports/mod.go
@@ -59,6 +59,8 @@
}
inv := gocommand.Invocation{
BuildFlags: r.env.BuildFlags,
+ ModFlag: r.env.ModFlag,
+ ModFile: r.env.ModFile,
Env: r.env.env(),
Logf: r.env.Logf,
WorkingDir: r.env.WorkingDir,
diff --git a/internal/lsp/cache/imports.go b/internal/lsp/cache/imports.go
index 6274792..5039eac 100644
--- a/internal/lsp/cache/imports.go
+++ b/internal/lsp/cache/imports.go
@@ -160,9 +160,7 @@
return cleanup, err
}
if modmod {
- // -mod isn't really a build flag, but we can get away with it given
- // the set of commands that goimports wants to run.
- pe.BuildFlags = append([]string{"-mod=mod"}, pe.BuildFlags...)
+ pe.ModFlag = "mod"
}
// Add -modfile to the build flags, if we are using it.
@@ -172,7 +170,7 @@
if err != nil {
return nil, err
}
- pe.BuildFlags = append(pe.BuildFlags, fmt.Sprintf("-modfile=%s", tmpURI.Filename()))
+ pe.ModFile = tmpURI.Filename()
}
return cleanup, nil
diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go
index 27cd534..a5ae427 100644
--- a/internal/lsp/cache/load.go
+++ b/internal/lsp/cache/load.go
@@ -92,7 +92,7 @@
cleanup := func() {}
wdir := s.view.rootURI.Filename()
- var buildFlags []string
+ var modFile string
var modURI span.URI
var modContent []byte
switch {
@@ -141,18 +141,17 @@
if err != nil {
return err
}
- buildFlags = append(buildFlags, fmt.Sprintf("-modfile=%s", tmpURI.Filename()))
+ modFile = tmpURI.Filename()
}
cfg := s.config(ctx, wdir)
- cfg.BuildFlags = append(cfg.BuildFlags, buildFlags...)
-
+ packagesinternal.SetModFile(cfg, modFile)
modMod, err := s.needsModEqualsMod(ctx, modURI, modContent)
if err != nil {
return err
}
if modMod {
- cfg.BuildFlags = append([]string{"-mod=mod"}, cfg.BuildFlags...)
+ packagesinternal.SetModFlag(cfg, "mod")
}
pkgs, err := packages.Load(cfg, query...)
diff --git a/internal/lsp/cache/mod.go b/internal/lsp/cache/mod.go
index 099b5ce..0c2a96b 100644
--- a/internal/lsp/cache/mod.go
+++ b/internal/lsp/cache/mod.go
@@ -23,6 +23,7 @@
"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"
)
@@ -329,7 +330,7 @@
if s.workspaceMode()&tempModfile == 0 || containsVendor(fh.URI()) {
// Use -mod=readonly if the module contains a vendor directory
// (see golang/go#38711).
- args = append([]string{"-mod", "readonly"}, args...)
+ packagesinternal.SetModFlag(cfg, "readonly")
}
stdout, err := snapshot.runGoCommandWithConfig(ctx, cfg, "list", args)
if err != nil {
diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go
index d141c51..93be75a 100644
--- a/internal/lsp/cache/snapshot.go
+++ b/internal/lsp/cache/snapshot.go
@@ -255,13 +255,14 @@
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) {
cleanup = func() {} // fallback
modURI := s.GoModForFile(ctx, span.URIFromPath(cfg.Dir))
- var buildFlags []string
- // `go mod`, `go env`, and `go version` don't take build flags.
- switch verb {
- case "mod", "env", "version":
- default:
- buildFlags = append(cfg.BuildFlags, buildFlags...)
+
+ inv = &gocommand.Invocation{
+ Verb: verb,
+ Args: args,
+ Env: cfg.Env,
+ WorkingDir: cfg.Dir,
}
+
if allowTempModfile && s.workspaceMode()&tempModfile != 0 {
if modURI == "" {
return "", nil, nil, cleanup, fmt.Errorf("no go.mod file found in %s", cfg.Dir)
@@ -277,40 +278,30 @@
if err != nil {
return "", nil, nil, cleanup, err
}
- buildFlags = append(buildFlags, fmt.Sprintf("-modfile=%s", tmpURI.Filename()))
+ inv.ModFile = tmpURI.Filename()
}
- // TODO(rstambler): Remove this when golang/go#41826 is resolved.
- // Don't add -mod=mod for `go mod` or `go get`.
- switch verb {
- case "mod", "get":
- default:
- var modContent []byte
- if modURI != "" {
- modFH, err := s.GetFile(ctx, modURI)
- if err != nil {
- return "", nil, nil, cleanup, err
- }
- modContent, err = modFH.Read()
- if err != nil {
- return "", nil, nil, nil, err
- }
- }
- modMod, err := s.needsModEqualsMod(ctx, modURI, modContent)
+
+ var modContent []byte
+ if modURI != "" {
+ modFH, err := s.GetFile(ctx, modURI)
if err != nil {
return "", nil, nil, cleanup, err
}
- if modMod {
- buildFlags = append([]string{"-mod=mod"}, buildFlags...)
+ modContent, err = modFH.Read()
+ if err != nil {
+ return "", nil, nil, nil, err
}
}
+ modMod, err := s.needsModEqualsMod(ctx, modURI, modContent)
+ if err != nil {
+ return "", nil, nil, cleanup, err
+ }
+ if modMod {
+ inv.ModFlag = "mod"
+ }
+
runner = packagesinternal.GetGoCmdRunner(cfg)
- return tmpURI, runner, &gocommand.Invocation{
- Verb: verb,
- Args: args,
- Env: cfg.Env,
- BuildFlags: buildFlags,
- WorkingDir: cfg.Dir,
- }, cleanup, nil
+ return tmpURI, runner, inv, cleanup, nil
}
func (s *snapshot) buildOverlay() map[string][]byte {
diff --git a/internal/packagesinternal/packages.go b/internal/packagesinternal/packages.go
index 2c4527f..1335a5e 100644
--- a/internal/packagesinternal/packages.go
+++ b/internal/packagesinternal/packages.go
@@ -12,3 +12,6 @@
var SetGoCmdRunner = func(config interface{}, runner *gocommand.Runner) {}
var TypecheckCgo int
+
+var SetModFlag = func(config interface{}, value string) {}
+var SetModFile = func(config interface{}, value string) {}
