worktree: Suppress bare .git events explained by filtered sibling events (#61636)
Fixes an infinite git-rescan loop on Windows introduced by #59876. On Windows, creating or deleting a file directly inside .git updates the directory's last-write time, so ReadDirectoryChangesW reports a bare .git Changed event alongside the file's own event. Since #59876, bare .git events schedule a git rescan (to cope with coalesced FSEvents on macOS), so every filtered-out lock-file event still triggered a rescan through its paired bare event. Because a rescan's own `git diff --numstat HEAD` can take .git/index.lock (even under --no-optional-locks, e.g. in jj-colocated repos whose index never refreshes clean), each rescan re-triggered the next one, spawning ~9 git processes per cycle, ~4 cycles/sec, indefinitely. Bare .git events are now deferred while processing an event batch and only trigger a rescan when the batch contains no filtered event for the same git dir that explains the directory change. A standalone bare .git event (the macOS coalescing case #59876 addressed) still triggers a rescan, as does any batch containing a meaningful .git change. Release Notes: - N/A or Added/Fixed/Improved ...
This commit is contained in:
1 parent
73be80748b
commit
fb813f70d6
2 files changed
+158
-9
No files matched your search
@@ -4773,6 +4773,18 @@ impl BackgroundScanner {
|
||||
let snapshot = &self.state.lock().await.snapshot;
|
||||
|
||||
let mut ranges_to_drop = SmallVec::<[Range<usize>; 4]>::new();
|
||||
// On Windows, creating or deleting a file directly inside `.git`
|
||||
// updates the directory's last-write time, so the watcher reports a
|
||||
// bare `.git` Changed event alongside the file's own event. When the
|
||||
// file event is filtered out (e.g. git's transient `index.lock`), the
|
||||
// bare event carries no information either, so acting on it would
|
||||
// turn every ignored lock file into a git rescan. Since a rescan's
|
||||
// own `git diff` can take `index.lock`, that feeds back into an
|
||||
// infinite loop of rescans. So bare `.git` events are deferred here
|
||||
// and only rescanned when nothing in the batch explains them; a
|
||||
// standalone bare event (macOS coalescing) still triggers a rescan.
|
||||
let mut bare_dot_git_abs_paths = Vec::new();
|
||||
let mut dot_git_dirs_with_ignored_events = Vec::new();
|
||||
|
||||
for (ix, event) in events.iter().enumerate() {
|
||||
let abs_path = SanitizedPath::new(&event.path);
|
||||
@@ -4816,6 +4828,21 @@ impl BackgroundScanner {
|
||||
log::debug!(
|
||||
"ignoring event {abs_path:?} as it's in the .git directory among skipped files or directories"
|
||||
);
|
||||
if !dot_git_dirs_with_ignored_events.contains(&dot_git_abs_path) {
|
||||
dot_git_dirs_with_ignored_events.push(dot_git_abs_path);
|
||||
}
|
||||
skip_ix(&mut ranges_to_drop, ix);
|
||||
continue;
|
||||
}
|
||||
|
||||
if is_dot_git {
|
||||
log::debug!(
|
||||
"ignoring event {abs_path:?} for .git directory itself (kind: {:?})",
|
||||
event.kind
|
||||
);
|
||||
if !bare_dot_git_abs_paths.contains(&dot_git_abs_path) {
|
||||
bare_dot_git_abs_paths.push(dot_git_abs_path);
|
||||
}
|
||||
skip_ix(&mut ranges_to_drop, ix);
|
||||
continue;
|
||||
}
|
||||
@@ -4827,15 +4854,6 @@ impl BackgroundScanner {
|
||||
dot_git_abs_paths.push(dot_git_abs_path);
|
||||
}
|
||||
|
||||
if is_dot_git {
|
||||
log::debug!(
|
||||
"ignoring event {abs_path:?} for .git directory itself (kind: {:?})",
|
||||
event.kind
|
||||
);
|
||||
skip_ix(&mut ranges_to_drop, ix);
|
||||
continue;
|
||||
}
|
||||
|
||||
// New directories can appear under the `refs` tree at any time, e.g. when a
|
||||
// remote is added or a branch name contains slashes. On platforms where the
|
||||
// native watcher is non-recursive they need their own watches, or subsequent
|
||||
@@ -4894,6 +4912,19 @@ impl BackgroundScanner {
|
||||
}
|
||||
}
|
||||
|
||||
for dot_git_abs_path in bare_dot_git_abs_paths {
|
||||
if dot_git_dirs_with_ignored_events.contains(&dot_git_abs_path) {
|
||||
log::debug!(
|
||||
"not reloading git repo at {dot_git_abs_path:?}: the bare .git event is explained by filtered events in the same batch"
|
||||
);
|
||||
} else if !dot_git_abs_paths.contains(&dot_git_abs_path) {
|
||||
log::debug!(
|
||||
"detected update within git repo at {dot_git_abs_path:?}: bare .git directory event"
|
||||
);
|
||||
dot_git_abs_paths.push(dot_git_abs_path);
|
||||
}
|
||||
}
|
||||
|
||||
for range_to_drop in ranges_to_drop.into_iter().rev() {
|
||||
events.drain(range_to_drop);
|
||||
}
|
||||
|
||||
@@ -5026,6 +5026,124 @@ async fn test_dot_git_dir_event_does_not_suppress_children(
|
||||
}
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_dot_git_event_explained_by_filtered_sibling_does_not_emit_git_repo_update(
|
||||
executor: BackgroundExecutor,
|
||||
cx: &mut TestAppContext,
|
||||
) {
|
||||
// On Windows, creating or deleting a file directly inside .git (such as
|
||||
// git's transient index.lock) updates the directory's last-write time, so
|
||||
// ReadDirectoryChangesW reports a Changed event for the .git directory
|
||||
// itself alongside the event for the file. Bare .git events schedule a git
|
||||
// rescan (to cope with coalesced events on macOS), but when the same batch
|
||||
// contains a filtered-out event that explains the directory change, acting
|
||||
// on the bare event turns every ignored lock file into a rescan. Since
|
||||
// Zed's own rescans take .git/index.lock via `git diff`, that feeds back
|
||||
// into an infinite loop of git scans.
|
||||
init_test(cx);
|
||||
|
||||
let fs = FakeFs::new(executor.clone());
|
||||
let project_dir = Path::new(path!("/project"));
|
||||
fs.insert_tree(
|
||||
project_dir,
|
||||
json!({
|
||||
".git": {},
|
||||
"src": {
|
||||
"main.rs": "fn main() {}",
|
||||
},
|
||||
}),
|
||||
)
|
||||
.await;
|
||||
|
||||
let worktree = Worktree::local(
|
||||
project_dir,
|
||||
true,
|
||||
fs.clone(),
|
||||
Default::default(),
|
||||
true,
|
||||
WorktreeId::from_proto(0),
|
||||
&mut cx.to_async(),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
worktree
|
||||
.update(cx, |worktree, _| {
|
||||
worktree.as_local().unwrap().scan_complete()
|
||||
})
|
||||
.await;
|
||||
cx.run_until_parked();
|
||||
|
||||
let dot_git = project_dir.join(DOT_GIT);
|
||||
|
||||
// The exact batch Windows delivers when git creates .git/index.lock.
|
||||
{
|
||||
let mut events = cx.events(&worktree);
|
||||
fs.pause_events();
|
||||
fs.emit_fs_event(dot_git.clone(), Some(PathEventKind::Changed));
|
||||
fs.emit_fs_event(dot_git.join("index.lock"), Some(PathEventKind::Created));
|
||||
fs.unpause_events_and_flush();
|
||||
executor.run_until_parked();
|
||||
|
||||
let got_git_update = drain_git_repo_updates(&mut events);
|
||||
assert!(
|
||||
!got_git_update,
|
||||
"a bare .git event accompanied only by a filtered index.lock event \
|
||||
should NOT emit UpdatedGitRepositories"
|
||||
);
|
||||
}
|
||||
|
||||
// Same batch with the events in the opposite order.
|
||||
{
|
||||
let mut events = cx.events(&worktree);
|
||||
fs.pause_events();
|
||||
fs.emit_fs_event(dot_git.join("index.lock"), Some(PathEventKind::Removed));
|
||||
fs.emit_fs_event(dot_git.clone(), Some(PathEventKind::Changed));
|
||||
fs.unpause_events_and_flush();
|
||||
executor.run_until_parked();
|
||||
|
||||
let got_git_update = drain_git_repo_updates(&mut events);
|
||||
assert!(
|
||||
!got_git_update,
|
||||
"event order within the batch should not matter for suppressing \
|
||||
the bare .git event"
|
||||
);
|
||||
}
|
||||
|
||||
// A meaningful change in the same batch must still trigger a rescan.
|
||||
{
|
||||
let mut events = cx.events(&worktree);
|
||||
fs.pause_events();
|
||||
fs.emit_fs_event(dot_git.clone(), Some(PathEventKind::Changed));
|
||||
fs.emit_fs_event(dot_git.join("index.lock"), Some(PathEventKind::Created));
|
||||
fs.emit_fs_event(dot_git.join("HEAD"), Some(PathEventKind::Changed));
|
||||
fs.unpause_events_and_flush();
|
||||
executor.run_until_parked();
|
||||
|
||||
let got_git_update = drain_git_repo_updates(&mut events);
|
||||
assert!(
|
||||
got_git_update,
|
||||
"a meaningful .git change in the same batch as a filtered event \
|
||||
should still emit UpdatedGitRepositories"
|
||||
);
|
||||
}
|
||||
|
||||
// A standalone bare .git event (macOS event coalescing) must still
|
||||
// trigger a rescan.
|
||||
{
|
||||
let mut events = cx.events(&worktree);
|
||||
fs.pause_events();
|
||||
fs.emit_fs_event(dot_git, Some(PathEventKind::Changed));
|
||||
fs.unpause_events_and_flush();
|
||||
executor.run_until_parked();
|
||||
|
||||
let got_git_update = drain_git_repo_updates(&mut events);
|
||||
assert!(
|
||||
got_git_update,
|
||||
"a standalone bare .git event should still emit UpdatedGitRepositories"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_ref_updates_in_dot_git_subdirectories_are_detected(cx: &mut TestAppContext) {
|
||||
// On Linux and FreeBSD the native file watcher is non-recursive: watching `.git`
|
||||
|
||||
Reference in new issue
Block a user