Fix accidental ignoring of git FS events

Max Brunsfeld created

Change summary

crates/project/src/worktree.rs       | 80 ++++++++++++++++-------------
crates/project/src/worktree_tests.rs | 50 +++++++++---------
2 files changed, 69 insertions(+), 61 deletions(-)

Detailed changes

crates/project/src/worktree.rs 🔗

@@ -2140,6 +2140,7 @@ impl LocalSnapshot {
 impl BackgroundScannerState {
     fn should_scan_directory(&self, entry: &Entry) -> bool {
         (!entry.is_external && !entry.is_ignored)
+            || entry.path.file_name() == Some(&*DOT_GIT)
             || self.scanned_dirs.contains(&entry.id) // If we've ever scanned it, keep scanning
             || self
                 .paths_to_scan
@@ -2319,6 +2320,7 @@ impl BackgroundScannerState {
                         .entry_for_id(entry_id)
                         .map(|entry| RepositoryWorkDirectory(entry.path.clone())) else { continue };
 
+                    log::info!("reload git repository {:?}", dot_git_dir);
                     let repository = repository.repo_ptr.lock();
                     let branch = repository.branch_name();
                     repository.reload_index();
@@ -2359,6 +2361,8 @@ impl BackgroundScannerState {
     }
 
     fn build_repository(&mut self, dot_git_path: Arc<Path>, fs: &dyn Fs) -> Option<()> {
+        log::info!("build git repository {:?}", dot_git_path);
+
         let work_dir_path: Arc<Path> = dot_git_path.parent().unwrap().into();
 
         // Guard against repositories inside the repository metadata
@@ -3138,8 +3142,6 @@ impl BackgroundScanner {
     }
 
     async fn process_events(&mut self, mut abs_paths: Vec<PathBuf>) {
-        log::debug!("received fs events {:?}", abs_paths);
-
         let root_path = self.state.lock().snapshot.abs_path.clone();
         let root_canonical_path = match self.fs.canonicalize(&root_path).await {
             Ok(path) => path,
@@ -3150,7 +3152,6 @@ impl BackgroundScanner {
         };
 
         let mut relative_paths = Vec::with_capacity(abs_paths.len());
-        let mut unloaded_relative_paths = Vec::new();
         abs_paths.sort_unstable();
         abs_paths.dedup_by(|a, b| a.starts_with(&b));
         abs_paths.retain(|abs_path| {
@@ -3173,7 +3174,6 @@ impl BackgroundScanner {
                 });
                 if !parent_dir_is_loaded {
                     log::debug!("ignoring event {relative_path:?} within unloaded directory");
-                    unloaded_relative_paths.push(relative_path);
                     return false;
                 }
 
@@ -3182,27 +3182,30 @@ impl BackgroundScanner {
             }
         });
 
-        if !relative_paths.is_empty() {
-            let (scan_job_tx, scan_job_rx) = channel::unbounded();
-            self.reload_entries_for_paths(
-                root_path,
-                root_canonical_path,
-                &relative_paths,
-                abs_paths,
-                Some(scan_job_tx.clone()),
-            )
-            .await;
-            drop(scan_job_tx);
-            self.scan_dirs(false, scan_job_rx).await;
-
-            let (scan_job_tx, scan_job_rx) = channel::unbounded();
-            self.update_ignore_statuses(scan_job_tx).await;
-            self.scan_dirs(false, scan_job_rx).await;
+        if relative_paths.is_empty() {
+            return;
         }
 
+        log::debug!("received fs events {:?}", relative_paths);
+
+        let (scan_job_tx, scan_job_rx) = channel::unbounded();
+        self.reload_entries_for_paths(
+            root_path,
+            root_canonical_path,
+            &relative_paths,
+            abs_paths,
+            Some(scan_job_tx.clone()),
+        )
+        .await;
+        drop(scan_job_tx);
+        self.scan_dirs(false, scan_job_rx).await;
+
+        let (scan_job_tx, scan_job_rx) = channel::unbounded();
+        self.update_ignore_statuses(scan_job_tx).await;
+        self.scan_dirs(false, scan_job_rx).await;
+
         {
             let mut state = self.state.lock();
-            relative_paths.extend(unloaded_relative_paths);
             state.reload_repositories(&relative_paths, self.fs.as_ref());
             state.snapshot.completed_scan_id = state.snapshot.scan_id;
             for (_, entry_id) in mem::take(&mut state.removed_entry_ids) {
@@ -3610,23 +3613,28 @@ impl BackgroundScanner {
                         }
                     }
 
-                    let fs_entry = state.insert_entry(fs_entry, self.fs.as_ref());
-
-                    if let Some(scan_queue_tx) = &scan_queue_tx {
-                        let mut ancestor_inodes = state.snapshot.ancestor_inodes_for_path(&path);
-                        if metadata.is_dir && !ancestor_inodes.contains(&metadata.inode) {
-                            ancestor_inodes.insert(metadata.inode);
-                            smol::block_on(scan_queue_tx.send(ScanJob {
-                                abs_path,
-                                path: path.clone(),
-                                ignore_stack,
-                                ancestor_inodes,
-                                is_external: fs_entry.is_external,
-                                scan_queue: scan_queue_tx.clone(),
-                            }))
-                            .unwrap();
+                    if let (Some(scan_queue_tx), true) = (&scan_queue_tx, fs_entry.is_dir()) {
+                        if state.should_scan_directory(&fs_entry) {
+                            let mut ancestor_inodes =
+                                state.snapshot.ancestor_inodes_for_path(&path);
+                            if !ancestor_inodes.contains(&metadata.inode) {
+                                ancestor_inodes.insert(metadata.inode);
+                                smol::block_on(scan_queue_tx.send(ScanJob {
+                                    abs_path,
+                                    path: path.clone(),
+                                    ignore_stack,
+                                    ancestor_inodes,
+                                    is_external: fs_entry.is_external,
+                                    scan_queue: scan_queue_tx.clone(),
+                                }))
+                                .unwrap();
+                            }
+                        } else {
+                            fs_entry.kind = EntryKind::UnloadedDir;
                         }
                     }
+
+                    state.insert_entry(fs_entry, self.fs.as_ref());
                 }
                 Ok(None) => {
                     self.remove_repo_path(&path, &mut state.snapshot);

crates/project/src/worktree_tests.rs 🔗

@@ -1654,37 +1654,37 @@ async fn test_git_status(deterministic: Arc<Deterministic>, cx: &mut TestAppCont
 
     }));
 
-    let tree = Worktree::local(
-        build_client(cx),
-        root.path(),
-        true,
-        Arc::new(RealFs),
-        Default::default(),
-        &mut cx.to_async(),
-    )
-    .await
-    .unwrap();
-
-    cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete())
-        .await;
-
     const A_TXT: &'static str = "a.txt";
     const B_TXT: &'static str = "b.txt";
     const E_TXT: &'static str = "c/d/e.txt";
     const F_TXT: &'static str = "f.txt";
     const DOTGITIGNORE: &'static str = ".gitignore";
     const BUILD_FILE: &'static str = "target/build_file";
-    let project_path: &Path = &Path::new("project");
+    let project_path = Path::new("project");
 
+    // Set up git repository before creating the worktree.
     let work_dir = root.path().join("project");
     let mut repo = git_init(work_dir.as_path());
     repo.add_ignore_rule(IGNORE_RULE).unwrap();
-    git_add(Path::new(A_TXT), &repo);
-    git_add(Path::new(E_TXT), &repo);
-    git_add(Path::new(DOTGITIGNORE), &repo);
+    git_add(A_TXT, &repo);
+    git_add(E_TXT, &repo);
+    git_add(DOTGITIGNORE, &repo);
     git_commit("Initial commit", &repo);
 
+    let tree = Worktree::local(
+        build_client(cx),
+        root.path(),
+        true,
+        Arc::new(RealFs),
+        Default::default(),
+        &mut cx.to_async(),
+    )
+    .await
+    .unwrap();
+
     tree.flush_fs_events(cx).await;
+    cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete())
+        .await;
     deterministic.run_until_parked();
 
     // Check that the right git state is observed on startup
@@ -1704,39 +1704,39 @@ async fn test_git_status(deterministic: Arc<Deterministic>, cx: &mut TestAppCont
         );
     });
 
+    // Modify a file in the working copy.
     std::fs::write(work_dir.join(A_TXT), "aa").unwrap();
-
     tree.flush_fs_events(cx).await;
     deterministic.run_until_parked();
 
+    // The worktree detects that the file's git status has changed.
     tree.read_with(cx, |tree, _cx| {
         let snapshot = tree.snapshot();
-
         assert_eq!(
             snapshot.status_for_file(project_path.join(A_TXT)),
             Some(GitFileStatus::Modified)
         );
     });
 
-    git_add(Path::new(A_TXT), &repo);
-    git_add(Path::new(B_TXT), &repo);
+    // Create a commit in the git repository.
+    git_add(A_TXT, &repo);
+    git_add(B_TXT, &repo);
     git_commit("Committing modified and added", &repo);
     tree.flush_fs_events(cx).await;
     deterministic.run_until_parked();
 
-    // Check that repo only changes are tracked
+    // The worktree detects that the files' git status have changed.
     tree.read_with(cx, |tree, _cx| {
         let snapshot = tree.snapshot();
-
         assert_eq!(
             snapshot.status_for_file(project_path.join(F_TXT)),
             Some(GitFileStatus::Added)
         );
-
         assert_eq!(snapshot.status_for_file(project_path.join(B_TXT)), None);
         assert_eq!(snapshot.status_for_file(project_path.join(A_TXT)), None);
     });
 
+    // Modify files in the working copy and perform git operations on other files.
     git_reset(0, &repo);
     git_remove_index(Path::new(B_TXT), &repo);
     git_stash(&mut repo);