Added git repository type infrastructure and moved git file system stuff into fs abstraction so we can test without touching the file system. Co-Authored-By: kay@zed.dev

crates/project/src/fs.rs

@@ -12,6 +12,7 @@ use std::{
     pin::Pin,
     time::{Duration, SystemTime},
 };
+
 use text::Rope;
 
 #[cfg(any(test, feature = "test-support"))]
@@ -21,6 +22,8 @@ use futures::lock::Mutex;
 #[cfg(any(test, feature = "test-support"))]
 use std::sync::{Arc, Weak};
 
+use crate::git_repository::{FakeGitRepository, GitRepository, RealGitRepository};
+
 #[async_trait::async_trait]
 pub trait Fs: Send + Sync {
     async fn create_dir(&self, path: &Path) -> Result<()>;
@@ -45,6 +48,11 @@ pub trait Fs: Send + Sync {
         path: &Path,
         latency: Duration,
     ) -> Pin<Box<dyn Send + Stream<Item = Vec<fsevent::Event>>>>;
+    fn open_git_repository(
+        &self,
+        abs_dotgit_path: &Path,
+        content_path: &Arc<Path>,
+    ) -> Option<Box<dyn GitRepository>>;
     fn is_fake(&self) -> bool;
     #[cfg(any(test, feature = "test-support"))]
     fn as_fake(&self) -> &FakeFs;
@@ -270,6 +278,14 @@ impl Fs for RealFs {
         })))
     }
 
+    fn open_git_repository(
+        &self,
+        abs_dotgit_path: &Path,
+        content_path: &Arc<Path>,
+    ) -> Option<Box<dyn GitRepository>> {
+        RealGitRepository::open(abs_dotgit_path, content_path)
+    }
+
     fn is_fake(&self) -> bool {
         false
     }
@@ -885,6 +901,17 @@ impl Fs for FakeFs {
         }))
     }
 
+    fn open_git_repository(
+        &self,
+        abs_dotgit_path: &Path,
+        content_path: &Arc<Path>,
+    ) -> Option<Box<dyn GitRepository>> {
+        Some(Box::new(FakeGitRepository::new(
+            abs_dotgit_path,
+            content_path,
+        )))
+    }
+
     fn is_fake(&self) -> bool {
         true
     }

crates/project/src/git_repository.rs

