From aca37d16a5a5c1d24e374245f0e5b6404379db96 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Wed, 22 Jun 2022 15:19:54 -0400 Subject: [PATCH] cmd/go: avoid indexing modules in GOROOT Scanning GOROOT modules for changes appears to be causing Windows builders to time out in x/tools tests. We may try a per-package index instead, but for now just skip GOROOT modules (as we do for main modules). Fixes #53493. Change-Id: Ic5bb90b4ce173a24fc6564e85fcde96e1f9b975f Reviewed-on: https://go-review.googlesource.com/c/go/+/413634 TryBot-Result: Gopher Robot Run-TryBot: Bryan Mills Reviewed-by: Michael Matloob --- src/cmd/go/go_test.go | 14 ------- src/cmd/go/internal/modindex/read.go | 58 ++++++---------------------- 2 files changed, 11 insertions(+), 61 deletions(-) diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go index bbcecf2b2e4..b39a62f3e4c 100644 --- a/src/cmd/go/go_test.go +++ b/src/cmd/go/go_test.go @@ -18,7 +18,6 @@ import ( "io" "io/fs" "log" - "math/rand" "os" "os/exec" "path/filepath" @@ -129,19 +128,6 @@ 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 e5761af6794..ea1ebb07c23 100644 --- a/src/cmd/go/internal/modindex/read.go +++ b/src/cmd/go/internal/modindex/read.go @@ -15,7 +15,6 @@ import ( "internal/godebug" "internal/goroot" "internal/unsafeheader" - "io/fs" "math" "path" "path/filepath" @@ -29,7 +28,6 @@ import ( "cmd/go/internal/base" "cmd/go/internal/cache" "cmd/go/internal/cfg" - "cmd/go/internal/fsys" "cmd/go/internal/imports" "cmd/go/internal/par" "cmd/go/internal/str" @@ -55,63 +53,29 @@ var fcache par.Cache var salt = godebug.Get("goindexsalt") +// moduleHash returns an ActionID corresponding to the state of the module +// located at filesystem path modroot. 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 modules within GOROOT to change only // infrequently (when the Go version changes). - if !ismodcache && !str.HasFilePathPrefix(modroot, cfg.GOROOT) { + if !ismodcache { // 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. + // + // Note that this is true even for modules in GOROOT/src: non-release builds + // of the Go toolchain may have arbitrary development changes on top of the + // commit reported by runtime.Version, or could be completly artificial due + // to lacking a `git` binary (like "devel gomote.XXXXX", as synthesized by + // "gomote push" as of 2022-06-15). (Release builds shouldn't have + // modifications, but we don't want to use a behavior for releases that we + // haven't tested during development.) return cache.ActionID{}, ErrNotIndexed } h := cache.NewHash("moduleIndex") fmt.Fprintf(h, "module index %s %s %s %v\n", runtime.Version(), salt, indexVersion, modroot) - - 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). - // - // 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 - // be mucking around with the import graphs in GOROOT often enough for mtime - // collisions to matter essentially ever. - // - // Note that fsys.Walk walks paths in deterministic order, so this hash - // should be completely deterministic if the files are unchanged. - err := fsys.Walk(modroot, func(path string, info fs.FileInfo, err error) error { - if err := moduleWalkErr(modroot, path, info, err); err != nil { - return err - } - - if info.IsDir() { - return nil - } - fmt.Fprintf(h, "file %v %v\n", info.Name(), info.ModTime()) - if info.Mode()&fs.ModeSymlink != 0 { - targ, err := fsys.Stat(path) - if err != nil { - return err - } - fmt.Fprintf(h, "target %v %v\n", targ.Name(), targ.ModTime()) - } - return nil - }) - if err != nil { - return cache.ActionID{}, err - } - } - return h.Sum(), nil }