Fix ancestor git repositories going missing (#28436)

Cole Miller created

Closes #ISSUE

Release Notes:

- Fixed a bug that caused Zed to sometimes not discover git repositories
above a worktree root.

Change summary

crates/worktree/src/worktree.rs       | 164 +++++++++++++++++-----------
crates/worktree/src/worktree_tests.rs |  64 +++++++++++
2 files changed, 164 insertions(+), 64 deletions(-)

Detailed changes

crates/worktree/src/worktree.rs 🔗

@@ -3768,69 +3768,21 @@ impl BackgroundScanner {
         // the git repository in an ancestor directory. Find any gitignore files
         // in ancestor directories.
         let root_abs_path = self.state.lock().snapshot.abs_path.clone();
-        let mut containing_git_repository = None;
-        for (index, ancestor) in root_abs_path.as_path().ancestors().enumerate() {
-            if index != 0 {
-                if Some(ancestor) == self.fs.home_dir().as_deref() {
-                    // Unless $HOME is itself the worktree root, don't consider it as a
-                    // containing git repository---expensive and likely unwanted.
-                    break;
-                } else if let Ok(ignore) =
-                    build_gitignore(&ancestor.join(*GITIGNORE), self.fs.as_ref()).await
-                {
-                    self.state
-                        .lock()
-                        .snapshot
-                        .ignores_by_parent_abs_path
-                        .insert(ancestor.into(), (ignore.into(), false));
-                }
-            }
-
-            let ancestor_dot_git = ancestor.join(*DOT_GIT);
-            log::trace!("considering ancestor: {ancestor_dot_git:?}");
-            // Check whether the directory or file called `.git` exists (in the
-            // case of worktrees it's a file.)
-            if self
-                .fs
-                .metadata(&ancestor_dot_git)
-                .await
-                .is_ok_and(|metadata| metadata.is_some())
-            {
-                if index != 0 {
-                    // We canonicalize, since the FS events use the canonicalized path.
-                    if let Some(ancestor_dot_git) =
-                        self.fs.canonicalize(&ancestor_dot_git).await.log_err()
-                    {
-                        let location_in_repo = root_abs_path
-                            .as_path()
-                            .strip_prefix(ancestor)
-                            .unwrap()
-                            .into();
-                        log::info!(
-                            "inserting parent git repo for this worktree: {location_in_repo:?}"
-                        );
-                        // We associate the external git repo with our root folder and
-                        // also mark where in the git repo the root folder is located.
-                        let local_repository = self.state.lock().insert_git_repository_for_path(
-                            WorkDirectory::AboveProject {
-                                absolute_path: ancestor.into(),
-                                location_in_repo,
-                            },
-                            ancestor_dot_git.clone().into(),
-                            self.fs.as_ref(),
-                            self.watcher.as_ref(),
-                        );
-
-                        if local_repository.is_some() {
-                            containing_git_repository = Some(ancestor_dot_git)
-                        }
-                    };
-                }
-
-                // Reached root of git repository.
-                break;
-            }
-        }
+        let (ignores, repo) = discover_ancestor_git_repo(self.fs.clone(), &root_abs_path).await;
+        self.state
+            .lock()
+            .snapshot
+            .ignores_by_parent_abs_path
+            .extend(ignores);
+        let containing_git_repository = repo.and_then(|(ancestor_dot_git, work_directory)| {
+            self.state.lock().insert_git_repository_for_path(
+                work_directory,
+                ancestor_dot_git.as_path().into(),
+                self.fs.as_ref(),
+                self.watcher.as_ref(),
+            )?;
+            Some(ancestor_dot_git)
+        });
 
         log::info!("containing git repository: {containing_git_repository:?}");
 
@@ -4482,6 +4434,15 @@ impl BackgroundScanner {
         )
         .await;
 
+        let mut new_ancestor_repo = if relative_paths
+            .iter()
+            .any(|path| path.as_ref() == Path::new(""))
+        {
+            Some(discover_ancestor_git_repo(self.fs.clone(), &root_abs_path).await)
+        } else {
+            None
+        };
+
         let mut state = self.state.lock();
         let doing_recursive_update = scan_queue_tx.is_some();
 
@@ -4533,6 +4494,21 @@ impl BackgroundScanner {
                     }
 
                     state.insert_entry(fs_entry.clone(), self.fs.as_ref(), self.watcher.as_ref());
+
+                    if path.as_ref() == Path::new("") {
+                        if let Some((ignores, repo)) = new_ancestor_repo.take() {
+                            log::trace!("updating ancestor git repository");
+                            state.snapshot.ignores_by_parent_abs_path.extend(ignores);
+                            if let Some((ancestor_dot_git, work_directory)) = repo {
+                                state.insert_git_repository_for_path(
+                                    work_directory,
+                                    ancestor_dot_git.as_path().into(),
+                                    self.fs.as_ref(),
+                                    self.watcher.as_ref(),
+                                );
+                            }
+                        }
+                    }
                 }
                 Ok(None) => {
                     self.remove_repo_path(path, &mut state.snapshot);
@@ -4811,6 +4787,68 @@ impl BackgroundScanner {
     }
 }
 
+async fn discover_ancestor_git_repo(
+    fs: Arc<dyn Fs>,
+    root_abs_path: &SanitizedPath,
+) -> (
+    HashMap<Arc<Path>, (Arc<Gitignore>, bool)>,
+    Option<(PathBuf, WorkDirectory)>,
+) {
+    let mut ignores = HashMap::default();
+    for (index, ancestor) in root_abs_path.as_path().ancestors().enumerate() {
+        if index != 0 {
+            if Some(ancestor) == fs.home_dir().as_deref() {
+                // Unless $HOME is itself the worktree root, don't consider it as a
+                // containing git repository---expensive and likely unwanted.
+                break;
+            } else if let Ok(ignore) =
+                build_gitignore(&ancestor.join(*GITIGNORE), fs.as_ref()).await
+            {
+                ignores.insert(ancestor.into(), (ignore.into(), false));
+            }
+        }
+
+        let ancestor_dot_git = ancestor.join(*DOT_GIT);
+        log::trace!("considering ancestor: {ancestor_dot_git:?}");
+        // Check whether the directory or file called `.git` exists (in the
+        // case of worktrees it's a file.)
+        if fs
+            .metadata(&ancestor_dot_git)
+            .await
+            .is_ok_and(|metadata| metadata.is_some())
+        {
+            if index != 0 {
+                // We canonicalize, since the FS events use the canonicalized path.
+                if let Some(ancestor_dot_git) = fs.canonicalize(&ancestor_dot_git).await.log_err() {
+                    let location_in_repo = root_abs_path
+                        .as_path()
+                        .strip_prefix(ancestor)
+                        .unwrap()
+                        .into();
+                    log::info!("inserting parent git repo for this worktree: {location_in_repo:?}");
+                    // We associate the external git repo with our root folder and
+                    // also mark where in the git repo the root folder is located.
+                    return (
+                        ignores,
+                        Some((
+                            ancestor_dot_git,
+                            WorkDirectory::AboveProject {
+                                absolute_path: ancestor.into(),
+                                location_in_repo,
+                            },
+                        )),
+                    );
+                };
+            }
+
+            // Reached root of git repository.
+            break;
+        }
+    }
+
+    (ignores, None)
+}
+
 fn build_diff(
     phase: BackgroundScannerPhase,
     old_snapshot: &Snapshot,

crates/worktree/src/worktree_tests.rs 🔗

@@ -5,7 +5,7 @@ use crate::{
 use anyhow::Result;
 use fs::{FakeFs, Fs, RealFs, RemoveOptions};
 use git::GITIGNORE;
-use gpui::{AppContext as _, BorrowAppContext, Context, Task, TestAppContext};
+use gpui::{AppContext as _, BackgroundExecutor, BorrowAppContext, Context, Task, TestAppContext};
 use parking_lot::Mutex;
 use postage::stream::Stream;
 use pretty_assertions::assert_eq;
@@ -1984,6 +1984,68 @@ fn test_unrelativize() {
     );
 }
 
+#[gpui::test]
+async fn test_repository_above_root(executor: BackgroundExecutor, cx: &mut TestAppContext) {
+    init_test(cx);
+
+    let fs = FakeFs::new(executor);
+    fs.insert_tree(
+        path!("/root"),
+        json!({
+            ".git": {},
+            "subproject": {
+                "a.txt": "A"
+            }
+        }),
+    )
+    .await;
+    let worktree = Worktree::local(
+        path!("/root/subproject").as_ref(),
+        true,
+        fs.clone(),
+        Arc::default(),
+        &mut cx.to_async(),
+    )
+    .await
+    .unwrap();
+    worktree
+        .update(cx, |worktree, _| {
+            worktree.as_local().unwrap().scan_complete()
+        })
+        .await;
+    cx.run_until_parked();
+    let repos = worktree.update(cx, |worktree, _| {
+        worktree
+            .as_local()
+            .unwrap()
+            .git_repositories
+            .values()
+            .map(|entry| entry.work_directory_abs_path.clone())
+            .collect::<Vec<_>>()
+    });
+    pretty_assertions::assert_eq!(repos, [Path::new(path!("/root")).into()]);
+
+    eprintln!(">>>>>>>>>> touch");
+    fs.touch_path(path!("/root/subproject")).await;
+    worktree
+        .update(cx, |worktree, _| {
+            worktree.as_local().unwrap().scan_complete()
+        })
+        .await;
+    cx.run_until_parked();
+
+    let repos = worktree.update(cx, |worktree, _| {
+        worktree
+            .as_local()
+            .unwrap()
+            .git_repositories
+            .values()
+            .map(|entry| entry.work_directory_abs_path.clone())
+            .collect::<Vec<_>>()
+    });
+    pretty_assertions::assert_eq!(repos, [Path::new(path!("/root")).into()]);
+}
+
 #[track_caller]
 fn check_worktree_entries(
     tree: &Worktree,