From 354bea8ca86859af66145c8ccba3d5722586cdaf Mon Sep 17 00:00:00 2001 From: Heschi Kreinick Date: Fri, 17 Jan 2020 15:33:57 -0500 Subject: [PATCH] internal/lsp/cache: let gopls track go.mod files We used to read the go.mod file information out of the imports.Resolver. Now that gopls tracks go.mod itself, we can use that instead. This is a slight regression, in that go.mods in replace targets will no longer be watched, but I don't think that's too important. This allows us to stop reading the ModuleResolver's internals, which were not sufficiently locked. Updates golang/go#36605. Change-Id: I42939e0248cba1f6b3850a003de67fcc11ab10b1 Reviewed-on: https://go-review.googlesource.com/c/tools/+/215319 Run-TryBot: Heschi Kreinick TryBot-Result: Gobot Gobot Reviewed-by: Rebecca Stambler --- internal/imports/mod.go | 46 ++++++++++++++++++------------------ internal/lsp/cache/view.go | 48 ++++++-------------------------------- 2 files changed, 30 insertions(+), 64 deletions(-) diff --git a/internal/imports/mod.go b/internal/imports/mod.go index 553f216450..3ae859ed23 100644 --- a/internal/imports/mod.go +++ b/internal/imports/mod.go @@ -29,10 +29,10 @@ type ModuleResolver struct { scanSema chan struct{} // scanSema prevents concurrent scans and guards scannedRoots. scannedRoots map[gopathwalk.Root]bool - Initialized bool - Main *ModuleJSON - ModsByModPath []*ModuleJSON // All modules, ordered by # of path components in module Path... - ModsByDir []*ModuleJSON // ...or Dir. + initialized bool + main *ModuleJSON + modsByModPath []*ModuleJSON // All modules, ordered by # of path components in module Path... + modsByDir []*ModuleJSON // ...or Dir. // moduleCacheCache stores information about the module cache. moduleCacheCache *dirInfoCache @@ -59,7 +59,7 @@ func newModuleResolver(e *ProcessEnv) *ModuleResolver { } func (r *ModuleResolver) init() error { - if r.Initialized { + if r.initialized { return nil } mainMod, vendorEnabled, err := vendorEnabled(r.env) @@ -70,13 +70,13 @@ func (r *ModuleResolver) init() error { if mainMod != nil && vendorEnabled { // Vendor mode is on, so all the non-Main modules are irrelevant, // and we need to search /vendor for everything. - r.Main = mainMod + r.main = mainMod r.dummyVendorMod = &ModuleJSON{ Path: "", Dir: filepath.Join(mainMod.Dir, "vendor"), } - r.ModsByModPath = []*ModuleJSON{mainMod, r.dummyVendorMod} - r.ModsByDir = []*ModuleJSON{mainMod, r.dummyVendorMod} + r.modsByModPath = []*ModuleJSON{mainMod, r.dummyVendorMod} + r.modsByDir = []*ModuleJSON{mainMod, r.dummyVendorMod} } else { // Vendor mode is off, so run go list -m ... to find everything. r.initAllMods() @@ -84,15 +84,15 @@ func (r *ModuleResolver) init() error { r.moduleCacheDir = filepath.Join(filepath.SplitList(r.env.GOPATH)[0], "/pkg/mod") - sort.Slice(r.ModsByModPath, func(i, j int) bool { + sort.Slice(r.modsByModPath, func(i, j int) bool { count := func(x int) int { - return strings.Count(r.ModsByModPath[x].Path, "/") + return strings.Count(r.modsByModPath[x].Path, "/") } return count(j) < count(i) // descending order }) - sort.Slice(r.ModsByDir, func(i, j int) bool { + sort.Slice(r.modsByDir, func(i, j int) bool { count := func(x int) int { - return strings.Count(r.ModsByDir[x].Dir, "/") + return strings.Count(r.modsByDir[x].Dir, "/") } return count(j) < count(i) // descending order }) @@ -100,8 +100,8 @@ func (r *ModuleResolver) init() error { r.roots = []gopathwalk.Root{ {filepath.Join(r.env.GOROOT, "/src"), gopathwalk.RootGOROOT}, } - if r.Main != nil { - r.roots = append(r.roots, gopathwalk.Root{r.Main.Dir, gopathwalk.RootCurrentModule}) + if r.main != nil { + r.roots = append(r.roots, gopathwalk.Root{r.main.Dir, gopathwalk.RootCurrentModule}) } if vendorEnabled { r.roots = append(r.roots, gopathwalk.Root{r.dummyVendorMod.Dir, gopathwalk.RootOther}) @@ -115,12 +115,12 @@ func (r *ModuleResolver) init() error { } } // Walk dependent modules before scanning the full mod cache, direct deps first. - for _, mod := range r.ModsByModPath { + for _, mod := range r.modsByModPath { if !mod.Indirect && !mod.Main { addDep(mod) } } - for _, mod := range r.ModsByModPath { + for _, mod := range r.modsByModPath { if mod.Indirect && !mod.Main { addDep(mod) } @@ -141,7 +141,7 @@ func (r *ModuleResolver) init() error { listeners: map[*int]cacheListener{}, } } - r.Initialized = true + r.initialized = true return nil } @@ -162,10 +162,10 @@ func (r *ModuleResolver) initAllMods() error { // Can't do anything with a module that's not downloaded. continue } - r.ModsByModPath = append(r.ModsByModPath, mod) - r.ModsByDir = append(r.ModsByDir, mod) + r.modsByModPath = append(r.modsByModPath, mod) + r.modsByDir = append(r.modsByDir, mod) if mod.Main { - r.Main = mod + r.main = mod } } return nil @@ -198,7 +198,7 @@ func (r *ModuleResolver) ClearForNewMod() { func (r *ModuleResolver) findPackage(importPath string) (*ModuleJSON, string) { // This can't find packages in the stdlib, but that's harmless for all // the existing code paths. - for _, m := range r.ModsByModPath { + for _, m := range r.modsByModPath { if !strings.HasPrefix(importPath, m.Path) { continue } @@ -292,7 +292,7 @@ func (r *ModuleResolver) findModuleByDir(dir string) *ModuleJSON { // - in /vendor/ in -mod=vendor mode. // - nested module? Dunno. // Rumor has it that replace targets cannot contain other replace targets. - for _, m := range r.ModsByDir { + for _, m := range r.modsByDir { if !strings.HasPrefix(dir, m.Dir) { continue } @@ -570,7 +570,7 @@ func (r *ModuleResolver) scanDirForPackage(root gopathwalk.Root, dir string) dir } switch root.Type { case gopathwalk.RootCurrentModule: - importPath = path.Join(r.Main.Path, filepath.ToSlash(subdir)) + importPath = path.Join(r.main.Path, filepath.ToSlash(subdir)) case gopathwalk.RootModuleCache: matches := modCacheRegexp.FindStringSubmatch(subdir) if len(matches) == 0 { diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index 6f2610672f..88a693435b 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -69,11 +69,7 @@ type view struct { processEnv *imports.ProcessEnv cacheRefreshDuration time.Duration cacheRefreshTimer *time.Timer - - // modFileVersions stores the last seen versions of the module files that are used - // by processEnvs resolver. - // TODO(suzmue): These versions may not actually be on disk. - modFileVersions map[string]string + cachedModFileVersion source.FileIdentity // keep track of files by uri and by basename, a single file may be mapped // to multiple uris, and the same basename may map to multiple files @@ -287,9 +283,12 @@ func (v *view) RunProcessEnvFunc(ctx context.Context, fn func(*imports.Options) } } - // Before running the user provided function, clear caches in the resolver. - if v.modFilesChanged() { - v.processEnv.GetResolver().(*imports.ModuleResolver).ClearForNewMod() + // In module mode, check if the mod file has changed. + if mod, _, err := v.Snapshot().ModFiles(ctx); err == nil && mod != nil { + if mod.Identity() != v.cachedModFileVersion { + v.processEnv.GetResolver().(*imports.ModuleResolver).ClearForNewMod() + } + v.cachedModFileVersion = mod.Identity() } // Run the user function. @@ -308,10 +307,6 @@ func (v *view) RunProcessEnvFunc(ctx context.Context, fn func(*imports.Options) return err } - // If applicable, store the file versions of the 'go.mod' files that are - // looked at by the resolver. - v.storeModFileVersions() - if v.cacheRefreshTimer == nil { // Don't refresh more than twice per minute. delay := 30 * time.Second @@ -377,35 +372,6 @@ func (v *view) buildProcessEnv(ctx context.Context) (*imports.ProcessEnv, error) return env, nil } -func (v *view) modFilesChanged() bool { - // Check the versions of the 'go.mod' files of the main module - // and modules included by a replace directive. Return true if - // any of these file versions do not match. - for filename, version := range v.modFileVersions { - if version != v.fileVersion(filename) { - return true - } - } - return false -} - -func (v *view) storeModFileVersions() { - // Store the mod files versions, if we are using a ModuleResolver. - r, moduleMode := v.processEnv.GetResolver().(*imports.ModuleResolver) - if !moduleMode || !r.Initialized { - return - } - v.modFileVersions = make(map[string]string) - - // Get the file versions of the 'go.mod' files of the main module - // and modules included by a replace directive in the resolver. - for _, mod := range r.ModsByModPath { - if (mod.Main || mod.Replace != nil) && mod.GoMod != "" { - v.modFileVersions[mod.GoMod] = v.fileVersion(mod.GoMod) - } - } -} - func (v *view) fileVersion(filename string) string { uri := span.FileURI(filename) fh := v.session.GetFile(uri)