gopls/internal/lsp/cache: clean up tracking of GO111MODULE
Gopls views stored at least 4 copies of GO111MODULE:
- the value in view.environmentVariables
- the value in view.goEnv
- the value in view.userGo111Module
- the value in view.effectiveGo111Module
All of these values may differ from eachother, depending on the user's
environment and go version, and their meaning is not clearly documented.
Try to clean this up, by having environmentVariables track precisely the
variables output by `go env`, and providing a method to implement the
derived logic of userGo111Module and effectiveGo111Module. Ignore
view.goEnv for now, but leave a TODO.
Confusingly, the name 'effectiveGO111MODULE' turned out to be a more
appropriate name for what was formerly 'userGo111Module', so the naming
has switched.
This change is intended to be a no-op cleanup.
Change-Id: I529cc005c1875915483ef119a465bf17a96dec3c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/451355
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
diff --git a/gopls/internal/lsp/cache/load.go b/gopls/internal/lsp/cache/load.go
index 0498264..46e2e89 100644
--- a/gopls/internal/lsp/cache/load.go
+++ b/gopls/internal/lsp/cache/load.go
@@ -307,7 +307,7 @@
}
// TODO(rfindley): both of the checks below should be delegated to the workspace.
- if s.view.userGo111Module == off {
+ if s.view.effectiveGO111MODULE() == off {
return nil
}
if s.workspace.moduleSource != legacyWorkspace {
diff --git a/gopls/internal/lsp/cache/session.go b/gopls/internal/lsp/cache/session.go
index fe88a46..9b43553 100644
--- a/gopls/internal/lsp/cache/session.go
+++ b/gopls/internal/lsp/cache/session.go
@@ -219,7 +219,7 @@
goworkURI := span.URIFromPath(explicitGowork)
// Build the gopls workspace, collecting active modules in the view.
- workspace, err := newWorkspace(ctx, root, goworkURI, s, pathExcludedByFilterFunc(root.Filename(), wsInfo.gomodcache, options), wsInfo.userGo111Module == off, options.ExperimentalWorkspaceModule)
+ workspace, err := newWorkspace(ctx, root, goworkURI, s, pathExcludedByFilterFunc(root.Filename(), wsInfo.gomodcache, options), wsInfo.effectiveGO111MODULE() == off, options.ExperimentalWorkspaceModule)
if err != nil {
return nil, nil, func() {}, err
}
diff --git a/gopls/internal/lsp/cache/snapshot.go b/gopls/internal/lsp/cache/snapshot.go
index 11288e5..634afdf 100644
--- a/gopls/internal/lsp/cache/snapshot.go
+++ b/gopls/internal/lsp/cache/snapshot.go
@@ -453,7 +453,7 @@
// this with a non-empty inv.Env?
//
// We should refactor to make it clearer that the correct env is being used.
- inv.Env = append(append(append(os.Environ(), s.view.options.EnvSlice()...), inv.Env...), "GO111MODULE="+s.view.effectiveGo111Module)
+ inv.Env = append(append(append(os.Environ(), s.view.options.EnvSlice()...), inv.Env...), "GO111MODULE="+s.view.GO111MODULE())
inv.BuildFlags = append([]string{}, s.view.options.BuildFlags...)
s.view.optionsMu.Unlock()
cleanup = func() {} // fallback
diff --git a/gopls/internal/lsp/cache/view.go b/gopls/internal/lsp/cache/view.go
index 5974fab..f373c00 100644
--- a/gopls/internal/lsp/cache/view.go
+++ b/gopls/internal/lsp/cache/view.go
@@ -125,22 +125,48 @@
hasGopackagesDriver bool
// `go env` variables that need to be tracked by gopls.
- environmentVariables
-
- // userGo111Module is the user's value of GO111MODULE.
//
- // TODO(rfindley): is there really value in memoizing this variable? It seems
- // simpler to make this a function/method.
- userGo111Module go111module
-
- // The value of GO111MODULE we want to run with.
- effectiveGo111Module string
+ // TODO(rfindley): eliminate this in favor of goEnv, or vice-versa.
+ environmentVariables
// goEnv is the `go env` output collected when a view is created.
// It includes the values of the environment variables above.
goEnv map[string]string
}
+// effectiveGO111MODULE reports the value of GO111MODULE effective in the go
+// command at this go version, accounting for default values at different go
+// versions.
+func (w workspaceInformation) effectiveGO111MODULE() go111module {
+ // Off by default until Go 1.12.
+ go111module := w.GO111MODULE()
+ if go111module == "off" || (w.goversion < 12 && go111module == "") {
+ return off
+ }
+ // On by default as of Go 1.16.
+ if go111module == "on" || (w.goversion >= 16 && go111module == "") {
+ return on
+ }
+ return auto
+}
+
+// GO111MODULE returns the value of GO111MODULE to use for running the go
+// command. It differs from the user's environment in order to allow for the
+// more forgiving default value "auto" when using recent go versions.
+//
+// TODO(rfindley): it is probably not worthwhile diverging from the go command
+// here. The extra forgiveness may be nice, but breaks the invariant that
+// running the go command from the command line produces the same build list.
+//
+// Put differently: we shouldn't go out of our way to make GOPATH work, when
+// the go command does not.
+func (w workspaceInformation) GO111MODULE() string {
+ if w.goversion >= 16 && w.go111module == "" {
+ return "auto"
+ }
+ return w.go111module
+}
+
type go111module int
const (
@@ -149,8 +175,15 @@
on
)
+// environmentVariables holds important environment variables captured by a
+// call to `go env`.
type environmentVariables struct {
- gocache, gopath, goroot, goprivate, gomodcache, go111module string
+ gocache, gopath, goroot, goprivate, gomodcache string
+
+ // Don't use go111module directly, because we choose to use a different
+ // default (auto) on Go 1.16 and later, to avoid spurious errors. Use
+ // the workspaceInformation.GO111MODULE method instead.
+ go111module string
}
// workspaceMode holds various flags defining how the gopls workspace should
@@ -804,20 +837,11 @@
return nil, err
}
- go111module := os.Getenv("GO111MODULE")
- if v, ok := options.Env["GO111MODULE"]; ok {
- go111module = v
- }
// Make sure to get the `go env` before continuing with initialization.
- envVars, env, err := s.getGoEnv(ctx, folder.Filename(), goversion, go111module, options.EnvSlice())
+ envVars, env, err := s.getGoEnv(ctx, folder.Filename(), goversion, options.EnvSlice())
if err != nil {
return nil, err
}
- // If using 1.16, change the default back to auto. The primary effect of
- // GO111MODULE=on is to break GOPATH, which we aren't too interested in.
- if goversion >= 16 && go111module == "" {
- go111module = "auto"
- }
// The value of GOPACKAGESDRIVER is not returned through the go command.
gopackagesdriver := os.Getenv("GOPACKAGESDRIVER")
// TODO(rfindley): this looks wrong, or at least overly defensive. If the
@@ -838,26 +862,12 @@
return &workspaceInformation{
hasGopackagesDriver: hasGopackagesDriver,
- effectiveGo111Module: go111module,
- userGo111Module: go111moduleForVersion(go111module, goversion),
goversion: goversion,
environmentVariables: envVars,
goEnv: env,
}, nil
}
-func go111moduleForVersion(go111module string, goversion int) go111module {
- // Off by default until Go 1.12.
- if go111module == "off" || (goversion < 12 && go111module == "") {
- return off
- }
- // On by default as of Go 1.16.
- if go111module == "on" || (goversion >= 16 && go111module == "") {
- return on
- }
- return auto
-}
-
// findWorkspaceRoot searches for the best workspace root according to the
// following heuristics:
// - First, look for a parent directory containing a gopls.mod file
@@ -938,7 +948,7 @@
}
// getGoEnv gets the view's various GO* values.
-func (s *Session) getGoEnv(ctx context.Context, folder string, goversion int, go111module string, configEnv []string) (environmentVariables, map[string]string, error) {
+func (s *Session) getGoEnv(ctx context.Context, folder string, goversion int, configEnv []string) (environmentVariables, map[string]string, error) {
envVars := environmentVariables{}
vars := map[string]*string{
"GOCACHE": &envVars.gocache,
@@ -984,13 +994,12 @@
}
// Old versions of Go don't have GOMODCACHE, so emulate it.
+ //
+ // TODO(rfindley): consistent with the treatment of go111module, we should
+ // provide a wrapper method rather than mutating this value.
if envVars.gomodcache == "" && envVars.gopath != "" {
envVars.gomodcache = filepath.Join(filepath.SplitList(envVars.gopath)[0], "pkg/mod")
}
- // GO111MODULE does not appear in `go env` output until Go 1.13.
- if goversion < 13 {
- envVars.go111module = go111module
- }
return envVars, env, err
}
