From 134275b0ae013d520d6ddb8a6b192a00a50e2fa8 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Sun, 26 Jul 2026 20:55:39 -0400 Subject: [PATCH] Avoid unnecessary git scan when the agent creates a checkpoint (#61271) Agent checkpointing is implemented by staging selected files to a temporary git index file, then creating a commit. To avoid checkpointing commonly-excluded files even when the user doesn't have them in their gitignore, we write a temporary override to .git/info/exclude while running the staging. However, this causes us to do an unnecessary git scan when a checkpoint happens, because in general we want to do this when `.git/info/exclude` changes (the user might have edited it). This PR fixes that by doing the staging in two steps: - First, we stage any tracked files - Then we get a list of untracked files to include and stage those explicitly. The list is obtained by running `git ls-files` with a custom ignore file in addition to the `--standard` ignores; we write that custom ignore file to a path ending in `tmp` under .git, which means that our worktree code will not start a git scan in response to its events (the same way it already works for the temporary index file). .git/info/exclude is never touched. Note that we already listed all untracked files before this PR, in order to exclude the large ones. Release Notes: - Fixed Zed doing unnecessary work when the agent created a checkpoint. --- crates/git/src/repository.rs | 255 ++++++++++++++--------------------- 1 file changed, 99 insertions(+), 156 deletions(-) diff --git a/crates/git/src/repository.rs b/crates/git/src/repository.rs index 528d1fdfeb..59b06fce32 100644 --- a/crates/git/src/repository.rs +++ b/crates/git/src/repository.rs @@ -676,110 +676,6 @@ impl std::fmt::Display for FetchOptions { } } -/// Modifies .git/info/exclude temporarily -pub struct GitExcludeOverride { - git_exclude_path: PathBuf, - original_excludes: Option, - added_excludes: Option, -} - -impl GitExcludeOverride { - const START_BLOCK_MARKER: &str = "\n\n# ====== Auto-added by Zed: =======\n"; - const END_BLOCK_MARKER: &str = "\n# ====== End of auto-added by Zed =======\n"; - - pub async fn new(git_exclude_path: PathBuf) -> Result { - let original_excludes = - smol::fs::read_to_string(&git_exclude_path) - .await - .ok() - .map(|content| { - // Auto-generated lines are normally cleaned up in - // `restore_original()` or `drop()`, but may stuck in rare cases. - // Make sure to remove them. - Self::remove_auto_generated_block(&content) - }); - - Ok(GitExcludeOverride { - git_exclude_path, - original_excludes, - added_excludes: None, - }) - } - - pub async fn add_excludes(&mut self, excludes: &str) -> Result<()> { - self.added_excludes = Some(if let Some(ref already_added) = self.added_excludes { - format!("{already_added}\n{excludes}") - } else { - excludes.to_string() - }); - - let mut content = self.original_excludes.clone().unwrap_or_default(); - - content.push_str(Self::START_BLOCK_MARKER); - content.push_str(self.added_excludes.as_ref().unwrap()); - content.push_str(Self::END_BLOCK_MARKER); - - smol::fs::write(&self.git_exclude_path, content).await?; - Ok(()) - } - - pub async fn restore_original(&mut self) -> Result<()> { - if let Some(ref original) = self.original_excludes { - smol::fs::write(&self.git_exclude_path, original).await?; - } else if self.git_exclude_path.exists() { - smol::fs::remove_file(&self.git_exclude_path).await?; - } - - self.added_excludes = None; - - Ok(()) - } - - fn remove_auto_generated_block(content: &str) -> String { - let start_marker = Self::START_BLOCK_MARKER; - let end_marker = Self::END_BLOCK_MARKER; - let mut content = content.to_string(); - - let start_index = content.find(start_marker); - let end_index = content.rfind(end_marker); - - if let (Some(start), Some(end)) = (start_index, end_index) { - if end > start { - content.replace_range(start..end + end_marker.len(), ""); - } - } - - // Older versions of Zed didn't have end-of-block markers, - // so it's impossible to determine auto-generated lines. - // Conservatively remove the standard list of excludes - let standard_excludes = format!( - "{}{}", - Self::START_BLOCK_MARKER, - include_str!("./checkpoint.gitignore") - ); - content = content.replace(&standard_excludes, ""); - - content - } -} - -impl Drop for GitExcludeOverride { - fn drop(&mut self) { - if self.added_excludes.is_some() { - let git_exclude_path = self.git_exclude_path.clone(); - let original_excludes = self.original_excludes.clone(); - smol::spawn(async move { - if let Some(original) = original_excludes { - smol::fs::write(&git_exclude_path, original).await - } else { - smol::fs::remove_file(&git_exclude_path).await - } - }) - .detach(); - } - } -} - #[derive(Clone, Debug, Default, PartialEq, Eq, Hash, Copy)] pub enum LogOrder { #[default] @@ -2850,9 +2746,10 @@ impl GitRepository for RealGitRepository { let mut git = git?.envs(checkpoint_author_envs()); git.with_temp_index(async |git| { let head_sha = git.run(&["rev-parse", "HEAD"]).await.ok(); - let mut excludes = exclude_files(git).await?; - git.run(&["add", "--all"]).await?; + git.run(&["add", "--update"]).await?; + let untracked_files = untracked_files_for_checkpoint(git).await?; + add_files_to_index(git, &untracked_files).await?; let tree = git.run(&["write-tree"]).await?; let checkpoint_sha = if let Some(head_sha) = head_sha.as_deref() { git.run(&["commit-tree", &tree, "-p", head_sha, "-m", "Checkpoint"]) @@ -2861,8 +2758,6 @@ impl GitRepository for RealGitRepository { git.run(&["commit-tree", &tree, "-m", "Checkpoint"]).await? }; - excludes.restore_original().await?; - Ok(GitRepositoryCheckpoint { commit_sha: checkpoint_sha.parse()?, }) @@ -3532,42 +3427,109 @@ fn git_status_args(path_prefixes: &[RepoPath]) -> Vec { args } -/// Temporarily git-ignore commonly ignored files and files over 2MB -async fn exclude_files(git: &GitBinary) -> Result { +/// Lists untracked files that should be included in a checkpoint, skipping +/// commonly ignored file types and files over 2MB. +async fn untracked_files_for_checkpoint(git: &GitBinary) -> Result> { const MAX_SIZE: u64 = 2 * 1024 * 1024; // 2 MB - let mut excludes = git.with_exclude_overrides().await?; - excludes - .add_excludes(include_str!("./checkpoint.gitignore")) - .await?; - let working_directory = git.working_directory.clone(); - let untracked_files = git.list_untracked_files().await?; - let excluded_paths = untracked_files.into_iter().map(|path| { - let working_directory = working_directory.clone(); - smol::spawn(async move { - let full_path = working_directory.join(path.clone()); - match smol::fs::metadata(&full_path).await { - Ok(metadata) if metadata.is_file() && metadata.len() >= MAX_SIZE => { - Some(PathBuf::from("/").join(path.clone())) - } - _ => None, - } - }) + // The extra checkpoint excludes are passed ad hoc via --exclude-from + // rather than by mutating .git/info/exclude, whose writes would trigger a + // rescan of the repository. The scratch file is placed directly in the + // .git directory with a .tmp extension so that the worktree scanner + // filters out the events it generates. + let excludes_file_path = git + .git_directory + .join(format!("checkpoint-excludes-{}.tmp", Uuid::new_v4())); + + let delete_excludes_file = util::defer({ + let excludes_file_path = excludes_file_path.clone(); + let executor = git.executor.clone(); + move || { + executor + .spawn(async move { + smol::fs::remove_file(excludes_file_path).await.log_err(); + }) + .detach(); + } }); - let excluded_paths = futures::future::join_all(excluded_paths).await; - let excluded_paths = excluded_paths.into_iter().flatten().collect::>(); + smol::fs::write(&excludes_file_path, include_str!("./checkpoint.gitignore")).await?; - if !excluded_paths.is_empty() { - let exclude_patterns = excluded_paths - .into_iter() - .map(|path| path.to_string_lossy().into_owned()) - .collect::>() - .join("\n"); - excludes.add_excludes(&exclude_patterns).await?; + let mut exclude_from_arg = OsString::from("--exclude-from="); + exclude_from_arg.push(&excludes_file_path); + let output = git + .run(&[ + OsStr::new("ls-files"), + OsStr::new("--others"), + OsStr::new("--exclude-standard"), + OsStr::new("-z"), + exclude_from_arg.as_os_str(), + ]) + .await; + + smol::fs::remove_file(&excludes_file_path).await.ok(); + delete_excludes_file.abort(); + let output = output?; + + let working_directory = git.working_directory.clone(); + let size_checks = output + .split('\0') + .filter(|path| !path.is_empty()) + .map(|path| { + let full_path = working_directory.join(path); + let path = path.to_string(); + smol::spawn(async move { + match smol::fs::metadata(&full_path).await { + Ok(metadata) if metadata.is_file() && metadata.len() >= MAX_SIZE => None, + _ => Some(path), + } + }) + }) + .collect::>(); + + let untracked_files = futures::future::join_all(size_checks) + .await + .into_iter() + .flatten() + .collect(); + Ok(untracked_files) +} + +async fn add_files_to_index(git: &GitBinary, files: &[String]) -> Result<()> { + if files.is_empty() { + return Ok(()); } - Ok(excludes) + let mut process = git + .build_command(&["update-index", "--add", "-z", "--stdin"]) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn()?; + + let mut stdin = BufWriter::new( + process + .stdin + .take() + .context("no stdin for git update-index subprocess")?, + ); + for file in files { + stdin.write_all(file.as_bytes()).await?; + stdin.write_all(b"\0").await?; + } + stdin.flush().await?; + drop(stdin); + + let output = process.output().await?; + anyhow::ensure!( + output.status.success(), + GitBinaryCommandError { + stdout: String::from_utf8_lossy(&output.stdout).to_string(), + stderr: String::from_utf8_lossy(&output.stderr).to_string(), + status: output.status, + } + ); + Ok(()) } pub(crate) struct GitBinary { @@ -3599,19 +3561,6 @@ impl GitBinary { } } - async fn list_untracked_files(&self) -> Result> { - let status_output = self - .run(&["status", "--porcelain=v1", "--untracked-files=all", "-z"]) - .await?; - - let paths = status_output - .split('\0') - .filter(|entry| entry.len() >= 3 && entry.starts_with("?? ")) - .map(|entry| PathBuf::from(&entry[3..])) - .collect::>(); - Ok(paths) - } - fn envs(mut self, envs: HashMap) -> Self { self.envs = envs; self @@ -3652,12 +3601,6 @@ impl GitBinary { Ok(result) } - pub async fn with_exclude_overrides(&self) -> Result { - let path = self.git_directory.join("info").join("exclude"); - - GitExcludeOverride::new(path).await - } - fn path_for_index_id(&self, id: Uuid) -> PathBuf { self.git_directory.join(format!("index-{}.tmp", id)) }