Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow glob patterns for native file watcher excludes #137872

Closed
ghuser opened this issue Nov 25, 2021 · 13 comments · Fixed by #169744
Closed

Allow glob patterns for native file watcher excludes #137872

ghuser opened this issue Nov 25, 2021 · 13 comments · Fixed by #169744
Assignees
Labels
feature-request Request for new features or functionality file-watcher File watcher insiders-released Patch has been released in VS Code Insiders on-testplan upstream Issue identified as 'upstream' component related (exists outside of VS Code) upstream-issue-linked This is an upstream issue that has been reported upstream
Milestone

Comments

@ghuser
Copy link

ghuser commented Nov 25, 2021

Update from bpasero

⚠️ VSCode SUPPORTS glob patterns as files.watcherExclude. You can use any glob pattern and file events will be ignored properly. ⚠️

This issue is about supporting these glob patterns on the level of the file watcher library natively. The only reason we want this is to help Linux users with the "out of handles" error when many file handles are used for file watching.

⚠️ ignore this issue if you are not using Linux or when you are not seeing the "out of handles error" ⚠️

Original issue from ghuser

Does this issue occur when all extensions are disabled?: Yes/No

  • VS Code Version: 1.62.3
  • OS Version: Ubuntu 18.04.6 LTS

(First of all a big acknowledgement on the great work done with watcher on linux!! It works great in most cases.)

