Fix submodules being incorrectly classified as linked worktrees (#52507)

Richard Feldman created 

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/<name>` 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.

Change summary

crates/git/src/repository.rs                      | 19 +++++++++++++++++
crates/project/src/git_store.rs                   | 14 ++++++++---
crates/project/tests/integration/project_tests.rs | 16 ++++++++++++++
crates/worktree/src/worktree.rs                   |  8 ++++--
4 files changed, 50 insertions(+), 7 deletions(-)

Detailed changes

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.
 ///

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<Path> =
-                    git::repository::original_repo_path_from_common_dir(common_dir_abs_path).into();
+                let original_repo_abs_path: Arc<Path> = 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()
     }

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)
     });

crates/worktree/src/worktree.rs 🔗 

@@ -289,9 +289,11 @@ struct LocalRepositoryEntry {
     dot_git_abs_path: Arc<Path>,
     /// 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<Path>,
     /// Absolute path to the directory holding the repository's state.
     ///