internal/imports: fix crash when adding stdlib imports
We failed to initialize the ProcessEnv when adding stdlib imports.
Somehow this went unnnoticed for a month or something. Initialize on
demand there too, and add a stronger check so there's a better error
message.
Fixes golang/go#40670.
Change-Id: I391c115f417390be4a0e18f92fbfbcbfbdcc052c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247797
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/internal/gopathwalk/walk.go b/internal/gopathwalk/walk.go
index 390cb9d..925ff53 100644
--- a/internal/gopathwalk/walk.go
+++ b/internal/gopathwalk/walk.go
@@ -10,7 +10,6 @@
"bufio"
"bytes"
"fmt"
- "go/build"
"io/ioutil"
"log"
"os"
@@ -47,16 +46,6 @@
Type RootType
}
-// SrcDirsRoots returns the roots from build.Default.SrcDirs(). Not modules-compatible.
-func SrcDirsRoots(ctx *build.Context) []Root {
- var roots []Root
- roots = append(roots, Root{filepath.Join(ctx.GOROOT, "src"), RootGOROOT})
- for _, p := range filepath.SplitList(ctx.GOPATH) {
- roots = append(roots, Root{filepath.Join(p, "src"), RootGOPATH})
- }
- return roots
-}
-
// Walk walks Go source directories ($GOROOT, $GOPATH, etc) to find packages.
// For each package found, add will be called (concurrently) with the absolute
// paths of the containing source directory and the package directory.
diff --git a/internal/imports/fix.go b/internal/imports/fix.go
index ecd13e8..0cb09e2 100644
--- a/internal/imports/fix.go
+++ b/internal/imports/fix.go
@@ -573,7 +573,9 @@
return fixes, nil
}
- addStdlibCandidates(p, p.missingRefs)
+ if err := addStdlibCandidates(p, p.missingRefs); err != nil {
+ return nil, err
+ }
p.assumeSiblingImportsValid()
if fixes, done := p.fix(); done {
return fixes, nil
@@ -601,10 +603,14 @@
notSelf := func(p *pkg) bool {
return p.packageName != filePkg || p.dir != filepath.Dir(filename)
}
+ goenv, err := env.goEnv()
+ if err != nil {
+ return err
+ }
// Start off with the standard library.
for importPath, exports := range stdlib {
p := &pkg{
- dir: filepath.Join(env.goroot(), "src", importPath),
+ dir: filepath.Join(goenv["GOROOT"], "src", importPath),
importPathShort: importPath,
packageName: path.Base(importPath),
relevance: MaxRelevance,
@@ -779,33 +785,44 @@
// If Logf is non-nil, debug logging is enabled through this function.
Logf func(format string, args ...interface{})
+ initialized bool
+
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))
+func (e *ProcessEnv) goEnv() (map[string]string, error) {
+ if err := e.init(); err != nil {
+ return nil, err
}
- return v
+ return e.Env, nil
+}
+
+func (e *ProcessEnv) matchFile(dir, name string) (bool, error) {
+ return build.Default.MatchFile(dir, name)
}
// CopyConfig copies the env's configuration into a new env.
func (e *ProcessEnv) CopyConfig() *ProcessEnv {
- copy := *e
- copy.resolver = nil
- return ©
+ copy := &ProcessEnv{
+ GocmdRunner: e.GocmdRunner,
+ initialized: e.initialized,
+ BuildFlags: e.BuildFlags,
+ Logf: e.Logf,
+ WorkingDir: e.WorkingDir,
+ resolver: nil,
+ Env: map[string]string{},
+ }
+ for k, v := range e.Env {
+ copy.Env[k] = v
+ }
+ return copy
}
func (e *ProcessEnv) init() error {
+ if e.initialized {
+ return nil
+ }
+
foundAllRequired := true
for _, k := range RequiredGoEnvVars {
if _, ok := e.Env[k]; !ok {
@@ -814,6 +831,7 @@
}
}
if foundAllRequired {
+ e.initialized = true
return nil
}
@@ -832,6 +850,7 @@
for k, v := range goEnv {
e.Env[k] = v
}
+ e.initialized = true
return nil
}
@@ -858,10 +877,14 @@
return e.resolver, nil
}
-func (e *ProcessEnv) buildContext() *build.Context {
+func (e *ProcessEnv) buildContext() (*build.Context, error) {
ctx := build.Default
- ctx.GOROOT = e.goroot()
- ctx.GOPATH = e.gopath()
+ goenv, err := e.goEnv()
+ if err != nil {
+ return nil, err
+ }
+ ctx.GOROOT = goenv["GOROOT"]
+ ctx.GOPATH = goenv["GOPATH"]
// As of Go 1.14, build.Context has a Dir field
// (see golang.org/issue/34860).
@@ -877,7 +900,7 @@
dir.SetString(e.WorkingDir)
}
- return &ctx
+ return &ctx, nil
}
func (e *ProcessEnv) invokeGo(ctx context.Context, verb string, args ...string) (*bytes.Buffer, error) {
@@ -892,10 +915,14 @@
return e.GocmdRunner.Run(ctx, inv)
}
-func addStdlibCandidates(pass *pass, refs references) {
+func addStdlibCandidates(pass *pass, refs references) error {
+ goenv, err := pass.env.goEnv()
+ if err != nil {
+ return err
+ }
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(goenv["GOROOT"], "src", pkg) == pass.srcDir {
return
}
exports := copyExports(stdlib[pkg])
@@ -916,6 +943,7 @@
}
}
}
+ return nil
}
// A Resolver does the build-system-specific parts of goimports.
@@ -1112,21 +1140,24 @@
func (r *gopathResolver) loadPackageNames(importPaths []string, srcDir string) (map[string]string, error) {
names := map[string]string{}
+ bctx, err := r.env.buildContext()
+ if err != nil {
+ return nil, err
+ }
for _, path := range importPaths {
- names[path] = importPathToName(r.env, path, srcDir)
+ names[path] = importPathToName(bctx, path, srcDir)
}
return names, nil
}
// importPathToName finds out the actual package name, as declared in its .go files.
-// If there's a problem, it returns "".
-func importPathToName(env *ProcessEnv, importPath, srcDir string) (packageName string) {
+func importPathToName(bctx *build.Context, importPath, srcDir string) string {
// Fast path for standard library without going to disk.
if _, ok := stdlib[importPath]; ok {
return path.Base(importPath) // stdlib packages always match their paths.
}
- buildPkg, err := env.buildContext().Import(importPath, srcDir, build.FindOnly)
+ buildPkg, err := bctx.Import(importPath, srcDir, build.FindOnly)
if err != nil {
return ""
}
@@ -1287,8 +1318,18 @@
}
stop := r.cache.ScanAndListen(ctx, processDir)
defer stop()
+
+ goenv, err := r.env.goEnv()
+ if err != nil {
+ return err
+ }
+ var roots []gopathwalk.Root
+ roots = append(roots, gopathwalk.Root{filepath.Join(goenv["GOROOT"], "src"), gopathwalk.RootGOROOT})
+ for _, p := range filepath.SplitList(goenv["GOPATH"]) {
+ roots = append(roots, gopathwalk.Root{filepath.Join(p, "src"), gopathwalk.RootGOPATH})
+ }
// The callback is not necessarily safe to use in the goroutine below. Process roots eagerly.
- roots := filterRoots(gopathwalk.SrcDirsRoots(r.env.buildContext()), callback.rootFound)
+ roots = filterRoots(roots, callback.rootFound)
// We can't cancel walks, because we need them to finish to have a usable
// cache. Instead, run them in a separate goroutine and detach.
scanDone := make(chan struct{})
@@ -1348,8 +1389,6 @@
}
func loadExportsFromFiles(ctx context.Context, env *ProcessEnv, dir string, includeTest bool) (string, []string, error) {
- var exports []string
-
// Look for non-test, buildable .go files which could provide exports.
all, err := ioutil.ReadDir(dir)
if err != nil {
@@ -1361,7 +1400,7 @@
if !strings.HasSuffix(name, ".go") || (!includeTest && strings.HasSuffix(name, "_test.go")) {
continue
}
- match, err := env.buildContext().MatchFile(dir, fi.Name())
+ match, err := env.matchFile(dir, fi.Name())
if err != nil || !match {
continue
}
@@ -1373,6 +1412,7 @@
}
var pkgName string
+ var exports []string
fset := token.NewFileSet()
for _, fi := range files {
select {
diff --git a/internal/imports/fix_test.go b/internal/imports/fix_test.go
index 170b6d8..e14f1d3 100644
--- a/internal/imports/fix_test.go
+++ b/internal/imports/fix_test.go
@@ -1568,8 +1568,8 @@
},
}.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.goroot, "src")
+ got, err := t.processNonModule(filepath.Join(t.goroot, "src/x.go"), []byte(input), nil)
if err != nil {
t.Fatalf("Process() = %v", err)
}
@@ -1591,7 +1591,7 @@
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.goroot, "src/crypto/ecdsa/foo.go"), []byte(input), nil)
if err != nil {
t.Fatalf("Process() = %v", err)
}
@@ -1646,6 +1646,7 @@
// packagestest clears out GOROOT to work around golang/go#32849,
// which isn't relevant here. Fill it back in so we can find the standard library.
it.env.Env["GOROOT"] = build.Default.GOROOT
+ it.goroot = build.Default.GOROOT
fn(it)
})
@@ -1662,6 +1663,7 @@
type goimportTest struct {
*testing.T
+ goroot string
env *ProcessEnv
exported *packagestest.Exported
}
diff --git a/internal/imports/mod.go b/internal/imports/mod.go
index 664fbbf..94880d6 100644
--- a/internal/imports/mod.go
+++ b/internal/imports/mod.go
@@ -53,6 +53,10 @@
return nil
}
+ goenv, err := r.env.goEnv()
+ if err != nil {
+ return err
+ }
inv := gocommand.Invocation{
BuildFlags: r.env.BuildFlags,
Env: r.env.env(),
@@ -82,7 +86,7 @@
if gmc := r.env.Env["GOMODCACHE"]; gmc != "" {
r.moduleCacheDir = gmc
} else {
- r.moduleCacheDir = filepath.Join(filepath.SplitList(r.env.gopath())[0], "/pkg/mod")
+ r.moduleCacheDir = filepath.Join(filepath.SplitList(goenv["GOPATH"])[0], "/pkg/mod")
}
sort.Slice(r.modsByModPath, func(i, j int) bool {
@@ -99,7 +103,7 @@
})
r.roots = []gopathwalk.Root{
- {filepath.Join(r.env.goroot(), "/src"), gopathwalk.RootGOROOT},
+ {filepath.Join(goenv["GOROOT"], "/src"), gopathwalk.RootGOROOT},
}
if r.main != nil {
r.roots = append(r.roots, gopathwalk.Root{r.main.Dir, gopathwalk.RootCurrentModule})
@@ -240,7 +244,7 @@
// files in that directory. If not, it could be provided by an
// outer module. See #29736.
for _, fi := range pkgFiles {
- if ok, _ := r.env.buildContext().MatchFile(pkgDir, fi.Name()); ok {
+ if ok, _ := r.env.matchFile(pkgDir, fi.Name()); ok {
return m, pkgDir
}
}
diff --git a/internal/imports/mod_test.go b/internal/imports/mod_test.go
index 9666fb8..254bc28 100644
--- a/internal/imports/mod_test.go
+++ b/internal/imports/mod_test.go
@@ -652,6 +652,7 @@
type modTest struct {
*testing.T
env *ProcessEnv
+ gopath string
resolver *ModuleResolver
cleanup func()
}
@@ -714,6 +715,7 @@
}
return &modTest{
T: t,
+ gopath: env.Env["GOPATH"],
env: env,
resolver: resolver.(*ModuleResolver),
cleanup: func() { removeDir(dir) },
@@ -841,7 +843,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.gopath, "pkg/mod", "rsc.io/quote@v1.5.2")
found := mt.assertScanFinds("rsc.io/quote", "quote")
modDir, _ := mt.resolver.modInfo(found.dir)
