Revert "internal/lsp: explicitly watch all known directories"
This reverts commit 3c3a81204b10d844dbacd6ca8ea5423628e00967.
Reason for revert: OpenBSD trybots are failing
Change-Id: I39f52ef07a667ed1457d2ae53863af15134eb78e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/270798
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Rebecca Stambler <rstambler@golang.org>
diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go
index cf74534..36bfc50 100644
--- a/internal/lsp/cache/session.go
+++ b/internal/lsp/cache/session.go
@@ -624,15 +624,3 @@
}
return overlays
}
-
-func (s *Session) FileWatchingGlobPatterns(ctx context.Context) map[string]struct{} {
- patterns := map[string]struct{}{}
- for _, view := range s.views {
- snapshot, release := view.getSnapshot(ctx)
- for k, v := range snapshot.fileWatchingGlobPatterns(ctx) {
- patterns[k] = v
- }
- release()
- }
- return patterns
-}
diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go
index 648fdbe..33266d7 100644
--- a/internal/lsp/cache/snapshot.go
+++ b/internal/lsp/cache/snapshot.go
@@ -604,77 +604,8 @@
return ids
}
-func (s *snapshot) fileWatchingGlobPatterns(ctx context.Context) map[string]struct{} {
- // Work-around microsoft/vscode#100870 by making sure that we are,
- // at least, watching the user's entire workspace. This will still be
- // applied to every folder in the workspace.
- patterns := map[string]struct{}{
- "**/*.{go,mod,sum}": {},
- }
- dirs := s.workspace.dirs(ctx, s)
- for _, dir := range dirs {
- dirName := dir.Filename()
-
- // If the directory is within the view's folder, we're already watching
- // it with the pattern above.
- if source.InDir(s.view.folder.Filename(), dirName) {
- continue
- }
- // TODO(rstambler): If microsoft/vscode#3025 is resolved before
- // microsoft/vscode#101042, we will need a work-around for Windows
- // drive letter casing.
- patterns[fmt.Sprintf("%s/**/*.{go,mod,sum}", dirName)] = struct{}{}
- }
-
- // Don't return results until the snapshot is loaded, otherwise it may not
- // yet "know" its files.
- if err := s.awaitLoaded(ctx); err != nil {
- return patterns
- }
-
- // Some clients do not send notifications for changes to directories that
- // contain Go code (golang/go#42348). To handle this, explicitly watch all
- // of the directories in the workspace. We find them by adding the
- // directories of every file in the snapshot's workspace directories.
- s.mu.Lock()
- defer s.mu.Unlock()
- seen := make(map[span.URI]struct{})
- for uri := range s.files {
- dir := filepath.Dir(uri.Filename())
- var matched bool
- for _, wsDir := range dirs {
- // Note: InDir handles symlinks, but InDirLex does not--it's too
- // expensive to call InDir on every file in the snapshot.
- if source.InDirLex(wsDir.Filename(), dir) {
- matched = true
- break
- }
- }
- // Don't watch any directory outside of the workspace directories.
- if !matched {
- continue
- }
- for {
- if dir == "" {
- break
- }
- uri := span.URIFromPath(dir)
- if _, ok := seen[uri]; ok {
- break
- }
- seen[uri] = struct{}{}
- dir = filepath.Dir(dir)
- }
- }
- var dirNames []string
- for uri := range seen {
- dirNames = append(dirNames, uri.Filename())
- }
- sort.Strings(dirNames)
- if len(dirNames) > 0 {
- patterns[fmt.Sprintf("{%s}", strings.Join(dirNames, ","))] = struct{}{}
- }
- return patterns
+func (s *snapshot) WorkspaceDirectories(ctx context.Context) []span.URI {
+ return s.workspace.dirs(ctx, s)
}
func (s *snapshot) WorkspacePackages(ctx context.Context) ([]source.Package, error) {
diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go
index bde908d..896e629 100644
--- a/internal/lsp/cache/view.go
+++ b/internal/lsp/cache/view.go
@@ -481,10 +481,6 @@
}
func (v *View) Snapshot(ctx context.Context) (source.Snapshot, func()) {
- return v.getSnapshot(ctx)
-}
-
-func (v *View) getSnapshot(ctx context.Context) (*snapshot, func()) {
v.snapshotMu.Lock()
defer v.snapshotMu.Unlock()
return v.snapshot, v.snapshot.generation.Acquire(ctx)
diff --git a/internal/lsp/general.go b/internal/lsp/general.go
index 60079b4..558767b 100644
--- a/internal/lsp/general.go
+++ b/internal/lsp/general.go
@@ -196,6 +196,7 @@
}()
}()
}
+ dirsToWatch := map[span.URI]struct{}{}
// Only one view gets to have a workspace.
assignedWorkspace := false
for _, folder := range folders {
@@ -234,6 +235,10 @@
work.end("Finished loading packages.")
}()
+ for _, dir := range snapshot.WorkspaceDirectories(ctx) {
+ dirsToWatch[dir] = struct{}{}
+ }
+
// Print each view's environment.
buf := &bytes.Buffer{}
if err := snapshot.WriteEnv(ctx, buf); err != nil {
@@ -251,12 +256,13 @@
wg.Done()
}()
}
-
// Register for file watching notifications, if they are supported.
- if err := s.updateWatchedDirectories(ctx); err != nil {
- event.Error(ctx, "failed to register for file watching notifications", err)
+ s.watchedDirectoriesMu.Lock()
+ err := s.registerWatchedDirectoriesLocked(ctx, dirsToWatch)
+ s.watchedDirectoriesMu.Unlock()
+ if err != nil {
+ return err
}
-
if len(viewErrors) > 0 {
errMsg := fmt.Sprintf("Error loading workspace folders (expected %v, got %v)\n", len(folders), len(s.session.Views())-originalViews)
for uri, err := range viewErrors {
@@ -274,14 +280,34 @@
// with the previously registered set of directories. If the set of directories
// has changed, we unregister and re-register for file watching notifications.
// updatedSnapshots is the set of snapshots that have been updated.
-func (s *Server) updateWatchedDirectories(ctx context.Context) error {
- patterns := s.session.FileWatchingGlobPatterns(ctx)
+func (s *Server) updateWatchedDirectories(ctx context.Context, updatedSnapshots map[source.View]source.Snapshot) error {
+ dirsToWatch := map[span.URI]struct{}{}
+ seenViews := map[source.View]struct{}{}
- s.watchedGlobPatternsMu.Lock()
- defer s.watchedGlobPatternsMu.Unlock()
+ // Collect all of the workspace directories from the updated snapshots.
+ for _, snapshot := range updatedSnapshots {
+ seenViews[snapshot.View()] = struct{}{}
+ for _, dir := range snapshot.WorkspaceDirectories(ctx) {
+ dirsToWatch[dir] = struct{}{}
+ }
+ }
+ // Not all views were necessarily updated, so check the remaining views.
+ for _, view := range s.session.Views() {
+ if _, ok := seenViews[view]; ok {
+ continue
+ }
+ snapshot, release := view.Snapshot(ctx)
+ for _, dir := range snapshot.WorkspaceDirectories(ctx) {
+ dirsToWatch[dir] = struct{}{}
+ }
+ release()
+ }
+
+ s.watchedDirectoriesMu.Lock()
+ defer s.watchedDirectoriesMu.Unlock()
// Nothing to do if the set of workspace directories is unchanged.
- if equalURISet(s.watchedGlobPatterns, patterns) {
+ if equalURISet(s.watchedDirectories, dirsToWatch) {
return nil
}
@@ -290,11 +316,11 @@
// period where no files are being watched. Still, if a user makes on-disk
// changes before these updates are complete, we may miss them for the new
// directories.
- prevID := s.watchRegistrationCount - 1
- if err := s.registerWatchedDirectoriesLocked(ctx, patterns); err != nil {
- return err
- }
- if prevID > 0 {
+ if s.watchRegistrationCount > 0 {
+ prevID := s.watchRegistrationCount - 1
+ if err := s.registerWatchedDirectoriesLocked(ctx, dirsToWatch); err != nil {
+ return err
+ }
return s.client.UnregisterCapability(ctx, &protocol.UnregistrationParams{
Unregisterations: []protocol.Unregistration{{
ID: watchedFilesCapabilityID(prevID),
@@ -309,7 +335,7 @@
return fmt.Sprintf("workspace/didChangeWatchedFiles-%d", id)
}
-func equalURISet(m1, m2 map[string]struct{}) bool {
+func equalURISet(m1, m2 map[span.URI]struct{}) bool {
if len(m1) != len(m2) {
return false
}
@@ -324,21 +350,44 @@
// registerWatchedDirectoriesLocked sends the workspace/didChangeWatchedFiles
// registrations to the client and updates s.watchedDirectories.
-func (s *Server) registerWatchedDirectoriesLocked(ctx context.Context, patterns map[string]struct{}) error {
+func (s *Server) registerWatchedDirectoriesLocked(ctx context.Context, dirs map[span.URI]struct{}) error {
if !s.session.Options().DynamicWatchedFilesSupported {
return nil
}
- for k := range s.watchedGlobPatterns {
- delete(s.watchedGlobPatterns, k)
+ for k := range s.watchedDirectories {
+ delete(s.watchedDirectories, k)
}
- var watchers []protocol.FileSystemWatcher
- for pattern := range patterns {
+ // Work-around microsoft/vscode#100870 by making sure that we are,
+ // at least, watching the user's entire workspace. This will still be
+ // applied to every folder in the workspace.
+ watchers := []protocol.FileSystemWatcher{{
+ GlobPattern: "**/*.{go,mod,sum}",
+ Kind: float64(protocol.WatchChange + protocol.WatchDelete + protocol.WatchCreate),
+ }}
+ for dir := range dirs {
+ filename := dir.Filename()
+
+ // If the directory is within a workspace folder, we're already
+ // watching it via the relative path above.
+ var matched bool
+ for _, view := range s.session.Views() {
+ if source.InDir(view.Folder().Filename(), filename) {
+ matched = true
+ break
+ }
+ }
+ if matched {
+ continue
+ }
+
+ // If microsoft/vscode#100870 is resolved before
+ // microsoft/vscode#104387, we will need a work-around for Windows
+ // drive letter casing.
watchers = append(watchers, protocol.FileSystemWatcher{
- GlobPattern: pattern,
+ GlobPattern: fmt.Sprintf("%s/**/*.{go,mod,sum}", filename),
Kind: float64(protocol.WatchChange + protocol.WatchDelete + protocol.WatchCreate),
})
}
-
if err := s.client.RegisterCapability(ctx, &protocol.RegistrationParams{
Registrations: []protocol.Registration{{
ID: watchedFilesCapabilityID(s.watchRegistrationCount),
@@ -352,8 +401,8 @@
}
s.watchRegistrationCount++
- for k, v := range patterns {
- s.watchedGlobPatterns[k] = v
+ for dir := range dirs {
+ s.watchedDirectories[dir] = struct{}{}
}
return nil
}
diff --git a/internal/lsp/server.go b/internal/lsp/server.go
index d4a6f0a..905a408 100644
--- a/internal/lsp/server.go
+++ b/internal/lsp/server.go
@@ -25,7 +25,7 @@
return &Server{
delivered: make(map[span.URI]sentDiagnostics),
gcOptimizationDetails: make(map[span.URI]struct{}),
- watchedGlobPatterns: make(map[string]struct{}),
+ watchedDirectories: make(map[span.URI]struct{}),
changedFiles: make(map[span.URI]struct{}),
session: session,
client: client,
@@ -79,11 +79,11 @@
// set of folders to build views for when we are ready
pendingFolders []protocol.WorkspaceFolder
- // watchedGlobPatterns is the set of glob patterns that we have requested
+ // watchedDirectories is the set of directories that we have requested that
// the client watch on disk. It will be updated as the set of directories
// that the server should watch changes.
- watchedGlobPatternsMu sync.Mutex
- watchedGlobPatterns map[string]struct{}
+ watchedDirectoriesMu sync.Mutex
+ watchedDirectories map[span.URI]struct{}
watchRegistrationCount uint64
// delivered is a cache of the diagnostics that the server has sent.
diff --git a/internal/lsp/source/util.go b/internal/lsp/source/util.go
index 1fffda5..72b074a 100644
--- a/internal/lsp/source/util.go
+++ b/internal/lsp/source/util.go
@@ -447,25 +447,25 @@
//
// Copied and slightly adjusted from go/src/cmd/go/internal/search/search.go.
func InDir(dir, path string) bool {
- if InDirLex(dir, path) {
+ if rel := inDirLex(path, dir); rel != "" {
return true
}
xpath, err := filepath.EvalSymlinks(path)
if err != nil || xpath == path {
xpath = ""
} else {
- if InDirLex(dir, xpath) {
+ if rel := inDirLex(xpath, dir); rel != "" {
return true
}
}
xdir, err := filepath.EvalSymlinks(dir)
if err == nil && xdir != dir {
- if InDirLex(xdir, path) {
+ if rel := inDirLex(path, xdir); rel != "" {
return true
}
if xpath != "" {
- if InDirLex(xdir, xpath) {
+ if rel := inDirLex(xpath, xdir); rel != "" {
return true
}
}
@@ -473,40 +473,43 @@
return false
}
-// InDirLex is like inDir but only checks the lexical form of the file names.
-// It does not consider symbolic links.
-//
// Copied from go/src/cmd/go/internal/search/search.go.
-func InDirLex(dir, path string) bool {
+//
+// inDirLex is like inDir but only checks the lexical form of the file names.
+// It does not consider symbolic links.
+// TODO(rsc): This is a copy of str.HasFilePathPrefix, modified to
+// return the suffix. Most uses of str.HasFilePathPrefix should probably
+// be calling InDir instead.
+func inDirLex(path, dir string) string {
pv := strings.ToUpper(filepath.VolumeName(path))
dv := strings.ToUpper(filepath.VolumeName(dir))
path = path[len(pv):]
dir = dir[len(dv):]
switch {
default:
- return false
+ return ""
case pv != dv:
- return false
+ return ""
case len(path) == len(dir):
if path == dir {
- return true
+ return "."
}
- return false
+ return ""
case dir == "":
- return path != ""
+ return path
case len(path) > len(dir):
if dir[len(dir)-1] == filepath.Separator {
if path[:len(dir)] == dir {
- return path[len(dir):] != ""
+ return path[len(dir):]
}
- return false
+ return ""
}
if path[len(dir)] == filepath.Separator && path[:len(dir)] == dir {
if len(path) == len(dir)+1 {
- return true
+ return "."
}
- return path[len(dir)+1:] != ""
+ return path[len(dir)+1:]
}
- return false
+ return ""
}
}
diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go
index 3ba82a2..fa307df 100644
--- a/internal/lsp/source/view.go
+++ b/internal/lsp/source/view.go
@@ -142,6 +142,10 @@
// WorkspacePackages returns the snapshot's top-level packages.
WorkspacePackages(ctx context.Context) ([]Package, error)
+
+ // WorkspaceDirectories returns any directory known by the view. For views
+ // within a module, this is the module root and any replace targets.
+ WorkspaceDirectories(ctx context.Context) []span.URI
}
// PackageFilter sets how a package is filtered out from a set of packages
@@ -297,11 +301,6 @@
// SetOptions sets the options of this session to new values.
SetOptions(*Options)
-
- // FileWatchingGlobPatterns returns glob patterns to watch every directory
- // known by the view. For views within a module, this is the module root,
- // any directory in the module root, and any replace targets.
- FileWatchingGlobPatterns(ctx context.Context) map[string]struct{}
}
// Overlay is the type for a file held in memory on a session.
diff --git a/internal/lsp/text_synchronization.go b/internal/lsp/text_synchronization.go
index 8839e88..fc35ff5 100644
--- a/internal/lsp/text_synchronization.go
+++ b/internal/lsp/text_synchronization.go
@@ -250,7 +250,7 @@
// After any file modifications, we need to update our watched files,
// in case something changed. Compute the new set of directories to watch,
// and if it differs from the current set, send updated registrations.
- if err := s.updateWatchedDirectories(ctx); err != nil {
+ if err := s.updateWatchedDirectories(ctx, snapshots); err != nil {
return err
}
return nil
