go/build: avoid setting Default.GOROOT when runtime.GOROOT() is empty

Previously, we called path.Clean on the value of runtime.GOROOT() even
if it was empty, which would set it explicitly to ".".
That would cause (*Context).importGo to assume that errors resolving
paths in GOROOT are fatal and return early:
https://cs.opensource.google/go/go/+/master:src/go/build/build.go;l=1121-1127;drc=38174b3a3514629b84dcd76878b2f536b189dd7b

If we instead leave it empty (and are in module mode), then importGo
will fall back to letting the 'go' command resolve the path, which may
succeed if the 'go' command can infer the correct GOROOT (from its own
stamped-in default GOROOT or executable path).

Fixes #51483

Change-Id: I44dce7cec6c3d1c86670e629ddfbca8be461130c
Reviewed-on: https://go-review.googlesource.com/c/go/+/391805
Trust: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>

diff --git a/src/cmd/go/testdata/script/build_trimpath_goroot.txt b/src/cmd/go/testdata/script/build_trimpath_goroot.txt
index 277a7a4bb6a69138ffb09cb2a8130355d02a5137..c0ce1f0457f891aa8005ea2c807e689da5dc10e6 100644 (file)
--- a/src/cmd/go/testdata/script/build_trimpath_goroot.txt
+++ b/src/cmd/go/testdata/script/build_trimpath_goroot.txt
@@ -23,11 +23,11 @@ stdout '^runtime '$TESTGO_GOROOT${/}src${/}runtime'$'
 
 ! go run -trimpath .
 stdout '^GOROOT $'
-stderr '^package runtime is not in GOROOT \(src'${/}'runtime\)$'
+stderr 'cannot find package "runtime" in any of:\n\t\(\$GOROOT not set\)\n\t'$WORK${/}gopath${/}src${/}runtime' \(from \$GOPATH\)\nexit status 1\n\z'
 
 ! go test -trimpath -v .
 stdout '^GOROOT $'
-stdout '^package runtime is not in GOROOT \(src'${/}'runtime\)$'
+stdout 'cannot find package "runtime" in any of:\n\t\(\$GOROOT not set\)\n\t'$WORK${/}gopath${/}src${/}runtime' \(from \$GOPATH\)$'
 
 -- go.mod --
 module example

diff --git a/src/go/build/build.go b/src/go/build/build.go
index b0842b3a1a689e72a1cdabac48e1643570e7ae6b..cacdbef4eee2557ed4f0ad63a7e08c2fe1130a78 100644 (file)
--- a/src/go/build/build.go
+++ b/src/go/build/build.go
@@ -35,7 +35,7 @@ type Context struct {
        GOARCH string // target architecture
        GOOS   string // target operating system
        GOROOT string // Go root
-       GOPATH string // Go path
+       GOPATH string // Go paths
 
        // Dir is the caller's working directory, or the empty string to use
        // the current directory of the running process. In module mode, this is used
@@ -302,7 +302,9 @@ func defaultContext() Context {
 
        c.GOARCH = buildcfg.GOARCH
        c.GOOS = buildcfg.GOOS
-       c.GOROOT = pathpkg.Clean(runtime.GOROOT())
+       if goroot := runtime.GOROOT(); goroot != "" {
+               c.GOROOT = filepath.Clean(goroot)
+       }
        c.GOPATH = envOr("GOPATH", defaultGOPATH())
        c.Compiler = runtime.Compiler
 
@@ -672,7 +674,7 @@ func (ctxt *Context) Import(path string, srcDir string, mode ImportMode) (*Packa
                                }
                                return false
                        }
-                       if ctxt.Compiler != "gccgo" && searchVendor(ctxt.GOROOT, true) {
+                       if ctxt.Compiler != "gccgo" && ctxt.GOROOT != "" && searchVendor(ctxt.GOROOT, true) {
                                goto Found
                        }
                        for _, root := range gopath {
@@ -706,12 +708,12 @@ func (ctxt *Context) Import(path string, srcDir string, mode ImportMode) (*Packa
                                }
                                tried.goroot = dir
                        }
-               }
-               if ctxt.Compiler == "gccgo" && goroot.IsStandardPackage(ctxt.GOROOT, ctxt.Compiler, path) {
-                       p.Dir = ctxt.joinPath(ctxt.GOROOT, "src", path)
-                       p.Goroot = true
-                       p.Root = ctxt.GOROOT
-                       goto Found
+                       if ctxt.Compiler == "gccgo" && goroot.IsStandardPackage(ctxt.GOROOT, ctxt.Compiler, path) {
+                               p.Dir = ctxt.joinPath(ctxt.GOROOT, "src", path)
+                               p.Goroot = true
+                               p.Root = ctxt.GOROOT
+                               goto Found
+                       }
                }
                for _, root := range gopath {
                        dir := ctxt.joinPath(root, "src", path)
@@ -1082,6 +1084,13 @@ func (ctxt *Context) importGo(p *Package, path, srcDir string, mode ImportMode)
                return errNoModules
        }
 
+       // If ctxt.GOROOT is not set, we don't know which go command to invoke,
+       // and even if we did we might return packages in GOROOT that we wouldn't otherwise find
+       // (because we don't know to search in 'go env GOROOT' otherwise).
+       if ctxt.GOROOT == "" {
+               return errNoModules
+       }
+
        // Predict whether module aware mode is enabled by checking the value of
        // GO111MODULE and looking for a go.mod file in the source directory or
        // one of its parents. Running 'go env GOMOD' in the source directory would
@@ -1119,11 +1128,8 @@ func (ctxt *Context) importGo(p *Package, path, srcDir string, mode ImportMode)
        }
 
        // For efficiency, if path is a standard library package, let the usual lookup code handle it.
