diff --git a/Cargo.lock b/Cargo.lock index 75dd5530c9..fa8f8acbdc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2235,6 +2235,7 @@ dependencies = [ "async-trait", "clock", "collections", + "futures", "git2", "lazy_static", "log", diff --git a/crates/collab/src/integration_tests.rs b/crates/collab/src/integration_tests.rs index d5a4c56b7d..168231a6b4 100644 --- a/crates/collab/src/integration_tests.rs +++ b/crates/collab/src/integration_tests.rs @@ -966,7 +966,8 @@ async fn test_git_head_text( .insert_tree( "/dir", json!({ - ".git": {}, + ".git": { + }, "a.txt": " one two @@ -983,9 +984,8 @@ async fn test_git_head_text( .unindent(); let new_head_text = " - 1 + one two - three " .unindent(); @@ -1041,7 +1041,6 @@ async fn test_git_head_text( ); }); - // TODO: Create a dummy file event client_a .fs .as_fake() @@ -1051,19 +1050,18 @@ async fn test_git_head_text( ) .await; - // TODO: Flush this file event - // Wait for buffer_a to receive it executor.run_until_parked(); // Smoke test new diffing buffer_a.read_with(cx_a, |buffer, _| { assert_eq!(buffer.head_text(), Some(new_head_text.as_ref())); + git::diff::assert_hunks( buffer.snapshot().git_diff_hunks_in_range(0..4), &buffer, &head_text, - &[(0..1, "1", "one\n")], + &[(2..3, "", "three\n")], ); }); diff --git a/crates/git/Cargo.toml b/crates/git/Cargo.toml index 744fdc8b99..b8f3aac0b9 100644 --- a/crates/git/Cargo.toml +++ b/crates/git/Cargo.toml @@ -19,6 +19,7 @@ log = { version = "0.4.16", features = ["kv_unstable_serde"] } smol = "1.2" parking_lot = "0.11.1" async-trait = "0.1" +futures = "0.3" [dev-dependencies] unindent = "0.1.7" diff --git a/crates/git/src/repository.rs b/crates/git/src/repository.rs index ba8faa4b2b..a49a1e0b60 100644 --- a/crates/git/src/repository.rs +++ b/crates/git/src/repository.rs @@ -1,39 +1,20 @@ use anyhow::Result; use collections::HashMap; -use git2::Repository as LibGitRepository; use parking_lot::Mutex; -use util::ResultExt; -use std::{path::{Path, PathBuf}, sync::Arc}; +use std::{ + path::{Path, PathBuf}, + sync::Arc, +}; + +pub use git2::Repository as LibGitRepository; #[async_trait::async_trait] pub trait GitRepository: Send { - // fn manages(&self, path: &Path) -> bool; - // fn reopen_git_repo(&mut self) -> bool; - // fn git_repo(&self) -> Arc>; - // fn boxed_clone(&self) -> Box; - fn load_head_text(&self, relative_file_path: &Path) -> Option; - - fn open_real(dotgit_path: &Path) -> Option>> - where Self: Sized - { - LibGitRepository::open(&dotgit_path) - .log_err() - .and_then::>, _>(|libgit_repository| { - Some(Arc::new(Mutex::new(libgit_repository))) - }) - } } #[async_trait::async_trait] impl GitRepository for LibGitRepository { - // fn manages(&self, path: &Path) -> bool { - // path.canonicalize() - // .map(|path| path.starts_with(&self.content_path)) - // .unwrap_or(false) - // } - - fn load_head_text(&self, relative_file_path: &Path) -> Option { fn logic(repo: &LibGitRepository, relative_file_path: &Path) -> Result> { const STAGE_NORMAL: i32 = 0; @@ -56,10 +37,8 @@ impl GitRepository for LibGitRepository { } } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Default)] pub struct FakeGitRepository { - content_path: Arc, - git_dir_path: Arc, state: Arc>, } @@ -69,47 +48,15 @@ pub struct FakeGitRepositoryState { } impl FakeGitRepository { - pub fn open(dotgit_path: &Path, state: Arc>) -> Box { - Box::new(FakeGitRepository { - content_path: dotgit_path.parent().unwrap().into(), - git_dir_path: dotgit_path.into(), - state, - }) + pub fn open(state: Arc>) -> Arc> { + Arc::new(Mutex::new(FakeGitRepository { state })) } } #[async_trait::async_trait] impl GitRepository for FakeGitRepository { - fn manages(&self, path: &Path) -> bool { - path.starts_with(self.content_path()) - } - - // fn in_dot_git(&self, path: &Path) -> bool { - // path.starts_with(self.git_dir_path()) - // } - - fn content_path(&self) -> &Path { - &self.content_path - } - - fn git_dir_path(&self) -> &Path { - &self.git_dir_path - } - - async fn load_head_text(&self, path: &Path) -> Option { + fn load_head_text(&self, path: &Path) -> Option { let state = self.state.lock(); state.index_contents.get(path).cloned() } - - fn reopen_git_repo(&mut self) -> bool { - true - } - - fn git_repo(&self) -> Arc> { - unimplemented!() - } - - fn boxed_clone(&self) -> Box { - Box::new(self.clone()) - } } diff --git a/crates/project/src/fs.rs b/crates/project/src/fs.rs index 1280fcb8bc..d0e549c0b5 100644 --- a/crates/project/src/fs.rs +++ b/crates/project/src/fs.rs @@ -1,8 +1,9 @@ use anyhow::{anyhow, Result}; use fsevent::EventStream; use futures::{future::BoxFuture, Stream, StreamExt}; -use git::repository::{FakeGitRepositoryState, GitRepository, Git2Repo}; +use git::repository::{FakeGitRepositoryState, GitRepository, LibGitRepository}; use language::LineEnding; +use parking_lot::Mutex as SyncMutex; use smol::io::{AsyncReadExt, AsyncWriteExt}; use std::{ io, @@ -11,6 +12,7 @@ use std::{ pin::Pin, time::{Duration, SystemTime}, }; +use util::ResultExt; use text::Rope; @@ -44,7 +46,7 @@ pub trait Fs: Send + Sync { path: &Path, latency: Duration, ) -> Pin>>>; - fn open_repo(&self, abs_dot_git: &Path) -> Option>; + fn open_repo(&self, abs_dot_git: &Path) -> Option>>; fn is_fake(&self) -> bool; #[cfg(any(test, feature = "test-support"))] fn as_fake(&self) -> &FakeFs; @@ -238,8 +240,12 @@ impl Fs for RealFs { }))) } - fn open_repo(&self, abs_dot_git: &Path) -> Option> { - Git2Repo::open(&abs_dot_git) + fn open_repo(&self, dotgit_path: &Path) -> Option>> { + LibGitRepository::open(&dotgit_path) + .log_err() + .and_then::>, _>(|libgit_repository| { + Some(Arc::new(SyncMutex::new(libgit_repository))) + }) } fn is_fake(&self) -> bool { @@ -277,7 +283,7 @@ enum FakeFsEntry { inode: u64, mtime: SystemTime, entries: BTreeMap>>, - git_repo_state: Option>>, + git_repo_state: Option>>, }, Symlink { target: PathBuf, @@ -488,7 +494,7 @@ impl FakeFs { head_state: &[(&Path, String)], ) { let content_path = dot_git.parent().unwrap(); - let state = self.state.lock().await; + let mut state = self.state.lock().await; let entry = state.read_path(dot_git).await.unwrap(); let mut entry = entry.lock().await; @@ -502,6 +508,8 @@ impl FakeFs { .iter() .map(|(path, content)| (content_path.join(path), content.clone())), ); + + state.emit_event([dot_git]); } else { panic!("not a directory"); } @@ -881,7 +889,7 @@ impl Fs for FakeFs { })) } - fn open_repo(&self, abs_dot_git: &Path) -> Option> { + fn open_repo(&self, abs_dot_git: &Path) -> Option>> { let executor = self.executor.upgrade().unwrap(); executor.block(async move { let state = self.state.lock().await; @@ -890,13 +898,10 @@ impl Fs for FakeFs { if let FakeFsEntry::Dir { git_repo_state, .. } = &mut *entry { let state = git_repo_state .get_or_insert_with(|| { - Arc::new(parking_lot::Mutex::new(FakeGitRepositoryState::default())) + Arc::new(SyncMutex::new(FakeGitRepositoryState::default())) }) .clone(); - Some(git::repository::FakeGitRepository::open( - abs_dot_git.into(), - state, - )) + Some(git::repository::FakeGitRepository::open(state)) } else { None } diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 1e8567d4d4..f1aa98c4e0 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -12,7 +12,7 @@ use client::{proto, Client, PeerId, TypedEnvelope, User, UserStore}; use clock::ReplicaId; use collections::{hash_map, BTreeMap, HashMap, HashSet}; use futures::{future::Shared, AsyncWriteExt, Future, FutureExt, StreamExt, TryFutureExt}; -use git::repository::GitRepository; + use gpui::{ AnyModelHandle, AppContext, AsyncAppContext, Entity, ModelContext, ModelHandle, MutableAppContext, Task, UpgradeModelHandle, WeakModelHandle, @@ -4648,7 +4648,7 @@ impl Project { fn update_local_worktree_buffers_git_repos( &mut self, - repos: &[Box], + repos: &[GitRepositoryEntry], cx: &mut ModelContext, ) { //TODO: Produce protos @@ -4663,12 +4663,15 @@ impl Project { let abs_path = file.abs_path(cx); let repo = match repos.iter().find(|repo| repo.manages(&abs_path)) { - Some(repo) => repo.boxed_clone(), + Some(repo) => repo.clone(), None => return, }; cx.spawn(|_, mut cx| async move { - let head_text = repo.load_head_text(&path).await; + let head_text = cx + .background() + .spawn(async move { repo.repo.lock().load_head_text(&path) }) + .await; buffer.update(&mut cx, |buffer, cx| { buffer.update_head_text(head_text, cx); }); diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 4f14ab6ad1..560f23d147 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -98,16 +98,15 @@ pub struct Snapshot { } #[derive(Clone)] -struct GitRepositoryEntry { - repo: Arc>, - - // repo: Box, - scan_id: usize, +pub struct GitRepositoryEntry { + pub(crate) repo: Arc>, + + pub(crate) scan_id: usize, // Path to folder containing the .git file or directory - content_path: Arc, + pub(crate) content_path: Arc, // 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, + pub(crate) git_dir_path: Arc, } impl std::fmt::Debug for GitRepositoryEntry { @@ -141,11 +140,7 @@ impl Clone for LocalSnapshot { Self { abs_path: self.abs_path.clone(), ignores_by_parent_abs_path: self.ignores_by_parent_abs_path.clone(), - git_repositories: self - .git_repositories - .iter() - .cloned() - .collect(), + git_repositories: self.git_repositories.iter().cloned().collect(), removed_entry_ids: self.removed_entry_ids.clone(), next_entry_id: self.next_entry_id.clone(), snapshot: self.snapshot.clone(), @@ -186,7 +181,7 @@ struct ShareState { pub enum Event { UpdatedEntries, - UpdatedGitRepositories(Vec>), + UpdatedGitRepositories(Vec), } impl Entity for Worktree { @@ -610,27 +605,26 @@ impl LocalWorktree { } fn changed_repos( - old_repos: &[Box], - new_repos: &[Box], - ) -> Vec> { + old_repos: &[GitRepositoryEntry], + new_repos: &[GitRepositoryEntry], + ) -> Vec { fn diff<'a>( - a: &'a [Box], - b: &'a [Box], - updated: &mut HashMap<&'a Path, Box>, + a: &'a [GitRepositoryEntry], + b: &'a [GitRepositoryEntry], + updated: &mut HashMap<&'a Path, GitRepositoryEntry>, ) { for a_repo in a { let matched = b.iter().find(|b_repo| { - a_repo.git_dir_path() == b_repo.git_dir_path() - && a_repo.scan_id() == b_repo.scan_id() + a_repo.git_dir_path == b_repo.git_dir_path && a_repo.scan_id == b_repo.scan_id }); if matched.is_none() { - updated.insert(a_repo.git_dir_path(), a_repo.boxed_clone()); + updated.insert(a_repo.git_dir_path.as_ref(), a_repo.clone()); } } } - let mut updated = HashMap::<&Path, Box>::default(); + let mut updated = HashMap::<&Path, GitRepositoryEntry>::default(); diff(old_repos, new_repos, &mut updated); diff(new_repos, old_repos, &mut updated); @@ -690,7 +684,12 @@ impl LocalWorktree { settings::GitFilesIncluded::All | settings::GitFilesIncluded::OnlyTracked ) { let results = if let Some(repo) = snapshot.repo_for(&abs_path) { - repo.load_head_text(&path).await + cx.background() + .spawn({ + let path = path.clone(); + async move { repo.repo.lock().load_head_text(&path) } + }) + .await } else { None }; @@ -1390,25 +1389,19 @@ impl LocalSnapshot { } // Gives the most specific git repository for a given path - pub(crate) fn repo_for(&self, path: &Path) -> Option> { + pub(crate) fn repo_for(&self, path: &Path) -> Option { self.git_repositories .iter() .rev() //git_repository is ordered lexicographically - .find(|repo| repo.manages(&self.abs_path.join(path))) - .map(|repo| repo.boxed_clone()) + .find(|repo| repo.manages(path)) + .cloned() } - pub(crate) fn in_dot_git(&mut self, path: &Path) -> Option<&mut Box> { + pub(crate) fn in_dot_git(&mut self, path: &Path) -> Option<&mut GitRepositoryEntry> { + // Git repositories cannot be nested, so we don't need to reverse the order self.git_repositories .iter_mut() - .rev() //git_repository is ordered lexicographically - .find(|repo| repo.in_dot_git(&self.abs_path.join(path))) - } - - pub(crate) fn _tracks_filepath(&self, repo: &dyn GitRepository, file_path: &Path) -> bool { - // Depends on git_repository_for_file_path returning the most specific git repository for a given path - self.repo_for(&self.abs_path.join(file_path)) - .map_or(false, |r| r.git_dir_path() == repo.git_dir_path()) + .find(|repo| repo.in_dot_git(path)) } #[cfg(test)] @@ -1575,12 +1568,21 @@ impl LocalSnapshot { if parent_path.file_name() == Some(&DOT_GIT) { let abs_path = self.abs_path.join(&parent_path); + let content_path: Arc = parent_path.parent().unwrap().into(); if let Err(ix) = self .git_repositories - .binary_search_by_key(&abs_path.as_path(), |repo| repo.git_dir_path()) + .binary_search_by_key(&&content_path, |repo| &repo.content_path) { - if let Some(repository) = fs.open_repo(abs_path.as_path()) { - self.git_repositories.insert(ix, repository); + if let Some(repo) = fs.open_repo(abs_path.as_path()) { + self.git_repositories.insert( + ix, + GitRepositoryEntry { + repo, + scan_id: 0, + content_path, + git_dir_path: parent_path, + }, + ); } } } @@ -1673,9 +1675,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.git_dir_path.as_ref()) { - self.git_repositories[ix].set_scan_id(self.snapshot.scan_id); + self.git_repositories[ix].scan_id = self.snapshot.scan_id; } } } @@ -1716,6 +1718,25 @@ impl LocalSnapshot { ignore_stack } + + pub fn git_repo_entries(&self) -> &[GitRepositoryEntry] { + &self.git_repositories + } +} +// Worktree root +// | +// git_dir_path: c/d/.git +//in_dot_git Query: c/d/.git/HEAD +// Manages Query: c/d/e/f/a.txt + +impl GitRepositoryEntry { + pub(crate) fn manages(&self, path: &Path) -> bool { + path.starts_with(self.content_path.as_ref()) + } + + pub(crate) fn in_dot_git(&self, path: &Path) -> bool { + path.starts_with(self.git_dir_path.as_ref()) + } } async fn build_gitignore(abs_path: &Path, fs: &dyn Fs) -> Result { @@ -2509,8 +2530,8 @@ impl BackgroundScanner { snapshot.insert_entry(fs_entry, self.fs.as_ref()); let scan_id = snapshot.scan_id; - if let Some(repo) = snapshot.in_dot_git(&abs_path) { - repo.set_scan_id(scan_id); + if let Some(repo) = snapshot.in_dot_git(&path) { + repo.scan_id = scan_id; } let mut ancestor_inodes = snapshot.ancestor_inodes_for_path(&path); @@ -2625,19 +2646,21 @@ impl BackgroundScanner { .await; } + // TODO: Clarify what is going on here because re-loading every git repository + // on every file system event seems wrong async fn update_git_repositories(&self) { let mut snapshot = self.snapshot.lock(); let new_repos = snapshot .git_repositories .iter() - .map(|repo| repo.boxed_clone()) - .filter_map(|mut repo| { - if repo.reopen_git_repo() { - Some(repo) - } else { - None - } + .cloned() + .filter_map(|mut repo_entry| { + let repo = self + .fs + .open_repo(&snapshot.abs_path.join(&repo_entry.git_dir_path))?; + repo_entry.repo = repo; + Some(repo_entry) }) .collect(); @@ -3262,34 +3285,17 @@ mod tests { assert!(tree.repo_for("c.txt".as_ref()).is_none()); let repo = tree.repo_for("dir1/src/b.txt".as_ref()).unwrap(); - - 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_eq!(repo.content_path.as_ref(), Path::new("dir1")); + assert_eq!(repo.git_dir_path.as_ref(), Path::new("dir1/.git")); let repo = tree.repo_for("dir1/deps/dep1/src/a.txt".as_ref()).unwrap(); - - 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() - ); + assert_eq!(repo.content_path.as_ref(), Path::new("dir1/deps/dep1")); + assert_eq!(repo.git_dir_path.as_ref(), Path::new("dir1/deps/dep1/.git"),); }); let original_scan_id = tree.read_with(cx, |tree, _cx| { let tree = tree.as_local().unwrap(); - tree.repo_for("dir1/src/b.txt".as_ref()).unwrap().scan_id() + tree.repo_for("dir1/src/b.txt".as_ref()).unwrap().scan_id }); std::fs::write(root.path().join("dir1/.git/random_new_file"), "hello").unwrap(); @@ -3297,7 +3303,7 @@ mod tests { tree.read_with(cx, |tree, _cx| { let tree = tree.as_local().unwrap(); - let new_scan_id = tree.repo_for("dir1/src/b.txt".as_ref()).unwrap().scan_id(); + let new_scan_id = tree.repo_for("dir1/src/b.txt".as_ref()).unwrap().scan_id; assert_ne!( original_scan_id, new_scan_id, "original {original_scan_id}, new {new_scan_id}" @@ -3316,44 +3322,51 @@ mod tests { #[test] fn test_changed_repos() { - // let prev_repos: Vec> = vec![ - // FakeGitRepository::open(Path::new("/.git"), 0, Default::default()), - // FakeGitRepository::open(Path::new("/a/.git"), 0, Default::default()), - // FakeGitRepository::open(Path::new("/a/b/.git"), 0, Default::default()), - // ]; + fn fake_entry(git_dir_path: impl AsRef, scan_id: usize) -> GitRepositoryEntry { + GitRepositoryEntry { + repo: Arc::new(Mutex::new(FakeGitRepository::default())), + scan_id, + content_path: git_dir_path.as_ref().parent().unwrap().into(), + git_dir_path: git_dir_path.as_ref().into(), + } + } - // let new_repos: Vec> = vec![ - // FakeGitRepository::open(Path::new("/a/.git"), 1, Default::default()), - // FakeGitRepository::open(Path::new("/a/b/.git"), 0, Default::default()), - // FakeGitRepository::open(Path::new("/a/c/.git"), 0, Default::default()), - // ]; + let prev_repos: Vec = vec![ + fake_entry("/.git", 0), + fake_entry("/a/.git", 0), + fake_entry("/a/b/.git", 0), + ]; + + let new_repos: Vec = vec![ + fake_entry("/a/.git", 1), + fake_entry("/a/b/.git", 0), + fake_entry("/a/c/.git", 0), + ]; let res = LocalWorktree::changed_repos(&prev_repos, &new_repos); - dbg!(&res); - // Deletion retained assert!(res .iter() - .find(|repo| repo.git_dir_path() == Path::new("/.git") && repo.scan_id() == 0) + .find(|repo| repo.git_dir_path.as_ref() == Path::new("/.git") && repo.scan_id == 0) .is_some()); // Update retained assert!(res .iter() - .find(|repo| repo.git_dir_path() == Path::new("/a/.git") && repo.scan_id() == 1) + .find(|repo| repo.git_dir_path.as_ref() == Path::new("/a/.git") && repo.scan_id == 1) .is_some()); // Addition retained assert!(res .iter() - .find(|repo| repo.git_dir_path() == Path::new("/a/c/.git") && repo.scan_id() == 0) + .find(|repo| repo.git_dir_path.as_ref() == Path::new("/a/c/.git") && repo.scan_id == 0) .is_some()); // Nochange, not retained assert!(res .iter() - .find(|repo| repo.git_dir_path() == Path::new("/a/b/.git") && repo.scan_id() == 0) + .find(|repo| repo.git_dir_path.as_ref() == Path::new("/a/b/.git") && repo.scan_id == 0) .is_none()); }