Clean up empty zed-created ancestor dirs of deleted worktrees (#54202)

Max Brunsfeld created

Release Notes:

- N/A

Change summary

crates/project/src/git_store.rs               | 74 +++++++++++++++++++-
crates/project/tests/integration/git_store.rs | 65 ++++++++++++++++++
2 files changed, 133 insertions(+), 6 deletions(-)

Detailed changes

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<Result<()>> {
         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

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