Skip to content

Commit

Permalink
go/packages: report an error from Load when GOROOT is misconfigured
Browse files Browse the repository at this point in the history
Load should return an error when 'go list' command returns the following error:

    err: exit status 2: stderr: go: no such tool "compile"

This occurs when internally running `go list` on a misconfigured GOROOT directory.

Fixes golang/go#69606

Change-Id: Ib7bf58293612e26aa33d2cf8230378747ad01820
Reviewed-on: https://go-review.googlesource.com/c/tools/+/615396
Reviewed-by: Michael Podtserkovskii <michaelpo@meta.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Commit-Queue: Tim King <taking@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Tim King <taking@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
  • Loading branch information
xieyuschen authored and Go LUCI committed Oct 10, 2024
1 parent 50179f2 commit feffeaa
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 0 deletions.
6 changes: 6 additions & 0 deletions go/packages/golist.go
Original file line number Diff line number Diff line change
Expand Up @@ -879,6 +879,12 @@ func (state *golistState) invokeGo(verb string, args ...string) (*bytes.Buffer,
return nil, friendlyErr
}

// Return an error if 'go list' failed due to missing tools in
// $GOROOT/pkg/tool/$GOOS_$GOARCH (#69606).
if len(stderr.String()) > 0 && strings.Contains(stderr.String(), `go: no such tool`) {
return nil, friendlyErr
}

// Is there an error running the C compiler in cgo? This will be reported in the "Error" field
// and should be suppressed by go list -e.
//
Expand Down
73 changes: 73 additions & 0 deletions go/packages/packages_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,14 @@ import (
"sort"
"strings"
"testing"
"testing/fstest"
"time"

"golang.org/x/tools/go/packages"
"golang.org/x/tools/go/packages/packagestest"
"golang.org/x/tools/internal/packagesinternal"
"golang.org/x/tools/internal/testenv"
"golang.org/x/tools/internal/testfiles"
)

// testCtx is canceled when the test binary is about to time out.
Expand Down Expand Up @@ -3103,3 +3105,74 @@ func TestLoadOverlayGoMod(t *testing.T) {
t.Errorf("Load: got %s, want %v", got, want)
}
}

func overlayFS(overlay map[string][]byte) fstest.MapFS {
fs := make(fstest.MapFS)
for name, data := range overlay {
fs[name] = &fstest.MapFile{Data: data}
}
return fs
}

// TestIssue69606a tests when tools in $GOROOT/pkg/tool/$GOOS_$GOARCH are missing,
// Load should return an error.
func TestIssue69606a(t *testing.T) {
testenv.NeedsTool(t, "go")
overlay := overlayFS(map[string][]byte{
"io/io.go": []byte("package io"),
"unsafe/unsafe.go": []byte("package unsafe"),
})
goroot := testfiles.CopyToTmp(t, overlay)

t.Logf("custom GOROOT: %s", goroot)

// load the std packages under a custom GOROOT
_, err := packages.Load(&packages.Config{
Mode: packages.NeedName |
packages.NeedFiles |
packages.NeedImports |
packages.NeedTypes,
Env: append(
os.Environ(),
"GO111MODULES=on",
"GOPATH=",
"GOWORK=off",
"GOPROXY=off",
fmt.Sprintf("GOROOT=%s", goroot)),
}, "std")

if err == nil {
t.Fatal("Expected to get an error because missing tool 'compile' but got a nil error")
}
}

// TestIssue69606b tests when loading std from a fake goroot without a unsafe package,
// Load should return an error.
func TestIssue69606b(t *testing.T) {
testenv.NeedsTool(t, "go")
overlay := overlayFS(map[string][]byte{
"io/io.go": []byte("package io"),
})
goroot := testfiles.CopyToTmp(t, overlay)

t.Logf("custom GOROOT: %s", goroot)

// load the std packages under a custom GOROOT
_, err := packages.Load(&packages.Config{
Mode: packages.NeedName |
packages.NeedFiles |
packages.NeedImports |
packages.NeedTypes,
Env: append(
os.Environ(),
"GO111MODULES=on",
"GOPATH=",
"GOWORK=off",
"GOPROXY=off",
fmt.Sprintf("GOROOT=%s", goroot)),
}, "std")

if err == nil {
t.Fatal("Expected to get an error because missing unsafe package but got a nil error")
}
}

0 comments on commit feffeaa

Please sign in to comment.