yenru0/zed
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

git: Use the real gitdir for checkpoints (#51563)

Browse Source 
## Summary

Zed's git checkpoint code was assuming that repository-local metadata
always lives under `<working-directory>/.git/...`.

That assumption breaks for repositories where `.git` is a file instead
of a directory, most notably:

- git submodules
- git worktrees

In those layouts, `.git` contains a `gitdir: ...` pointer to the real
git directory. When checkpoint creation or restore tries to build paths
like `.git/index-<uuid>.tmp` or `.git/info/exclude`, those path
operations fail with `Not a directory (os error 20)`.

In practice, that shows up as checkpoint failures in ACP threads for
worktrees and submodules.

## Root Cause

`GitBinary` tracked the repository working directory, but it did not
track the resolved git directory.

As a result, checkpoint-related helpers derived temporary index and
exclude paths from the working tree:

- temp index path creation
- copying the default index into the temp index
- exclude override handling

That works for a plain repository where `.git` is a directory, but not
when `.git` is a pointer file.

## Changes

- Thread the resolved git directory from `RealGitRepository::path()`
into `GitBinary`.
- Add a dedicated `git_directory` field to `GitBinary`.
- Use `git_directory` instead of `<working-directory>/.git` for:
  - temp index file paths
  - copying the default `index`
  - `info/exclude` override paths
- Keep the working directory unchanged for command execution; only
metadata path resolution changes.
- Add a regression test that verifies temp index files are created under
the real gitdir, not under `<working-directory>/.git`.

## Why this approach

The minimal correct fix is to distinguish between:

- the working directory used to run git commands, and
- the actual git directory used to store repository metadata

Git itself already makes that distinction, and
`git2::Repository::path()` gives us the resolved gitdir directly.
Reusing that value keeps the fix small and avoids special-casing
submodules or worktrees elsewhere.

## User-visible impact

Before this change, checkpoint operations could fail in repositories
where `.git` is not a directory, producing errors like:

    Not a directory (os error 20)

After this change, checkpoint-related git metadata is read and written
from the real gitdir, so ACP checkpoints work in submodules and
worktrees the same way they do in regular repositories.

## Testing

- `cargo fmt -p git`
- `cargo test -p git --lib`
- `cargo clippy -p git --lib --tests -- -D warnings`

Added regression coverage:

- `test_path_for_index_id_uses_real_git_directory`

If you want, I can also compress this into a more GitHub-PR-native
version with sections like `## Problem`, `## Fix`, and `## Validation`.

Release Notes
- Fixed `.git` handling when `.git` is a file instead of directory, e.g.
when using worktrees or submodules.
This commit is contained in:
Stefan Junker authored and GitHub committed 2026-03-19 14:20:08 +00:00
1 parent 1b44159346
commit eb7f568455
1 file changed
+50 -15
Download Patch File Download Diff File Expand all files Collapse all files
crates/git/src/repository.rs
+50 -15
View File
@@ -943,6 +943,7 @@ impl RealGitRepository {
self.any_git_binary_path.clone(),
self.working_directory()
.with_context(|| "Can't run git commands without a working directory")?,
self.path(),
self.executor.clone(),
self.is_trusted(),
))
@@ -997,6 +998,7 @@ pub async fn get_git_committer(cx: &AsyncApp) -> GitCommitter {
let git = GitBinary::new(
git_binary_path.unwrap_or(PathBuf::from("git")),
paths::home_dir().clone(),
paths::home_dir().join(".git"),
cx.background_executor().clone(),
true,
);
@@ -2154,6 +2156,7 @@ impl GitRepository for RealGitRepository {
cx: AsyncApp,
) -> BoxFuture<'_, Result<RemoteCommandOutput>> {
let working_directory = self.working_directory();
let git_directory = self.path();
let executor = cx.background_executor().clone();
let git_binary_path = self.system_git_binary_path.clone();
let is_trusted = self.is_trusted();
@@ -2165,6 +2168,7 @@ impl GitRepository for RealGitRepository {
let git = GitBinary::new(
git_binary_path,
working_directory,
git_directory,
executor.clone(),
is_trusted,
);
@@ -2196,6 +2200,7 @@ impl GitRepository for RealGitRepository {
cx: AsyncApp,
) -> BoxFuture<'_, Result<RemoteCommandOutput>> {
let working_directory = self.working_directory();
let git_directory = self.path();
let executor = cx.background_executor().clone();
let git_binary_path = self.system_git_binary_path.clone();
let is_trusted = self.is_trusted();
@@ -2207,6 +2212,7 @@ impl GitRepository for RealGitRepository {
let git = GitBinary::new(
git_binary_path,
working_directory,
git_directory,
executor.clone(),
is_trusted,
);
@@ -2236,6 +2242,7 @@ impl GitRepository for RealGitRepository {
cx: AsyncApp,
) -> BoxFuture<'_, Result<RemoteCommandOutput>> {
let working_directory = self.working_directory();
let git_directory = self.path();
let remote_name = format!("{}", fetch_options);
let git_binary_path = self.system_git_binary_path.clone();
let executor = cx.background_executor().clone();
@@ -2248,6 +2255,7 @@ impl GitRepository for RealGitRepository {
let git = GitBinary::new(
git_binary_path,
working_directory,
git_directory,
executor.clone(),
is_trusted,
);
@@ -2900,6 +2908,7 @@ async fn exclude_files(git: &GitBinary) -> Result<GitExcludeOverride> {
pub(crate) struct GitBinary {
git_binary_path: PathBuf,
working_directory: PathBuf,
git_directory: PathBuf,
executor: BackgroundExecutor,
index_file_path: Option<PathBuf>,
envs: HashMap<String, String>,
@@ -2910,12 +2919,14 @@ impl GitBinary {
pub(crate) fn new(
git_binary_path: PathBuf,
working_directory: PathBuf,
git_directory: PathBuf,
executor: BackgroundExecutor,
is_trusted: bool,
) -> Self {
Self {
git_binary_path,
working_directory,
git_directory,
executor,
index_file_path: None,
envs: HashMap::default(),
@@ -2961,12 +2972,9 @@ impl GitBinary {
// Copy the default index file so that Git doesn't have to rebuild the
// whole index from scratch. This might fail if this is an empty repository.
smol::fs::copy(
self.working_directory.join(".git").join("index"),
&index_file_path,
)
.await
.ok();
smol::fs::copy(self.git_directory.join("index"), &index_file_path)
.await
.ok();
self.index_file_path = Some(index_file_path.clone());
let result = f(self).await;
@@ -2980,19 +2988,13 @@ impl GitBinary {
}
pub async fn with_exclude_overrides(&self) -> Result<GitExcludeOverride> {
let path = self
.working_directory
.join(".git")
.join("info")
.join("exclude");
let path = self.git_directory.join("info").join("exclude");
GitExcludeOverride::new(path).await
}
fn path_for_index_id(&self, id: Uuid) -> PathBuf {
self.working_directory
.join(".git")
.join(format!("index-{}.tmp", id))
self.git_directory.join(format!("index-{}.tmp", id))
}
pub async fn run<S>(&self, args: &[S]) -> Result<String>
@@ -3317,6 +3319,7 @@ mod tests {
let git = GitBinary::new(
PathBuf::from("git"),
dir.path().to_path_buf(),
dir.path().join(".git"),
cx.executor(),
false,
);
@@ -3330,6 +3333,7 @@ mod tests {
let git = GitBinary::new(
PathBuf::from("git"),
dir.path().to_path_buf(),
dir.path().join(".git"),
cx.executor(),
false,
);
@@ -3349,6 +3353,7 @@ mod tests {
let git = GitBinary::new(
PathBuf::from("git"),
dir.path().to_path_buf(),
dir.path().join(".git"),
cx.executor(),
false,
);
@@ -3374,6 +3379,7 @@ mod tests {
let git = GitBinary::new(
PathBuf::from("git"),
dir.path().to_path_buf(),
dir.path().join(".git"),
cx.executor(),
true,
);
@@ -3392,6 +3398,7 @@ mod tests {
let git = GitBinary::new(
PathBuf::from("git"),
dir.path().to_path_buf(),
dir.path().join(".git"),
cx.executor(),
true,
);
@@ -3406,6 +3413,27 @@ mod tests {
);
}
#[gpui::test]
async fn test_path_for_index_id_uses_real_git_directory(cx: &mut TestAppContext) {
cx.executor().allow_parking();
let working_directory = PathBuf::from("/code/worktree");
let git_directory = PathBuf::from("/code/repo/.git/modules/worktree");
let git = GitBinary::new(
PathBuf::from("git"),
working_directory,
git_directory.clone(),
cx.executor(),
false,
);
let path = git.path_for_index_id(Uuid::nil());
assert_eq!(
path,
git_directory.join(format!("index-{}.tmp", Uuid::nil()))
);
}
#[gpui::test]
async fn test_checkpoint_basic(cx: &mut TestAppContext) {
disable_git_global_config();
@@ -4090,13 +4118,20 @@ mod tests {
/// Force a Git garbage collection on the repository.
fn gc(&self) -> BoxFuture<'_, Result<()>> {
let working_directory = self.working_directory();
let git_directory = self.path();
let git_binary_path = self.any_git_binary_path.clone();
let executor = self.executor.clone();
self.executor
.spawn(async move {
let git_binary_path = git_binary_path.clone();
let working_directory = working_directory?;
let git = GitBinary::new(git_binary_path, working_directory, executor, true);
let git = GitBinary::new(
git_binary_path,
working_directory,
git_directory,
executor,
true,
);
git.run(&["gc", "--prune"]).await?;
Ok(())
})