One Issue: I have a multiroot workspace. One of the roots contains the following subdir ./node/node_modules (instead of ./node_modules). The existing exclusion pattern **/node_modules/*/** does not exclude the nested node_modules directory from being watched.

I have tried adding the following exclusion patterns:

  • **/*/node_modules/*/**
  • **/**/node_modules/*/**
  • */**/node_modules/*/**
  • */node_modules/*/**

but without luck.

What works though is: **/node/node_modules/*/**, but this implies that we know the exact subdirectory path inside which the node_modules directory will exist.

Steps to Reproduce:

  1. Make sure a multi-root workspace has been loaded to vscode before closing it. Also at least one of the root workspace directories should contain inside a node/node_modules directory.
  2. execute: strace -f -etrace=inotify_add_watch /path/to/code 2>&1 | grep node_modules

You will see paths containing node_modules being printed. e.g.

[pid  4634] inotify_add_watch(58, "/projects/myproject/node/node_modules/modulename", IN_MODIFY|IN_ATTRIB|IN_MOVED_FROM|IN_MOVED_TO|IN_CREATE|IN_DELETE|IN_DELETE_SELF|IN_MOVE_SELF|IN_ONLYDIR|IN_DONT_FOLLOW|IN_EXCL_UNLINK) = 3024
@bpasero
Copy link
Member

bpasero commented Nov 25, 2021

@ghuser the file watcher we are moving to does currently not yet support glob patterns for native excludes, so we have some code on our end to try best to convert glob patterns to absolute file paths for the watcher.

If you configure a relative path such as node/node_modules it should work better. Relative paths will automatically be joined with the workspace folder path.

@bpasero bpasero self-assigned this Nov 25, 2021
@bpasero bpasero added file-watcher File watcher info-needed Issue requires more information from poster labels Nov 25, 2021
@ghuser
Copy link
Author

ghuser commented Nov 27, 2021

Indeed node/node_modules works and it is simpler, though it does not seem to have any difference to **/node/node_modules/*/**. It still requires the user to know the full path to the excluded dir, even if it is relative.

To sum up, the issue reported here is: the lack of a way to exclude a directory that could exist anywhere in the path.
Imagine a maven project with multiple sub-modules (something very common) where we need to exclude the target/ directory from each sub-module. Currently this does not seem to be possible unless we manually enter each one of the sub-modules' target/ directory to that exclusion list.

@bpasero bpasero added feature-request Request for new features or functionality upstream Issue identified as 'upstream' component related (exists outside of VS Code) upstream-issue-linked This is an upstream issue that has been reported upstream and removed info-needed Issue requires more information from poster labels Nov 27, 2021
@bpasero bpasero removed their assignment Nov 27, 2021
@bpasero bpasero added this to the Backlog milestone Nov 27, 2021
@bpasero
Copy link
Member

bpasero commented Nov 27, 2021

Needs parcel-bundler/watcher#64

@bpasero bpasero changed the title File watcher: cannot exclude directory name via wildcards if it is not a direct child of a workspace root (multi-root workspace). Allow glob patterns for native file watcher excludes Nov 27, 2021
@unional
Copy link

unional commented Feb 11, 2022

FYI the default files.watcherExclude is still using glob pattern.

https://code.visualstudio.com/docs/getstarted/settings#_default-settings

  // Configure paths or glob patterns to exclude from file watching.
  "files.watcherExclude": {
    "**/.git/objects/**": true,
    "**/.git/subtree-cache/**": true,
    "**/node_modules/*/**": true,
    "**/.hg/store/**": true
  },

The tooltips is also still saying that.

Configure paths or glob patterns to exclude from file watching. Paths that are relative (for example build/output) will be resolved to an absolute path using the currently opened workspace. Glob patterns must match on absolute paths (i.e. prefix with **/ or the full path and suffix with /** to match files within a path) to match properly (for example **/build/output/** or /Users/name/workspaces/project/build/output/**). When you experience the file watcher process consuming a lot of CPU, make sure to exclude large folders that are of less interest (such as build output folders).

@unional
Copy link

unional commented Feb 11, 2022

Adding a use case for glob pattern here:

I'm working on a large repository that has code in C.

The build system builds the binary to a relative .bld folder.
There are hundreds of them.
So having a glob **/.bld/** is likely the only option.

@bpasero
Copy link
Member

bpasero commented Feb 12, 2022

We have a heuristic to convert "simple" glob patterns into absolute paths given that. See this test for what is supported:

@bpasero
Copy link
Member

bpasero commented Feb 12, 2022

Btw just to clarify: we do support glob patterns for watcher excludes. This issue is about supporting them on the native layer in the library itself (parcel watcher). Supporting them natively means you are not hitting the ENOSPC issue on Linux. But conceptually file changes are properly excluded even when you use glob patterns, just on a different layer.

@jaypea
Copy link

jaypea commented Mar 27, 2022

I had to downgrade and pin VSCode to version 1.62 on my Ubuntu system to keep using the monorepo i'm working with daily.
I also had to turn this setting on to keep using the older filewatcher.

"files.legacyWatcher": "on"

@bpasero
Copy link
Member

bpasero commented Mar 27, 2022

Just to clarify: the previous file watcher behaved the same as the current file watcher in that excludes were not supported natively. So for this issue (running out of file handles), it makes no difference whether you use old VSCode or new VSCode. But when you are able to configure non-glob patterns for excludes, the new VSCode will behave better.

Edit
Actually not entirely true, for as long as we did use chokidar, exclude patterns would work even when glob patterns are used and reduce the handle count on Linux.

@jaypea
Copy link

jaypea commented Mar 27, 2022

@bpasero I run in to the "out of handles" error as soon as I change to the parcel watcher. my system setting is 524288 already.
I've also tried non-glob patterns as far as its possible in my workspace without any luck.

@bpasero
Copy link
Member

bpasero commented Mar 27, 2022

Hm, does the workspace contain many folders with large structures or just a few? You would have to build absolute path patterns to exclude them unfortunately. I am open for suggestions how to make this any easier.

@ghuser
Copy link
Author

ghuser commented Mar 30, 2022

@jaypea If possible use the command described on top to find out whether you have missed some exclusion pattern and also be able to provide some detailed feedback to the team with specific examples on when a directory is not excluded.

For example the specific command helped me fix my problem because I found out that I had to also add **/node/node_modules/*/** in the exclusion list.

What is described on this issue is that it would be even better to be able to exclude a directory (e.g node_modules) anywhere in the path without the need to specify the relative path to it.

@bpasero bpasero linked a pull request Dec 21, 2022 that will close this issue
bpasero added a commit that referenced this issue Jan 6, 2023
* watcher - update settings description

* watcher - directly pass on exclude patterns

* more setting updates and exclude test

* 💄

* 💄
@bpasero bpasero self-assigned this Jan 6, 2023
@bpasero bpasero modified the milestones: Backlog, January 2023 Jan 6, 2023
@vscodenpa vscodenpa added the unreleased Patch has not yet been released in VS Code Insiders label Jan 6, 2023
@bpasero
Copy link
Member

bpasero commented Jan 6, 2023

Given parcel-bundler/watcher#106 landed and we updated to 2.1.0, glob patterns for files.watcherExclude setting are now supported all the way down to the file watcher. You can:

  • use relative paths: they are applied relative to the watch root
  • use absolute paths: they are applied absolute
  • use globs: they are applied relative to the watch root

Thanks @joaomoreno for the clever idea of using a node.js module for the heavy lifting of creating the glob regex and then plugging that into parcel watcher. This was a fun journey that even resulted in a StackOverflow question regarding RegEx on different platforms that remains unanswered and a mystery: https://stackoverflow.com/questions/74796098/c-regular-expression-with-negative-lookahead-and-matches-on-macos-but-not-l

@vscodenpa vscodenpa added insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Jan 9, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Feb 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality file-watcher File watcher insiders-released Patch has been released in VS Code Insiders on-testplan upstream Issue identified as 'upstream' component related (exists outside of VS Code) upstream-issue-linked This is an upstream issue that has been reported upstream
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants