[RFC PATCH v2] builtin/worktree: enhance worktree removal

[Kaartic Sivaraam]

"git worktree remove" removes both the named worktree
directory and the administrative information for it after
checking that there is no local modifications that would be
lost (which is a handy safety measure). However, due to a
possible oversight, it aborts with an error if the worktree
directory is _already_ removed.

The user could use "git worktree prune" after seeing the
error and realizing the situation, but at that point, there
is nothing gained by leaving only the administrative data
behind. Teach "git worktree remove" to go ahead and remove
the trace of the worktree in such a case.

Helped-by: Eric Sunshine <sunsh...@sunshineco.com>
Signed-off-by: Kaartic Sivaraam <kaartic.sivar...@gmail.com>
---
 Changes in v2:

  - incorporated the suggestion to avoid quieting `validate_worktree()`
    to detect inexistent directory (thanks, Eric!)

  - used the suggested (much better) commit message

 builtin/worktree.c | 35 ++++++++++++++++++++++++++++-------
 1 file changed, 28 insertions(+), 7 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index b5afba164..6eab91889 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -605,6 +605,23 @@ static int move_worktree(int ac, const char **av, const 
char *prefix)
        return update_worktree_location(wt, dst.buf);
 }
 
+/* Removes the .git/worktrees/worktree_id directory for
+ * the given worktree_id
+ *
+ * Returns 0 on success and non-zero value in case of failure
+ */
+static int remove_worktree_entry(char *worktree_id) {
+       int ret = 0;
+       struct strbuf we_path = STRBUF_INIT;
+       strbuf_addstr(&we_path, git_common_path("worktrees/%s", worktree_id));
+       if (remove_dir_recursively(&we_path, 0)) {
+               error_errno(_("failed to delete '%s'"), we_path.buf);
+               ret = -1;
+       }
+       strbuf_release(&we_path);
+       return ret;
+}
+
 static int remove_worktree(int ac, const char **av, const char *prefix)
 {
        int force = 0;
@@ -634,6 +651,16 @@ static int remove_worktree(int ac, const char **av, const 
char *prefix)
                        die(_("already locked, reason: %s"), reason);
                die(_("already locked, no reason"));
        }
+
+       if (!file_exists(wt->path)) {
+       /* There's a worktree entry but the worktree directory
+        * doesn't exist. So, just remove the worktree entry.
+        */
+               ret = remove_worktree_entry(wt->id);
+               free_worktrees(worktrees);
+               return ret;
+       }
+
        if (validate_worktree(wt, 0))
                return -1;
 
@@ -670,13 +697,7 @@ static int remove_worktree(int ac, const char **av, const 
char *prefix)
                error_errno(_("failed to delete '%s'"), sb.buf);
                ret = -1;
        }
-       strbuf_reset(&sb);
-       strbuf_addstr(&sb, git_common_path("worktrees/%s", wt->id));
-       if (remove_dir_recursively(&sb, 0)) {
-               error_errno(_("failed to delete '%s'"), sb.buf);
-               ret = -1;
-       }
-       strbuf_release(&sb);
+       ret = remove_worktree_entry(wt->id);
        free_worktrees(worktrees);
        return ret;
 }
-- 
2.15.0.345.gf926f18f3