Find repos under worktree & return correct results for repo queries

Julia and Mikayla Maki created

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

Change summary

crates/project/src/fs.rs             |  27 +-----
crates/project/src/git_repository.rs | 121 +++++------------------------
crates/project/src/worktree.rs       |  98 +++++++++--------------
3 files changed, 66 insertions(+), 180 deletions(-)

Detailed changes

crates/project/src/fs.rs 🔗

@@ -22,7 +22,7 @@ use futures::lock::Mutex;
 #[cfg(any(test, feature = "test-support"))]
 use std::sync::{Arc, Weak};
 
-use crate::git_repository::{FakeGitRepository, GitRepository, RealGitRepository};
+use crate::git_repository::GitRepository;
 
 #[async_trait::async_trait]
 pub trait Fs: Send + Sync {
@@ -48,11 +48,7 @@ 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 open_git_repository(&self, abs_dotgit_path: &Path) -> Option<GitRepository>;
     fn is_fake(&self) -> bool;
     #[cfg(any(test, feature = "test-support"))]
     fn as_fake(&self) -> &FakeFs;
@@ -278,12 +274,8 @@ 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 open_git_repository(&self, abs_dotgit_path: &Path) -> Option<GitRepository> {
+        GitRepository::open(abs_dotgit_path)
     }
 
     fn is_fake(&self) -> bool {
@@ -901,15 +893,8 @@ 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 open_git_repository(&self, _: &Path) -> Option<GitRepository> {
+        None
     }
 
     fn is_fake(&self) -> bool {

crates/project/src/git_repository.rs 🔗

@@ -3,19 +3,8 @@ 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);
-    fn with_repo(&mut self, f: Box<dyn FnOnce(&mut git2::Repository)>);
-}
-
 #[derive(Clone)]
-pub struct RealGitRepository {
+pub struct GitRepository {
     // Path to folder containing the .git file or directory
     content_path: Arc<Path>,
     // Path to the actual .git folder.
@@ -25,118 +14,50 @@ pub struct RealGitRepository {
     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)
+impl GitRepository {
+    pub fn open(dotgit_path: &Path) -> Option<GitRepository> {
+        Repository::open(&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(),
+            .and_then(|libgit_repository| {
+                Some(Self {
+                    content_path: libgit_repository.workdir()?.into(),
+                    git_dir_path: dotgit_path.canonicalize().log_err()?.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)
+    pub fn is_path_managed_by(&self, path: &Path) -> bool {
+        path.canonicalize()
+            .map(|path| path.starts_with(&self.content_path))
+            .unwrap_or(false)
     }
 
-    fn is_path_in_git_folder(&self, path: &Path) -> bool {
-        path.starts_with(&self.git_dir_path)
+    pub fn is_path_in_git_folder(&self, path: &Path) -> bool {
+        path.canonicalize()
+            .map(|path| path.starts_with(&self.git_dir_path))
+            .unwrap_or(false)
     }
 
-    fn content_path(&self) -> &Path {
+    pub fn content_path(&self) -> &Path {
         self.content_path.as_ref()
     }
 
-    fn git_dir_path(&self) -> &Path {
+    pub fn git_dir_path(&self) -> &Path {
         self.git_dir_path.as_ref()
     }
 
-    fn last_scan_id(&self) -> usize {
+    pub fn last_scan_id(&self) -> usize {
         self.last_scan_id
     }
 
-    fn set_scan_id(&mut self, scan_id: usize) {
+    pub fn set_scan_id(&mut self, scan_id: usize) {
         self.last_scan_id = scan_id;
     }
 
-    fn with_repo(&mut self, f: Box<dyn FnOnce(&mut git2::Repository)>) {
+    pub fn with_repo(&mut self, f: Box<dyn FnOnce(&mut git2::Repository)>) {
         let mut git2 = self.libgit_repository.lock();
         f(&mut git2)
     }
 }
-
-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;
-    }
-
-    fn with_repo(&mut self, _: Box<dyn FnOnce(&mut git2::Repository)>) {
-        unimplemented!();
-    }
-}

crates/project/src/worktree.rs 🔗

@@ -100,7 +100,7 @@ pub struct Snapshot {
 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<GitRepository>,
     removed_entry_ids: HashMap<u64, ProjectEntryId>,
     next_entry_id: Arc<AtomicUsize>,
     snapshot: Snapshot,
@@ -115,7 +115,7 @@ impl Clone for LocalSnapshot {
             git_repositories: self
                 .git_repositories
                 .iter()
-                .map(|repo| repo.boxed_clone())
+                .map(|repo| repo.clone())
                 .collect(),
             removed_entry_ids: self.removed_entry_ids.clone(),
             next_entry_id: self.next_entry_id.clone(),
@@ -157,7 +157,7 @@ struct ShareState {
 
 pub enum Event {
     UpdatedEntries,
-    UpdatedGitRepositories(Vec<Box<dyn GitRepository>>),
+    UpdatedGitRepositories(Vec<GitRepository>),
 }
 
 impl Entity for Worktree {
@@ -1307,53 +1307,35 @@ impl LocalSnapshot {
     }
 
     // Gives the most specific git repository for a given path
-    pub(crate) fn git_repository_for_file_path(
-        &self,
-        path: &Path,
-    ) -> Option<Box<dyn GitRepository>> {
-        let repos = self
-            .git_repositories
-            .iter()
-            .map(|repo| repo.content_path().to_str().unwrap().to_string())
-            .collect::<Vec<String>>();
-        dbg!(repos);
-
+    pub(crate) fn git_repository_for_file_path(&self, path: &Path) -> Option<GitRepository> {
         self.git_repositories
             .iter()
             .rev() //git_repository is ordered lexicographically
-            .find(|repo| repo.is_path_managed_by(path))
-            .map(|repo| repo.boxed_clone())
+            .find(|repo| {
+                repo.is_path_managed_by(&self.abs_path.join(path))
+            })
+            .map(|repo| repo.clone())
     }
 
     //  ~/zed:
     //   - src
     //   - crates
     //   - .git -> /usr/.git
-    pub(crate) fn git_repository_for_git_data(
-        &self,
-        path: &Path,
-    ) -> Option<Box<dyn GitRepository>> {
+    pub(crate) fn git_repository_for_git_data(&self, path: &Path) -> Option<GitRepository> {
         self.git_repositories
             .iter()
-            .find(|repo| repo.is_path_in_git_folder(path))
-            .map(|repo| repo.boxed_clone())
+            .find(|repo| repo.is_path_in_git_folder(&self.abs_path.join(path)))
+            .map(|repo| repo.clone())
     }
 
     pub(crate) fn does_git_repository_track_file_path(
         &self,
-        repo: &Box<dyn GitRepository>,
+        repo: &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 == repo)
+        self.git_repository_for_file_path(&self.abs_path.join(file_path))
+            .map_or(false, |r| r.git_dir_path() == repo.git_dir_path())
     }
 
     #[cfg(test)]
@@ -1438,7 +1420,6 @@ impl LocalSnapshot {
     }
 
     fn insert_entry(&mut self, mut entry: Entry, fs: &dyn Fs) -> Entry {
-        dbg!(&entry.path);
         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)) {
@@ -1456,19 +1437,6 @@ impl LocalSnapshot {
                     );
                 }
             }
-        } else if entry.path.file_name() == Some(&DOT_GIT) {
-            dbg!(&entry.path);
-
-            let abs_path = self.abs_path.join(&entry.path);
-            let content_path: Arc<Path> = entry.path.parent().unwrap().into();
-            if let Err(ix) = self
-                .git_repositories
-                .binary_search_by_key(&content_path.as_ref(), |repo| repo.content_path())
-            {
-                if let Some(repository) = fs.open_git_repository(&abs_path, &content_path) {
-                    self.git_repositories.insert(ix, repository);
-                }
-            }
         }
 
         self.reuse_entry_id(&mut entry);
@@ -1506,6 +1474,7 @@ impl LocalSnapshot {
         parent_path: Arc<Path>,
         entries: impl IntoIterator<Item = Entry>,
         ignore: Option<Arc<Gitignore>>,
+        fs: &dyn Fs,
     ) {
         let mut parent_entry = if let Some(parent_entry) =
             self.entries_by_path.get(&PathKey(parent_path.clone()), &())
@@ -1531,6 +1500,18 @@ impl LocalSnapshot {
             unreachable!();
         }
 
+        if parent_path.file_name() == Some(&DOT_GIT) {
+            let abs_path = self.abs_path.join(&parent_path);
+            if let Err(ix) = self
+                .git_repositories
+                .binary_search_by_key(&abs_path.as_path(), |repo| repo.git_dir_path())
+            {
+                if let Some(repository) = fs.open_git_repository(&abs_path) {
+                    self.git_repositories.insert(ix, repository);
+                }
+            }
+        }
+
         let mut entries_by_path_edits = vec![Edit::Insert(parent_entry)];
         let mut entries_by_id_edits = Vec::new();
 
@@ -2227,7 +2208,6 @@ impl BackgroundScanner {
             if ignore_stack.is_all() {
                 if let Some(mut root_entry) = snapshot.root_entry().cloned() {
                     root_entry.is_ignored = true;
-                    dbg!("scan dirs entry");
                     snapshot.insert_entry(root_entry, self.fs.as_ref());
                 }
             }
@@ -2375,9 +2355,12 @@ impl BackgroundScanner {
             new_entries.push(child_entry);
         }
 
-        self.snapshot
-            .lock()
-            .populate_dir(job.path.clone(), new_entries, new_ignore);
+        self.snapshot.lock().populate_dir(
+            job.path.clone(),
+            new_entries,
+            new_ignore,
+            self.fs.as_ref(),
+        );
         for new_job in new_jobs {
             job.scan_queue.send(new_job).await.unwrap();
         }
@@ -2450,7 +2433,6 @@ impl BackgroundScanner {
                             snapshot.root_char_bag,
                         );
                         fs_entry.is_ignored = ignore_stack.is_all();
-                        dbg!("process_events entry");
                         snapshot.insert_entry(fs_entry, self.fs.as_ref());
 
                         let mut ancestor_inodes = snapshot.ancestor_inodes_for_path(&path);
@@ -3189,8 +3171,6 @@ mod tests {
         tree.read_with(cx, |tree, _cx| {
             let tree = tree.as_local().unwrap();
 
-            dbg!(tree);
-
             assert!(tree
                 .git_repository_for_file_path("c.txt".as_ref())
                 .is_none());
@@ -3202,22 +3182,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(), Path::new("dir1"));
-            assert_eq!(repo.git_dir_path(), Path::new("dir1/.git"));
+            assert_eq!(repo.content_path(), root.path().join("dir1").canonicalize().unwrap());
+            assert_eq!(repo.git_dir_path(), root.path().join("dir1/.git").canonicalize().unwrap());
 
             let repo = tree
                 .git_repository_for_file_path("dir1/deps/dep1/src/a.txt".as_ref())
                 .unwrap();
 
-            assert_eq!(repo.content_path(), Path::new("dir1/deps/dep1"));
-            assert_eq!(repo.git_dir_path(), Path::new("dir1/deps/dep1"));
+            assert_eq!(repo.content_path(), root.path().join("dir1/deps/dep1").canonicalize().unwrap());
+            assert_eq!(repo.git_dir_path(), root.path().join("dir1/deps/dep1/.git").canonicalize().unwrap());
 
             let repo = tree
                 .git_repository_for_git_data("dir1/.git/HEAD".as_ref())
                 .unwrap();
 
-            assert_eq!(repo.content_path(), Path::new("dir1"));
-            assert_eq!(repo.git_dir_path(), Path::new("dir1/.git"));
+            assert_eq!(repo.content_path(), root.path().join("dir1").canonicalize().unwrap());
+            assert_eq!(repo.git_dir_path(), root.path().join("dir1/.git").canonicalize().unwrap());
 
             assert!(tree.does_git_repository_track_file_path(&repo, "dir1/src/b.txt".as_ref()));
             assert!(!tree