fix(plugin-ext): skip plugin watches rooted at an ancestor of the workspace#17633
fix(plugin-ext): skip plugin watches rooted at an ancestor of the workspace#17633safisa wants to merge 4 commits into
Conversation
…kspace Theia's backend ignores the `recursive` flag and always watches recursively, so a non-recursive watch rooted at a strict ancestor of a workspace root - e.g. a language server (`redhat.java` / JDT-LS) watching the PARENT of the workspace folder via `RelativePattern(parentDir, folderName)` to detect deletion of the folder itself - is turned into a recursive crawl of every sibling subtree, which can exhaust the OS file-watch budget. `files.watcherExclude` cannot bound it because the watch root is outside the workspace. `MainFileSystemEventService.$watch` now skips such watches: no OS watch is registered. Explicit recursive requests and watches on or inside a workspace root are unchanged. Closes eclipse-theia#17632
colin-grant-work
left a comment
There was a problem hiding this comment.
This is an expedient fix, but I wonder whether we could, without too much difficulty, follow VSCode's example and add non-recursive watching as a feature, rather than dropping these watches?
| const isAncestorOfWorkspace = this.workspaceService.tryGetRoots().some( | ||
| root => uri.isEqualOrParent(root.resource) && !uri.isEqual(root.resource) | ||
| ); |
There was a problem hiding this comment.
In a multi-root workspace where one root is a parent of another, a watch of the outer folder would fail here.
There was a problem hiding this comment.
Good catch — fixed in 3cfc636. A folder that is itself a workspace root is now never skipped; the check only drops watches rooted strictly above every root. So in your nested multi-root example (/projects and /projects/my-app), a non-recursive watch on /projects is still registered. Added a regression test (still registers a non-recursive watch on an outer root that is itself the parent of another root).
# Conflicts: # CHANGELOG.md
…f another root In a multi-root workspace where one root is nested inside another (e.g. roots `/projects` and `/projects/my-app`), `shouldSkipWatch` dropped a non-recursive watch on the outer root: for the inner root the outer folder is a strict ancestor, so the `.some(...)` check matched and the watch was skipped even though the outer folder is itself a workspace root the user opened. Treat a folder that equals any workspace root as never-skippable, and only drop watches rooted strictly above every root. Adds a regression test for the nested multi-root case. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Re: adding non-recursive watching as a first-class feature instead of dropping these watches — I looked into the feasibility. The obstacle is the backend watcher: Theia's recursive watcher is So matching VSCode's approach would mean adding a separate |
What it does
Fixes #17632
The backend ignores the
recursiveflag and always watches recursively, so a non-recursive watch rooted outside the workspace becomes a full recursive crawl. JDT-LS (redhat.java) watches the parent of the workspace folder (sent asrecursive: false) only to detect its deletion — Theia then crawls every sibling of the workspace folder and can exhaustfs.inotify.max_user_watches;files.watcherExcludecan't help since the root is outside the workspace.MainFileSystemEventService.$watchnow skips a non-recursive watch rooted at a strict ancestor of a workspace root; recursive requests and on/inside-workspace watches are unchanged. The root-cause alternative (a real non-recursive watcher, à la VS Code) is larger — both options are in #17632.How to test
Linux, workspace alongside large siblings under a shared parent, with Red Hat Java (JDT-LS, non-
LightWeight): via the diagnostic on #17630 orinotify-consumers, the parent's sibling trees are no longer watched. Covered by unit tests inmain-file-system-event-service.spec.ts.Follow-ups
Root-cause non-recursive watcher tracked in #17632; trade-off: loses workspace-folder-self-deletion detection for the skipped watchers.
Breaking changes
Review checklist
nlsservice (for details, please see the Internationalization/Localization section in the Coding Guidelines)