fs: Handle removed Linux watch paths#47462
fs: Handle removed Linux watch paths#47462iNViTiON wants to merge 3 commits intozed-industries:mainfrom
Conversation
|
We require contributors to sign our Contributor License Agreement, and we don't have @iNViTiON on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'. |
|
@cla-bot check |
|
The cla-bot has been summoned, and re-checked this pull request! |
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug in the Linux filesystem watcher where changes to directories that were removed and then recreated with the same name were not being detected. The issue occurs because inotify watches on Linux become invalid when a directory is deleted, and the watcher needs to be re-registered for a newly created directory with the same path.
Changes:
- Added tracking of removed watched paths on Linux using a HashSet
- Modified the add logic to detect when a path was previously removed and re-register the watcher
- Updated the remove event handler to mark paths as removed when they are deleted
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
There are already two pull requests that fix this same issue, each in a different way. I don't know what is the best way. The tests in my pull request also passed with these changes. I don't think this closes issue #46580 or #46709 or #38109, because there are multiple issues with file watching. For example, it doesn't handle rescan events and it doesn't work on filesystems that don't support notify events, see https://github.com/lilith/zed/tree/feature/pollwatcher-support. |
|
Thanks @AdamJedl. I missed these PRs! |
0eaf256 to
62225d6
Compare
Applied 5 PRs to fix Zed getting out of sync with filesystem changes: - zed-industries#48691: Include file size in DiskState to fix stale buffer reload - zed-industries#48695: Map EventKind::Other to Changed instead of None - zed-industries#48698: Reload after undo when file changed while dirty - zed-industries#48704: Rescan repositories when window regains focus - zed-industries#47462: Handle removed Linux watch paths
Track paths that were removed on Linux to work around inotify behavior. Add a removed_paths HashSet and mark a path when a Remove event contains the watched root. If a subsequent add finds the path already registered but marked removed, remove the old registration before re-adding. Initialize and clone removed_paths into the watcher callback.
62225d6 to
993bf8f
Compare
|
I just added the integration test that failed with the current code and pass with this fix. |
993bf8f to
8057f15
Compare
|
Sorry to nagging you a bit @probably-neb . But last week I found the CI test were running for one time, but after I fix the code, it's not running again. Do I have to trigger it somehow? |
|
The test failed for unused import on Windows and macOS. Fixed by doing conditional import only on Linux. cc @probably-neb |
|
Thanks @probably-neb . That's one look straighter forward than this one and I favor that more too! |
fs_watcherfailed to track change of removed watching path on Linux. Turns out that the removed directory broke the watcher by treating newly created same-named directory as different one.This PR track those watching but removed directory in order to re-register the watcher.
Closes #46580 #46709
Release Notes:
Stable release on the left | This PR build on the right
Project Panel after running issue reproducer script.
Git Panel after running issue reproducer script.
