Skip to content

Commit 5b300bd

Browse files
committed
gopls/internal/lsp/cache: clean up view workspace information
Tracking of workspace information in the View contained several inconsistencies and redundancies. Clean this up, with the following changes: - eliminate the View.rootURI, opting to derive it instead - eliminate the View.explicitGowork field, instead using the view.gowork field and checking if it is outside of the workspace. - eliminate many places where directory filters were interpreted relative to the rootURI. This is wrong: directory filters are expressed relative to the workspace folder. - remove special handling of GOMODCACHE, now that we're on Go 1.16+ - rewrite the locateTemplateFiles function to use view.filterFunc and filepath.WalkDir (now that we're on Go 1.16+). - don't request goimports env vars when loading environment variables for the view. They weren't being propagated to goimports anyway, and goimports will load them as needed. For golang/go#55331 Change-Id: I5e7f7e77e86d9ae425d2feaff31030278fed8240 Reviewed-on: https://go-review.googlesource.com/c/tools/+/459789 Reviewed-by: Alan Donovan <[email protected]> gopls-CI: kokoro <[email protected]> Run-TryBot: Robert Findley <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
1 parent 97d5de5 commit 5b300bd

File tree

7 files changed

+189
-198
lines changed

7 files changed

+189
-198
lines changed

