From 0c80ba376b31d193c153ed6951197c4c67406443 Mon Sep 17 00:00:00 2001 From: Rob Findley Date: Tue, 30 Jan 2024 16:52:21 -0500 Subject: [PATCH] internal/imports: remove the unused ProcessEnv.ModFile field Remove the ProcessEnv.ModFile field, which was never set. Also add additional comments and superficial cleanup. Change-Id: I45d63e5039043d01d4280d2d916df6794e437d8a Reviewed-on: https://go-review.googlesource.com/c/tools/+/559506 Reviewed-by: Alan Donovan LUCI-TryBot-Result: Go LUCI Auto-Submit: Robert Findley --- gopls/internal/cache/session.go | 8 +++++--- internal/imports/fix.go | 11 ++++++++++- internal/imports/mod.go | 3 +-- internal/imports/mod_test.go | 6 +++++- 4 files changed, 21 insertions(+), 7 deletions(-) diff --git a/gopls/internal/cache/session.go b/gopls/internal/cache/session.go index 17c7514664d..82474789f7c 100644 --- a/gopls/internal/cache/session.go +++ b/gopls/internal/cache/session.go @@ -201,9 +201,11 @@ func (s *Session) createView(ctx context.Context, def *viewDefinition) (*View, * } } pe = &imports.ProcessEnv{ - GocmdRunner: s.gocmdRunner, - BuildFlags: slices.Clone(def.folder.Options.BuildFlags), - ModFlag: "readonly", // processEnv operations should not mutate the modfile + GocmdRunner: s.gocmdRunner, + BuildFlags: slices.Clone(def.folder.Options.BuildFlags), + // TODO(rfindley): an old comment said "processEnv operations should not mutate the modfile" + // But shouldn't we honor the default behavior of mod vendoring? + ModFlag: "readonly", SkipPathInScan: skipPath, Env: env, WorkingDir: def.root.Path(), diff --git a/internal/imports/fix.go b/internal/imports/fix.go index c12d42368dd..5cf1aff3cb3 100644 --- a/internal/imports/fix.go +++ b/internal/imports/fix.go @@ -852,7 +852,6 @@ type ProcessEnv struct { BuildFlags []string ModFlag string - ModFile string // SkipPathInScan returns true if the path should be skipped from scans of // the RootCurrentModule root type. The function argument is a clean, @@ -960,6 +959,12 @@ func (e *ProcessEnv) GetResolver() (Resolver, error) { if err := e.init(); err != nil { return nil, err } + // TODO(rfindley): we should only use a gopathResolver here if the working + // directory is actually *in* GOPATH. (I seem to recall an open gopls issue + // for this behavior, but I can't find it). + // + // For gopls, we can optionally explicitly choose a resolver type, since we + // already know the view type. if len(e.Env["GOMOD"]) == 0 && len(e.Env["GOWORK"]) == 0 { e.resolver = newGopathResolver(e) return e.resolver, nil @@ -968,6 +973,10 @@ func (e *ProcessEnv) GetResolver() (Resolver, error) { return e.resolver, nil } +// buildContext returns the build.Context to use for matching files. +// +// TODO(rfindley): support dynamic GOOS, GOARCH here, when doing cross-platform +// development. func (e *ProcessEnv) buildContext() (*build.Context, error) { ctx := build.Default goenv, err := e.goEnv() diff --git a/internal/imports/mod.go b/internal/imports/mod.go index 013616609f9..2287d4a8f5e 100644 --- a/internal/imports/mod.go +++ b/internal/imports/mod.go @@ -34,7 +34,7 @@ import ( // - Split scanning the module cache from other ModuleResolver functionality, // as it is the source of performance woes (and inconsistency). // - Allow sharing module cache state across multiple ModuleResolvers. -// - Optimize the scan itself, as there is a lot of redundancy statting and +// - Optimize the scan itself, as there is some redundancy statting and // reading go.mod files. // - Make it possible to reuse the current state while running a refresh in // the background. @@ -109,7 +109,6 @@ func (r *ModuleResolver) init() error { inv := gocommand.Invocation{ BuildFlags: r.env.BuildFlags, ModFlag: r.env.ModFlag, - ModFile: r.env.ModFile, Env: r.env.env(), Logf: r.env.Logf, WorkingDir: r.env.WorkingDir, diff --git a/internal/imports/mod_test.go b/internal/imports/mod_test.go index 3a4cd048aa8..98aa678f1bb 100644 --- a/internal/imports/mod_test.go +++ b/internal/imports/mod_test.go @@ -17,6 +17,7 @@ import ( "strings" "sync" "testing" + "time" "golang.org/x/mod/module" "golang.org/x/tools/internal/gocommand" @@ -1291,14 +1292,17 @@ import ( func BenchmarkScanModCache(b *testing.B) { env := &ProcessEnv{ GocmdRunner: &gocommand.Runner{}, - Logf: b.Logf, + // Uncomment for verbose logging (too verbose to enable by default). + // Logf: b.Logf, } exclude := []gopathwalk.RootType{gopathwalk.RootGOROOT} resolver, err := env.GetResolver() if err != nil { b.Fatal(err) } + start := time.Now() scanToSlice(resolver, exclude) + b.Logf("warming the mod cache took %v", time.Since(start)) b.ResetTimer() for i := 0; i < b.N; i++ { scanToSlice(resolver, exclude)