-       if ctxt.GOROOT != "" {
-               dir := ctxt.joinPath(ctxt.GOROOT, "src", path)
-               if ctxt.isDir(dir) {
-                       return errNoModules
-               }
+       if dir := ctxt.joinPath(ctxt.GOROOT, "src", path); ctxt.isDir(dir) {
+               return errNoModules
        }
 
        // If GO111MODULE=auto, look to see if there is a go.mod.
@@ -1165,7 +1171,8 @@ func (ctxt *Context) importGo(p *Package, path, srcDir string, mode ImportMode)
                }
        }
 
-       cmd := exec.Command("go", "list", "-e", "-compiler="+ctxt.Compiler, "-tags="+strings.Join(ctxt.BuildTags, ","), "-installsuffix="+ctxt.InstallSuffix, "-f={{.Dir}}\n{{.ImportPath}}\n{{.Root}}\n{{.Goroot}}\n{{if .Error}}{{.Error}}{{end}}\n", "--", path)
+       goCmd := filepath.Join(ctxt.GOROOT, "bin", "go")
+       cmd := exec.Command(goCmd, "list", "-e", "-compiler="+ctxt.Compiler, "-tags="+strings.Join(ctxt.BuildTags, ","), "-installsuffix="+ctxt.InstallSuffix, "-f={{.Dir}}\n{{.ImportPath}}\n{{.Root}}\n{{.Goroot}}\n{{if .Error}}{{.Error}}{{end}}\n", "--", path)
 
        if ctxt.Dir != "" {
                cmd.Dir = ctxt.Dir

diff --git a/src/go/build/build_test.go b/src/go/build/build_test.go
index cfe9c5e4bb286b82a944e41929adb72c080bfd2a..6cd7f9b58903d4ef0101616f97b875483a3af338 100644 (file)
--- a/src/go/build/build_test.go
+++ b/src/go/build/build_test.go
@@ -5,7 +5,6 @@
 package build
 
 import (
-       "flag"
        "internal/testenv"
        "io"
        "os"
@@ -17,10 +16,7 @@ import (
 )
 
 func TestMain(m *testing.M) {
-       flag.Parse()
-       if goTool, err := testenv.GoTool(); err == nil {
-               os.Setenv("PATH", filepath.Dir(goTool)+string(os.PathListSeparator)+os.Getenv("PATH"))
-       }
+       Default.GOROOT = testenv.GOROOT(nil)
        os.Exit(m.Run())
 }
 
@@ -84,7 +80,7 @@ func TestDotSlashImport(t *testing.T) {
 }
 
 func TestEmptyImport(t *testing.T) {
-       p, err := Import("", Default.GOROOT, FindOnly)
+       p, err := Import("", testenv.GOROOT(t), FindOnly)
        if err == nil {
                t.Fatal(`Import("") returned nil error.`)
        }
@@ -658,7 +654,7 @@ func TestImportDirTarget(t *testing.T) {
        testenv.MustHaveGoBuild(t) // really must just have source
        ctxt := Default
        ctxt.GOPATH = ""
-       p, err := ctxt.ImportDir(filepath.Join(ctxt.GOROOT, "src/path"), 0)
+       p, err := ctxt.ImportDir(filepath.Join(testenv.GOROOT(t), "src/path"), 0)
        if err != nil {
                t.Fatal(err)
        }