gopls/internal/lsp/cache/imports.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ func (s *importsState) populateProcessEnv(ctx context.Context, snapshot *snapsho
138138
// and has led to memory leaks in the past, when the snapshot was
139139
// unintentionally held past its lifetime.
140140
_, inv, cleanupInvocation, err := snapshot.goCommandInvocation(ctx, source.LoadWorkspace, &gocommand.Invocation{
141-
WorkingDir: snapshot.view.rootURI.Filename(),
141+
WorkingDir: snapshot.view.workingDir().Filename(),
142142
})
143143
if err != nil {
144144
return nil, err
@@ -175,7 +175,7 @@ func (s *importsState) populateProcessEnv(ctx context.Context, snapshot *snapsho
175175
os.RemoveAll(tmpDir) // ignore error
176176
}
177177
} else {
178-
pe.WorkingDir = snapshot.view.rootURI.Filename()
178+
pe.WorkingDir = snapshot.view.workingDir().Filename()
179179
}
180180

181181
return cleanup, nil

gopls/internal/lsp/cache/load.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ func (s *snapshot) load(ctx context.Context, allowNetwork bool, scopes ...loadSc
113113
flags |= source.AllowNetwork
114114
}
115115
_, inv, cleanup, err := s.goCommandInvocation(ctx, flags, &gocommand.Invocation{
116-
WorkingDir: s.view.rootURI.Filename(),
116+
WorkingDir: s.view.workingDir().Filename(),
117117
})
118118
if err != nil {
119119
return err
@@ -152,7 +152,7 @@ func (s *snapshot) load(ctx context.Context, allowNetwork bool, scopes ...loadSc
152152
}
153153

154154
moduleErrs := make(map[string][]packages.Error) // module path -> errors
155-
filterer := buildFilterer(s.view.rootURI.Filename(), s.view.gomodcache, s.view.Options())
155+
filterFunc := s.view.filterFunc()
156156
newMetadata := make(map[PackageID]*source.Metadata)
157157
for _, pkg := range pkgs {
158158
// The Go command returns synthetic list results for module queries that
@@ -199,7 +199,8 @@ func (s *snapshot) load(ctx context.Context, allowNetwork bool, scopes ...loadSc
199199
//
200200
// TODO(rfindley): why exclude metadata arbitrarily here? It should be safe
201201
// to capture all metadata.
202-
if s.view.allFilesExcluded(pkg, filterer) {
202+
// TODO(rfindley): what about compiled go files?
203+
if allFilesExcluded(pkg.GoFiles, filterFunc) {
203204
continue
204205
}
205206
if err := buildMetadata(ctx, pkg, cfg, query, newMetadata, nil); err != nil {

gopls/internal/lsp/cache/session.go

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ package cache
77
import (
88
"context"
99
"fmt"
10-
"os"
1110
"strconv"
1211
"strings"
1312
"sync"
@@ -180,6 +179,8 @@ func (s *Session) NewView(ctx context.Context, name string, folder span.URI, opt
180179
return view, snapshot, release, nil
181180
}
182181

182+
// TODO(rfindley): clarify that createView can never be cancelled (with the
183+
// possible exception of server shutdown).
183184
func (s *Session) createView(ctx context.Context, name string, folder span.URI, options *source.Options, seqID uint64) (*View, *snapshot, func(), error) {
184185
index := atomic.AddInt64(&viewIndex, 1)
185186

@@ -188,15 +189,15 @@ func (s *Session) createView(ctx context.Context, name string, folder span.URI,
188189
// TODO(rfindley): this info isn't actually immutable. For example, GOWORK
189190
// could be changed, or a user's environment could be modified.
190191
// We need a mechanism to invalidate it.
191-
wsInfo, err := s.getWorkspaceInformation(ctx, folder, options)
192+
info, err := s.getWorkspaceInformation(ctx, folder, options)
192193
if err != nil {
193194
return nil, nil, func() {}, err
194195
}
195196

196197
root := folder
197198
// filterFunc is the path filter function for this workspace folder. Notably,
198199
// it is relative to folder (which is specified by the user), not root.
199-
filterFunc := pathExcludedByFilterFunc(folder.Filename(), wsInfo.gomodcache, options)
200+
filterFunc := pathExcludedByFilterFunc(folder.Filename(), info.gomodcache, options)
200201
rootSrc, err := findWorkspaceModuleSource(ctx, root, s, filterFunc, options.ExperimentalWorkspaceModule)
201202
if err != nil {
202203
return nil, nil, func() {}, err
@@ -205,14 +206,8 @@ func (s *Session) createView(ctx context.Context, name string, folder span.URI,
205206
root = span.Dir(rootSrc)
206207
}
207208

208-
explicitGowork := os.Getenv("GOWORK")
209-
if v, ok := options.Env["GOWORK"]; ok {
210-
explicitGowork = v
211-
}
212-
goworkURI := span.URIFromPath(explicitGowork)
213-
214209
// Build the gopls workspace, collecting active modules in the view.
215-
workspace, err := newWorkspace(ctx, root, goworkURI, s, filterFunc, wsInfo.effectiveGO111MODULE() == off, options.ExperimentalWorkspaceModule)
210+
workspace, err := newWorkspace(ctx, root, info.effectiveGOWORK(), s, filterFunc, info.effectiveGO111MODULE() == off, options.ExperimentalWorkspaceModule)
216211
if err != nil {
217212
return nil, nil, func() {}, err
218213
}
@@ -236,10 +231,8 @@ func (s *Session) createView(ctx context.Context, name string, folder span.URI,
236231
vulns: map[span.URI]*govulncheck.Result{},
237232
filesByURI: make(map[span.URI]span.URI),
238233
filesByBase: make(map[string][]canonicalURI),
239-
rootURI: root,
240234
rootSrc: rootSrc,
241-
explicitGowork: goworkURI,
242-
workspaceInformation: *wsInfo,
235+
workspaceInformation: info,
243236
}
244237
v.importsState = &importsState{
245238
ctx: backgroundCtx,
@@ -506,11 +499,12 @@ func (s *Session) DidModifyFiles(ctx context.Context, changes []source.FileModif
506499
// Re-create views whose root may have changed.
507500
//
508501
// checkRoots controls whether to re-evaluate view definitions when
509-
// collecting views below. Any change to a go.mod or go.work file may have
510-
// affected the definition of the view.
502+
// collecting views below. Any addition or deletion of a go.mod or go.work
503+
// file may have affected the definition of the view.
511504
checkRoots := false
512505
for _, c := range changes {
513506
if isGoMod(c.URI) || isGoWork(c.URI) {
507+
// TODO(rfindley): only consider additions or deletions here.
514508
checkRoots = true
515509
break
516510
}

gopls/internal/lsp/cache/snapshot.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,7 @@ func (s *snapshot) ValidBuildConfiguration() bool {
302302
// Check if the workspace is within any of them.
303303
// TODO(rfindley): this should probably be subject to "if GO111MODULES = off {...}".
304304
for _, gp := range filepath.SplitList(s.view.gopath) {
305-
if source.InDir(filepath.Join(gp, "src"), s.view.rootURI.Filename()) {
305+
if source.InDir(filepath.Join(gp, "src"), s.view.folder.Filename()) {
306306
return true
307307
}
308308
}
@@ -820,8 +820,10 @@ func (s *snapshot) fileWatchingGlobPatterns(ctx context.Context) map[string]stru
820820
fmt.Sprintf("**/*.{%s}", extensions): {},
821821
}
822822

823-
if s.view.explicitGowork != "" {
824-
patterns[s.view.explicitGowork.Filename()] = struct{}{}
823+
// If GOWORK is outside the folder, ensure we are watching it.
824+
gowork := s.view.effectiveGOWORK()
825+
if gowork != "" && !source.InDir(s.view.folder.Filename(), gowork.Filename()) {
826+
patterns[gowork.Filename()] = struct{}{}
825827
}
826828

827829
// Add a pattern for each Go module in the workspace that is not within the view.
@@ -2178,7 +2180,7 @@ func (s *snapshot) setBuiltin(path string) {
21782180
// BuildGoplsMod generates a go.mod file for all modules in the workspace. It
21792181
// bypasses any existing gopls.mod.
21802182
func (s *snapshot) BuildGoplsMod(ctx context.Context) (*modfile.File, error) {
2181-
allModules, err := findModules(s.view.folder, pathExcludedByFilterFunc(s.view.rootURI.Filename(), s.view.gomodcache, s.View().Options()), 0)
2183+
allModules, err := findModules(s.view.folder, pathExcludedByFilterFunc(s.view.folder.Filename(), s.view.gomodcache, s.View().Options()), 0)
21822184
if err != nil {
21832185
return nil, err
21842186
}

0 commit comments

Comments
 (0)