Directly parse .git when it's a file instead of using libgit2 (#27885)

Cole Miller created

Avoids building a whole git2 repository object at the worktree layer
just to watch some additional paths.

- [x] Tidy up names of the various paths
- [x] Tests for worktrees and submodules

Release Notes:

- N/A

Change summary

crates/fs/src/fake_git_repo.rs      |  22 +--
crates/fs/src/fs.rs                 | 163 +++++++++++++++++++++------
crates/git/src/repository.rs        |   6 -
crates/project/src/git_store.rs     |  33 ++++-
crates/project/src/project.rs       |  26 ++++
crates/project/src/project_tests.rs | 180 +++++++++++++++++++++++-------
crates/worktree/src/worktree.rs     | 133 +++++++++++++---------
7 files changed, 402 insertions(+), 161 deletions(-)

Detailed changes

crates/fs/src/fake_git_repo.rs 🔗

@@ -21,11 +21,12 @@ pub struct FakeGitRepository {
     pub(crate) fs: Arc<FakeFs>,
     pub(crate) executor: BackgroundExecutor,
     pub(crate) dot_git_path: PathBuf,
+    pub(crate) repository_dir_path: PathBuf,
+    pub(crate) common_dir_path: PathBuf,
 }
 
 #[derive(Debug, Clone)]
 pub struct FakeGitRepositoryState {
-    pub path: PathBuf,
     pub event_emitter: smol::channel::Sender<PathBuf>,
     pub unmerged_paths: HashMap<RepoPath, UnmergedStatus>,
     pub head_contents: HashMap<RepoPath, String>,
@@ -37,9 +38,8 @@ pub struct FakeGitRepositoryState {
 }
 
 impl FakeGitRepositoryState {
-    pub fn new(path: PathBuf, event_emitter: smol::channel::Sender<PathBuf>) -> Self {
+    pub fn new(event_emitter: smol::channel::Sender<PathBuf>) -> Self {
         FakeGitRepositoryState {
-            path,
             event_emitter,
             head_contents: Default::default(),
             index_contents: Default::default(),
@@ -53,15 +53,6 @@ impl FakeGitRepositoryState {
 }
 
 impl FakeGitRepository {
-    fn with_state<F, T>(&self, f: F) -> T
-    where
-        F: FnOnce(&mut FakeGitRepositoryState) -> T,
-    {
-        self.fs
-            .with_git_state(&self.dot_git_path, false, f)
-            .unwrap()
-    }
-
     fn with_state_async<F, T>(&self, write: bool, f: F) -> BoxFuture<'static, Result<T>>
     where
         F: 'static + Send + FnOnce(&mut FakeGitRepositoryState) -> Result<T>,
@@ -172,11 +163,11 @@ impl GitRepository for FakeGitRepository {
     }
 
     fn path(&self) -> PathBuf {
-        self.with_state(|state| state.path.clone())
+        self.repository_dir_path.clone()
     }
 
     fn main_repository_path(&self) -> PathBuf {
-        self.path()
+        self.common_dir_path.clone()
     }
 
     fn merge_message(&self) -> BoxFuture<Option<String>> {
@@ -207,8 +198,9 @@ impl GitRepository for FakeGitRepository {
             .files()
             .iter()
             .filter_map(|path| {
+                // TODO better simulate git status output in the case of submodules and worktrees
                 let repo_path = path.strip_prefix(workdir_path).ok()?;
-                let mut is_ignored = false;
+                let mut is_ignored = repo_path.starts_with(".git");
                 for ignore in &ignores {
                     match ignore.matched_path_or_any_parents(path, false) {
                         ignore::Match::None => {}

crates/fs/src/fs.rs 🔗

@@ -851,7 +851,7 @@ impl Watcher for RealWatcher {
 pub struct FakeFs {
     this: std::sync::Weak<Self>,
     // Use an unfair lock to ensure tests are deterministic.
-    state: Mutex<FakeFsState>,
+    state: Arc<Mutex<FakeFsState>>,
     executor: gpui::BackgroundExecutor,
 }
 
@@ -878,6 +878,8 @@ enum FakeFsEntry {
         mtime: MTime,
         len: u64,
         content: Vec<u8>,
+        // The path to the repository state directory, if this is a gitfile.
+        git_dir_path: Option<PathBuf>,
     },
     Dir {
         inode: u64,
@@ -1036,7 +1038,7 @@ impl FakeFs {
         let this = Arc::new_cyclic(|this| Self {
             this: this.clone(),
             executor: executor.clone(),
-            state: Mutex::new(FakeFsState {
+            state: Arc::new(Mutex::new(FakeFsState {
                 root: Arc::new(Mutex::new(FakeFsEntry::Dir {
                     inode: 0,
                     mtime: MTime(UNIX_EPOCH),
@@ -1054,7 +1056,7 @@ impl FakeFs {
                 metadata_call_count: 0,
                 moves: Default::default(),
                 home_dir: None,
-            }),
+            })),
         });
 
         executor.spawn({
@@ -1097,6 +1099,7 @@ impl FakeFs {
                             mtime: new_mtime,
                             content: Vec::new(),
                             len: 0,
+                            git_dir_path: None,
                         })));
                     }
                     btree_map::Entry::Occupied(mut e) => match &mut *e.get_mut().lock() {
@@ -1154,6 +1157,7 @@ impl FakeFs {
                         mtime: new_mtime,
                         len: new_len,
                         content: new_content,
+                        git_dir_path: None,
                     })));
                 }
                 btree_map::Entry::Occupied(mut e) => {
@@ -1278,9 +1282,14 @@ impl FakeFs {
         .boxed()
     }
 
-    pub fn with_git_state<T, F>(&self, dot_git: &Path, emit_git_event: bool, f: F) -> Result<T>
+    pub fn with_git_state_and_paths<T, F>(
+        &self,
+        dot_git: &Path,
+        emit_git_event: bool,
+        f: F,
+    ) -> Result<T>
     where
-        F: FnOnce(&mut FakeGitRepositoryState) -> T,
+        F: FnOnce(&mut FakeGitRepositoryState, &Path, &Path) -> T,
     {
         let mut state = self.state.lock();
         let entry = state.read_path(dot_git).context("open .git")?;
@@ -1288,25 +1297,75 @@ impl FakeFs {
 
         if let FakeFsEntry::Dir { git_repo_state, .. } = &mut *entry {
             let repo_state = git_repo_state.get_or_insert_with(|| {
+                log::debug!("insert git state for {dot_git:?}");
                 Arc::new(Mutex::new(FakeGitRepositoryState::new(
-                    dot_git.to_path_buf(),
                     state.git_event_tx.clone(),
                 )))
             });
             let mut repo_state = repo_state.lock();
 
-            let result = f(&mut repo_state);
+            let result = f(&mut repo_state, dot_git, dot_git);
 
             if emit_git_event {
                 state.emit_event([(dot_git, None)]);
             }
 
+            Ok(result)
+        } else if let FakeFsEntry::File {
+            content,
+            git_dir_path,
+            ..
+        } = &mut *entry
+        {
+            let path = match git_dir_path {
+                Some(path) => path,
+                None => {
+                    let path = std::str::from_utf8(content)
+                        .ok()
+                        .and_then(|content| content.strip_prefix("gitdir:"))
+                        .ok_or_else(|| anyhow!("not a valid gitfile"))?
+                        .trim();
+                    git_dir_path.insert(normalize_path(&dot_git.parent().unwrap().join(path)))
+                }
+            }
+            .clone();
+            drop(entry);
+            let Some((git_dir_entry, canonical_path)) = state.try_read_path(&path, true) else {
+                anyhow::bail!("pointed-to git dir {path:?} not found")
+            };
+            let FakeFsEntry::Dir { git_repo_state, .. } = &mut *git_dir_entry.lock() else {
+                anyhow::bail!("gitfile points to a non-directory")
+            };
+            let common_dir = canonical_path
+                .ancestors()
+                .find(|ancestor| ancestor.ends_with(".git"))
+                .ok_or_else(|| anyhow!("repository dir not contained in any .git"))?;
+            let repo_state = git_repo_state.get_or_insert_with(|| {
+                Arc::new(Mutex::new(FakeGitRepositoryState::new(
+                    state.git_event_tx.clone(),
+                )))
+            });
+            let mut repo_state = repo_state.lock();
+
+            let result = f(&mut repo_state, &canonical_path, common_dir);
+
+            if emit_git_event {
+                state.emit_event([(canonical_path, None)]);
+            }
+
             Ok(result)
         } else {
-            Err(anyhow!("not a directory"))
+            Err(anyhow!("not a valid git repository"))
         }
     }
 
+    pub fn with_git_state<T, F>(&self, dot_git: &Path, emit_git_event: bool, f: F) -> Result<T>
+    where
+        F: FnOnce(&mut FakeGitRepositoryState) -> T,
+    {
+        self.with_git_state_and_paths(dot_git, emit_git_event, |state, _, _| f(state))
+    }
+
     pub fn set_branch_name(&self, dot_git: &Path, branch: Option<impl Into<String>>) {
         self.with_git_state(dot_git, true, |state| {
             let branch = branch.map(Into::into);
@@ -1663,11 +1722,25 @@ impl FakeFsEntry {
 }
 
 #[cfg(any(test, feature = "test-support"))]
-struct FakeWatcher {}
+struct FakeWatcher {
+    tx: smol::channel::Sender<Vec<PathEvent>>,
+    original_path: PathBuf,
+    fs_state: Arc<Mutex<FakeFsState>>,
+    prefixes: Mutex<Vec<PathBuf>>,
+}
 
 #[cfg(any(test, feature = "test-support"))]
 impl Watcher for FakeWatcher {
-    fn add(&self, _: &Path) -> Result<()> {
+    fn add(&self, path: &Path) -> Result<()> {
+        if path.starts_with(&self.original_path) {
+            return Ok(());
+        }
+        self.fs_state
+            .try_lock()
+            .unwrap()
+            .event_txs
+            .push((path.to_owned(), self.tx.clone()));
+        self.prefixes.lock().push(path.to_owned());
         Ok(())
     }
 
@@ -1745,6 +1818,7 @@ impl Fs for FakeFs {
             mtime,
             len: 0,
             content: Vec::new(),
+            git_dir_path: None,
         }));
         let mut kind = Some(PathEventKind::Created);
         state.write_path(path, |entry| {
@@ -1901,6 +1975,7 @@ impl Fs for FakeFs {
                     mtime,
                     len: content.len() as u64,
                     content,
+                    git_dir_path: None,
                 })))
                 .clone(),
             )),
@@ -2137,42 +2212,54 @@ impl Fs for FakeFs {
         self.simulate_random_delay().await;
         let (tx, rx) = smol::channel::unbounded();
         let path = path.to_path_buf();
-        self.state.lock().event_txs.push((path.clone(), tx));
+        self.state.lock().event_txs.push((path.clone(), tx.clone()));
         let executor = self.executor.clone();
+        let watcher = Arc::new(FakeWatcher {
+            tx,
+            original_path: path.to_owned(),
+            fs_state: self.state.clone(),
+            prefixes: Mutex::new(vec![path.to_owned()]),
+        });
         (
-            Box::pin(futures::StreamExt::filter(rx, move |events| {
-                let result = events
-                    .iter()
-                    .any(|evt_path| evt_path.path.starts_with(&path));
-                let executor = executor.clone();
-                async move {
-                    executor.simulate_random_delay().await;
-                    result
+            Box::pin(futures::StreamExt::filter(rx, {
+                let watcher = watcher.clone();
+                move |events| {
+                    let result = events.iter().any(|evt_path| {
+                        let result = watcher
+                            .prefixes
+                            .lock()
+                            .iter()
+                            .any(|prefix| evt_path.path.starts_with(prefix));
+                        result
+                    });
+                    let executor = executor.clone();
+                    async move {
+                        executor.simulate_random_delay().await;
+                        result
+                    }
                 }
             })),
-            Arc::new(FakeWatcher {}),
+            watcher,
         )
     }
 
     fn open_repo(&self, abs_dot_git: &Path) -> Option<Arc<dyn GitRepository>> {
-        let state = self.state.lock();
-        let entry = state.read_path(abs_dot_git).unwrap();
-        let mut entry = entry.lock();
-        if let FakeFsEntry::Dir { git_repo_state, .. } = &mut *entry {
-            git_repo_state.get_or_insert_with(|| {
-                Arc::new(Mutex::new(FakeGitRepositoryState::new(
-                    abs_dot_git.to_path_buf(),
-                    state.git_event_tx.clone(),
-                )))
-            });
-            Some(Arc::new(fake_git_repo::FakeGitRepository {
-                fs: self.this.upgrade().unwrap(),
-                executor: self.executor.clone(),
-                dot_git_path: abs_dot_git.to_path_buf(),
-            }))
-        } else {
-            None
-        }
+        use util::ResultExt as _;
+
+        self.with_git_state_and_paths(
+            abs_dot_git,
+            false,
+            |_, repository_dir_path, common_dir_path| {
+                Arc::new(fake_git_repo::FakeGitRepository {
+                    fs: self.this.upgrade().unwrap(),
+                    executor: self.executor.clone(),
+                    dot_git_path: abs_dot_git.to_path_buf(),
+                    repository_dir_path: repository_dir_path.to_owned(),
+                    common_dir_path: common_dir_path.to_owned(),
+                }) as _
+            },
+        )
+        .log_err()
     }
 
     fn git_init(

crates/git/src/repository.rs 🔗

@@ -229,12 +229,6 @@ pub trait GitRepository: Send + Sync {
     /// worktree's gitdir within the main repository (typically `.git/worktrees/<name>`).
     fn path(&self) -> PathBuf;
 
-    /// Returns the absolute path to the ".git" dir for the main repository, typically a `.git`
-    /// folder. For worktrees, this will be the path to the repository the worktree was created
-    /// from. Otherwise, this is the same value as `path()`.
-    ///
-    /// Git documentation calls this the "commondir", and for git CLI is overridden by
-    /// `GIT_COMMON_DIR`.
     fn main_repository_path(&self) -> PathBuf;
 
     /// Updates the index to match the worktree at the given paths.

crates/project/src/git_store.rs 🔗

@@ -61,7 +61,8 @@ use sum_tree::{Edit, SumTree, TreeSet};
 use text::{Bias, BufferId};
 use util::{ResultExt, debug_panic, post_inc};
 use worktree::{
-    File, PathKey, PathProgress, PathSummary, PathTarget, UpdatedGitRepositoriesSet, Worktree,
+    File, PathKey, PathProgress, PathSummary, PathTarget, UpdatedGitRepositoriesSet,
+    UpdatedGitRepository, Worktree,
 };
 
 pub struct GitStore {
@@ -1144,18 +1145,23 @@ impl GitStore {
                 } else {
                     removed_ids.push(*id);
                 }
-            } else if let Some((work_directory_abs_path, dot_git_abs_path)) = update
-                .new_work_directory_abs_path
-                .clone()
-                .zip(update.dot_git_abs_path.clone())
+            } else if let UpdatedGitRepository {
+                new_work_directory_abs_path: Some(work_directory_abs_path),
+                dot_git_abs_path: Some(dot_git_abs_path),
+                repository_dir_abs_path: Some(repository_dir_abs_path),
+                common_dir_abs_path: Some(common_dir_abs_path),
+                ..
+            } = update
             {
                 let id = RepositoryId(next_repository_id.fetch_add(1, atomic::Ordering::Release));
                 let git_store = cx.weak_entity();
                 let repo = cx.new(|cx| {
                     let mut repo = Repository::local(
                         id,
-                        work_directory_abs_path,
-                        dot_git_abs_path,
+                        work_directory_abs_path.clone(),
+                        dot_git_abs_path.clone(),
+                        repository_dir_abs_path.clone(),
+                        common_dir_abs_path.clone(),
                         project_environment.downgrade(),
                         fs.clone(),
                         git_store,
@@ -2542,6 +2548,8 @@ impl Repository {
         id: RepositoryId,
         work_directory_abs_path: Arc<Path>,
         dot_git_abs_path: Arc<Path>,
+        repository_dir_abs_path: Arc<Path>,
+        common_dir_abs_path: Arc<Path>,
         project_environment: WeakEntity<ProjectEnvironment>,
         fs: Arc<dyn Fs>,
         git_store: WeakEntity<GitStore>,
@@ -2559,6 +2567,8 @@ impl Repository {
             job_sender: Repository::spawn_local_git_worker(
                 work_directory_abs_path,
                 dot_git_abs_path,
+                repository_dir_abs_path,
+                common_dir_abs_path,
                 project_environment,
                 fs,
                 cx,
@@ -3836,6 +3846,8 @@ impl Repository {
     fn spawn_local_git_worker(
         work_directory_abs_path: Arc<Path>,
         dot_git_abs_path: Arc<Path>,
+        repository_dir_abs_path: Arc<Path>,
+        common_dir_abs_path: Arc<Path>,
         project_environment: WeakEntity<ProjectEnvironment>,
         fs: Arc<dyn Fs>,
         cx: &mut Context<Self>,
@@ -3861,6 +3873,9 @@ impl Repository {
                 })
                 .await?;
 
+            debug_assert_eq!(backend.path().as_path(), repository_dir_abs_path.as_ref());
+            debug_assert_eq!(backend.main_repository_path().as_path(), common_dir_abs_path.as_ref());
+
             if let Some(git_hosting_provider_registry) =
                 cx.update(|cx| GitHostingProviderRegistry::try_global(cx))?
             {
@@ -4092,6 +4107,10 @@ impl Repository {
     pub fn current_job(&self) -> Option<JobInfo> {
         self.active_jobs.values().next().cloned()
     }
+
+    pub fn barrier(&mut self) -> oneshot::Receiver<()> {
+        self.send_job(None, |_, _| async {})
+    }
 }
 
 fn get_permalink_in_rust_registry_src(

crates/project/src/project.rs 🔗

@@ -48,6 +48,8 @@ use debugger::{
     session::Session,
 };
 pub use environment::ProjectEnvironment;
+#[cfg(test)]
+use futures::future::join_all;
 use futures::{
     StreamExt,
     channel::mpsc::{self, UnboundedReceiver},
@@ -4808,6 +4810,30 @@ impl Project {
         &self.git_store
     }
 
+    #[cfg(test)]
+    fn git_scans_complete(&self, cx: &Context<Self>) -> Task<()> {
+        cx.spawn(async move |this, cx| {
+            let scans_complete = this
+                .read_with(cx, |this, cx| {
+                    this.worktrees(cx)
+                        .filter_map(|worktree| Some(worktree.read(cx).as_local()?.scan_complete()))
+                        .collect::<Vec<_>>()
+                })
+                .unwrap();
+            join_all(scans_complete).await;
+            let barriers = this
+                .update(cx, |this, cx| {
+                    let repos = this.repositories(cx).values().cloned().collect::<Vec<_>>();
+                    repos
+                        .into_iter()
+                        .map(|repo| repo.update(cx, |repo, _| repo.barrier()))
+                        .collect::<Vec<_>>()
+                })
+                .unwrap();
+            join_all(barriers).await;
+        })
+    }
+
     pub fn active_repository(&self, cx: &App) -> Option<Entity<Repository>> {
         self.git_store.read(cx).active_repository()
     }

crates/project/src/project_tests.rs 🔗

@@ -7192,7 +7192,7 @@ async fn test_repository_and_path_for_project_path(
     let tree_id = tree.read_with(cx, |tree, _| tree.id());
     tree.read_with(cx, |tree, _| tree.as_local().unwrap().scan_complete())
         .await;
-    tree.flush_fs_events(cx).await;
+    cx.run_until_parked();
 
     project.read_with(cx, |project, cx| {
         let git_store = project.git_store().read(cx);
@@ -7233,7 +7233,7 @@ async fn test_repository_and_path_for_project_path(
     fs.remove_dir(path!("/root/dir1/.git").as_ref(), RemoveOptions::default())
         .await
         .unwrap();
-    tree.flush_fs_events(cx).await;
+    cx.run_until_parked();
 
     project.read_with(cx, |project, cx| {
         let git_store = project.git_store().read(cx);
@@ -7493,49 +7493,51 @@ async fn test_git_status_postprocessing(cx: &mut gpui::TestAppContext) {
 }
 
 #[gpui::test]
-async fn test_repository_subfolder_git_status(cx: &mut gpui::TestAppContext) {
+async fn test_repository_subfolder_git_status(
+    executor: gpui::BackgroundExecutor,
+    cx: &mut gpui::TestAppContext,
+) {
     init_test(cx);
-    cx.executor().allow_parking();
 
-    let root = TempTree::new(json!({
-        "my-repo": {
-            // .git folder will go here
-            "a.txt": "a",
-            "sub-folder-1": {
-                "sub-folder-2": {
-                    "c.txt": "cc",
-                    "d": {
-                        "e.txt": "eee"
-                    }
-                },
-            }
-        },
-    }));
+    let fs = FakeFs::new(executor);
+    fs.insert_tree(
+        path!("/root"),
+        json!({
+            "my-repo": {
+                ".git": {},
+                "a.txt": "a",
+                "sub-folder-1": {
+                    "sub-folder-2": {
+                        "c.txt": "cc",
+                        "d": {
+                            "e.txt": "eee"
+                        }
+                    },
+                }
+            },
+        }),
+    )
+    .await;
 
     const C_TXT: &str = "sub-folder-1/sub-folder-2/c.txt";
     const E_TXT: &str = "sub-folder-1/sub-folder-2/d/e.txt";
 
-    // Set up git repository before creating the worktree.
-    let git_repo_work_dir = root.path().join("my-repo");
-    let repo = git_init(git_repo_work_dir.as_path());
-    git_add(C_TXT, &repo);
-    git_commit("Initial commit", &repo);
-
-    // Open the worktree in subfolder
-    let project_root = Path::new("my-repo/sub-folder-1/sub-folder-2");
+    fs.set_status_for_repo(
+        path!("/root/my-repo/.git").as_ref(),
+        &[(E_TXT.as_ref(), FileStatus::Untracked)],
+    );
 
     let project = Project::test(
-        Arc::new(RealFs::new(None, cx.executor())),
-        [root.path().join(project_root).as_path()],
+        fs.clone(),
+        [path!("/root/my-repo/sub-folder-1/sub-folder-2").as_ref()],
         cx,
     )
     .await;
 
-    let tree = project.read_with(cx, |project, cx| project.worktrees(cx).next().unwrap());
-    tree.flush_fs_events(cx).await;
-    cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete())
+    project
+        .update(cx, |project, cx| project.git_scans_complete(cx))
         .await;
-    cx.executor().run_until_parked();
+    cx.run_until_parked();
 
     let repository = project.read_with(cx, |project, cx| {
         project.repositories(cx).values().next().unwrap().clone()
@@ -7544,8 +7546,8 @@ async fn test_repository_subfolder_git_status(cx: &mut gpui::TestAppContext) {
     // Ensure that the git status is loaded correctly
     repository.read_with(cx, |repository, _cx| {
         assert_eq!(
-            repository.work_directory_abs_path.canonicalize().unwrap(),
-            root.path().join("my-repo").canonicalize().unwrap()
+            repository.work_directory_abs_path,
+            Path::new(path!("/root/my-repo")).into()
         );
 
         assert_eq!(repository.status_for_path(&C_TXT.into()), None);
@@ -7555,13 +7557,11 @@ async fn test_repository_subfolder_git_status(cx: &mut gpui::TestAppContext) {
         );
     });
 
-    // Now we simulate FS events, but ONLY in the .git folder that's outside
-    // of out project root.
-    // Meaning: we don't produce any FS events for files inside the project.
-    git_add(E_TXT, &repo);
-    git_commit("Second commit", &repo);
-    tree.flush_fs_events_in_root_git_repository(cx).await;
-    cx.executor().run_until_parked();
+    fs.set_status_for_repo(path!("/root/my-repo/.git").as_ref(), &[]);
+    project
+        .update(cx, |project, cx| project.git_scans_complete(cx))
+        .await;
+    cx.run_until_parked();
 
     repository.read_with(cx, |repository, _cx| {
         assert_eq!(repository.status_for_path(&C_TXT.into()), None);
@@ -8182,6 +8182,104 @@ async fn test_rescan_with_gitignore(cx: &mut gpui::TestAppContext) {
     });
 }
 
+#[gpui::test]
+async fn test_git_worktrees_and_submodules(cx: &mut gpui::TestAppContext) {
+    init_test(cx);
+
+    let fs = FakeFs::new(cx.executor());
+    fs.insert_tree(
+        path!("/project"),
+        json!({
+            ".git": {
+                "worktrees": {
+                    "some-worktree": {}
+                },
+            },
+            "src": {
+                "a.txt": "A",
+            },
+            "some-worktree": {
+                ".git": "gitdir: ../.git/worktrees/some-worktree",
+                "src": {
+                    "b.txt": "B",
+                }
+            }
+        }),
+    )
+    .await;
+
+    let project = Project::test(fs.clone(), [path!("/project").as_ref()], cx).await;
+    let scan_complete = project.update(cx, |project, cx| {
+        project
+            .worktrees(cx)
+            .next()
+            .unwrap()
+            .read(cx)
+            .as_local()
+            .unwrap()
+            .scan_complete()
+    });
+    scan_complete.await;
+
+    let mut repositories = project.update(cx, |project, cx| {
+        project
+            .repositories(cx)
+            .values()
+            .map(|repo| repo.read(cx).work_directory_abs_path.clone())
+            .collect::<Vec<_>>()
+    });
+    repositories.sort();
+    pretty_assertions::assert_eq!(
+        repositories,
+        [
+            Path::new(path!("/project")).into(),
+            Path::new(path!("/project/some-worktree")).into(),
+        ]
+    );
+
+    fs.with_git_state(
+        path!("/project/some-worktree/.git").as_ref(),
+        true,
+        |state| {
+            state
+                .head_contents
+                .insert("src/b.txt".into(), "b".to_owned());
+            state
+                .index_contents
+                .insert("src/b.txt".into(), "b".to_owned());
+        },
+    )
+    .unwrap();
+    cx.run_until_parked();
+
+    let buffer = project
+        .update(cx, |project, cx| {
+            project.open_local_buffer(path!("/project/some-worktree/src/b.txt"), cx)
+        })
+        .await
+        .unwrap();
+    let (worktree_repo, barrier) = project.update(cx, |project, cx| {
+        let (repo, _) = project
+            .git_store()
+            .read(cx)
+            .repository_and_path_for_buffer_id(buffer.read(cx).remote_id(), cx)
+            .unwrap();
+        pretty_assertions::assert_eq!(
+            repo.read(cx).work_directory_abs_path,
+            Path::new(path!("/project/some-worktree")).into(),
+        );
+        let barrier = repo.update(cx, |repo, _| repo.barrier());
+        (repo.clone(), barrier)
+    });
+    barrier.await.unwrap();
+    worktree_repo.update(cx, |repo, _| {
+        pretty_assertions::assert_eq!(
+            repo.status_for_path(&"src/b.txt".into()).unwrap().status,
+            StatusCode::Modified.worktree(),
+        );
+    });
+}
+
 #[gpui::test]
 async fn test_repository_deduplication(cx: &mut gpui::TestAppContext) {
     init_test(cx);

crates/worktree/src/worktree.rs 🔗

@@ -384,12 +384,23 @@ struct LocalRepositoryEntry {
     work_directory: WorkDirectory,
     work_directory_abs_path: Arc<Path>,
     git_dir_scan_id: usize,
-    original_dot_git_abs_path: Arc<Path>,
-    /// Absolute path to the actual .git folder.
-    /// Note: if .git is a file, this points to the folder indicated by the .git file
-    dot_git_dir_abs_path: Arc<Path>,
-    /// Absolute path to the .git file, if we're in a git worktree.
-    dot_git_worktree_abs_path: Option<Arc<Path>>,
+    /// Absolute path to the original .git entry that caused us to create this repository.
+    ///
+    /// This is normally a directory, but may be a "gitfile" that points to a directory elsewhere
+    /// (whose path we then store in `repository_dir_abs_path`).
+    dot_git_abs_path: Arc<Path>,
+    /// Absolute path to the "commondir" for this repository.
+    ///
+    /// This is always a directory. For a normal repository, this is the same as dot_git_abs_path,
+    /// but in the case of a submodule or a worktree it is the path to the "parent" .git directory
+    /// from which the submodule/worktree was derived.
+    common_dir_abs_path: Arc<Path>,
+    /// Absolute path to the directory holding the repository's state.
+    ///
+    /// For a normal repository, this is a directory and coincides with `dot_git_abs_path` and
+    /// `common_dir_abs_path`. For a submodule or worktree, this is some subdirectory of the
+    /// commondir like `/project/.git/modules/foo`.
+    repository_dir_abs_path: Arc<Path>,
 }
 
 impl sum_tree::Item for LocalRepositoryEntry {
@@ -1351,7 +1362,11 @@ impl LocalWorktree {
                                 new_work_directory_abs_path: Some(
                                     new_repo.work_directory_abs_path.clone(),
                                 ),
-                                dot_git_abs_path: Some(new_repo.original_dot_git_abs_path.clone()),
+                                dot_git_abs_path: Some(new_repo.dot_git_abs_path.clone()),
+                                repository_dir_abs_path: Some(
+                                    new_repo.repository_dir_abs_path.clone(),
+                                ),
+                                common_dir_abs_path: Some(new_repo.common_dir_abs_path.clone()),
                             });
                             new_repos.next();
                         }
@@ -1368,9 +1383,11 @@ impl LocalWorktree {
                                     new_work_directory_abs_path: Some(
                                         new_repo.work_directory_abs_path.clone(),
                                     ),
-                                    dot_git_abs_path: Some(
-                                        new_repo.original_dot_git_abs_path.clone(),
+                                    dot_git_abs_path: Some(new_repo.dot_git_abs_path.clone()),
+                                    repository_dir_abs_path: Some(
+                                        new_repo.repository_dir_abs_path.clone(),
                                     ),
+                                    common_dir_abs_path: Some(new_repo.common_dir_abs_path.clone()),
                                 });
                             }
                             new_repos.next();
@@ -1384,6 +1401,8 @@ impl LocalWorktree {
                                 ),
                                 new_work_directory_abs_path: None,
                                 dot_git_abs_path: None,
+                                repository_dir_abs_path: None,
+                                common_dir_abs_path: None,
                             });
                             old_repos.next();
                         }
@@ -1394,7 +1413,9 @@ impl LocalWorktree {
                         work_directory_id: entry_id,
                         old_work_directory_abs_path: None,
                         new_work_directory_abs_path: Some(repo.work_directory_abs_path.clone()),
-                        dot_git_abs_path: Some(repo.original_dot_git_abs_path.clone()),
+                        dot_git_abs_path: Some(repo.dot_git_abs_path.clone()),
+                        repository_dir_abs_path: Some(repo.repository_dir_abs_path.clone()),
+                        common_dir_abs_path: Some(repo.common_dir_abs_path.clone()),
                     });
                     new_repos.next();
                 }
@@ -1403,7 +1424,9 @@ impl LocalWorktree {
                         work_directory_id: entry_id,
                         old_work_directory_abs_path: Some(repo.work_directory_abs_path.clone()),
                         new_work_directory_abs_path: None,
-                        dot_git_abs_path: Some(repo.original_dot_git_abs_path.clone()),
+                        dot_git_abs_path: Some(repo.dot_git_abs_path.clone()),
+                        repository_dir_abs_path: Some(repo.repository_dir_abs_path.clone()),
+                        common_dir_abs_path: Some(repo.common_dir_abs_path.clone()),
                     });
                     old_repos.next();
                 }
@@ -3042,9 +3065,6 @@ impl BackgroundScannerState {
                     );
                     return;
                 };
-                log::debug!(
-                    "building git repository, `.git` path in the worktree: {dot_git_path:?}"
-                );
 
                 parent_dir.into()
             }
@@ -3075,7 +3095,6 @@ impl BackgroundScannerState {
         fs: &dyn Fs,
         watcher: &dyn Watcher,
     ) -> Option<LocalRepositoryEntry> {
-        log::trace!("insert git repository for {dot_git_path:?}");
         let work_dir_entry = self.snapshot.entry_for_path(work_directory.path_key().0)?;
         let work_directory_abs_path = self
             .snapshot
@@ -3092,46 +3111,51 @@ impl BackgroundScannerState {
             return None;
         }
 
-        let dot_git_abs_path = self.snapshot.abs_path.as_path().join(&dot_git_path);
-
-        // TODO add these watchers without building a whole repository by parsing .git-with-indirection
-        let t0 = Instant::now();
-        let repository = fs.open_repo(&dot_git_abs_path)?;
-        log::trace!("opened git repo for {dot_git_abs_path:?}");
-
-        let repository_path = repository.path();
-        watcher.add(&repository_path).log_err()?;
-
-        let actual_dot_git_dir_abs_path = repository.main_repository_path();
-        let dot_git_worktree_abs_path = if actual_dot_git_dir_abs_path == dot_git_abs_path {
-            None
-        } else {
-            // The two paths could be different because we opened a git worktree.
-            // When that happens:
-            //
-            // * `dot_git_abs_path` is a file that points to the worktree-subdirectory in the actual
-            // .git directory.
-            //
-            // * `repository_path` is the worktree-subdirectory.
-            //
-            // * `actual_dot_git_dir_abs_path` is the path to the actual .git directory. In git
-            // documentation this is called the "commondir".
-            watcher.add(&dot_git_abs_path).log_err()?;
-            Some(Arc::from(dot_git_abs_path.as_path()))
+        let dot_git_abs_path: Arc<Path> = self
+            .snapshot
+            .abs_path
+            .as_path()
+            .join(&dot_git_path)
+            .as_path()
+            .into();
+
+        let mut common_dir_abs_path = dot_git_abs_path.clone();
+        let mut repository_dir_abs_path = dot_git_abs_path.clone();
+        // Parse .git if it's a "gitfile" pointing to a repository directory elsewhere.
+        if let Some(dot_git_contents) = smol::block_on(fs.load(&dot_git_abs_path)).ok() {
+            if let Some(path) = dot_git_contents.strip_prefix("gitdir:") {
+                let path = path.trim();
+                let path = dot_git_abs_path
+                    .parent()
+                    .unwrap_or(Path::new(""))
+                    .join(path);
+                if let Some(path) = smol::block_on(fs.canonicalize(&path)).log_err() {
+                    repository_dir_abs_path = Path::new(&path).into();
+                    common_dir_abs_path = repository_dir_abs_path.clone();
+                    if let Some(ancestor_dot_git) = path
+                        .ancestors()
+                        .skip(1)
+                        .find(|ancestor| smol::block_on(is_git_dir(ancestor, fs)))
+                    {
+                        common_dir_abs_path = ancestor_dot_git.into();
+                    }
+                }
+            } else {
+                log::error!("failed to parse contents of .git file: {dot_git_contents:?}");
+            }
         };
-
-        log::trace!("constructed libgit2 repo in {:?}", t0.elapsed());
+        watcher.add(&common_dir_abs_path).log_err();
 
         let work_directory_id = work_dir_entry.id;
 
         let local_repository = LocalRepositoryEntry {
             work_directory_id,
             work_directory,
-            git_dir_scan_id: 0,
-            original_dot_git_abs_path: dot_git_abs_path.as_path().into(),
-            dot_git_dir_abs_path: actual_dot_git_dir_abs_path.into(),
             work_directory_abs_path: work_directory_abs_path.as_path().into(),
-            dot_git_worktree_abs_path,
+            git_dir_scan_id: 0,
+            dot_git_abs_path,
+            common_dir_abs_path,
+            repository_dir_abs_path,
         };
 
         self.snapshot
@@ -3454,6 +3478,8 @@ pub struct UpdatedGitRepository {
     /// For a normal git repository checkout, the absolute path to the .git directory.
     /// For a worktree, the absolute path to the worktree's subdirectory inside the .git directory.
     pub dot_git_abs_path: Option<Arc<Path>>,
+    pub repository_dir_abs_path: Option<Arc<Path>>,
+    pub common_dir_abs_path: Option<Arc<Path>>,
 }
 
 pub type UpdatedEntriesSet = Arc<[(Arc<Path>, ProjectEntryId, PathChange)]>;
@@ -4010,8 +4036,8 @@ impl BackgroundScanner {
 
                 if abs_path.0.file_name() == Some(*GITIGNORE) {
                     for (_, repo) in snapshot.git_repositories.iter().filter(|(_, repo)| repo.directory_contains(&relative_path)) {
-                        if !dot_git_abs_paths.iter().any(|dot_git_abs_path| dot_git_abs_path == repo.dot_git_dir_abs_path.as_ref()) {
-                            dot_git_abs_paths.push(repo.dot_git_dir_abs_path.to_path_buf());
+                        if !dot_git_abs_paths.iter().any(|dot_git_abs_path| dot_git_abs_path == repo.common_dir_abs_path.as_ref()) {
+                            dot_git_abs_paths.push(repo.common_dir_abs_path.to_path_buf());
                         }
                     }
                 }
@@ -4070,7 +4096,6 @@ impl BackgroundScanner {
             }
         }
         self.send_status_update(false, SmallVec::new());
-        // send_status_update_inner(phase, state, status_update_tx, false, SmallVec::new());
     }
 
     async fn forcibly_load_paths(&self, paths: &[Arc<Path>]) -> bool {
@@ -4709,8 +4734,8 @@ impl BackgroundScanner {
                     .git_repositories
                     .iter()
                     .find_map(|(_, repo)| {
-                        if repo.dot_git_dir_abs_path.as_ref() == &dot_git_dir
-                            || repo.dot_git_worktree_abs_path.as_deref() == Some(&dot_git_dir)
+                        if repo.common_dir_abs_path.as_ref() == &dot_git_dir
+                            || repo.repository_dir_abs_path.as_ref() == &dot_git_dir
                         {
                             Some(repo.clone())
                         } else {
@@ -4752,7 +4777,7 @@ impl BackgroundScanner {
 
             if exists_in_snapshot
                 || matches!(
-                    smol::block_on(self.fs.metadata(&entry.dot_git_dir_abs_path)),
+                    smol::block_on(self.fs.metadata(&entry.common_dir_abs_path)),
                     Ok(Some(_))
                 )
             {
@@ -5081,7 +5106,7 @@ impl WorktreeModelHandle for Entity<Worktree> {
                 .unwrap();
             (
                 tree.fs.clone(),
-                local_repo_entry.dot_git_dir_abs_path.clone(),
+                local_repo_entry.common_dir_abs_path.clone(),
                 local_repo_entry.git_dir_scan_id,
             )
         });