all: rework goimports environment, support GOMODCACHE
This CL got away from me a little.
For a number of reasons, the existing goimports API of passing in values
for various GO* values was not working. For one, the number of necessary
variables kept growing. For another, we tried to avoid calling `go env`
in GOPATH mode by using `build.Default`, but that turns out to be buggy;
see golang/go#39838. And finally, it created massive confusion about
whether the values were intended to be read from the OS environment, or
fully evaluated by the `go` command.
There are only two users of the internal imports API, so there really
shouldn't need to be more than two modes. For the command line tool, we
have to call `go env` to deal with the `go/build` bug. So we just do it.
Tests use that same path, but can augment the enviroment to set
themselves up. In contrast, `gopls` needs to fully specify the
environment. It can simply pass in the fully evaluated GO* values.
Finally, make the change I was actually here to make: propagate
GOMODCACHE and use it where appropriate.
Fixes golang/go#39761.
Change-Id: I720c69839d91d66d98e94dfc5f065ba0279c5542
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239754
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/cmd/goimports/goimports.go b/cmd/goimports/goimports.go
index c195caa..f7fc3d5 100644
--- a/cmd/goimports/goimports.go
+++ b/cmd/goimports/goimports.go
@@ -10,7 +10,6 @@
"errors"
"flag"
"fmt"
- "go/build"
"go/scanner"
"io"
"io/ioutil"
@@ -43,15 +42,6 @@
TabIndent: true,
Comments: true,
Fragment: true,
- // This environment, and its caches, will be reused for the whole run.
- Env: &imports.ProcessEnv{
- GOPATH: build.Default.GOPATH,
- GOROOT: build.Default.GOROOT,
- GOFLAGS: os.Getenv("GOFLAGS"),
- GO111MODULE: os.Getenv("GO111MODULE"),
- GOPROXY: os.Getenv("GOPROXY"),
- GOSUMDB: os.Getenv("GOSUMDB"),
- },
}
exitCode = 0
)
diff --git a/imports/forward.go b/imports/forward.go
index dbe5b49..83f4e44 100644
--- a/imports/forward.go
+++ b/imports/forward.go
@@ -3,9 +3,7 @@
package imports // import "golang.org/x/tools/imports"
import (
- "go/build"
"log"
- "os"
intimp "golang.org/x/tools/internal/imports"
)
@@ -42,12 +40,6 @@
}
intopt := &intimp.Options{
Env: &intimp.ProcessEnv{
- GOPATH: build.Default.GOPATH,
- GOROOT: build.Default.GOROOT,
- GOFLAGS: os.Getenv("GOFLAGS"),
- GO111MODULE: os.Getenv("GO111MODULE"),
- GOPROXY: os.Getenv("GOPROXY"),
- GOSUMDB: os.Getenv("GOSUMDB"),
LocalPrefix: LocalPrefix,
},
AllErrors: opt.AllErrors,
diff --git a/internal/gocommand/invoke.go b/internal/gocommand/invoke.go
index 83b654b..f516e17 100644
--- a/internal/gocommand/invoke.go
+++ b/internal/gocommand/invoke.go
@@ -185,7 +185,6 @@
cmd.Env = append(cmd.Env, "PWD="+i.WorkingDir)
cmd.Dir = i.WorkingDir
}
-
defer func(start time.Time) { log("%s for %v", time.Since(start), cmdDebugStr(cmd)) }(time.Now())
return runCmdContext(ctx, cmd)
diff --git a/internal/imports/fix.go b/internal/imports/fix.go
index 36292d7..2c70307 100644
--- a/internal/imports/fix.go
+++ b/internal/imports/fix.go
@@ -7,6 +7,7 @@
import (
"bytes"
"context"
+ "encoding/json"
"fmt"
"go/ast"
"go/build"
@@ -598,7 +599,7 @@
// Start off with the standard library.
for importPath, exports := range stdlib {
p := &pkg{
- dir: filepath.Join(env.GOROOT, "src", importPath),
+ dir: filepath.Join(env.goroot(), "src", importPath),
importPathShort: importPath,
packageName: path.Base(importPath),
relevance: MaxRelevance,
@@ -743,6 +744,8 @@
return getCandidatePkgs(ctx, callback, filename, filePkg, env)
}
+var RequiredGoEnvVars = []string{"GO111MODULE", "GOFLAGS", "GOINSECURE", "GOMOD", "GOMODCACHE", "GONOPROXY", "GONOSUMDB", "GOPATH", "GOPROXY", "GOROOT", "GOSUMDB"}
+
// ProcessEnv contains environment variables and settings that affect the use of
// the go command, the go/build package, etc.
type ProcessEnv struct {
@@ -752,10 +755,13 @@
BuildFlags []string
- // If non-empty, these will be used instead of the
- // process-wide values.
- GOPATH, GOROOT, GO111MODULE, GOPROXY, GOFLAGS, GOSUMDB string
- WorkingDir string
+ // Env overrides the OS environment, and can be used to specify
+ // GOPROXY, GO111MODULE, etc. PATH cannot be set here, because
+ // exec.Command will not honor it.
+ // Specifying all of RequiredGoEnvVars avoids a call to `go env`.
+ Env map[string]string
+
+ WorkingDir string
// If Logf is non-nil, debug logging is enabled through this function.
Logf func(format string, args ...interface{})
@@ -763,6 +769,22 @@
resolver Resolver
}
+func (e *ProcessEnv) goroot() string {
+ return e.mustGetEnv("GOROOT")
+}
+
+func (e *ProcessEnv) gopath() string {
+ return e.mustGetEnv("GOPATH")
+}
+
+func (e *ProcessEnv) mustGetEnv(k string) string {
+ v, ok := e.Env[k]
+ if !ok {
+ panic(fmt.Sprintf("%v not set in evaluated environment", k))
+ }
+ return v
+}
+
// CopyConfig copies the env's configuration into a new env.
func (e *ProcessEnv) CopyConfig() *ProcessEnv {
copy := *e
@@ -770,21 +792,40 @@
return ©
}
-func (e *ProcessEnv) env() []string {
- env := os.Environ()
- add := func(k, v string) {
- if v != "" {
- env = append(env, k+"="+v)
+func (e *ProcessEnv) init() error {
+ foundAllRequired := true
+ for _, k := range RequiredGoEnvVars {
+ if _, ok := e.Env[k]; !ok {
+ foundAllRequired = false
+ break
}
}
- add("GOPATH", e.GOPATH)
- add("GOROOT", e.GOROOT)
- add("GO111MODULE", e.GO111MODULE)
- add("GOPROXY", e.GOPROXY)
- add("GOFLAGS", e.GOFLAGS)
- add("GOSUMDB", e.GOSUMDB)
- if e.WorkingDir != "" {
- add("PWD", e.WorkingDir)
+ if foundAllRequired {
+ return nil
+ }
+
+ if e.Env == nil {
+ e.Env = map[string]string{}
+ }
+
+ goEnv := map[string]string{}
+ stdout, err := e.invokeGo(context.TODO(), "env", append([]string{"-json"}, RequiredGoEnvVars...)...)
+ if err != nil {
+ return err
+ }
+ if err := json.Unmarshal(stdout.Bytes(), &goEnv); err != nil {
+ return err
+ }
+ for k, v := range goEnv {
+ e.Env[k] = v
+ }
+ return nil
+}
+
+func (e *ProcessEnv) env() []string {
+ var env []string // the gocommand package will prepend os.Environ.
+ for k, v := range e.Env {
+ env = append(env, k+"="+v)
}
return env
}
@@ -793,8 +834,7 @@
if e.resolver != nil {
return e.resolver
}
- out, err := e.invokeGo(context.TODO(), "env", "GOMOD")
- if err != nil || len(bytes.TrimSpace(out.Bytes())) == 0 {
+ if len(e.Env["GOMOD"]) == 0 {
e.resolver = newGopathResolver(e)
return e.resolver
}
@@ -804,8 +844,8 @@
func (e *ProcessEnv) buildContext() *build.Context {
ctx := build.Default
- ctx.GOROOT = e.GOROOT
- ctx.GOPATH = e.GOPATH
+ ctx.GOROOT = e.goroot()
+ ctx.GOPATH = e.gopath()
// As of Go 1.14, build.Context has a Dir field
// (see golang.org/issue/34860).
@@ -839,7 +879,7 @@
func addStdlibCandidates(pass *pass, refs references) {
add := func(pkg string) {
// Prevent self-imports.
- if path.Base(pkg) == pass.f.Name.Name && filepath.Join(pass.env.GOROOT, "src", pkg) == pass.srcDir {
+ if path.Base(pkg) == pass.f.Name.Name && filepath.Join(pass.env.goroot(), "src", pkg) == pass.srcDir {
return
}
exports := copyExports(stdlib[pkg])
diff --git a/internal/imports/fix_test.go b/internal/imports/fix_test.go
index 1e78687..7800cce 100644
--- a/internal/imports/fix_test.go
+++ b/internal/imports/fix_test.go
@@ -8,7 +8,6 @@
"context"
"flag"
"fmt"
- "go/build"
"log"
"path/filepath"
"reflect"
@@ -1562,12 +1561,13 @@
testConfig{
module: packagestest.Module{
- Name: "ignored.com",
+ Name: "ignored.com",
+ Files: fm{"x.go": "package x"},
},
}.test(t, func(t *goimportTest) {
// Run in GOROOT/src so that the std module shows up in go list -m all.
- t.env.WorkingDir = filepath.Join(t.env.GOROOT, "src")
- got, err := t.processNonModule(filepath.Join(t.env.GOROOT, "src/x.go"), []byte(input), nil)
+ t.env.WorkingDir = filepath.Join(t.env.goroot(), "src")
+ got, err := t.processNonModule(filepath.Join(t.env.goroot(), "src/x.go"), []byte(input), nil)
if err != nil {
t.Fatalf("Process() = %v", err)
}
@@ -1585,10 +1585,11 @@
testConfig{
module: packagestest.Module{
- Name: "ignored.com",
+ Name: "ignored.com",
+ Files: fm{"x.go": "package x"},
},
}.test(t, func(t *goimportTest) {
- got, err := t.processNonModule(filepath.Join(t.env.GOROOT, "src/crypto/ecdsa/foo.go"), []byte(input), nil)
+ got, err := t.processNonModule(filepath.Join(t.env.goroot(), "src/crypto/ecdsa/foo.go"), []byte(input), nil)
if err != nil {
t.Fatalf("Process() = %v", err)
}
@@ -1623,33 +1624,26 @@
exported := packagestest.Export(t, exporter, c.modules)
defer exported.Cleanup()
- env := make(map[string]string)
+ env := map[string]string{}
for _, kv := range exported.Config.Env {
- split := strings.Split(kv, "=")
- k, v := split[0], split[1]
- env[k] = v
+ split := strings.SplitN(kv, "=", 2)
+ env[split[0]] = split[1]
}
-
it := &goimportTest{
T: t,
env: &ProcessEnv{
- GOROOT: env["GOROOT"],
- GOPATH: env["GOPATH"],
- GO111MODULE: env["GO111MODULE"],
- GOSUMDB: env["GOSUMDB"],
+ Env: env,
WorkingDir: exported.Config.Dir,
GocmdRunner: &gocommand.Runner{},
},
exported: exported,
}
+ if err := it.env.init(); err != nil {
+ t.Fatal(err)
+ }
if *testDebug {
it.env.Logf = log.Printf
}
- if it.env.GOROOT == "" {
- // packagestest clears out GOROOT to work around https://golang.org/issue/32849,
- // which isn't relevant here. Fill it back in so we can find the standard library.
- it.env.GOROOT = build.Default.GOROOT
- }
fn(it)
})
}
diff --git a/internal/imports/imports.go b/internal/imports/imports.go
index f43d6b2..04ecdfd 100644
--- a/internal/imports/imports.go
+++ b/internal/imports/imports.go
@@ -14,14 +14,12 @@
"context"
"fmt"
"go/ast"
- "go/build"
"go/format"
"go/parser"
"go/printer"
"go/token"
"io"
"io/ioutil"
- "os"
"regexp"
"strconv"
"strings"
@@ -146,19 +144,16 @@
// Set the env if the user has not provided it.
if opt.Env == nil {
- opt.Env = &ProcessEnv{
- GOPATH: build.Default.GOPATH,
- GOROOT: build.Default.GOROOT,
- GOFLAGS: os.Getenv("GOFLAGS"),
- GO111MODULE: os.Getenv("GO111MODULE"),
- GOPROXY: os.Getenv("GOPROXY"),
- GOSUMDB: os.Getenv("GOSUMDB"),
- }
+ opt.Env = &ProcessEnv{}
}
// Set the gocmdRunner if the user has not provided it.
if opt.Env.GocmdRunner == nil {
opt.Env.GocmdRunner = &gocommand.Runner{}
}
+ if err := opt.Env.init(); err != nil {
+ return nil, nil, err
+ }
+
if src == nil {
b, err := ioutil.ReadFile(filename)
if err != nil {
diff --git a/internal/imports/imports_test.go b/internal/imports/imports_test.go
index 70f66ab..ad16622 100644
--- a/internal/imports/imports_test.go
+++ b/internal/imports/imports_test.go
@@ -1,7 +1,6 @@
package imports
import (
- "go/build"
"os"
"testing"
@@ -32,8 +31,6 @@
name: "default",
opt: &Options{
Env: &ProcessEnv{
- GOPATH: build.Default.GOPATH,
- GOROOT: build.Default.GOROOT,
GocmdRunner: &gocommand.Runner{},
},
Comments: true,
diff --git a/internal/imports/mod.go b/internal/imports/mod.go
index 4e816e8..664fbbf 100644
--- a/internal/imports/mod.go
+++ b/internal/imports/mod.go
@@ -79,7 +79,11 @@
r.initAllMods()
}
- r.moduleCacheDir = filepath.Join(filepath.SplitList(r.env.GOPATH)[0], "/pkg/mod")
+ if gmc := r.env.Env["GOMODCACHE"]; gmc != "" {
+ r.moduleCacheDir = gmc
+ } else {
+ r.moduleCacheDir = filepath.Join(filepath.SplitList(r.env.gopath())[0], "/pkg/mod")
+ }
sort.Slice(r.modsByModPath, func(i, j int) bool {
count := func(x int) int {
@@ -95,7 +99,7 @@
})
r.roots = []gopathwalk.Root{
- {filepath.Join(r.env.GOROOT, "/src"), gopathwalk.RootGOROOT},
+ {filepath.Join(r.env.goroot(), "/src"), gopathwalk.RootGOROOT},
}
if r.main != nil {
r.roots = append(r.roots, gopathwalk.Root{r.main.Dir, gopathwalk.RootCurrentModule})
diff --git a/internal/imports/mod_test.go b/internal/imports/mod_test.go
index fa1a9c6..9d26aff 100644
--- a/internal/imports/mod_test.go
+++ b/internal/imports/mod_test.go
@@ -4,7 +4,6 @@
"archive/zip"
"context"
"fmt"
- "go/build"
"io/ioutil"
"log"
"os"
@@ -240,7 +239,7 @@
// Clear out the resolver's cache, since we've changed the environment.
mt.resolver = newModuleResolver(mt.env)
- mt.env.GOFLAGS = "-mod=vendor"
+ mt.env.Env["GOFLAGS"] = "-mod=vendor"
mt.assertModuleFoundInDir("rsc.io/sampler", "sampler", `/vendor/`)
}
@@ -686,17 +685,21 @@
}
env := &ProcessEnv{
- GOROOT: build.Default.GOROOT,
- GOPATH: filepath.Join(dir, "gopath"),
- GO111MODULE: "on",
- GOPROXY: proxydir.ToURL(proxyDir),
- GOSUMDB: "off",
+ Env: map[string]string{
+ "GOPATH": filepath.Join(dir, "gopath"),
+ "GO111MODULE": "on",
+ "GOSUMDB": "off",
+ "GOPROXY": proxydir.ToURL(proxyDir),
+ },
WorkingDir: filepath.Join(mainDir, wd),
GocmdRunner: &gocommand.Runner{},
}
if *testDebug {
env.Logf = log.Printf
}
+ if err := env.init(); err != nil {
+ t.Fatal(err)
+ }
// go mod download gets mad if we don't have a go.mod, so make sure we do.
_, err = os.Stat(filepath.Join(mainDir, "go.mod"))
@@ -838,7 +841,7 @@
import _ "rsc.io/quote"
`, "")
defer mt.cleanup()
- want := filepath.Join(mt.resolver.env.GOPATH, "pkg/mod", "rsc.io/quote@v1.5.2")
+ want := filepath.Join(mt.resolver.env.gopath(), "pkg/mod", "rsc.io/quote@v1.5.2")
found := mt.assertScanFinds("rsc.io/quote", "quote")
modDir, _ := mt.resolver.modInfo(found.dir)
@@ -864,13 +867,17 @@
t.Fatal(err)
}
env := &ProcessEnv{
- GOROOT: build.Default.GOROOT,
- GOPATH: filepath.Join(dir, "gopath"),
- GO111MODULE: "on",
- GOSUMDB: "off",
+ Env: map[string]string{
+ "GOPATH": filepath.Join(dir, "gopath"),
+ "GO111MODULE": "on",
+ "GOSUMDB": "off",
+ },
GocmdRunner: &gocommand.Runner{},
WorkingDir: dir,
}
+ if err := env.init(); err != nil {
+ t.Fatal(err)
+ }
resolver := newModuleResolver(env)
scanToSlice(resolver, nil)
}
@@ -938,11 +945,12 @@
func BenchmarkScanModCache(b *testing.B) {
testenv.NeedsGo1Point(b, 11)
env := &ProcessEnv{
- GOPATH: build.Default.GOPATH,
- GOROOT: build.Default.GOROOT,
GocmdRunner: &gocommand.Runner{},
Logf: log.Printf,
}
+ if err := env.init(); err != nil {
+ b.Fatal(err)
+ }
exclude := []gopathwalk.RootType{gopathwalk.RootGOROOT}
scanToSlice(env.GetResolver(), exclude)
b.ResetTimer()
diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go
index 8faab5b..f78dbcd 100644
--- a/internal/lsp/cache/view.go
+++ b/internal/lsp/cache/view.go
@@ -114,8 +114,10 @@
// build system.
goCommand bool
- // `go env` variables that need to be tracked.
- gopath, gocache, goprivate string
+ // `go env` variables that need to be tracked by gopls.
+ gocache, gomodcache, gopath, goprivate string
+
+ goEnv map[string]string
// gocmdRunner guards go command calls from concurrency errors.
gocmdRunner *gocommand.Runner
@@ -464,13 +466,16 @@
cleanup = func() {}
v.optionsMu.Lock()
- env, buildFlags := v.envLocked()
+ _, buildFlags := v.envLocked()
localPrefix, verboseOutput := v.options.LocalPrefix, v.options.VerboseOutput
v.optionsMu.Unlock()
pe := v.processEnv
-
+ pe.LocalPrefix = localPrefix
+ pe.GocmdRunner = v.gocmdRunner
pe.BuildFlags = buildFlags
+ pe.Env = v.goEnv
+ pe.WorkingDir = v.folder.Filename()
// Add -modfile to the build flags, if we are using it.
if modFH != nil {
@@ -482,46 +487,18 @@
pe.BuildFlags = append(pe.BuildFlags, fmt.Sprintf("-modfile=%s", tmpURI.Filename()))
}
- pe.WorkingDir = v.folder.Filename()
- pe.LocalPrefix = localPrefix
- pe.GocmdRunner = v.gocmdRunner
-
if verboseOutput {
pe.Logf = func(format string, args ...interface{}) {
event.Log(ctx, fmt.Sprintf(format, args...))
}
}
- for _, kv := range env {
- split := strings.SplitN(kv, "=", 2)
- if len(split) < 2 {
- continue
- }
- switch split[0] {
- case "GOPATH":
- pe.GOPATH = split[1]
- case "GOROOT":
- pe.GOROOT = split[1]
- case "GO111MODULE":
- pe.GO111MODULE = split[1]
- case "GOPROXY":
- pe.GOPROXY = split[1]
- case "GOFLAGS":
- pe.GOFLAGS = split[1]
- case "GOSUMDB":
- pe.GOSUMDB = split[1]
- }
- }
- if pe.GOPATH == "" {
- return nil, fmt.Errorf("no GOPATH for view %s", v.folder)
- }
return cleanup, nil
}
// 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 := []string{fmt.Sprintf("GOPATH=%s", v.gopath)}
- env = append(env, v.options.Env...)
+ env := append([]string{}, v.options.Env...)
buildFlags := append([]string{}, v.options.BuildFlags...)
return env, buildFlags
}
@@ -650,8 +627,7 @@
}
} else {
mainMod := filepath.Dir(v.modURI.Filename())
- modCache := filepath.Join(filepath.SplitList(v.gopath)[0], "/pkg/mod")
- prefixes = []string{mainMod, modCache}
+ prefixes = []string{mainMod, v.gomodcache}
}
for _, prefix := range prefixes {
@@ -747,11 +723,10 @@
return fmt.Errorf("invalid workspace configuration: %w", err)
}
// Make sure to get the `go env` before continuing with initialization.
- gomod, err := v.getGoEnv(ctx, env)
+ modFile, err := v.setGoEnv(ctx, env)
if err != nil {
return err
}
- modFile := strings.TrimSpace(gomod)
if modFile == os.DevNull {
return nil
}
@@ -813,80 +788,52 @@
return err == nil && !strings.HasPrefix(rel, "..")
}
-// getGoEnv sets the view's build information's GOPATH, GOCACHE, GOPRIVATE, and
-// GOPACKAGESDRIVER values. It also returns the view's GOMOD value, which need
-// not be cached.
-func (v *View) getGoEnv(ctx context.Context, env []string) (string, error) {
- var gocache, gopath, gopackagesdriver, goprivate bool
- isGoCommand := func(gopackagesdriver string) bool {
- return gopackagesdriver == "" || gopackagesdriver == "off"
+// setGoEnv sets the view's various GO* values. It also returns the view's
+// GOMOD value, which need not be cached.
+func (v *View) setGoEnv(ctx context.Context, configEnv []string) (string, error) {
+ var gomod string
+ vars := map[string]*string{
+ "GOCACHE": &v.gocache,
+ "GOPATH": &v.gopath,
+ "GOPRIVATE": &v.goprivate,
+ "GOMODCACHE": &v.gomodcache,
+ "GOMOD": &gomod,
}
- for _, e := range env {
- split := strings.Split(e, "=")
- if len(split) != 2 {
- continue
- }
- switch split[0] {
- case "GOCACHE":
- v.gocache = split[1]
- gocache = true
- case "GOPATH":
- v.gopath = split[1]
- gopath = true
- case "GOPRIVATE":
- v.goprivate = split[1]
- goprivate = true
- case "GOPACKAGESDRIVER":
- v.goCommand = isGoCommand(split[1])
- gopackagesdriver = true
- }
+ // We can save ~200 ms by requesting only the variables we care about.
+ args := append([]string{"-json"}, imports.RequiredGoEnvVars...)
+ for k := range vars {
+ args = append(args, k)
+ }
+
+ inv := gocommand.Invocation{
+ Verb: "env",
+ Args: args,
+ Env: configEnv,
+ WorkingDir: v.Folder().Filename(),
}
// Don't go through runGoCommand, as we don't need a temporary -modfile to
// run `go env`.
- inv := gocommand.Invocation{
- Verb: "env",
- Args: []string{"-json"},
- Env: env,
- WorkingDir: v.Folder().Filename(),
- }
stdout, err := v.gocmdRunner.Run(ctx, inv)
if err != nil {
return "", err
}
- envMap := make(map[string]string)
- decoder := json.NewDecoder(stdout)
- if err := decoder.Decode(&envMap); err != nil {
+ if err := json.Unmarshal(stdout.Bytes(), &v.goEnv); err != nil {
return "", err
}
- if !gopath {
- if gopath, ok := envMap["GOPATH"]; ok {
- v.gopath = gopath
- } else {
- return "", errors.New("unable to determine GOPATH")
- }
- }
- if !gocache {
- if gocache, ok := envMap["GOCACHE"]; ok {
- v.gocache = gocache
- } else {
- return "", errors.New("unable to determine GOCACHE")
- }
- }
- if !goprivate {
- if goprivate, ok := envMap["GOPRIVATE"]; ok {
- v.goprivate = goprivate
- }
- // No error here: GOPRIVATE is not essential.
- }
- // The value of GOPACKAGESDRIVER is not returned through the go command.
- if !gopackagesdriver {
- v.goCommand = isGoCommand(os.Getenv("GOPACKAGESDRIVER"))
- }
- if gomod, ok := envMap["GOMOD"]; ok {
- return gomod, nil
- }
- return "", nil
+ for key, ptr := range vars {
+ *ptr = v.goEnv[key]
+ }
+
+ // Old versions of Go don't have GOMODCACHE, so emulate it.
+ if v.gomodcache == "" && v.gopath != "" {
+ v.gomodcache = filepath.Join(filepath.SplitList(v.gopath)[0], "pkg/mod")
+ }
+
+ // The value of GOPACKAGESDRIVER is not returned through the go command.
+ gopackagesdriver := os.Getenv("GOPACKAGESDRIVER")
+ v.goCommand = gopackagesdriver == "" || gopackagesdriver == "off"
+ return gomod, nil
}
func (v *View) IsGoPrivatePath(target string) bool {
diff --git a/internal/lsp/regtest/diagnostics_test.go b/internal/lsp/regtest/diagnostics_test.go
index 39eb67c..72a5313 100644
--- a/internal/lsp/regtest/diagnostics_test.go
+++ b/internal/lsp/regtest/diagnostics_test.go
@@ -418,7 +418,7 @@
})
}
-// Tests golang/go#37984.
+// Tests golang/go#37984: GOPATH should be read from the go command.
func TestNoGOPATH_Issue37984(t *testing.T) {
const missingImport = `
-- main.go --
@@ -431,9 +431,8 @@
runner.Run(t, missingImport, func(t *testing.T, env *Env) {
env.OpenFile("main.go")
env.Await(env.DiagnosticAtRegexp("main.go", "fmt"))
- if err := env.Editor.OrganizeImports(env.Ctx, "main.go"); err == nil {
- t.Fatalf("organize imports should fail with an empty GOPATH")
- }
+ env.SaveBuffer("main.go")
+ env.Await(EmptyDiagnostics("main.go"))
}, WithEditorConfig(fake.EditorConfig{Env: []string{"GOPATH="}}))
}
diff --git a/internal/lsp/regtest/imports_test.go b/internal/lsp/regtest/imports_test.go
index fd35f34..0a66a21 100644
--- a/internal/lsp/regtest/imports_test.go
+++ b/internal/lsp/regtest/imports_test.go
@@ -1,10 +1,19 @@
package regtest
import (
+ "io/ioutil"
+ "os"
+ "path/filepath"
+ "strings"
"testing"
+ "time"
+
+ "golang.org/x/tools/internal/lsp/fake"
+ "golang.org/x/tools/internal/testenv"
)
-const needs = `
+func TestIssue38815(t *testing.T) {
+ const needs = `
-- go.mod --
module foo
@@ -12,12 +21,12 @@
package main
func f() {}
`
-const ntest = `package main
+ const ntest = `package main
func TestZ(t *testing.T) {
f()
}
`
-const want = `package main
+ const want = `package main
import "testing"
@@ -26,7 +35,6 @@
}
`
-func TestIssue38815(t *testing.T) {
// it was returning
// "package main\nimport \"testing\"\npackage main..."
runner.Run(t, needs, func(t *testing.T, env *Env) {
@@ -39,7 +47,8 @@
})
}
-const vim1 = `package main
+func TestVim1(t *testing.T) {
+ const vim1 = `package main
import "fmt"
@@ -53,10 +62,9 @@
}
`
-func TestVim1(t *testing.T) {
// The file remains unchanged, but if there are any CodeActions returned, they confuse vim.
// Therefore check for no CodeActions
- runner.Run(t, vim1, func(t *testing.T, env *Env) {
+ runner.Run(t, "", func(t *testing.T, env *Env) {
env.CreateBuffer("main.go", vim1)
env.OrganizeImports("main.go")
actions := env.CodeAction("main.go")
@@ -73,7 +81,8 @@
})
}
-const vim2 = `package main
+func TestVim2(t *testing.T) {
+ const vim2 = `package main
import (
"fmt"
@@ -88,8 +97,7 @@
}
`
-func TestVim2(t *testing.T) {
- runner.Run(t, vim1, func(t *testing.T, env *Env) {
+ runner.Run(t, "", func(t *testing.T, env *Env) {
env.CreateBuffer("main.go", vim2)
env.OrganizeImports("main.go")
actions := env.CodeAction("main.go")
@@ -98,3 +106,51 @@
}
})
}
+
+func TestGOMODCACHE(t *testing.T) {
+ const proxy = `
+-- example.com@v1.2.3/go.mod --
+module example.com
+
+go 1.12
+-- example.com@v1.2.3/x/x.go --
+package x
+
+const X = 1
+-- example.com@v1.2.3/y/y.go --
+package y
+
+const Y = 2
+`
+ const ws = `
+-- go.mod --
+module mod.com
+
+require example.com v1.2.3
+
+-- main.go --
+package main
+
+import "example.com/x"
+
+var _, _ = x.X, y.Y
+`
+ testenv.NeedsGo1Point(t, 15)
+
+ modcache, err := ioutil.TempDir("", "TestGOMODCACHE-modcache")
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer os.RemoveAll(modcache)
+
+ runner.Run(t, ws, func(t *testing.T, env *Env) {
+ env.OpenFile("main.go")
+ env.Await(env.DiagnosticAtRegexp("main.go", `y.Y`))
+ env.SaveBuffer("main.go")
+ env.Await(EmptyDiagnostics("main.go"))
+ path, _ := env.GoToDefinition("main.go", env.RegexpSearch("main.go", `y.(Y)`))
+ if !strings.HasPrefix(path, filepath.ToSlash(modcache)) {
+ t.Errorf("found module dependency outside of GOMODCACHE: got %v, wanted subdir of %v", path, filepath.ToSlash(modcache))
+ }
+ }, WithProxy(proxy), WithEditorConfig(fake.EditorConfig{Env: []string{"GOMODCACHE=" + modcache}}), WithTimeout(5*time.Second))
+}
diff --git a/internal/lsp/regtest/modfile_test.go b/internal/lsp/regtest/modfile_test.go
index 47a0ed2..188818e 100644
--- a/internal/lsp/regtest/modfile_test.go
+++ b/internal/lsp/regtest/modfile_test.go
@@ -45,7 +45,7 @@
env.DiagnosticAtRegexp("main.go", "\"example.com/blah\""),
)
if got := env.ReadWorkspaceFile("go.mod"); got != goModContent {
- t.Fatalf("go.mod changed on disk:\n%s", tests.Diff(want, got))
+ t.Fatalf("go.mod changed on disk:\n%s", tests.Diff(goModContent, got))
}
// Save the buffer, which will format and organize imports.
// Confirm that the go.mod file still does not change.
@@ -54,7 +54,7 @@
env.DiagnosticAtRegexp("main.go", "\"example.com/blah\""),
)
if got := env.ReadWorkspaceFile("go.mod"); got != goModContent {
- t.Fatalf("go.mod changed on disk:\n%s", tests.Diff(want, got))
+ t.Fatalf("go.mod changed on disk:\n%s", tests.Diff(goModContent, got))
}
}, WithProxy(proxy))
}
