From 055df307571af95bd91968c92a2e7024fcd2e650 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Fri, 11 Apr 2025 20:35:14 -0400 Subject: [PATCH] Directly parse .git when it's a file instead of using libgit2 (#27885) 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 --- 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 | 131 ++++++++++++-------- 7 files changed, 401 insertions(+), 160 deletions(-) diff --git a/crates/fs/src/fake_git_repo.rs b/crates/fs/src/fake_git_repo.rs index 584abd4cf7..15750bfccc 100644 --- a/crates/fs/src/fake_git_repo.rs +++ b/crates/fs/src/fake_git_repo.rs @@ -21,11 +21,12 @@ pub struct FakeGitRepository { pub(crate) fs: Arc, 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, pub unmerged_paths: HashMap, pub head_contents: HashMap, @@ -37,9 +38,8 @@ pub struct FakeGitRepositoryState { } impl FakeGitRepositoryState { - pub fn new(path: PathBuf, event_emitter: smol::channel::Sender) -> Self { + pub fn new(event_emitter: smol::channel::Sender) -> Self { FakeGitRepositoryState { - path, event_emitter, head_contents: Default::default(), index_contents: Default::default(), @@ -53,15 +53,6 @@ impl FakeGitRepositoryState { } impl FakeGitRepository { - fn with_state(&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(&self, write: bool, f: F) -> BoxFuture<'static, Result> where F: 'static + Send + FnOnce(&mut FakeGitRepositoryState) -> Result, @@ -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> { @@ -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 => {} diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index 272a05e9b8..bc60e8a2fd 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -851,7 +851,7 @@ impl Watcher for RealWatcher { pub struct FakeFs { this: std::sync::Weak, // Use an unfair lock to ensure tests are deterministic. - state: Mutex, + state: Arc>, executor: gpui::BackgroundExecutor, } @@ -878,6 +878,8 @@ enum FakeFsEntry { mtime: MTime, len: u64, content: Vec, + // The path to the repository state directory, if this is a gitfile. + git_dir_path: Option, }, 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(&self, dot_git: &Path, emit_git_event: bool, f: F) -> Result + pub fn with_git_state_and_paths( + &self, + dot_git: &Path, + emit_git_event: bool, + f: F, + ) -> Result 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(&self, dot_git: &Path, emit_git_event: bool, f: F) -> Result + 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>) { 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>, + original_path: PathBuf, + fs_state: Arc>, + prefixes: Mutex>, +} #[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> { - 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( diff --git a/crates/git/src/repository.rs b/crates/git/src/repository.rs index da68a532e3..060c14ffcc 100644 --- a/crates/git/src/repository.rs +++ b/crates/git/src/repository.rs @@ -229,12 +229,6 @@ pub trait GitRepository: Send + Sync { /// worktree's gitdir within the main repository (typically `.git/worktrees/`). 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. diff --git a/crates/project/src/git_store.rs b/crates/project/src/git_store.rs index b0b35c52bc..b917295ec1 100644 --- a/crates/project/src/git_store.rs +++ b/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, dot_git_abs_path: Arc, + repository_dir_abs_path: Arc, + common_dir_abs_path: Arc, project_environment: WeakEntity, fs: Arc, git_store: WeakEntity, @@ -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, dot_git_abs_path: Arc, + repository_dir_abs_path: Arc, + common_dir_abs_path: Arc, project_environment: WeakEntity, fs: Arc, cx: &mut Context, @@ -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 { 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( diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 7ce0d92379..d27ffc070c 100644 --- a/crates/project/src/project.rs +++ b/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) -> 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::>() + }) + .unwrap(); + join_all(scans_complete).await; + let barriers = this + .update(cx, |this, cx| { + let repos = this.repositories(cx).values().cloned().collect::>(); + repos + .into_iter() + .map(|repo| repo.update(cx, |repo, _| repo.barrier())) + .collect::>() + }) + .unwrap(); + join_all(barriers).await; + }) + } + pub fn active_repository(&self, cx: &App) -> Option> { self.git_store.read(cx).active_repository() } diff --git a/crates/project/src/project_tests.rs b/crates/project/src/project_tests.rs index 93537e4e46..fd1106b11d 100644 --- a/crates/project/src/project_tests.rs +++ b/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::>() + }); + 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); diff --git a/crates/worktree/src/worktree.rs b/crates/worktree/src/worktree.rs index 920b89770b..eff02a5cfa 100644 --- a/crates/worktree/src/worktree.rs +++ b/crates/worktree/src/worktree.rs @@ -384,12 +384,23 @@ struct LocalRepositoryEntry { work_directory: WorkDirectory, work_directory_abs_path: Arc, git_dir_scan_id: usize, - original_dot_git_abs_path: Arc, - /// 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, - /// Absolute path to the .git file, if we're in a git worktree. - dot_git_worktree_abs_path: Option>, + /// 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, + /// 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, + /// 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, } 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 { - 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); + let dot_git_abs_path: Arc = self + .snapshot + .abs_path + .as_path() + .join(&dot_git_path) + .as_path() + .into(); - // 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 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>, + pub repository_dir_abs_path: Option>, + pub common_dir_abs_path: Option>, } pub type UpdatedEntriesSet = Arc<[(Arc, 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]) -> 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 { .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, ) });