cmd/go/internal/modindex: avoid walking modules when not needed

Due to a missed condition in CL 412394, we were walking all modules
(instead of just the ones contained in GOROOT) at each invocation of a
devel version of cmd/go.

Moreover, while we were running cmd/go tests, we were re-walking
GOROOT at each 'go' invocation in the test even though we expect
GOROOT to be stable within a test run.

This change always avoids walking non-GOROOT modules, and also adds a
salt (configurable via GODEBUG) and uses it to avoid walking GOROOT
modules when GOROOT is known to be stable (such as over the course of
a 'cmd/go' test run).

This should fix the cmd/go test timeouts that are currently occurring
on the dragonfly-amd64 builder, such as this one:
https://build.golang.org/log/21c01c3ae5490d387d84abeaf872b3a0a76ab8e5

Updates #53290.

Change-Id: Ic807d215831a3cd21c63e0bccd3d7acd10d8f2b7
Reviewed-on: https://go-review.googlesource.com/c/go/+/412779
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>

diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go
index b39a62f3e4cc1898536b13dbf21ce1aaad12d23c..bbcecf2b2e4bff77837532380e256ddb0317bf8e 100644 (file)
--- a/src/cmd/go/go_test.go
+++ b/src/cmd/go/go_test.go
@@ -18,6 +18,7 @@ import (
        "io"
        "io/fs"
        "log"
+       "math/rand"
        "os"
        "os/exec"
        "path/filepath"
@@ -128,6 +129,19 @@ func TestMain(m *testing.M) {
        }
        os.Setenv("CMDGO_TEST_RUN_MAIN", "true")
 
+       if strings.HasPrefix(runtime.Version(), "devel ") && godebug.Get("goindexsalt") == "" {
+               // We're going to execute a lot of cmd/go tests, so set a consistent salt
+               // via GODEBUG so that the modindex package can avoid walking an entire
+               // GOROOT module whenever it tries to use an index for that module.
+               indexSalt := rand.Int63()
+               v := os.Getenv("GODEBUG")
+               if v == "" {
+                       os.Setenv("GODEBUG", fmt.Sprintf("goindexsalt=%d", indexSalt))
+               } else {
+                       os.Setenv("GODEBUG", fmt.Sprintf("%s,goindexsalt=%d", v, indexSalt))
+               }
+       }
+
        // $GO_GCFLAGS a compiler debug flag known to cmd/dist, make.bash, etc.
        // It is not a standard go command flag; use os.Getenv, not cfg.Getenv.
        if os.Getenv("GO_GCFLAGS") != "" {

diff --git a/src/cmd/go/internal/modindex/read.go b/src/cmd/go/internal/modindex/read.go
index ffa091df41b680fcf56f376279e8ed45b5be5d57..e5761af679480a7a91039b496c4a2778bcd96cfa 100644 (file)
--- a/src/cmd/go/internal/modindex/read.go
+++ b/src/cmd/go/internal/modindex/read.go
@@ -12,11 +12,11 @@ import (
        "go/build"
        "go/build/constraint"
        "go/token"
+       "internal/godebug"
        "internal/goroot"
        "internal/unsafeheader"
        "io/fs"
        "math"
-       "os"
        "path"
        "path/filepath"
        "runtime"
@@ -39,15 +39,7 @@ import (
 // It will be removed before the release.
 // TODO(matloob): Remove enabled once we have more confidence on the
 // module index.
-var enabled = func() bool {
-       debug := strings.Split(os.Getenv("GODEBUG"), ",")
-       for _, f := range debug {
-               if f == "goindex=0" {
-                       return false
-               }
-       }
-       return true
-}()
+var enabled bool = godebug.Get("goindex") != "0"
 
 // ModuleIndex represents and encoded module index file. It is used to
 // do the equivalent of build.Import of packages in the module and answer other
@@ -61,26 +53,34 @@ type ModuleIndex struct {
 
 var fcache par.Cache
 
+var salt = godebug.Get("goindexsalt")
+
 func moduleHash(modroot string, ismodcache bool) (cache.ActionID, error) {
        // We expect modules stored within the module cache to be checksummed and
-       // immutable, and we expect released Go modules to change only infrequently
-       // (when the Go version changes).
-       if !ismodcache || !str.HasFilePathPrefix(modroot, cfg.GOROOT) {
+       // immutable, and we expect released modules within GOROOT to change only
+       // infrequently (when the Go version changes).
+       if !ismodcache && !str.HasFilePathPrefix(modroot, cfg.GOROOT) {
+               // The contents of this module may change over time. We don't want to pay
+               // the cost to detect changes and re-index whenever they occur, so just
+               // don't index it at all.
                return cache.ActionID{}, ErrNotIndexed
        }
 
        h := cache.NewHash("moduleIndex")
-       fmt.Fprintf(h, "module index %s %s %v\n", runtime.Version(), indexVersion, modroot)
+       fmt.Fprintf(h, "module index %s %s %s %v\n", runtime.Version(), salt, indexVersion, modroot)
 
-       if strings.HasPrefix(runtime.Version(), "devel ") {
+       if str.HasFilePathPrefix(modroot, cfg.GOROOT) && strings.HasPrefix(runtime.Version(), "devel ") && salt == "" {
                // This copy of the standard library is a development version, not a
-               // release. It could be based on a Git commit (like "devel go1.19-2a78e8afc0
-               // Wed Jun 15 00:06:24 2022 +0000") with or without changes on top of that
-               // commit, or it could be completly artificial due to lacking a `git` binary
-               // (like "devel gomote.XXXXX", as synthesized by "gomote push" as of
-               // 2022-06-15). Compute an inexpensive hash of its files using mtimes so
-               // that during development we can continue to exercise the logic for cached
-               // GOROOT indexes.
+               // release. It could be based on a Git commit (like
+               // "devel go1.19-2a78e8afc0 Wed Jun 15 00:06:24 2022 +0000") with or
+               // without changes on top of that commit, or it could be completly
+               // artificial due to lacking a `git` binary (like "devel gomote.XXXXX", as
+               // synthesized by "gomote push" as of 2022-06-15).
+               //
+               // If the user provided a unique salt via GODEBUG, we can trust that it is
+               // unique and just go with it. Otherwise, we compute an inexpensive hash of
+               // its files using mtimes so that during development we can continue to
+               // exercise the logic for cached GOROOT indexes.
                //
                // mtimes may be granular, imprecise, and loosely updated (see
                // https://apenwarr.ca/log/20181113), but we don't expect Go contributors to