From 7cf26f4046774ba8be6276aebef24347a4164694 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Tue, 31 Mar 2026 12:40:01 -0400 Subject: [PATCH] Fix submodules being incorrectly classified as linked worktrees (#52507) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For submodules, `common_dir_abs_path` equals `repository_dir_abs_path` (since submodules don't have a `commondir` file). The previous code passed this path to `original_repo_path_from_common_dir` unconditionally, which returned the `.git/modules/` path as the `original_repo_abs_path` — causing `linked_worktree_path()` to return a false positive for submodules. Now we detect linked worktrees by checking whether `common_dir` differs from `repository_dir` (only true for actual linked worktrees that have a `commondir` file). For normal repos and submodules, `original_repo_abs_path` is simply `work_directory_abs_path`. Also fixes the misleading doc comment on `common_dir_abs_path` in `LocalRepositoryEntry` and adds test assertions for `original_repo_abs_path` and `linked_worktree_path()` on both worktrees and submodules. Closes AI-102 Release Notes: - Fixed git submodules being incorrectly classified as linked worktrees, which could cause issues with worktree-related operations. --- crates/git/src/repository.rs | 19 +++++++++++++++++++ crates/project/src/git_store.rs | 14 ++++++++++---- .../tests/integration/project_tests.rs | 16 ++++++++++++++++ crates/worktree/src/worktree.rs | 8 +++++--- 4 files changed, 50 insertions(+), 7 deletions(-) diff --git a/crates/git/src/repository.rs b/crates/git/src/repository.rs index 036ceeb620e1aa0345b6f9a296c16069c0fa09bf..8b0f189d869fe2438aecbac14895d5f30deaf308 100644 --- a/crates/git/src/repository.rs +++ b/crates/git/src/repository.rs @@ -60,6 +60,25 @@ pub const GRAPH_CHUNK_SIZE: usize = 1000; /// Default value for the `git.worktree_directory` setting. pub const DEFAULT_WORKTREE_DIRECTORY: &str = "../worktrees"; +/// Determine the original (main) repository's working directory. +/// +/// For linked worktrees, `common_dir` differs from `repository_dir` and +/// points to the main repo's `.git` directory, so we can derive the main +/// repo's working directory from it. For normal repos and submodules, +/// `common_dir` equals `repository_dir`, and the original repo is simply +/// `work_directory` itself. +pub fn original_repo_path( + work_directory: &Path, + common_dir: &Path, + repository_dir: &Path, +) -> PathBuf { + if common_dir != repository_dir { + original_repo_path_from_common_dir(common_dir) + } else { + work_directory.to_path_buf() + } +} + /// Given the git common directory (from `commondir()`), derive the original /// repository's working directory. /// diff --git a/crates/project/src/git_store.rs b/crates/project/src/git_store.rs index f439c5da157cdcdaec813a1fd63ea119af78cb83..1be256a50a1029e54a6002f1301f248b88b80850 100644 --- a/crates/project/src/git_store.rs +++ b/crates/project/src/git_store.rs @@ -1532,13 +1532,17 @@ impl GitStore { } 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), + repository_dir_abs_path: Some(repository_dir_abs_path), common_dir_abs_path: Some(common_dir_abs_path), .. } = update { - let original_repo_abs_path: Arc = - git::repository::original_repo_path_from_common_dir(common_dir_abs_path).into(); + let original_repo_abs_path: Arc = git::repository::original_repo_path( + work_directory_abs_path, + common_dir_abs_path, + repository_dir_abs_path, + ) + .into(); let id = RepositoryId(next_repository_id.fetch_add(1, atomic::Ordering::Release)); let is_trusted = TrustedWorktrees::try_get_global(cx) .map(|trusted_worktrees| { @@ -3711,6 +3715,8 @@ impl RepositorySnapshot { /// /// For example, if you had both `~/code/zed` and `~/code/worktrees/zed-2`, /// then `~/code/zed` is the main worktree and `~/code/worktrees/zed-2` is a linked worktree. + /// + /// Submodules also return `true` here, since they are not linked worktrees. pub fn is_main_worktree(&self) -> bool { self.work_directory_abs_path == self.original_repo_abs_path } @@ -3718,7 +3724,7 @@ impl RepositorySnapshot { /// Returns true if this repository is a linked worktree, that is, one that /// was created from another worktree. /// - /// This is by definition the opposite of [`Self::is_main_worktree`]. + /// Returns `false` for both the main worktree and submodules. pub fn is_linked_worktree(&self) -> bool { !self.is_main_worktree() } diff --git a/crates/project/tests/integration/project_tests.rs b/crates/project/tests/integration/project_tests.rs index 3230df665557077ed2f50142815242e7caef85a4..8603a904acd2c0cd52fcdc9d102be0f2efeb0636 100644 --- a/crates/project/tests/integration/project_tests.rs +++ b/crates/project/tests/integration/project_tests.rs @@ -11496,6 +11496,14 @@ async fn test_git_worktrees_and_submodules(cx: &mut gpui::TestAppContext) { repo.read(cx).work_directory_abs_path, Path::new(path!("/project/some-worktree")).into(), ); + pretty_assertions::assert_eq!( + repo.read(cx).original_repo_abs_path, + Path::new(path!("/project")).into(), + ); + assert!( + repo.read(cx).linked_worktree_path().is_some(), + "linked worktree should be detected as a linked worktree" + ); let barrier = repo.update(cx, |repo, _| repo.barrier()); (repo.clone(), barrier) }); @@ -11541,6 +11549,14 @@ async fn test_git_worktrees_and_submodules(cx: &mut gpui::TestAppContext) { repo.read(cx).work_directory_abs_path, Path::new(path!("/project/subdir/some-submodule")).into(), ); + pretty_assertions::assert_eq!( + repo.read(cx).original_repo_abs_path, + Path::new(path!("/project/subdir/some-submodule")).into(), + ); + assert!( + repo.read(cx).linked_worktree_path().is_none(), + "submodule should not be detected as a linked worktree" + ); let barrier = repo.update(cx, |repo, _| repo.barrier()); (repo.clone(), barrier) }); diff --git a/crates/worktree/src/worktree.rs b/crates/worktree/src/worktree.rs index 07f01e21758aa79509e7d6466e2f3b798eb7b8d3..1ad3e7ecaa694326aa479b5b69ccd3206fbf1e8d 100644 --- a/crates/worktree/src/worktree.rs +++ b/crates/worktree/src/worktree.rs @@ -289,9 +289,11 @@ struct LocalRepositoryEntry { 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. + /// This is always a directory. For a normal repository, this is the same as + /// `dot_git_abs_path`. For a linked worktree, this is the main repo's `.git` + /// directory (resolved from the worktree's `commondir` file). For a submodule, + /// this equals `repository_dir_abs_path` (submodules don't have a `commondir` + /// file). common_dir_abs_path: Arc, /// Absolute path to the directory holding the repository's state. ///