Do less git metadata rescans on FS events (#24034)

Kirill Bulatov created 11 months ago

A preparation for collaborative commit message editing.

Before, almost any `.git`-contained file FS update, except
`.git/fsmonitor--daemon/cookies/**` caused git metadata rescan.
This included `index.lock` that was created after any git operation,
e.g. `git status`, which was unnecessary.
Collaborative editing aims to share `.git/COMMIT_EDITMSG` between
multiple users, so there are potentially multiple users editing the file
and causing excessive events.

The change makes worktree to ignore .git/COMMIT_EDITMSG`,
`.git/index.lock` and `.git/fsmonitor--daemon/**` paths and adjusts the
logic to be more extensible: there's much more files Zed can ignore and
still have its git metadata up to date.

Release Notes:

- N/A

Change summary

crates/git/src/git.rs                |  6 ++-
crates/project/src/worktree_store.rs |  1 
crates/worktree/src/worktree.rs      | 50 +++++++++++++----------------
3 files changed, 27 insertions(+), 30 deletions(-)

Detailed changes

crates/git/src/git.rs

@@ -20,10 +20,12 @@ pub use git2 as libgit;
 pub use repository::WORK_DIRECTORY_REPO_PATH;
 
 pub static DOT_GIT: LazyLock<&'static OsStr> = LazyLock::new(|| OsStr::new(".git"));
-pub static COOKIES: LazyLock<&'static OsStr> = LazyLock::new(|| OsStr::new("cookies"));
+pub static GITIGNORE: LazyLock<&'static OsStr> = LazyLock::new(|| OsStr::new(".gitignore"));
 pub static FSMONITOR_DAEMON: LazyLock<&'static OsStr> =
     LazyLock::new(|| OsStr::new("fsmonitor--daemon"));
-pub static GITIGNORE: LazyLock<&'static OsStr> = LazyLock::new(|| OsStr::new(".gitignore"));
+pub static COMMIT_MESSAGE: LazyLock<&'static OsStr> =
+    LazyLock::new(|| OsStr::new("COMMIT_EDITMSG"));
+pub static INDEX_LOCK: LazyLock<&'static OsStr> = LazyLock::new(|| OsStr::new("index.lock"));
 
 actions!(
     git,

crates/project/src/worktree_store.rs

@@ -56,6 +56,7 @@ pub struct WorktreeStore {
     state: WorktreeStoreState,
 }
 
+#[derive(Debug)]
 pub enum WorktreeStoreEvent {
     WorktreeAdded(Entity<Worktree>),
     WorktreeRemoved(EntityId, WorktreeId),

crates/worktree/src/worktree.rs

@@ -23,7 +23,7 @@ use git::{
     status::{
         FileStatus, GitSummary, StatusCode, TrackedStatus, UnmergedStatus, UnmergedStatusCode,
     },
-    GitHostingProviderRegistry, COOKIES, DOT_GIT, FSMONITOR_DAEMON, GITIGNORE,
+    GitHostingProviderRegistry, COMMIT_MESSAGE, DOT_GIT, FSMONITOR_DAEMON, GITIGNORE, INDEX_LOCK,
 };
 use gpui::{
     App, AppContext as _, AsyncApp, BackgroundExecutor, Context, Entity, EventEmitter, Task,
@@ -4317,47 +4317,41 @@ impl BackgroundScanner {
             }
         };
 
+        // Certain directories may have FS changes, but do not lead to git data changes that Zed cares about.
+        // Ignore these, to avoid Zed unnecessarily rescanning git metadata.
+        let skipped_files_in_dot_git = HashSet::from_iter([*COMMIT_MESSAGE, *INDEX_LOCK]);
+        let skipped_dirs_in_dot_git = [*FSMONITOR_DAEMON];
+
         let mut relative_paths = Vec::with_capacity(abs_paths.len());
         let mut dot_git_abs_paths = Vec::new();
         abs_paths.sort_unstable();
         abs_paths.dedup_by(|a, b| a.starts_with(b));
         abs_paths.retain(|abs_path| {
             let abs_path = SanitizedPath::from(abs_path);
+
             let snapshot = &self.state.lock().snapshot;
             {
                 let mut is_git_related = false;
 
-                // We don't want to trigger .git rescan for events within .git/fsmonitor--daemon/cookies directory.
-                #[derive(PartialEq)]
-                enum FsMonitorParseState {
-                    Cookies,
-                    FsMonitor
-                }
-                let mut fsmonitor_parse_state = None;
-                if let Some(dot_git_abs_path) = abs_path.as_path()
-                    .ancestors()
-                    .find(|ancestor| {
-                        let file_name = ancestor.file_name();
-                        if file_name == Some(*COOKIES) {
-                            fsmonitor_parse_state = Some(FsMonitorParseState::Cookies);
-                            false
-                        } else if fsmonitor_parse_state == Some(FsMonitorParseState::Cookies) && file_name == Some(*FSMONITOR_DAEMON) {
-                            fsmonitor_parse_state = Some(FsMonitorParseState::FsMonitor);
-                            false
-                        } else if fsmonitor_parse_state != Some(FsMonitorParseState::FsMonitor) && smol::block_on(is_git_dir(ancestor, self.fs.as_ref())) {
-                            true
-                        } else {
-                            fsmonitor_parse_state.take();
-                            false
-                        }
+                let dot_git_paths = abs_path.as_path().ancestors().find_map(|ancestor| {
+                    if smol::block_on(is_git_dir(ancestor, self.fs.as_ref())) {
+                        let path_in_git_dir = abs_path.as_path().strip_prefix(ancestor).expect("stripping off the ancestor");
+                        Some((ancestor.to_owned(), path_in_git_dir.to_owned()))
+                    } else {
+                        None
+                    }
+                });
 
-                    })
-                {
-                    let dot_git_abs_path = dot_git_abs_path.to_path_buf();
+                if let Some((dot_git_abs_path, path_in_git_dir)) = dot_git_paths {
+                    if skipped_files_in_dot_git.contains(path_in_git_dir.as_os_str()) || skipped_dirs_in_dot_git.iter().any(|skipped_git_subdir| path_in_git_dir.starts_with(skipped_git_subdir)) {
+                        log::debug!("ignoring event {abs_path:?} as it's in the .git directory among skipped files or directories");
+                        return false;
+                    }
+
+                    is_git_related = true;
                     if !dot_git_abs_paths.contains(&dot_git_abs_path) {
                         dot_git_abs_paths.push(dot_git_abs_path);
                     }
-                    is_git_related = true;
                 }
 
                 let relative_path: Arc<Path> =