From 4976606a2f9203f4520c28d775e67fc32f5853ed Mon Sep 17 00:00:00 2001 From: Ian Alexander Date: Mon, 24 Nov 2025 20:24:51 -0500 Subject: [PATCH] cmd/go: remove final references to modfetch.Fetcher_ This commit removes the final references to the global Fetcher_ variable from the modfetch and modload packages. This completes the removal of global state from the modfetch package. Change-Id: Ibb5309acdc7d05f1a7591ddcf890b44b6cc4cb2f Reviewed-on: https://go-review.googlesource.com/c/go/+/724249 Reviewed-by: Michael Matloob Reviewed-by: Michael Matloob LUCI-TryBot-Result: Go LUCI --- src/cmd/go/internal/load/pkg.go | 6 +- src/cmd/go/internal/load/test.go | 2 +- src/cmd/go/internal/modfetch/cache.go | 19 ++--- src/cmd/go/internal/modfetch/coderepo_test.go | 14 ++-- src/cmd/go/internal/modfetch/fetch.go | 72 +++++++++---------- src/cmd/go/internal/modfetch/repo.go | 6 +- src/cmd/go/internal/modload/build.go | 4 +- src/cmd/go/internal/modload/init.go | 7 +- src/cmd/go/internal/modload/load.go | 6 +- 9 files changed, 67 insertions(+), 69 deletions(-) diff --git a/src/cmd/go/internal/load/pkg.go b/src/cmd/go/internal/load/pkg.go index 91602aa5bff..e2a77d7d7df 100644 --- a/src/cmd/go/internal/load/pkg.go +++ b/src/cmd/go/internal/load/pkg.go @@ -2050,7 +2050,7 @@ func (p *Package) load(loaderstate *modload.State, ctx context.Context, opts Pac // Consider starting this as a background goroutine and retrieving the result // asynchronously when we're actually ready to build the package, or when we // actually need to evaluate whether the package's metadata is stale. - p.setBuildInfo(ctx, opts.AutoVCS) + p.setBuildInfo(ctx, loaderstate.Fetcher(), opts.AutoVCS) } // If cgo is not enabled, ignore cgo supporting sources @@ -2323,7 +2323,7 @@ func appendBuildSetting(info *debug.BuildInfo, key, value string) { // // Note that the GoVersion field is not set here to avoid encoding it twice. // It is stored separately in the binary, mostly for historical reasons. -func (p *Package) setBuildInfo(ctx context.Context, autoVCS bool) { +func (p *Package) setBuildInfo(ctx context.Context, f *modfetch.Fetcher, autoVCS bool) { setPkgErrorf := func(format string, args ...any) { if p.Error == nil { p.Error = &PackageError{Err: fmt.Errorf(format, args...)} @@ -2595,7 +2595,7 @@ func (p *Package) setBuildInfo(ctx context.Context, autoVCS bool) { if !ok { goto omitVCS } - repo := modfetch.LookupLocal(ctx, codeRoot, p.Module.Path, repoDir) + repo := f.LookupLocal(ctx, codeRoot, p.Module.Path, repoDir) revInfo, err := repo.Stat(ctx, st.Revision) if err != nil { goto omitVCS diff --git a/src/cmd/go/internal/load/test.go b/src/cmd/go/internal/load/test.go index c7c58fd5487..e5c074fa193 100644 --- a/src/cmd/go/internal/load/test.go +++ b/src/cmd/go/internal/load/test.go @@ -301,7 +301,7 @@ func TestPackagesAndErrors(loaderstate *modload.State, ctx context.Context, done // pmain won't have buildinfo set (since we copy it from the package under test). If the default GODEBUG // used for the package under test is different from that of the test main, the BuildInfo assigned above from the package // under test incorrect for the test main package. Either set or correct pmain's build info. - pmain.setBuildInfo(ctx, opts.AutoVCS) + pmain.setBuildInfo(ctx, loaderstate.Fetcher(), opts.AutoVCS) } // The generated main also imports testing, regexp, and os. diff --git a/src/cmd/go/internal/modfetch/cache.go b/src/cmd/go/internal/modfetch/cache.go index 3886d3b1feb..f035c359b13 100644 --- a/src/cmd/go/internal/modfetch/cache.go +++ b/src/cmd/go/internal/modfetch/cache.go @@ -156,7 +156,7 @@ func lockVersion(ctx context.Context, mod module.Version) (unlock func(), err er if err != nil { return nil, err } - if err := os.MkdirAll(filepath.Dir(path), 0777); err != nil { + if err := os.MkdirAll(filepath.Dir(path), 0o777); err != nil { return nil, err } return lockedfile.MutexAt(path).Lock() @@ -172,7 +172,7 @@ func SideLock(ctx context.Context) (unlock func(), err error) { } path := filepath.Join(cfg.GOMODCACHE, "cache", "lock") - if err := os.MkdirAll(filepath.Dir(path), 0777); err != nil { + if err := os.MkdirAll(filepath.Dir(path), 0o777); err != nil { return nil, fmt.Errorf("failed to create cache directory: %w", err) } @@ -194,12 +194,14 @@ type cachingRepo struct { once sync.Once initRepo func(context.Context) (Repo, error) r Repo + fetcher *Fetcher } -func newCachingRepo(ctx context.Context, path string, initRepo func(context.Context) (Repo, error)) *cachingRepo { +func newCachingRepo(ctx context.Context, fetcher *Fetcher, path string, initRepo func(context.Context) (Repo, error)) *cachingRepo { return &cachingRepo{ path: path, initRepo: initRepo, + fetcher: fetcher, } } @@ -226,7 +228,6 @@ func (r *cachingRepo) Versions(ctx context.Context, prefix string) (*Versions, e v, err := r.versionsCache.Do(prefix, func() (*Versions, error) { return r.repo(ctx).Versions(ctx, prefix) }) - if err != nil { return nil, err } @@ -309,7 +310,7 @@ func (r *cachingRepo) GoMod(ctx context.Context, version string) ([]byte, error) return r.repo(ctx).GoMod(ctx, version) } text, err := r.gomodCache.Do(version, func() ([]byte, error) { - file, text, err := Fetcher_.readDiskGoMod(ctx, r.path, version) + file, text, err := r.fetcher.readDiskGoMod(ctx, r.path, version) if err == nil { // Note: readDiskGoMod already called checkGoMod. return text, nil @@ -317,7 +318,7 @@ func (r *cachingRepo) GoMod(ctx context.Context, version string) ([]byte, error) text, err = r.repo(ctx).GoMod(ctx, version) if err == nil { - if err := checkGoMod(Fetcher_, r.path, version, text); err != nil { + if err := checkGoMod(r.fetcher, r.path, version, text); err != nil { return text, err } if err := writeDiskGoMod(ctx, file, text); err != nil { @@ -653,13 +654,13 @@ func writeDiskCache(ctx context.Context, file string, data []byte) error { return nil } // Make sure directory for file exists. - if err := os.MkdirAll(filepath.Dir(file), 0777); err != nil { + if err := os.MkdirAll(filepath.Dir(file), 0o777); err != nil { return err } // Write the file to a temporary location, and then rename it to its final // path to reduce the likelihood of a corrupt file existing at that final path. - f, err := tempFile(ctx, filepath.Dir(file), filepath.Base(file), 0666) + f, err := tempFile(ctx, filepath.Dir(file), filepath.Base(file), 0o666) if err != nil { return err } @@ -812,7 +813,7 @@ func checkCacheDir(ctx context.Context) error { statCacheErr = fmt.Errorf("could not create module cache: %w", err) return } - if err := os.MkdirAll(cfg.GOMODCACHE, 0777); err != nil { + if err := os.MkdirAll(cfg.GOMODCACHE, 0o777); err != nil { statCacheErr = fmt.Errorf("could not create module cache: %w", err) return } diff --git a/src/cmd/go/internal/modfetch/coderepo_test.go b/src/cmd/go/internal/modfetch/coderepo_test.go index df6e3ed0445..a403e83b847 100644 --- a/src/cmd/go/internal/modfetch/coderepo_test.go +++ b/src/cmd/go/internal/modfetch/coderepo_test.go @@ -36,7 +36,6 @@ func TestMain(m *testing.M) { } func testMain(m *testing.M) (err error) { - cfg.GOPROXY = "direct" // The sum database is populated using a released version of the go command, @@ -56,7 +55,7 @@ func testMain(m *testing.M) (err error) { }() cfg.GOMODCACHE = filepath.Join(dir, "modcache") - if err := os.Mkdir(cfg.GOMODCACHE, 0755); err != nil { + if err := os.Mkdir(cfg.GOMODCACHE, 0o755); err != nil { return err } @@ -589,6 +588,7 @@ var codeRepoTests = []codeRepoTest{ func TestCodeRepo(t *testing.T) { testenv.MustHaveExternalNetwork(t) tmpdir := t.TempDir() + fetcher := NewFetcher() for _, tt := range codeRepoTests { f := func(tt codeRepoTest) func(t *testing.T) { @@ -603,7 +603,7 @@ func TestCodeRepo(t *testing.T) { } ctx := context.Background() - repo := Fetcher_.Lookup(ctx, "direct", tt.path) + repo := fetcher.Lookup(ctx, "direct", tt.path) if tt.mpath == "" { tt.mpath = tt.path @@ -817,7 +817,7 @@ var codeRepoVersionsTests = []struct { func TestCodeRepoVersions(t *testing.T) { testenv.MustHaveExternalNetwork(t) - + fetcher := NewFetcher() for _, tt := range codeRepoVersionsTests { tt := tt t.Run(strings.ReplaceAll(tt.path, "/", "_"), func(t *testing.T) { @@ -831,7 +831,7 @@ func TestCodeRepoVersions(t *testing.T) { } ctx := context.Background() - repo := Fetcher_.Lookup(ctx, "direct", tt.path) + repo := fetcher.Lookup(ctx, "direct", tt.path) list, err := repo.Versions(ctx, tt.prefix) if err != nil { t.Fatalf("Versions(%q): %v", tt.prefix, err) @@ -898,7 +898,7 @@ var latestTests = []struct { func TestLatest(t *testing.T) { testenv.MustHaveExternalNetwork(t) - + fetcher := NewFetcher() for _, tt := range latestTests { name := strings.ReplaceAll(tt.path, "/", "_") t.Run(name, func(t *testing.T) { @@ -909,7 +909,7 @@ func TestLatest(t *testing.T) { } ctx := context.Background() - repo := Fetcher_.Lookup(ctx, "direct", tt.path) + repo := fetcher.Lookup(ctx, "direct", tt.path) info, err := repo.Latest(ctx) if err != nil { if tt.err != "" { diff --git a/src/cmd/go/internal/modfetch/fetch.go b/src/cmd/go/internal/modfetch/fetch.go index 87f69156256..7c9280f1d09 100644 --- a/src/cmd/go/internal/modfetch/fetch.go +++ b/src/cmd/go/internal/modfetch/fetch.go @@ -475,8 +475,6 @@ type Fetcher struct { sumState sumState } -var Fetcher_ *Fetcher = NewFetcher() - func NewFetcher() *Fetcher { f := new(Fetcher) f.lookupCache = new(par.Cache[lookupCacheKey, Repo]) @@ -498,15 +496,15 @@ func (f *Fetcher) AddWorkspaceGoSumFile(file string) { // Reset resets globals in the modfetch package, so previous loads don't affect // contents of go.sum files. -func Reset() { - SetState(NewFetcher()) +func (f *Fetcher) Reset() { + f.SetState(NewFetcher()) } // SetState sets the global state of the modfetch package to the newState, and returns the previous // global state. newState should have been returned by SetState, or be an empty State. // There should be no concurrent calls to any of the exported functions of this package with // a call to SetState because it will modify the global state in a non-thread-safe way. -func SetState(newState *Fetcher) (oldState *Fetcher) { +func (f *Fetcher) SetState(newState *Fetcher) (oldState *Fetcher) { if newState.lookupCache == nil { newState.lookupCache = new(par.Cache[lookupCacheKey, Repo]) } @@ -514,26 +512,26 @@ func SetState(newState *Fetcher) (oldState *Fetcher) { newState.downloadCache = new(par.ErrCache[module.Version, string]) } - Fetcher_.mu.Lock() - defer Fetcher_.mu.Unlock() + f.mu.Lock() + defer f.mu.Unlock() oldState = &Fetcher{ - goSumFile: Fetcher_.goSumFile, - workspaceGoSumFiles: Fetcher_.workspaceGoSumFiles, - lookupCache: Fetcher_.lookupCache, - downloadCache: Fetcher_.downloadCache, - sumState: Fetcher_.sumState, + goSumFile: f.goSumFile, + workspaceGoSumFiles: f.workspaceGoSumFiles, + lookupCache: f.lookupCache, + downloadCache: f.downloadCache, + sumState: f.sumState, } - Fetcher_.SetGoSumFile(newState.goSumFile) - Fetcher_.workspaceGoSumFiles = newState.workspaceGoSumFiles + f.SetGoSumFile(newState.goSumFile) + f.workspaceGoSumFiles = newState.workspaceGoSumFiles // Uses of lookupCache and downloadCache both can call checkModSum, // which in turn sets the used bit on goSum.status for modules. // Set (or reset) them so used can be computed properly. - Fetcher_.lookupCache = newState.lookupCache - Fetcher_.downloadCache = newState.downloadCache + f.lookupCache = newState.lookupCache + f.downloadCache = newState.downloadCache // Set, or reset all fields on goSum. If being reset to empty, it will be initialized later. - Fetcher_.sumState = newState.sumState + f.sumState = newState.sumState return oldState } @@ -666,20 +664,20 @@ func HaveSum(f *Fetcher, mod module.Version) bool { // The entry's hash must be generated with a known hash algorithm. // mod.Version may have a "/go.mod" suffix to distinguish sums for // .mod and .zip files. -func RecordedSum(mod module.Version) (sum string, ok bool) { - Fetcher_.mu.Lock() - defer Fetcher_.mu.Unlock() - inited, err := Fetcher_.initGoSum() +func (f *Fetcher) RecordedSum(mod module.Version) (sum string, ok bool) { + f.mu.Lock() + defer f.mu.Unlock() + inited, err := f.initGoSum() foundSum := "" if err != nil || !inited { return "", false } - for _, goSums := range Fetcher_.sumState.w { + for _, goSums := range f.sumState.w { for _, h := range goSums[mod] { if !strings.HasPrefix(h, "h1:") { continue } - if !Fetcher_.sumState.status[modSum{mod, h}].dirty { + if !f.sumState.status[modSum{mod, h}].dirty { if foundSum != "" && foundSum != h { // conflicting sums exist return "", false } @@ -687,11 +685,11 @@ func RecordedSum(mod module.Version) (sum string, ok bool) { } } } - for _, h := range Fetcher_.sumState.m[mod] { + for _, h := range f.sumState.m[mod] { if !strings.HasPrefix(h, "h1:") { continue } - if !Fetcher_.sumState.status[modSum{mod, h}].dirty { + if !f.sumState.status[modSum{mod, h}].dirty { if foundSum != "" && foundSum != h { // conflicting sums exist return "", false } @@ -977,14 +975,14 @@ Outer: // TidyGoSum returns a tidy version of the go.sum file. // A missing go.sum file is treated as if empty. -func TidyGoSum(keep map[module.Version]bool) (before, after []byte) { - Fetcher_.mu.Lock() - defer Fetcher_.mu.Unlock() - before, err := lockedfile.Read(Fetcher_.goSumFile) +func (f *Fetcher) TidyGoSum(keep map[module.Version]bool) (before, after []byte) { + f.mu.Lock() + defer f.mu.Unlock() + before, err := lockedfile.Read(f.goSumFile) if err != nil && !errors.Is(err, fs.ErrNotExist) { base.Fatalf("reading go.sum: %v", err) } - after = tidyGoSum(Fetcher_, before, keep) + after = tidyGoSum(f, before, keep) return before, after } @@ -1041,10 +1039,10 @@ func sumInWorkspaceModulesLocked(f *Fetcher, m module.Version) bool { // keep is used to check whether a sum should be retained in go.mod. It should // have entries for both module content sums and go.mod sums (version ends // with "/go.mod"). -func TrimGoSum(keep map[module.Version]bool) { - Fetcher_.mu.Lock() - defer Fetcher_.mu.Unlock() - inited, err := Fetcher_.initGoSum() +func (f *Fetcher) TrimGoSum(keep map[module.Version]bool) { + f.mu.Lock() + defer f.mu.Unlock() + inited, err := f.initGoSum() if err != nil { base.Fatalf("%s", err) } @@ -1052,12 +1050,12 @@ func TrimGoSum(keep map[module.Version]bool) { return } - for m, hs := range Fetcher_.sumState.m { + for m, hs := range f.sumState.m { if !keep[m] { for _, h := range hs { - Fetcher_.sumState.status[modSum{m, h}] = modSumStatus{used: false, dirty: true} + f.sumState.status[modSum{m, h}] = modSumStatus{used: false, dirty: true} } - Fetcher_.sumState.overwrite = true + f.sumState.overwrite = true } } } diff --git a/src/cmd/go/internal/modfetch/repo.go b/src/cmd/go/internal/modfetch/repo.go index b1e197284fe..5ed2f259a00 100644 --- a/src/cmd/go/internal/modfetch/repo.go +++ b/src/cmd/go/internal/modfetch/repo.go @@ -206,7 +206,7 @@ func (f *Fetcher) Lookup(ctx context.Context, proxy, path string) Repo { } return f.lookupCache.Do(lookupCacheKey{proxy, path}, func() Repo { - return newCachingRepo(ctx, path, func(ctx context.Context) (Repo, error) { + return newCachingRepo(ctx, f, path, func(ctx context.Context) (Repo, error) { r, err := lookup(f, ctx, proxy, path) if err == nil && traceRepo { r = newLoggingRepo(r) @@ -223,13 +223,13 @@ var lookupLocalCache = new(par.Cache[string, Repo]) // path, Repo // codeRoot is the module path of the root module in the repository. // path is the module path of the module being looked up. // dir is the file system path of the repository containing the module. -func LookupLocal(ctx context.Context, codeRoot string, path string, dir string) Repo { +func (f *Fetcher) LookupLocal(ctx context.Context, codeRoot string, path string, dir string) Repo { if traceRepo { defer logCall("LookupLocal(%q)", path)() } return lookupLocalCache.Do(path, func() Repo { - return newCachingRepo(ctx, path, func(ctx context.Context) (Repo, error) { + return newCachingRepo(ctx, f, path, func(ctx context.Context) (Repo, error) { repoDir, vcsCmd, err := vcs.FromDir(dir, "") if err != nil { return nil, err diff --git a/src/cmd/go/internal/modload/build.go b/src/cmd/go/internal/modload/build.go index e89a34e0ce9..b560dd6a617 100644 --- a/src/cmd/go/internal/modload/build.go +++ b/src/cmd/go/internal/modload/build.go @@ -368,7 +368,7 @@ func moduleInfo(loaderstate *State, ctx context.Context, rs *Requirements, m mod m.GoMod = gomod } } - if gomodsum, ok := modfetch.RecordedSum(modkey(mod)); ok { + if gomodsum, ok := loaderstate.fetcher.RecordedSum(modkey(mod)); ok { m.GoModSum = gomodsum } } @@ -377,7 +377,7 @@ func moduleInfo(loaderstate *State, ctx context.Context, rs *Requirements, m mod if err == nil { m.Dir = dir } - if sum, ok := modfetch.RecordedSum(mod); ok { + if sum, ok := loaderstate.fetcher.RecordedSum(mod); ok { m.Sum = sum } } diff --git a/src/cmd/go/internal/modload/init.go b/src/cmd/go/internal/modload/init.go index d78a41d3c61..8bfae266928 100644 --- a/src/cmd/go/internal/modload/init.go +++ b/src/cmd/go/internal/modload/init.go @@ -60,7 +60,7 @@ func EnterModule(loaderstate *State, ctx context.Context, enterModroot string) { loaderstate.MainModules = nil // reset MainModules loaderstate.requirements = nil loaderstate.workFilePath = "" // Force module mode - modfetch.Reset() + loaderstate.Fetcher().Reset() loaderstate.modRoots = []string{enterModroot} LoadModFile(loaderstate, ctx) @@ -411,8 +411,7 @@ func (s *State) setState(new *State) (old *State) { // The modfetch package's global state is used to compute // the go.sum file, so save and restore it along with the // modload state. - s.fetcher = new.fetcher - old.fetcher = modfetch.SetState(s.fetcher) // TODO(jitsu): remove after completing global state elimination + old.fetcher = s.fetcher.SetState(new.fetcher) return old } @@ -457,7 +456,7 @@ type State struct { func NewState() *State { s := new(State) - s.fetcher = modfetch.Fetcher_ + s.fetcher = modfetch.NewFetcher() return s } diff --git a/src/cmd/go/internal/modload/load.go b/src/cmd/go/internal/modload/load.go index bbc81263d5a..a432862429b 100644 --- a/src/cmd/go/internal/modload/load.go +++ b/src/cmd/go/internal/modload/load.go @@ -462,13 +462,13 @@ func LoadPackages(loaderstate *State, ctx context.Context, opts PackageOpts, pat } goModDiff := diff.Diff("current/go.mod", currentGoMod, "tidy/go.mod", updatedGoMod) - modfetch.TrimGoSum(keep) + loaderstate.Fetcher().TrimGoSum(keep) // Dropping compatibility for 1.16 may result in a strictly smaller go.sum. // Update the keep map with only the loaded.requirements. if gover.Compare(compatVersion, "1.16") > 0 { keep = keepSums(loaderstate, ctx, loaded, loaderstate.requirements, addBuildListZipSums) } - currentGoSum, tidyGoSum := modfetch.TidyGoSum(keep) + currentGoSum, tidyGoSum := loaderstate.fetcher.TidyGoSum(keep) goSumDiff := diff.Diff("current/go.sum", currentGoSum, "tidy/go.sum", tidyGoSum) if len(goModDiff) > 0 { @@ -483,7 +483,7 @@ func LoadPackages(loaderstate *State, ctx context.Context, opts PackageOpts, pat } if !ExplicitWriteGoMod { - modfetch.TrimGoSum(keep) + loaderstate.Fetcher().TrimGoSum(keep) // commitRequirements below will also call WriteGoSum, but the "keep" map // we have here could be strictly larger: commitRequirements only commits