From 7b0f59bd0444a146856d3318275bb8a743246f2c Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 17 Apr 2026 12:57:50 -0700 Subject: [PATCH] Clean up empty zed-created ancestor dirs of deleted worktrees (#54202) Release Notes: - N/A --- crates/project/src/git_store.rs | 74 +++++++++++++++++-- crates/project/tests/integration/git_store.rs | 65 +++++++++++++++- 2 files changed, 133 insertions(+), 6 deletions(-) diff --git a/crates/project/src/git_store.rs b/crates/project/src/git_store.rs index 1b7ead6dba9e680ff62ef85835d83d0ff0229729..d35a13e7df3e55df662ab09270b5ec5a5203c2a8 100644 --- a/crates/project/src/git_store.rs +++ b/crates/project/src/git_store.rs @@ -6,6 +6,7 @@ pub mod pending_op; use crate::{ ProjectEnvironment, ProjectItem, ProjectPath, buffer_store::{BufferStore, BufferStoreEvent}, + project_settings::ProjectSettings, trusted_worktrees::{ PathTrust, TrustedWorktrees, TrustedWorktreesEvent, TrustedWorktreesStore, }, @@ -60,7 +61,7 @@ use rpc::{ proto::{self, git_reset, split_repository_update}, }; use serde::Deserialize; -use settings::WorktreeId; +use settings::{Settings, WorktreeId}; use smol::future::yield_now; use std::{ cmp::Ordering, @@ -6346,9 +6347,10 @@ impl Repository { pub fn remove_worktree(&mut self, path: PathBuf, force: bool) -> oneshot::Receiver> { let id = self.id; + let original_repo_abs_path = self.snapshot.original_repo_abs_path.clone(); self.send_job( Some(format!("git worktree remove: {}", path.display()).into()), - move |repo, _cx| async move { + move |repo, cx| async move { match repo { RepositoryState::Local(LocalRepositoryState { backend, fs, .. }) => { // When forcing, delete the worktree directory ourselves before @@ -6363,8 +6365,13 @@ impl Repository { // `GitRemoveWorktree` RPC is handled against the local repo on // the headless server) using its own filesystem. // - // Non-force removals are left untouched: `git worktree remove` - // must see the dirty working tree to refuse the operation. + // After a successful removal, also delete any empty ancestor + // directories between the worktree path and the configured + // base directory used when creating linked worktrees. + // + // Non-force removals are left untouched before git runs: + // `git worktree remove` must see the dirty working tree to + // refuse the operation. if force { fs.remove_dir( &path, @@ -6378,7 +6385,24 @@ impl Repository { format!("failed to delete worktree directory '{}'", path.display()) })?; } - backend.remove_worktree(path, force).await + + backend.remove_worktree(path.clone(), force).await?; + + let managed_worktree_base = cx.update(|cx| { + let setting = &ProjectSettings::get_global(cx).git.worktree_directory; + worktrees_directory_for_repo(&original_repo_abs_path, setting).log_err() + }); + + if let Some(managed_worktree_base) = managed_worktree_base { + remove_empty_managed_worktree_ancestors( + fs.as_ref(), + &path, + &managed_worktree_base, + ) + .await; + } + + Ok(()) } RepositoryState::Remote(RemoteRepositoryState { project_id, client }) => { client @@ -7403,6 +7427,46 @@ pub fn worktrees_directory_for_repo( Ok(resolved) } +async fn remove_empty_managed_worktree_ancestors(fs: &dyn Fs, child_path: &Path, base_path: &Path) { + let mut current = child_path; + while let Some(parent) = current.parent() { + if parent == base_path { + break; + } + if !parent.starts_with(base_path) { + break; + } + + let result = fs + .remove_dir( + parent, + RemoveOptions { + recursive: false, + ignore_if_not_exists: true, + }, + ) + .await; + + match result { + Ok(()) => { + log::info!( + "Removed empty managed worktree directory: {}", + parent.display() + ); + } + Err(error) => { + log::debug!( + "Stopped removing managed worktree parent directories at {}: {error}", + parent.display() + ); + break; + } + } + + current = parent; + } +} + /// Returns a short name for a linked worktree suitable for UI display /// /// Uses the main worktree path to come up with a short name that disambiguates diff --git a/crates/project/tests/integration/git_store.rs b/crates/project/tests/integration/git_store.rs index bbe5c64d7cf7f5b2ffa9160df6130cd88ddc5d69..766e41b100dc8b8855d25d6c81107fdc88246b76 100644 --- a/crates/project/tests/integration/git_store.rs +++ b/crates/project/tests/integration/git_store.rs @@ -1176,13 +1176,14 @@ mod git_traversal { } mod git_worktrees { - use fs::FakeFs; + use fs::{FakeFs, Fs}; use gpui::TestAppContext; use project::worktrees_directory_for_repo; use serde_json::json; use settings::SettingsStore; use std::path::{Path, PathBuf}; use util::path; + fn init_test(cx: &mut gpui::TestAppContext) { zlog::init_test(); @@ -1335,6 +1336,68 @@ mod git_worktrees { assert_eq!(worktree_2.sha.as_ref(), "fake-sha"); } + #[gpui::test] + async fn test_remove_worktree_removes_managed_parent_directories(cx: &mut TestAppContext) { + init_test(cx); + let fs = FakeFs::new(cx.background_executor.clone()); + fs.insert_tree( + path!("/root"), + json!({ + ".git": {}, + "file.txt": "content", + }), + ) + .await; + + let project = Project::test(fs.clone(), [path!("/root").as_ref()], cx).await; + cx.executor().run_until_parked(); + + let repository = project.read_with(cx, |project, cx| { + project.repositories(cx).values().next().unwrap().clone() + }); + + let worktree_path = PathBuf::from(path!("/worktrees/root/feature/nested/root")); + let worktree_parent = PathBuf::from(path!("/worktrees/root/feature/nested")); + let worktree_intermediate_parent = PathBuf::from(path!("/worktrees/root/feature")); + let worktree_base = PathBuf::from(path!("/worktrees/root")); + + cx.update(|cx| { + repository.update(cx, |repository, _| { + repository.create_worktree( + git::repository::CreateWorktreeTarget::NewBranch { + branch_name: "feature/nested".to_string(), + base_sha: Some("abc123".to_string()), + }, + worktree_path.clone(), + ) + }) + }) + .await + .unwrap() + .unwrap(); + + assert!(Fs::is_dir(fs.as_ref(), &worktree_path).await); + assert!(Fs::is_dir(fs.as_ref(), &worktree_parent).await); + assert!(Fs::is_dir(fs.as_ref(), &worktree_intermediate_parent).await); + assert!(Fs::is_dir(fs.as_ref(), &worktree_base).await); + + cx.update(|cx| { + repository.update(cx, |repository, _| { + repository.remove_worktree(worktree_path.clone(), false) + }) + }) + .await + .unwrap() + .unwrap(); + + cx.executor().run_until_parked(); + + assert!(!Fs::is_dir(fs.as_ref(), &worktree_path).await); + assert!(!Fs::is_dir(fs.as_ref(), &worktree_parent).await); + assert!(!Fs::is_dir(fs.as_ref(), &worktree_intermediate_parent).await); + assert!(Fs::is_dir(fs.as_ref(), &worktree_base).await); + } + use crate::Project; }