@@ -0,0 +1,132 @@
+use git2::Repository;
+use parking_lot::Mutex;
+use std::{path::Path, sync::Arc};
+use util::ResultExt;
+
+pub trait GitRepository: Send + Sync {
+    fn boxed_clone(&self) -> Box<dyn GitRepository>;
+    fn is_path_managed_by(&self, path: &Path) -> bool;
+    fn is_path_in_git_folder(&self, path: &Path) -> bool;
+    fn content_path(&self) -> &Path;
+    fn git_dir_path(&self) -> &Path;
+    fn last_scan_id(&self) -> usize;
+    fn set_scan_id(&mut self, scan_id: usize);
+}
+
+#[derive(Clone)]
+pub struct RealGitRepository {
+    // Path to folder containing the .git file or directory
+    content_path: Arc<Path>,
+    // Path to the actual .git folder.
+    // Note: if .git is a file, this points to the folder indicated by the .git file
+    git_dir_path: Arc<Path>,
+    last_scan_id: usize,
+    libgit_repository: Arc<Mutex<git2::Repository>>,
+}
+
+impl RealGitRepository {
+    pub fn open(
+        abs_dotgit_path: &Path,
+        content_path: &Arc<Path>,
+    ) -> Option<Box<dyn GitRepository>> {
+        Repository::open(&abs_dotgit_path)
+            .log_err()
+            .map::<Box<dyn GitRepository>, _>(|libgit_repository| {
+                Box::new(Self {
+                    content_path: content_path.clone(),
+                    git_dir_path: libgit_repository.path().into(),
+                    last_scan_id: 0,
+                    libgit_repository: Arc::new(parking_lot::Mutex::new(libgit_repository)),
+                })
+            })
+    }
+}
+
+impl GitRepository for RealGitRepository {
+    fn boxed_clone(&self) -> Box<dyn GitRepository> {
+        Box::new(self.clone())
+    }
+
+    fn is_path_managed_by(&self, path: &Path) -> bool {
+        path.starts_with(&self.content_path)
+    }
+
+    fn is_path_in_git_folder(&self, path: &Path) -> bool {
+        path.starts_with(&self.git_dir_path)
+    }
+
+    fn content_path(&self) -> &Path {
+        self.content_path.as_ref()
+    }
+
+    fn git_dir_path(&self) -> &Path {
+        self.git_dir_path.as_ref()
+    }
+
+    fn last_scan_id(&self) -> usize {
+        self.last_scan_id
+    }
+
+    fn set_scan_id(&mut self, scan_id: usize) {
+        self.last_scan_id = scan_id;
+    }
+}
+
+impl PartialEq for &Box<dyn GitRepository> {
+    fn eq(&self, other: &Self) -> bool {
+        self.content_path() == other.content_path()
+    }
+}
+impl Eq for &Box<dyn GitRepository> {}
+
+#[cfg(any(test, feature = "test-support"))]
+#[derive(Clone)]
+pub struct FakeGitRepository {
+    // Path to folder containing the .git file or directory
+    content_path: Arc<Path>,
+    // Path to the actual .git folder.
+    // Note: if .git is a file, this points to the folder indicated by the .git file
+    git_dir_path: Arc<Path>,
+    last_scan_id: usize,
+}
+
+impl FakeGitRepository {
+    pub fn new(abs_dotgit_path: &Path, content_path: &Arc<Path>) -> FakeGitRepository {
+        Self {
+            content_path: content_path.clone(),
+            git_dir_path: abs_dotgit_path.into(),
+            last_scan_id: 0,
+        }
+    }
+}
+
+#[cfg(any(test, feature = "test-support"))]
+impl GitRepository for FakeGitRepository {
+    fn boxed_clone(&self) -> Box<dyn GitRepository> {
+        Box::new(self.clone())
+    }
+
+    fn is_path_managed_by(&self, path: &Path) -> bool {
+        path.starts_with(&self.content_path)
+    }
+
+    fn is_path_in_git_folder(&self, path: &Path) -> bool {
+        path.starts_with(&self.git_dir_path)
+    }
+
+    fn content_path(&self) -> &Path {
+        self.content_path.as_ref()
+    }
+
+    fn git_dir_path(&self) -> &Path {
+        self.git_dir_path.as_ref()
+    }
+
+    fn last_scan_id(&self) -> usize {
+        self.last_scan_id
+    }
+
+    fn set_scan_id(&mut self, scan_id: usize) {
+        self.last_scan_id = scan_id;
+    }
+}

crates/project/src/project.rs

@@ -1,4 +1,5 @@
 pub mod fs;
+mod git_repository;
 mod ignore;
 mod lsp_command;
 pub mod search;

crates/project/src/worktree.rs

