Skip to content

Commit

Permalink
go/build: avoid setting Default.GOROOT when runtime.GOROOT() is empty
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Bryan C. Mills committed Mar 18, 2022
1 parent 58631ba commit 6378c0e
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 24 deletions.
4 changes: 2 additions & 2 deletions src/cmd/go/testdata/script/build_trimpath_goroot.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
37 changes: 22 additions & 15 deletions src/go/build/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
10 changes: 3 additions & 7 deletions src/go/build/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
package build

import (
"flag"
"internal/testenv"
"io"
"os"
Expand All @@ -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())
}

Expand Down Expand Up @@ -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.`)
}
Expand Down Expand Up @@ -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)
}
Expand Down

0 comments on commit 6378c0e

Please sign in to comment.