From 864c1ff00c9be8368d1cb567171558ca72b2a7e4 Mon Sep 17 00:00:00 2001 From: Michael Sloan Date: Thu, 6 Feb 2025 21:38:09 -0700 Subject: [PATCH] Use `commondir` from libgit2 instead of walking fs (#22028) Release Notes: - N/A --- crates/git/src/repository.rs | 38 ++++++++++++++++++++++++--------- crates/worktree/src/worktree.rs | 32 ++++++++++++--------------- 2 files changed, 42 insertions(+), 28 deletions(-) diff --git a/crates/git/src/repository.rs b/crates/git/src/repository.rs index 58dc9a9dce724167e242165a4dae6e6f6369efdb..9bf4c4da12d60d5aa0533c54c69a279a29172c66 100644 --- a/crates/git/src/repository.rs +++ b/crates/git/src/repository.rs @@ -58,8 +58,17 @@ pub trait GitRepository: Send + Sync { fn blame(&self, path: &Path, content: Rope) -> Result; - /// Returns the path to the repository, typically the `.git` folder. - fn dot_git_dir(&self) -> PathBuf; + /// Returns the absolute path to the repository. For worktrees, this will be the path to the + /// 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. /// @@ -109,11 +118,16 @@ impl GitRepository for RealGitRepository { } } - fn dot_git_dir(&self) -> PathBuf { + fn path(&self) -> PathBuf { let repo = self.repository.lock(); repo.path().into() } + fn main_repository_path(&self) -> PathBuf { + let repo = self.repository.lock(); + repo.commondir().into() + } + fn load_index_text(&self, path: &RepoPath) -> Option { fn logic(repo: &git2::Repository, path: &RepoPath) -> Result> { const STAGE_NORMAL: i32 = 0; @@ -344,7 +358,7 @@ pub struct FakeGitRepository { #[derive(Debug, Clone)] pub struct FakeGitRepositoryState { - pub dot_git_dir: PathBuf, + pub path: PathBuf, pub event_emitter: smol::channel::Sender, pub head_contents: HashMap, pub index_contents: HashMap, @@ -361,9 +375,9 @@ impl FakeGitRepository { } impl FakeGitRepositoryState { - pub fn new(dot_git_dir: PathBuf, event_emitter: smol::channel::Sender) -> Self { + pub fn new(path: PathBuf, event_emitter: smol::channel::Sender) -> Self { FakeGitRepositoryState { - dot_git_dir, + path, event_emitter, head_contents: Default::default(), index_contents: Default::default(), @@ -405,9 +419,13 @@ impl GitRepository for FakeGitRepository { vec![] } - fn dot_git_dir(&self) -> PathBuf { + fn path(&self) -> PathBuf { let state = self.state.lock(); - state.dot_git_dir.clone() + state.path.clone() + } + + fn main_repository_path(&self) -> PathBuf { + self.path() } fn status(&self, path_prefixes: &[RepoPath]) -> Result { @@ -458,7 +476,7 @@ impl GitRepository for FakeGitRepository { state.current_branch_name = Some(name.to_owned()); state .event_emitter - .try_send(state.dot_git_dir.clone()) + .try_send(state.path.clone()) .expect("Dropped repo change event"); Ok(()) } @@ -468,7 +486,7 @@ impl GitRepository for FakeGitRepository { state.branches.insert(name.to_owned()); state .event_emitter - .try_send(state.dot_git_dir.clone()) + .try_send(state.path.clone()) .expect("Dropped repo change event"); Ok(()) } diff --git a/crates/worktree/src/worktree.rs b/crates/worktree/src/worktree.rs index fc2603bd442f40261799f915b15b94c3e3c94cd6..915e4e9954a723361800de27e5faaef05b0c0e1c 100644 --- a/crates/worktree/src/worktree.rs +++ b/crates/worktree/src/worktree.rs @@ -3354,18 +3354,23 @@ impl BackgroundScannerState { let t0 = Instant::now(); let repository = fs.open_repo(&dot_git_abs_path)?; - let actual_repo_path = repository.dot_git_dir(); + let repository_path = repository.path(); + watcher.add(&repository_path).log_err()?; - let actual_dot_git_dir_abs_path = smol::block_on(find_git_dir(&actual_repo_path, fs))?; - watcher.add(&actual_repo_path).log_err()?; - - let dot_git_worktree_abs_path = if actual_dot_git_dir_abs_path.as_ref() == dot_git_abs_path - { + 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, the .git path in the worktree (`dot_git_abs_path`) is a file that - // points to the worktree-subdirectory in the actual .git directory (`git_dir_path`) + // 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)) }; @@ -3400,7 +3405,7 @@ impl BackgroundScannerState { git_dir_scan_id: 0, status_scan_id: 0, repo_ptr: repository.clone(), - dot_git_dir_abs_path: actual_dot_git_dir_abs_path, + dot_git_dir_abs_path: actual_dot_git_dir_abs_path.into(), dot_git_worktree_abs_path, current_merge_head_shas: Default::default(), }; @@ -3429,15 +3434,6 @@ async fn is_git_dir(path: &Path, fs: &dyn Fs) -> bool { matches!(config_metadata, Ok(Some(_))) } -async fn find_git_dir(path: &Path, fs: &dyn Fs) -> Option> { - for ancestor in path.ancestors() { - if is_git_dir(ancestor, fs).await { - return Some(Arc::from(ancestor)); - } - } - None -} - async fn build_gitignore(abs_path: &Path, fs: &dyn Fs) -> Result { let contents = fs.load(abs_path).await?; let parent = abs_path.parent().unwrap_or_else(|| Path::new("/"));