@@ -1,4 +1,4 @@
-use crate::{copy_recursive, ProjectEntryId, RemoveOptions};
+use crate::{copy_recursive, git_repository::GitRepository, ProjectEntryId, RemoveOptions};
 
 use super::{
     fs::{self, Fs},
@@ -18,7 +18,6 @@ use futures::{
     Stream, StreamExt,
 };
 use fuzzy::CharBag;
-use git2::Repository;
 use gpui::{
     executor, AppContext, AsyncAppContext, Entity, ModelContext, ModelHandle, MutableAppContext,
     Task,
@@ -104,41 +103,32 @@ pub struct Snapshot {
     is_complete: bool,
 }
 
-//
-
-// 'GitResolver'
-// File paths <-> Repository Paths  -> git_repository_path() -> First .git in an ancestor in a path
-// Repository Paths <-> Repository Pointers -> git_repository_open()
-// fs.watch()  ^
-//
-// Folder: where all the git magic happens
-// .git IT
-// OR it can be a file that points somewhere else
-
-// 1. Walk through the file tree, looking for .git files or folders
-// 2. When we discover them, open and save a libgit2 pointer to the repository
-// 2a. Use git_repository_path() to start a watch on the repository (if not already watched)
-//
-// File paths -> Git repository == Ancestor check (is there a .git in an ancestor folder)
-// Git repository -> Files == Descendent check (subtracting out any intersecting .git folders)
-
-#[derive(Clone)]
 pub struct LocalSnapshot {
     abs_path: Arc<Path>,
     ignores_by_parent_abs_path: HashMap<Arc<Path>, (Arc<Gitignore>, usize)>,
-    git_repositories: Vec<GitRepositoryState>,
+    git_repositories: Vec<Box<dyn GitRepository>>,
     removed_entry_ids: HashMap<u64, ProjectEntryId>,
     next_entry_id: Arc<AtomicUsize>,
     snapshot: Snapshot,
     extension_counts: HashMap<OsString, usize>,
 }
 
-#[derive(Clone)]
+impl Clone for LocalSnapshot {
-pub struct GitRepositoryState {
+    fn clone(&self) -> Self {
-    content_path: Arc<Path>,
+        Self {
-    git_dir_path: Arc<Path>,
+            abs_path: self.abs_path.clone(),
-    scan_id: usize,
+            ignores_by_parent_abs_path: self.ignores_by_parent_abs_path.clone(),
-    repository: Arc<Mutex<git2::Repository>>,
+            git_repositories: self
+                .git_repositories
+                .iter()
+                .map(|repo| repo.boxed_clone())
+                .collect(),
+            removed_entry_ids: self.removed_entry_ids.clone(),
+            next_entry_id: self.next_entry_id.clone(),
+            snapshot: self.snapshot.clone(),
+            extension_counts: self.extension_counts.clone(),
+        }
+    }
 }
 
 impl Deref for LocalSnapshot {
@@ -173,7 +163,7 @@ struct ShareState {
 
 pub enum Event {
     UpdatedEntries,
-    UpdatedGitRepositories(Vec<GitRepositoryState>),
+    UpdatedGitRepositories(Vec<Box<dyn GitRepository>>),
 }
 
 impl Entity for Worktree {
@@ -1322,31 +1312,47 @@ impl LocalSnapshot {
         &self.extension_counts
     }
 
-    pub(crate) fn git_repository_for_file_path(&self, path: &Path) -> Option<GitRepositoryState> {
+    // Gives the most specific git repository for a given path
-        for repository in self.git_repositories.iter().rev() {
+    pub(crate) fn git_repository_for_file_path(
-            if path.starts_with(&repository.content_path) {
+        &self,
-                return Some(repository.clone());
+        path: &Path,
-            }
+    ) -> Option<Box<dyn GitRepository>> {
-        }
+        self.git_repositories
-        None
+            .iter()
+            .rev() //git_repository is ordered lexicographically
+            .find(|repo| repo.is_path_managed_by(path))
+            .map(|repo| repo.boxed_clone())
     }
 
-    pub(crate) fn git_repository_for_git_data(&self, path: &Path) -> Option<GitRepositoryState> {
+    //  ~/zed:
-        for repository in self.git_repositories.iter() {
+    //   - src
-            if path.starts_with(&repository.git_dir_path) {
+    //   - crates
-                return Some(repository.clone());
+    //   - .git -> /usr/.git
-            }
+    pub(crate) fn git_repository_for_git_data(
-        }
+        &self,
-        None
+        path: &Path,
+    ) -> Option<Box<dyn GitRepository>> {
+        self.git_repositories
+            .iter()
+            .find(|repo| repo.is_path_in_git_folder(path))
+            .map(|repo| repo.boxed_clone())
     }
 
     pub(crate) fn does_git_repository_track_file_path(
         &self,
-        repo: &GitRepositoryState,
+        repo: &Box<dyn GitRepository>,
         file_path: &Path,
     ) -> bool {
+        // /zed
+        //  - .git
+        //  - a.txt
+        //  - /dep
+        //      - b.txt
+        //      - .git
+
+        // Depends on git_repository_for_file_path returning the most specific git repository for a given path
         self.git_repository_for_file_path(file_path)
-            .map_or(false, |r| r.content_path == repo.content_path)
+            .map_or(false, |r| &r == repo)
     }
 
     #[cfg(test)]
@@ -1431,7 +1437,7 @@ impl LocalSnapshot {
     }
 
     fn insert_entry(&mut self, mut entry: Entry, fs: &dyn Fs) -> Entry {
-        if !entry.is_dir() && entry.path.file_name() == Some(&GITIGNORE) {
+        if entry.is_file() && entry.path.file_name() == Some(&GITIGNORE) {
             let abs_path = self.abs_path.join(&entry.path);
             match smol::block_on(build_gitignore(&abs_path, fs)) {
                 Ok(ignore) => {
@@ -1453,18 +1459,10 @@ impl LocalSnapshot {
             let content_path: Arc<Path> = entry.path.parent().unwrap().into();
             if let Err(ix) = self
                 .git_repositories
-                .binary_search_by_key(&&content_path, |repo| &repo.content_path)
+                .binary_search_by_key(&content_path.as_ref(), |repo| repo.content_path())
             {
-                if let Some(repository) = Repository::open(&abs_path).log_err() {
+                if let Some(repository) = fs.open_git_repository(&abs_path, &content_path) {
-                    self.git_repositories.insert(
+                    self.git_repositories.insert(ix, repository);
-                        ix,
-                        GitRepositoryState {
-                            content_path,
-                            git_dir_path: repository.path().into(),
-                            scan_id: self.scan_id,
-                            repository: Arc::new(Mutex::new(repository)),
-                        },
-                    );
                 }
             }
         }
@@ -1617,9 +1615,9 @@ impl LocalSnapshot {
             let parent_path = path.parent().unwrap();
             if let Ok(ix) = self
                 .git_repositories
-                .binary_search_by_key(&parent_path, |repo| repo.content_path.as_ref())
+                .binary_search_by_key(&parent_path, |repo| repo.content_path().as_ref())
             {
-                self.git_repositories[ix].scan_id = self.snapshot.scan_id;
+                self.git_repositories[ix].set_scan_id(self.snapshot.scan_id);
             }
         }
     }
@@ -2565,7 +2563,7 @@ impl BackgroundScanner {
         let mut snapshot = self.snapshot();
         let mut git_repositories = mem::take(&mut snapshot.git_repositories);
         git_repositories.retain(|git_repository| {
-            let dot_git_path = git_repository.content_path.join(&*DOT_GIT);
+            let dot_git_path = git_repository.content_path().join(&*DOT_GIT);
             snapshot.entry_for_path(dot_git_path).is_some()
         });
         snapshot.git_repositories = git_repositories;
@@ -2925,6 +2923,7 @@ mod tests {
         fmt::Write,
         time::{SystemTime, UNIX_EPOCH},
     };
+
     use util::test::temp_tree;
 
     #[gpui::test]
@@ -3147,6 +3146,7 @@ mod tests {
     #[gpui::test]
     async fn test_git_repository_for_path(cx: &mut TestAppContext) {
         let fs = FakeFs::new(cx.background());
+
         fs.insert_tree(
             "/root",
             json!({
@@ -3200,17 +3200,22 @@ mod tests {
             // Need to update the file system for anything involving git
             // Goal: Make this test pass
             // Up Next: Invalidating git repos!
-            assert_eq!(repo.content_path.as_ref(), Path::new("dir1"));
+            assert_eq!(repo.content_path(), Path::new("dir1"));
-            assert_eq!(repo.git_dir_path.as_ref(), Path::new("dir1/.git"));
+            assert_eq!(repo.git_dir_path(), Path::new("dir1/.git"));
 
             let repo = tree
                 .git_repository_for_file_path("dir1/deps/dep1/src/a.txt".as_ref())
                 .unwrap();
 
-            assert_eq!(repo.content_path.as_ref(), Path::new("dir1/deps/dep1"));
+            assert_eq!(repo.content_path(), Path::new("dir1/deps/dep1"));
-            assert_eq!( repo = tree    .git_repository_for_git_data("dir/.git/HEAD".as_ref())
+            assert_eq!(repo.git_dir_path(), Path::new("dir1/deps/dep1"));
+
+            let repo = tree
+                .git_repository_for_git_data("dir1/.git/HEAD".as_ref())
                 .unwrap();
-            assert_eq!(repo.content_path.as_ref(), Path::new("dir1/deps/dep1"));
+
+            assert_eq!(repo.content_path(), Path::new("dir1"));
+            assert_eq!(repo.git_dir_path(), Path::new("dir1/.git"));
 
             assert!(tree.does_git_repository_track_file_path(&repo, "dir1/src/b.txt".as_ref()));
             assert!(!tree