worktree: Fix git ignored directories dropping their contents when they are refreshed (#44143) (cherry-pick to preview) (#44170)

zed-zippy[bot] and Lukas Wirth created

Cherry-pick of #44143 to preview

----
Closes https://github.com/zed-industries/zed/issues/38653

Release Notes:

- Fixed git ignored directories appearing as empty when their content
changes on windows

Co-authored by: Smit Barmase <smit@zed.dev>

Co-authored-by: Lukas Wirth <lukas@zed.dev>

Change summary

crates/worktree/src/worktree.rs       |  44 +++++--
crates/worktree/src/worktree_tests.rs | 169 +++++++++++++++++++++++++++++
2 files changed, 201 insertions(+), 12 deletions(-)

Detailed changes

crates/worktree/src/worktree.rs 🔗

@@ -428,7 +428,7 @@ impl Worktree {
                 let mut entry = Entry::new(
                     RelPath::empty().into(),
                     &metadata,
-                    &next_entry_id,
+                    ProjectEntryId::new(&next_entry_id),
                     snapshot.root_char_bag,
                     None,
                 );
@@ -2736,13 +2736,30 @@ impl BackgroundScannerState {
         }
     }
 
-    async fn insert_entry(
+    fn entry_id_for(
         &mut self,
-        mut entry: Entry,
-        fs: &dyn Fs,
-        watcher: &dyn Watcher,
-    ) -> Entry {
-        self.reuse_entry_id(&mut entry);
+        next_entry_id: &AtomicUsize,
+        path: &RelPath,
+        metadata: &fs::Metadata,
+    ) -> ProjectEntryId {
+        // If an entry with the same inode was removed from the worktree during this scan,
+        // then it *might* represent the same file or directory. But the OS might also have
+        // re-used the inode for a completely different file or directory.
+        //
+        // Conditionally reuse the old entry's id:
+        // * if the mtime is the same, the file was probably been renamed.
+        // * if the path is the same, the file may just have been updated
+        if let Some(removed_entry) = self.removed_entries.remove(&metadata.inode) {
+            if removed_entry.mtime == Some(metadata.mtime) || *removed_entry.path == *path {
+                return removed_entry.id;
+            }
+        } else if let Some(existing_entry) = self.snapshot.entry_for_path(path) {
+            return existing_entry.id;
+        }
+        ProjectEntryId::new(next_entry_id)
+    }
+
+    async fn insert_entry(&mut self, entry: Entry, fs: &dyn Fs, watcher: &dyn Watcher) -> Entry {
         let entry = self.snapshot.insert_entry(entry, fs);
         if entry.path.file_name() == Some(&DOT_GIT) {
             self.insert_git_repository(entry.path.clone(), fs, watcher)
@@ -3389,13 +3406,13 @@ impl Entry {
     fn new(
         path: Arc<RelPath>,
         metadata: &fs::Metadata,
-        next_entry_id: &AtomicUsize,
+        id: ProjectEntryId,
         root_char_bag: CharBag,
         canonical_path: Option<Arc<Path>>,
     ) -> Self {
         let char_bag = char_bag_for_path(root_char_bag, &path);
         Self {
-            id: ProjectEntryId::new(next_entry_id),
+            id,
             kind: if metadata.is_dir {
                 EntryKind::PendingDir
             } else {
@@ -3682,8 +3699,10 @@ impl BackgroundScanner {
                     .await;
                 if ignore_stack.is_abs_path_ignored(root_abs_path.as_path(), true) {
                     root_entry.is_ignored = true;
+                    let mut root_entry = root_entry.clone();
+                    state.reuse_entry_id(&mut root_entry);
                     state
-                        .insert_entry(root_entry.clone(), self.fs.as_ref(), self.watcher.as_ref())
+                        .insert_entry(root_entry, self.fs.as_ref(), self.watcher.as_ref())
                         .await;
                 }
                 if root_entry.is_dir() {
@@ -4289,7 +4308,7 @@ impl BackgroundScanner {
             let mut child_entry = Entry::new(
                 child_path.clone(),
                 &child_metadata,
-                &next_entry_id,
+                ProjectEntryId::new(&next_entry_id),
                 root_char_bag,
                 None,
             );
@@ -4476,10 +4495,11 @@ impl BackgroundScanner {
                         .ignore_stack_for_abs_path(&abs_path, metadata.is_dir, self.fs.as_ref())
                         .await;
                     let is_external = !canonical_path.starts_with(&root_canonical_path);
+                    let entry_id = state.entry_id_for(self.next_entry_id.as_ref(), path, &metadata);
                     let mut fs_entry = Entry::new(
                         path.clone(),
                         &metadata,
-                        self.next_entry_id.as_ref(),
+                        entry_id,
                         state.snapshot.root_char_bag,
                         if metadata.is_symlink {
                             Some(canonical_path.as_path().to_path_buf().into())

crates/worktree/src/worktree_tests.rs 🔗

@@ -1533,6 +1533,175 @@ async fn test_create_dir_all_on_create_entry(cx: &mut TestAppContext) {
     });
 }
 
+#[gpui::test]
+async fn test_create_file_in_expanded_gitignored_dir(cx: &mut TestAppContext) {
+    // Tests the behavior of our worktree refresh when a file in a gitignored directory
+    // is created.
+    init_test(cx);
+    let fs = FakeFs::new(cx.background_executor.clone());
+    fs.insert_tree(
+        "/root",
+        json!({
+            ".gitignore": "ignored_dir\n",
+            "ignored_dir": {
+                "existing_file.txt": "existing content",
+                "another_file.txt": "another content",
+            },
+        }),
+    )
+    .await;
+
+    let tree = Worktree::local(
+        Path::new("/root"),
+        true,
+        fs.clone(),
+        Default::default(),
+        &mut cx.to_async(),
+    )
+    .await
+    .unwrap();
+
+    cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete())
+        .await;
+
+    tree.read_with(cx, |tree, _| {
+        let ignored_dir = tree.entry_for_path(rel_path("ignored_dir")).unwrap();
+        assert!(ignored_dir.is_ignored);
+        assert_eq!(ignored_dir.kind, EntryKind::UnloadedDir);
+    });
+
+    tree.update(cx, |tree, cx| {
+        tree.load_file(rel_path("ignored_dir/existing_file.txt"), cx)
+    })
+    .await
+    .unwrap();
+
+    tree.read_with(cx, |tree, _| {
+        let ignored_dir = tree.entry_for_path(rel_path("ignored_dir")).unwrap();
+        assert!(ignored_dir.is_ignored);
+        assert_eq!(ignored_dir.kind, EntryKind::Dir);
+
+        assert!(
+            tree.entry_for_path(rel_path("ignored_dir/existing_file.txt"))
+                .is_some()
+        );
+        assert!(
+            tree.entry_for_path(rel_path("ignored_dir/another_file.txt"))
+                .is_some()
+        );
+    });
+
+    let entry = tree
+        .update(cx, |tree, cx| {
+            tree.create_entry(rel_path("ignored_dir/new_file.txt").into(), false, None, cx)
+        })
+        .await
+        .unwrap();
+    assert!(entry.into_included().is_some());
+
+    cx.executor().run_until_parked();
+
+    tree.read_with(cx, |tree, _| {
+        let ignored_dir = tree.entry_for_path(rel_path("ignored_dir")).unwrap();
+        assert!(ignored_dir.is_ignored);
+        assert_eq!(
+            ignored_dir.kind,
+            EntryKind::Dir,
+            "ignored_dir should still be loaded, not UnloadedDir"
+        );
+
+        assert!(
+            tree.entry_for_path(rel_path("ignored_dir/existing_file.txt"))
+                .is_some(),
+            "existing_file.txt should still be visible"
+        );
+        assert!(
+            tree.entry_for_path(rel_path("ignored_dir/another_file.txt"))
+                .is_some(),
+            "another_file.txt should still be visible"
+        );
+        assert!(
+            tree.entry_for_path(rel_path("ignored_dir/new_file.txt"))
+                .is_some(),
+            "new_file.txt should be visible"
+        );
+    });
+}
+
+#[gpui::test]
+async fn test_fs_event_for_gitignored_dir_does_not_lose_contents(cx: &mut TestAppContext) {
+    // Tests the behavior of our worktree refresh when a directory modification for a gitignored directory
+    // is triggered.
+    init_test(cx);
+    let fs = FakeFs::new(cx.background_executor.clone());
+    fs.insert_tree(
+        "/root",
+        json!({
+            ".gitignore": "ignored_dir\n",
+            "ignored_dir": {
+                "file1.txt": "content1",
+                "file2.txt": "content2",
+            },
+        }),
+    )
+    .await;
+
+    let tree = Worktree::local(
+        Path::new("/root"),
+        true,
+        fs.clone(),
+        Default::default(),
+        &mut cx.to_async(),
+    )
+    .await
+    .unwrap();
+
+    cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete())
+        .await;
+
+    // Load a file to expand the ignored directory
+    tree.update(cx, |tree, cx| {
+        tree.load_file(rel_path("ignored_dir/file1.txt"), cx)
+    })
+    .await
+    .unwrap();
+
+    tree.read_with(cx, |tree, _| {
+        let ignored_dir = tree.entry_for_path(rel_path("ignored_dir")).unwrap();
+        assert_eq!(ignored_dir.kind, EntryKind::Dir);
+        assert!(
+            tree.entry_for_path(rel_path("ignored_dir/file1.txt"))
+                .is_some()
+        );
+        assert!(
+            tree.entry_for_path(rel_path("ignored_dir/file2.txt"))
+                .is_some()
+        );
+    });
+
+    fs.emit_fs_event("/root/ignored_dir", Some(fs::PathEventKind::Changed));
+    tree.flush_fs_events(cx).await;
+
+    tree.read_with(cx, |tree, _| {
+        let ignored_dir = tree.entry_for_path(rel_path("ignored_dir")).unwrap();
+        assert_eq!(
+            ignored_dir.kind,
+            EntryKind::Dir,
+            "ignored_dir should still be loaded (Dir), not UnloadedDir"
+        );
+        assert!(
+            tree.entry_for_path(rel_path("ignored_dir/file1.txt"))
+                .is_some(),
+            "file1.txt should still be visible after directory fs event"
+        );
+        assert!(
+            tree.entry_for_path(rel_path("ignored_dir/file2.txt"))
+                .is_some(),
+            "file2.txt should still be visible after directory fs event"
+        );
+    });
+}
+
 #[gpui::test(iterations = 100)]
 async fn test_random_worktree_operations_during_initial_scan(
     cx: &mut TestAppContext,