Fix handling of git repositories with an external git directory (#55402)

Closes https://github.com/zed-industries/zed/issues/54824

Previously, we always assumed that `gitdir` was an absolute path. Also,
we did not correctly handle custom gitignore files that were configured
via separate git directories.

Release Notes:

- Fixed failure to recognize git repositories where `gitdir` was
expressed as a relative path.
- Fixed handling of gitignores in git repositories that use a separate
git dir.
This commit is contained in:
Max Brunsfeld 2026-05-04 09:20:25 -07:00 committed by GitHub
parent 8f2ab516d0
commit 42f9420437
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 353 additions and 183 deletions

View file

@ -3964,19 +3964,22 @@ impl BackgroundScanner {
let repo = if scanning_enabled {
let (ignores, exclude, repo) =
discover_ancestor_git_repo(self.fs.clone(), &root_abs_path).await;
self.state
.lock()
.await
.snapshot
.ignores_by_parent_abs_path
.extend(ignores);
let mut state = self.state.lock().await;
state.snapshot.ignores_by_parent_abs_path.extend(ignores);
if let Some(exclude) = exclude {
self.state
.lock()
.await
let work_directory_abs_path: Arc<Path> = repo
.as_ref()
.map(|(_, work_directory)| {
state
.snapshot
.work_directory_abs_path(work_directory)
.into()
})
.unwrap_or_else(|| root_abs_path.as_path().into());
state
.snapshot
.repo_exclude_by_work_dir_abs_path
.insert(root_abs_path.as_path().into(), (exclude, false));
.insert(work_directory_abs_path, (exclude, false));
}
repo
@ -5078,12 +5081,8 @@ impl BackgroundScanner {
state.snapshot.ignores_by_parent_abs_path.extend(ignores);
if let Some((ancestor_dot_git, work_directory)) = repo {
if let Some(exclude) = exclude {
let work_directory_abs_path = self
.state
.lock()
.await
.snapshot
.work_directory_abs_path(&work_directory);
let work_directory_abs_path =
state.snapshot.work_directory_abs_path(&work_directory);
state
.snapshot
@ -5201,7 +5200,11 @@ impl BackgroundScanner {
if *needs_update {
*needs_update = false;
ignores_to_update.push(work_dir_abs_path.clone());
if work_dir_abs_path.starts_with(abs_path.as_path()) {
ignores_to_update.push(work_dir_abs_path.clone());
} else {
ignores_to_update.push(abs_path.as_path().into());
}
if let Some((_, repository)) = repository {
let exclude_abs_path = repository.common_dir_abs_path.join(REPO_EXCLUDE);
@ -5543,39 +5546,45 @@ async fn discover_ancestor_git_repo(
.await
.is_ok_and(|metadata| metadata.is_some())
{
if index != 0 {
let dot_git_abs_path = if index != 0 {
// We canonicalize, since the FS events use the canonicalized path.
if let Some(ancestor_dot_git) = fs.canonicalize(&ancestor_dot_git).await.log_err() {
let location_in_repo = root_abs_path
.as_path()
.strip_prefix(ancestor)
.unwrap()
.into();
log::info!("inserting parent git repo for this worktree: {location_in_repo:?}");
// We associate the external git repo with our root folder and
// also mark where in the git repo the root folder is located.
return (
ignores,
exclude,
Some((
ancestor_dot_git,
WorkDirectory::AboveProject {
absolute_path: ancestor.into(),
location_in_repo,
},
)),
);
};
}
match fs.canonicalize(&ancestor_dot_git).await.log_err() {
Some(path) => path,
None => continue,
}
} else {
ancestor_dot_git.clone()
};
let dot_git_abs_path: Arc<Path> = dot_git_abs_path.as_path().into();
let (_, common_dir_abs_path) = discover_git_paths(&dot_git_abs_path, fs.as_ref()).await;
let dot_git_path: Arc<Path> = ancestor_dot_git.into();
let (_, common_dir_abs_path) = discover_git_paths(&dot_git_path, fs.as_ref()).await;
let repo_exclude_abs_path = common_dir_abs_path.join(REPO_EXCLUDE);
if let Ok(repo_exclude) = build_gitignore(&repo_exclude_abs_path, fs.as_ref()).await {
exclude = Some(Arc::new(repo_exclude));
}
// Reached root of git repository.
if index != 0 {
let location_in_repo = root_abs_path
.as_path()
.strip_prefix(ancestor)
.unwrap()
.into();
log::info!("inserting parent git repo for this worktree: {location_in_repo:?}");
// We associate the external git repo with our root folder and
// also mark where in the git repo the root folder is located.
return (
ignores,
exclude,
Some((
dot_git_abs_path.as_ref().into(),
WorkDirectory::AboveProject {
absolute_path: ancestor.into(),
location_in_repo,
},
)),
);
}
break;
}
}
@ -6285,6 +6294,26 @@ fn parse_gitfile(content: &str) -> anyhow::Result<&Path> {
Ok(Path::new(path.trim()))
}
fn resolve_gitfile_path(dot_git_abs_path: &Path, gitfile_path: &Path) -> PathBuf {
if gitfile_path.is_absolute() {
gitfile_path.into()
} else {
dot_git_abs_path
.parent()
.unwrap_or_else(|| Path::new(""))
.join(gitfile_path)
}
}
fn resolve_commondir_path(repository_dir_abs_path: &Path, commondir_path: &str) -> PathBuf {
let commondir_path = Path::new(commondir_path.trim());
if commondir_path.is_absolute() {
commondir_path.into()
} else {
repository_dir_abs_path.join(commondir_path)
}
}
pub async fn discover_root_repo_common_dir(root_abs_path: &Path, fs: &dyn Fs) -> Option<Arc<Path>> {
let root_dot_git = root_abs_path.join(DOT_GIT);
if !fs.metadata(&root_dot_git).await.is_ok_and(|m| m.is_some()) {
@ -6306,17 +6335,14 @@ async fn discover_git_paths(dot_git_abs_path: &Arc<Path>, fs: &dyn Fs) -> (Arc<P
.as_ref()
.and_then(|contents| parse_gitfile(contents).log_err())
{
let path = dot_git_abs_path
.parent()
.unwrap_or(Path::new(""))
.join(path);
let path = resolve_gitfile_path(dot_git_abs_path, path);
if let Some(path) = fs.canonicalize(&path).await.log_err() {
repository_dir_abs_path = Path::new(&path).into();
common_dir_abs_path = repository_dir_abs_path.clone();
if let Some(commondir_contents) = fs.load(&path.join("commondir")).await.ok()
&& let Some(commondir_path) = fs
.canonicalize(&path.join(commondir_contents.trim()))
.canonicalize(&resolve_commondir_path(&path, &commondir_contents))
.await
.log_err()
{

View file

@ -1220,15 +1220,17 @@ async fn test_file_scan_inclusions(cx: &mut TestAppContext) {
// Assert that file_scan_inclusions overrides file_scan_exclusions.
check_worktree_entries(
tree,
&[],
&["target", "node_modules"],
&["src/lib.rs", "src/bar/bar.rs", ".gitignore"],
&[
"node_modules/prettier/package.json",
".DS_Store",
"node_modules/.DS_Store",
"src/.DS_Store",
],
WorktreeExpectations {
excluded_paths: &[],
ignored_paths: &["target", "node_modules"],
tracked_paths: &["src/lib.rs", "src/bar/bar.rs", ".gitignore"],
included_paths: &[
"node_modules/prettier/package.json",
".DS_Store",
"node_modules/.DS_Store",
"src/.DS_Store",
],
},
)
});
}
@ -1287,10 +1289,12 @@ async fn test_file_scan_exclusions_overrules_inclusions(cx: &mut TestAppContext)
// Assert that file_scan_inclusions overrides file_scan_exclusions.
check_worktree_entries(
tree,
&[".DS_Store, src/.DS_Store"],
&["target", "node_modules"],
&["src/foo/another.rs", "src/foo/foo.rs", ".gitignore"],
&[],
WorktreeExpectations {
excluded_paths: &[".DS_Store, src/.DS_Store"],
ignored_paths: &["target", "node_modules"],
tracked_paths: &["src/foo/another.rs", "src/foo/foo.rs", ".gitignore"],
included_paths: &[],
},
)
});
}
@ -1433,16 +1437,18 @@ async fn test_file_scan_exclusions(cx: &mut TestAppContext) {
tree.read_with(cx, |tree, _| {
check_worktree_entries(
tree,
&[
"src/foo/foo.rs",
"src/foo/another.rs",
"node_modules/.DS_Store",
"src/.DS_Store",
".DS_Store",
],
&["target", "node_modules"],
&["src/lib.rs", "src/bar/bar.rs", ".gitignore"],
&[],
WorktreeExpectations {
excluded_paths: &[
"src/foo/foo.rs",
"src/foo/another.rs",
"node_modules/.DS_Store",
"src/.DS_Store",
".DS_Store",
],
ignored_paths: &["target", "node_modules"],
tracked_paths: &["src/lib.rs", "src/bar/bar.rs", ".gitignore"],
included_paths: &[],
},
)
});
@ -1459,22 +1465,24 @@ async fn test_file_scan_exclusions(cx: &mut TestAppContext) {
tree.read_with(cx, |tree, _| {
check_worktree_entries(
tree,
&[
"node_modules/prettier/package.json",
"node_modules/.DS_Store",
"node_modules",
],
&["target"],
&[
".gitignore",
"src/lib.rs",
"src/bar/bar.rs",
"src/foo/foo.rs",
"src/foo/another.rs",
"src/.DS_Store",
".DS_Store",
],
&[],
WorktreeExpectations {
excluded_paths: &[
"node_modules/prettier/package.json",
"node_modules/.DS_Store",
"node_modules",
],
ignored_paths: &["target"],
tracked_paths: &[
".gitignore",
"src/lib.rs",
"src/bar/bar.rs",
"src/foo/foo.rs",
"src/foo/another.rs",
"src/.DS_Store",
".DS_Store",
],
included_paths: &[],
},
)
});
}
@ -1628,25 +1636,27 @@ async fn test_fs_events_in_exclusions(cx: &mut TestAppContext) {
tree.read_with(cx, |tree, _| {
check_worktree_entries(
tree,
&[
".git/HEAD",
".git/foo",
"node_modules",
"node_modules/.DS_Store",
"node_modules/prettier",
"node_modules/prettier/package.json",
],
&["target"],
&[
".DS_Store",
"src/.DS_Store",
"src/lib.rs",
"src/foo/foo.rs",
"src/foo/another.rs",
"src/bar/bar.rs",
".gitignore",
],
&[],
WorktreeExpectations {
excluded_paths: &[
".git/HEAD",
".git/foo",
"node_modules",
"node_modules/.DS_Store",
"node_modules/prettier",
"node_modules/prettier/package.json",
],
ignored_paths: &["target"],
tracked_paths: &[
".DS_Store",
"src/.DS_Store",
"src/lib.rs",
"src/foo/foo.rs",
"src/foo/another.rs",
"src/bar/bar.rs",
".gitignore",
],
included_paths: &[],
},
)
});
@ -1683,31 +1693,33 @@ async fn test_fs_events_in_exclusions(cx: &mut TestAppContext) {
tree.read_with(cx, |tree, _| {
check_worktree_entries(
tree,
&[
".git/HEAD",
".git/foo",
".git/new_file",
"node_modules",
"node_modules/.DS_Store",
"node_modules/prettier",
"node_modules/prettier/package.json",
"node_modules/new_file",
"build_output",
"build_output/new_file",
"test_output/new_file",
],
&["target", "test_output"],
&[
".DS_Store",
"src/.DS_Store",
"src/lib.rs",
"src/foo/foo.rs",
"src/foo/another.rs",
"src/bar/bar.rs",
"src/new_file",
".gitignore",
],
&[],
WorktreeExpectations {
excluded_paths: &[
".git/HEAD",
".git/foo",
".git/new_file",
"node_modules",
"node_modules/.DS_Store",
"node_modules/prettier",
"node_modules/prettier/package.json",
"node_modules/new_file",
"build_output",
"build_output/new_file",
"test_output/new_file",
],
ignored_paths: &["target", "test_output"],
tracked_paths: &[
".DS_Store",
"src/.DS_Store",
"src/lib.rs",
"src/foo/foo.rs",
"src/foo/another.rs",
"src/bar/bar.rs",
"src/new_file",
".gitignore",
],
included_paths: &[],
},
)
});
}
@ -1739,14 +1751,26 @@ async fn test_fs_events_in_dot_git_worktree(cx: &mut TestAppContext) {
.await;
tree.flush_fs_events(cx).await;
tree.read_with(cx, |tree, _| {
check_worktree_entries(tree, &[], &["HEAD", "foo"], &[], &[])
check_worktree_entries(
tree,
WorktreeExpectations {
ignored_paths: &["HEAD", "foo"],
..Default::default()
},
)
});
std::fs::write(dot_git_worktree_dir.join("new_file"), "new file contents")
.unwrap_or_else(|e| panic!("Failed to create in {dot_git_worktree_dir:?} a new file: {e}"));
tree.flush_fs_events(cx).await;
tree.read_with(cx, |tree, _| {
check_worktree_entries(tree, &[], &["HEAD", "foo", "new_file"], &[], &[])
check_worktree_entries(
tree,
WorktreeExpectations {
ignored_paths: &["HEAD", "foo", "new_file"],
..Default::default()
},
)
});
}
@ -2785,10 +2809,11 @@ async fn test_global_gitignore(executor: BackgroundExecutor, cx: &mut TestAppCon
worktree.update(cx, |worktree, _cx| {
check_worktree_entries(
worktree,
&[],
&["foo", "bar", "subrepo/bar"],
&["sub/bar", "baz"],
&[],
WorktreeExpectations {
ignored_paths: &["foo", "bar", "subrepo/bar"],
tracked_paths: &["sub/bar", "baz"],
..Default::default()
},
);
});
@ -2809,10 +2834,11 @@ async fn test_global_gitignore(executor: BackgroundExecutor, cx: &mut TestAppCon
worktree.update(cx, |worktree, _cx| {
check_worktree_entries(
worktree,
&[],
&["bar", "subrepo/bar"],
&["foo", "sub/bar", "baz"],
&[],
WorktreeExpectations {
ignored_paths: &["bar", "subrepo/bar"],
tracked_paths: &["foo", "sub/bar", "baz"],
..Default::default()
},
);
});
@ -2836,10 +2862,11 @@ async fn test_global_gitignore(executor: BackgroundExecutor, cx: &mut TestAppCon
worktree.update(cx, |worktree, _cx| {
check_worktree_entries(
worktree,
&[],
&["bar"],
&["foo", "sub/bar", "baz", "subrepo/bar"],
&[],
WorktreeExpectations {
ignored_paths: &["bar"],
tracked_paths: &["foo", "sub/bar", "baz", "subrepo/bar"],
..Default::default()
},
);
});
}
@ -2899,17 +2926,13 @@ async fn test_repo_exclude_in_worktree(executor: BackgroundExecutor, cx: &mut Te
// .env.local should be ignored via info/exclude from the repo's exclude
worktree.update(cx, |worktree, _cx| {
let expected_excluded_paths = [];
let expected_ignored_paths = [".env.local"];
let expected_tracked_paths = ["not-ignored.txt"];
let expected_included_paths = [];
check_worktree_entries(
worktree,
&expected_excluded_paths,
&expected_ignored_paths,
&expected_tracked_paths,
&expected_included_paths,
WorktreeExpectations {
ignored_paths: &[".env.local"],
tracked_paths: &["not-ignored.txt"],
..Default::default()
},
);
});
}
@ -2959,17 +2982,13 @@ async fn test_repo_exclude(executor: BackgroundExecutor, cx: &mut TestAppContext
// .gitignore overrides .git/info/exclude
worktree.update(cx, |worktree, _cx| {
let expected_excluded_paths = [];
let expected_ignored_paths = [".env.local"];
let expected_tracked_paths = [".env.example", "README.md", "src/main.rs"];
let expected_included_paths = [];
check_worktree_entries(
worktree,
&expected_excluded_paths,
&expected_ignored_paths,
&expected_tracked_paths,
&expected_included_paths,
WorktreeExpectations {
ignored_paths: &[".env.local"],
tracked_paths: &[".env.example", "README.md", "src/main.rs"],
..Default::default()
},
);
});
@ -2988,37 +3007,34 @@ async fn test_repo_exclude(executor: BackgroundExecutor, cx: &mut TestAppContext
cx.run_until_parked();
worktree.update(cx, |worktree, _cx| {
let expected_excluded_paths = [];
let expected_ignored_paths = [];
let expected_tracked_paths = [".env.example", ".env.local", "README.md", "src/main.rs"];
let expected_included_paths = [];
check_worktree_entries(
worktree,
&expected_excluded_paths,
&expected_ignored_paths,
&expected_tracked_paths,
&expected_included_paths,
WorktreeExpectations {
tracked_paths: &[".env.example", ".env.local", "README.md", "src/main.rs"],
..Default::default()
},
);
});
}
#[derive(Default)]
struct WorktreeExpectations {
excluded_paths: &'static [&'static str],
ignored_paths: &'static [&'static str],
tracked_paths: &'static [&'static str],
included_paths: &'static [&'static str],
}
#[track_caller]
fn check_worktree_entries(
tree: &Worktree,
expected_excluded_paths: &[&str],
expected_ignored_paths: &[&str],
expected_tracked_paths: &[&str],
expected_included_paths: &[&str],
) {
for path in expected_excluded_paths {
fn check_worktree_entries(tree: &Worktree, expectations: WorktreeExpectations) {
for path in expectations.excluded_paths {
let entry = tree.entry_for_path(rel_path(path));
assert!(
entry.is_none(),
"expected path '{path}' to be excluded, but got entry: {entry:?}",
);
}
for path in expected_ignored_paths {
for path in expectations.ignored_paths {
let entry = tree
.entry_for_path(rel_path(path))
.unwrap_or_else(|| panic!("Missing entry for expected ignored path '{path}'"));
@ -3027,7 +3043,7 @@ fn check_worktree_entries(
"expected path '{path}' to be ignored, but got entry: {entry:?}",
);
}
for path in expected_tracked_paths {
for path in expectations.tracked_paths {
let entry = tree
.entry_for_path(rel_path(path))
.unwrap_or_else(|| panic!("Missing entry for expected tracked path '{path}'"));
@ -3036,7 +3052,7 @@ fn check_worktree_entries(
"expected path '{path}' to be tracked, but got entry: {entry:?}",
);
}
for path in expected_included_paths {
for path in expectations.included_paths {
let entry = tree
.entry_for_path(rel_path(path))
.unwrap_or_else(|| panic!("Missing entry for expected included path '{path}'"));
@ -3047,6 +3063,134 @@ fn check_worktree_entries(
}
}
#[gpui::test]
async fn test_root_repo_common_dir_for_relative_gitdir(
executor: BackgroundExecutor,
cx: &mut TestAppContext,
) {
init_test(cx);
let fs = FakeFs::new(executor);
fs.insert_tree(
path!("/repo"),
json!({
".git": {
"HEAD": "ref: refs/heads/main",
"config": "[core]\n\tbare = false\n",
"info": {
"exclude": "ignored.txt\n",
},
"worktrees": {
"feature-a": {
"HEAD": "ref: refs/heads/feature-a",
"commondir": "../..",
"gitdir": "/repo/feature-a/.git",
},
},
},
"feature-a": {
".git": "gitdir: ../.git/worktrees/feature-a",
"file.txt": "content",
"ignored.txt": "ignored",
"subdir": {
"file.txt": "content",
"ignored.txt": "ignored",
},
},
}),
)
.await;
let feature_tree = Worktree::local(
path!("/repo/feature-a").as_ref(),
true,
fs.clone(),
Arc::default(),
true,
WorktreeId::from_proto(0),
&mut cx.to_async(),
)
.await
.unwrap();
feature_tree
.update(cx, |tree, _| tree.as_local().unwrap().scan_complete())
.await;
cx.run_until_parked();
feature_tree.read_with(cx, |tree, _| {
assert_eq!(
tree.snapshot()
.root_repo_common_dir()
.map(|path| path.as_ref()),
Some(Path::new(path!("/repo/.git"))),
);
check_worktree_entries(
tree,
WorktreeExpectations {
ignored_paths: &["ignored.txt"],
tracked_paths: &["file.txt"],
..Default::default()
},
);
});
let nested_tree = Worktree::local(
path!("/repo/feature-a/subdir").as_ref(),
true,
fs.clone(),
Arc::default(),
true,
WorktreeId::from_proto(1),
&mut cx.to_async(),
)
.await
.unwrap();
nested_tree
.update(cx, |tree, _| tree.as_local().unwrap().scan_complete())
.await;
cx.run_until_parked();
nested_tree.read_with(cx, |tree, _| {
check_worktree_entries(
tree,
WorktreeExpectations {
ignored_paths: &["ignored.txt"],
tracked_paths: &["file.txt"],
..Default::default()
},
);
});
fs.write(
Path::new(path!("/repo/.git")).join(REPO_EXCLUDE).as_ref(),
"file.txt\n".as_bytes(),
)
.await
.unwrap();
cx.run_until_parked();
feature_tree.read_with(cx, |tree, _| {
check_worktree_entries(
tree,
WorktreeExpectations {
ignored_paths: &["file.txt", "subdir/file.txt"],
tracked_paths: &["ignored.txt", "subdir/ignored.txt"],
..Default::default()
},
);
});
nested_tree.read_with(cx, |tree, _| {
check_worktree_entries(
tree,
WorktreeExpectations {
ignored_paths: &["file.txt"],
tracked_paths: &["ignored.txt"],
..Default::default()
},
);
});
}
#[gpui::test]
async fn test_root_repo_common_dir(executor: BackgroundExecutor, cx: &mut TestAppContext) {
init_test(cx);