From 8fe5e852cc5eca782226dcc41628bf23969a5ea0 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Fri, 3 Apr 2026 10:18:16 -0400 Subject: [PATCH] Simplify worktree deletion: use git worktree remove instead of rename-to-temp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- crates/sidebar/src/sidebar.rs | 80 +++++++++++++++-------------------- 1 file changed, 33 insertions(+), 47 deletions(-) diff --git a/crates/sidebar/src/sidebar.rs b/crates/sidebar/src/sidebar.rs index 3182154edc81678d94192776ff16683cc3488481..e936d1370a03aa0bd377218e4c8dc26b781a1f9e 100644 --- a/crates/sidebar/src/sidebar.rs +++ b/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 {