From 7bf6cd4ccfefb4547d91287d52845bee18c1639a Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Wed, 9 Apr 2025 12:44:29 -0400 Subject: [PATCH] Fix ancestor git repositories going missing (#28436) Closes #ISSUE Release Notes: - Fixed a bug that caused Zed to sometimes not discover git repositories above a worktree root. --- crates/worktree/src/worktree.rs | 164 ++++++++++++++++---------- crates/worktree/src/worktree_tests.rs | 64 +++++++++- 2 files changed, 164 insertions(+), 64 deletions(-) diff --git a/crates/worktree/src/worktree.rs b/crates/worktree/src/worktree.rs index 93e6ed25fc..5deaaa4267 100644 --- a/crates/worktree/src/worktree.rs +++ b/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, + root_abs_path: &SanitizedPath, +) -> ( + HashMap, (Arc, 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, diff --git a/crates/worktree/src/worktree_tests.rs b/crates/worktree/src/worktree_tests.rs index 45ffc22892..21a67e3b49 100644 --- a/crates/worktree/src/worktree_tests.rs +++ b/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::>() + }); + 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::>() + }); + pretty_assertions::assert_eq!(repos, [Path::new(path!("/root")).into()]); +} + #[track_caller] fn check_worktree_entries( tree: &Worktree,