WIP Start refactoring separation of concerns for repo metadata

Co-Authored-By: Max Brunsfeld <max@zed.dev>
Co-Authored-By: Mikayla Maki <mikayla@zed.dev>

crates/collab/src/integration_tests.rs

@@ -1008,9 +1008,7 @@ async fn test_git_head_text(
         .unwrap();
 
     // Wait for it to catch up to the new diff
-    buffer_a
+    executor.run_until_parked();
-        .condition(cx_a, |buffer, _| !buffer.is_recalculating_git_diff())
-        .await;
 
     // Smoke test diffing
     buffer_a.read_with(cx_a, |buffer, _| {
@@ -1029,7 +1027,8 @@ async fn test_git_head_text(
         .await
         .unwrap();
 
-    //TODO: WAIT FOR REMOTE UPDATES TO FINISH
+    // Wait remote buffer to catch up to the new diff
+    executor.run_until_parked();
 
     // Smoke test diffing
     buffer_b.read_with(cx_b, |buffer, _| {
@@ -1055,9 +1054,7 @@ async fn test_git_head_text(
     // TODO: Flush this file event
 
     // Wait for buffer_a to receive it
-    buffer_a
+    executor.run_until_parked();
-        .condition(cx_a, |buffer, _| !buffer.is_recalculating_git_diff())
-        .await;
 
     // Smoke test new diffing
     buffer_a.read_with(cx_a, |buffer, _| {
@@ -1071,6 +1068,7 @@ async fn test_git_head_text(
     });
 
     //TODO: WAIT FOR REMOTE UPDATES TO FINISH on B
+    executor.run_until_parked();
 
     // Smoke test B
     buffer_b.read_with(cx_b, |buffer, _| {

crates/git/src/repository.rs

@@ -2,88 +2,39 @@ use anyhow::Result;
 use collections::HashMap;
 use git2::Repository as LibGitRepository;
 use parking_lot::Mutex;
-use std::{
-    path::{Path, PathBuf},
-    sync::Arc,
-};
 use util::ResultExt;
+use std::{path::{Path, PathBuf}, sync::Arc};
 
 #[async_trait::async_trait]
-pub trait GitRepository: Send + Sync + std::fmt::Debug {
+pub trait GitRepository: Send {
-    fn manages(&self, path: &Path) -> bool;
+    // fn manages(&self, path: &Path) -> bool;
+    // fn reopen_git_repo(&mut self) -> bool;
+    // fn git_repo(&self) -> Arc<Mutex<LibGitRepository>>;
+    // fn boxed_clone(&self) -> Box<dyn GitRepository>;
 
-    fn in_dot_git(&self, path: &Path) -> bool;
+    fn load_head_text(&self, relative_file_path: &Path) -> Option<String>;
-
+    
-    fn content_path(&self) -> &Path;
+    fn open_real(dotgit_path: &Path) -> Option<Arc<Mutex<dyn GitRepository>>>
-
+    where Self: Sized
-    fn git_dir_path(&self) -> &Path;
+    {
-
-    fn scan_id(&self) -> usize;
-
-    fn set_scan_id(&mut self, scan_id: usize);
-
-    fn reopen_git_repo(&mut self) -> bool;
-
-    fn git_repo(&self) -> Arc<Mutex<LibGitRepository>>;
-
-    fn boxed_clone(&self) -> Box<dyn GitRepository>;
-
-    async fn load_head_text(&self, relative_file_path: &Path) -> Option<String>;
-}
-
-#[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>,
-    scan_id: usize,
-    libgit_repository: Arc<Mutex<LibGitRepository>>,
-}
-
-impl RealGitRepository {
-    pub fn open(dotgit_path: &Path) -> Option<Box<dyn GitRepository>> {
         LibGitRepository::open(&dotgit_path)
             .log_err()
-            .and_then::<Box<dyn GitRepository>, _>(|libgit_repository| {
+            .and_then::<Arc<Mutex<dyn GitRepository>>, _>(|libgit_repository| {
-                Some(Box::new(Self {
+                Some(Arc::new(Mutex::new(libgit_repository)))
-                    content_path: libgit_repository.workdir()?.into(),
-                    git_dir_path: dotgit_path.canonicalize().log_err()?.into(),
-                    scan_id: 0,
-                    libgit_repository: Arc::new(parking_lot::Mutex::new(libgit_repository)),
-                }))
             })
     }
 }
 
 #[async_trait::async_trait]
-impl GitRepository for RealGitRepository {
+impl GitRepository for LibGitRepository {
-    fn manages(&self, path: &Path) -> bool {
+    // fn manages(&self, path: &Path) -> bool {
-        path.canonicalize()
+    //     path.canonicalize()
-            .map(|path| path.starts_with(&self.content_path))
+    //         .map(|path| path.starts_with(&self.content_path))
-            .unwrap_or(false)
+    //         .unwrap_or(false)
-    }
+    // }
 
-    fn in_dot_git(&self, path: &Path) -> bool {
-        path.canonicalize()
-            .map(|path| path.starts_with(&self.git_dir_path))
-            .unwrap_or(false)
-    }
 
-    fn content_path(&self) -> &Path {
+    fn load_head_text(&self, relative_file_path: &Path) -> Option<String> {
-        self.content_path.as_ref()
-    }
-
-    fn git_dir_path(&self) -> &Path {
-        self.git_dir_path.as_ref()
-    }
-
-    fn scan_id(&self) -> usize {
-        self.scan_id
-    }
-
-    async fn load_head_text(&self, relative_file_path: &Path) -> Option<String> {
         fn logic(repo: &LibGitRepository, relative_file_path: &Path) -> Result<Option<String>> {
             const STAGE_NORMAL: i32 = 0;
             let index = repo.index()?;
@@ -97,53 +48,18 @@ impl GitRepository for RealGitRepository {
             Ok(Some(head_text))
         }
 
-        match logic(&self.libgit_repository.as_ref().lock(), relative_file_path) {
+        match logic(&self, relative_file_path) {
             Ok(value) => return value,
             Err(err) => log::error!("Error loading head text: {:?}", err),
         }
         None
     }
-
-    fn reopen_git_repo(&mut self) -> bool {
-        match LibGitRepository::open(&self.git_dir_path) {
-            Ok(repo) => {
-                self.libgit_repository = Arc::new(Mutex::new(repo));
-                true
-            }
-
-            Err(_) => false,
-        }
-    }
-
-    fn git_repo(&self) -> Arc<Mutex<LibGitRepository>> {
-        self.libgit_repository.clone()
-    }
-
-    fn set_scan_id(&mut self, scan_id: usize) {
-        self.scan_id = scan_id;
-    }
-
-    fn boxed_clone(&self) -> Box<dyn GitRepository> {
-        Box::new(self.clone())
-    }
-}
-
-impl std::fmt::Debug for RealGitRepository {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        f.debug_struct("GitRepository")
-            .field("content_path", &self.content_path)
-            .field("git_dir_path", &self.git_dir_path)
-            .field("scan_id", &self.scan_id)
-            .field("libgit_repository", &"LibGitRepository")
-            .finish()
-    }
 }
 
 #[derive(Debug, Clone)]
 pub struct FakeGitRepository {
     content_path: Arc<Path>,
     git_dir_path: Arc<Path>,
-    scan_id: usize,
     state: Arc<Mutex<FakeGitRepositoryState>>,
 }
 
@@ -153,15 +69,10 @@ pub struct FakeGitRepositoryState {
 }
 
 impl FakeGitRepository {
-    pub fn open(
+    pub fn open(dotgit_path: &Path, state: Arc<Mutex<FakeGitRepositoryState>>) -> Box<dyn GitRepository> {
-        dotgit_path: &Path,
-        scan_id: usize,
-        state: Arc<Mutex<FakeGitRepositoryState>>,
-    ) -> Box<dyn GitRepository> {
         Box::new(FakeGitRepository {
             content_path: dotgit_path.parent().unwrap().into(),
             git_dir_path: dotgit_path.into(),
-            scan_id,
             state,
         })
     }
@@ -173,9 +84,9 @@ impl GitRepository for FakeGitRepository {
         path.starts_with(self.content_path())
     }
 
-    fn in_dot_git(&self, path: &Path) -> bool {
+    // fn in_dot_git(&self, path: &Path) -> bool {
-        path.starts_with(self.git_dir_path())
+    //     path.starts_with(self.git_dir_path())
-    }
+    // }
 
     fn content_path(&self) -> &Path {
         &self.content_path
@@ -185,10 +96,6 @@ impl GitRepository for FakeGitRepository {
         &self.git_dir_path
     }
 
-    fn scan_id(&self) -> usize {
-        self.scan_id
-    }
-
     async fn load_head_text(&self, path: &Path) -> Option<String> {
         let state = self.state.lock();
         state.index_contents.get(path).cloned()
@@ -202,10 +109,6 @@ impl GitRepository for FakeGitRepository {
         unimplemented!()
     }
 
-    fn set_scan_id(&mut self, scan_id: usize) {
-        self.scan_id = scan_id;
-    }
-
     fn boxed_clone(&self) -> Box<dyn GitRepository> {
         Box::new(self.clone())
     }

crates/language/src/buffer.rs

@@ -676,10 +676,6 @@ impl Buffer {
         self.git_diff_status.diff.needs_update(self)
     }
 
-    pub fn is_recalculating_git_diff(&self) -> bool {
-        self.git_diff_status.update_in_progress
-    }
-
     pub fn git_diff_recalc(&mut self, cx: &mut ModelContext<Self>) {
         if self.git_diff_status.update_in_progress {
             self.git_diff_status.update_requested = true;

crates/project/src/fs.rs

@@ -1,7 +1,7 @@
 use anyhow::{anyhow, Result};
 use fsevent::EventStream;
 use futures::{future::BoxFuture, Stream, StreamExt};
-use git::repository::{FakeGitRepositoryState, GitRepository, RealGitRepository};
+use git::repository::{FakeGitRepositoryState, GitRepository, Git2Repo};
 use language::LineEnding;
 use smol::io::{AsyncReadExt, AsyncWriteExt};
 use std::{
@@ -239,7 +239,7 @@ impl Fs for RealFs {
     }
 
     fn open_repo(&self, abs_dot_git: &Path) -> Option<Box<dyn GitRepository>> {
-        RealGitRepository::open(&abs_dot_git)
+        Git2Repo::open(&abs_dot_git)
     }
 
     fn is_fake(&self) -> bool {
@@ -895,7 +895,6 @@ impl Fs for FakeFs {
                     .clone();
                 Some(git::repository::FakeGitRepository::open(
                     abs_dot_git.into(),
-                    0,
                     state,
                 ))
             } else {

crates/project/src/worktree.rs

@@ -97,10 +97,39 @@ pub struct Snapshot {
     is_complete: bool,
 }
 
+#[derive(Clone)]
+struct GitRepositoryEntry {
+    repo: Arc<Mutex<dyn GitRepository>>,
+    
+    // repo: Box<dyn GitRepository>,
+    scan_id: usize,
+    // 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>,
+}
+
+impl std::fmt::Debug for GitRepositoryEntry {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("GitRepositoryEntry")
+            .field("content_path", &self.content_path)
+            .field("git_dir_path", &self.git_dir_path)
+            .field("libgit_repository", &"LibGitRepository")
+            .finish()
+    }
+}
+
+// impl Clone for GitRepositoryEntry {
+//     fn clone(&self) -> Self {
+//         GitRepositoryEntry { repo: self.repo.boxed_clone(), scan_id: self.scan_id }
+//     }
+// }
+
 pub struct LocalSnapshot {
     abs_path: Arc<Path>,
     ignores_by_parent_abs_path: HashMap<Arc<Path>, (Arc<Gitignore>, usize)>,
-    git_repositories: Vec<Box<dyn GitRepository>>,
+    git_repositories: Vec<GitRepositoryEntry>,
     removed_entry_ids: HashMap<u64, ProjectEntryId>,
     next_entry_id: Arc<AtomicUsize>,
     snapshot: Snapshot,
@@ -115,7 +144,7 @@ impl Clone for LocalSnapshot {
             git_repositories: self
                 .git_repositories
                 .iter()
-                .map(|repo| repo.boxed_clone())
+                .cloned()
                 .collect(),
             removed_entry_ids: self.removed_entry_ids.clone(),
             next_entry_id: self.next_entry_id.clone(),
@@ -3287,17 +3316,17 @@ mod tests {
 
     #[test]
     fn test_changed_repos() {
-        let prev_repos: Vec<Box<dyn GitRepository>> = vec![
+        // let prev_repos: Vec<Box<dyn GitRepository>> = vec![
-            FakeGitRepository::open(Path::new("/.git"), 0, Default::default()),
+        //     FakeGitRepository::open(Path::new("/.git"), 0, Default::default()),
-            FakeGitRepository::open(Path::new("/a/.git"), 0, Default::default()),
+        //     FakeGitRepository::open(Path::new("/a/.git"), 0, Default::default()),
-            FakeGitRepository::open(Path::new("/a/b/.git"), 0, Default::default()),
+        //     FakeGitRepository::open(Path::new("/a/b/.git"), 0, Default::default()),
-        ];
+        // ];
 
-        let new_repos: Vec<Box<dyn GitRepository>> = vec![
+        // let new_repos: Vec<Box<dyn GitRepository>> = vec![
-            FakeGitRepository::open(Path::new("/a/.git"), 1, Default::default()),
+        //     FakeGitRepository::open(Path::new("/a/.git"), 1, Default::default()),
-            FakeGitRepository::open(Path::new("/a/b/.git"), 0, Default::default()),
+        //     FakeGitRepository::open(Path::new("/a/b/.git"), 0, Default::default()),
-            FakeGitRepository::open(Path::new("/a/c/.git"), 0, Default::default()),
+        //     FakeGitRepository::open(Path::new("/a/c/.git"), 0, Default::default()),
-        ];
+        // ];
 
         let res = LocalWorktree::changed_repos(&prev_repos, &new_repos);