Simplify worktree deletion: use git worktree remove instead of rename-to-temp

Richard Feldman created 1 week ago

The old approach renamed the worktree directory to a temp path before
deleting it, which raced with the filesystem watcher — the project's
worktree scanner would try to canonicalize the temp path while it was
being deleted, producing 'root path could not be canonicalized' errors.

Now we call git worktree remove --force first, which handles both the
git bookkeeping and directory deletion atomically. Manual fs removal
is only used as a fallback if git can't do it.

Change summary

crates/sidebar/src/sidebar.rs | 80 +++++++++++++++---------------------
1 file changed, 33 insertions(+), 47 deletions(-)

Detailed changes

crates/sidebar/src/sidebar.rs

@@ -3322,44 +3322,42 @@ impl Sidebar {
             }
         }
 
-        let timestamp = std::time::SystemTime::now()
-            .duration_since(std::time::UNIX_EPOCH)
-            .unwrap_or_default()
-            .as_nanos();
-        let temp_path = std::env::temp_dir().join(format!("zed-removing-worktree-{timestamp}"));
-
-        let dir_removed = if fs
-            .rename(
-                &worktree_path,
-                &temp_path,
-                fs::RenameOptions {
-                    overwrite: false,
-                    ..Default::default()
-                },
-            )
-            .await
-            .is_ok()
-        {
-            if let Some(main_repo) = &main_repo {
-                let receiver = main_repo.update(cx, |repo, _cx| {
-                    repo.remove_worktree(worktree_path.clone(), true)
-                });
-                if let Ok(result) = receiver.await {
-                    result.log_err();
+        // Use `git worktree remove --force` which handles both the git
+        // bookkeeping and directory deletion. Fall back to manual removal
+        // only if git can't do it.
+        let dir_removed = if let Some(main_repo) = &main_repo {
+            let receiver = main_repo.update(cx, |repo, _cx| {
+                repo.remove_worktree(worktree_path.clone(), true)
+            });
+            match receiver.await {
+                Ok(Ok(())) => true,
+                Ok(Err(err)) => {
+                    log::warn!("git worktree remove failed: {err}, trying manual removal");
+                    fs.remove_dir(
+                        &worktree_path,
+                        fs::RemoveOptions {
+                            recursive: true,
+                            ignore_if_not_exists: true,
+                        },
+                    )
+                    .await
+                    .is_ok()
+                }
+                Err(_) => {
+                    log::warn!("git worktree remove was canceled, trying manual removal");
+                    fs.remove_dir(
+                        &worktree_path,
+                        fs::RemoveOptions {
+                            recursive: true,
+                            ignore_if_not_exists: true,
+                        },
+                    )
+                    .await
+                    .is_ok()
                 }
             }
+        } else {
             fs.remove_dir(
-                &temp_path,
-                fs::RemoveOptions {
-                    recursive: true,
-                    ignore_if_not_exists: true,
-                },
-            )
-            .await
-            .log_err();
-            true
-        } else if fs
-            .remove_dir(
                 &worktree_path,
                 fs::RemoveOptions {
                     recursive: true,
@@ -3368,18 +3366,6 @@ impl Sidebar {
             )
             .await
             .is_ok()
-        {
-            if let Some(main_repo) = &main_repo {
-                let receiver = main_repo.update(cx, |repo, _cx| {
-                    repo.remove_worktree(worktree_path.clone(), true)
-                });
-                if let Ok(result) = receiver.await {
-                    result.log_err();
-                }
-            }
-            true
-        } else {
-            false
         };
 
         if !dir_removed {