[RFC PATCH] builtin/worktree: enhance worktree removal

From: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
Date: Nov 21, 2017, 15:09 UTC
Message-ID: <20171121150954.3227-1-kaartic.sivaraam@gmail.com>

The new feature to 'remove' worktree was handy to remove specific worktrees. It didn't cover one particular case of removal. Specifically, if there is an "entry" (a directory in <main_worktree>/.git/worktrees) for a worktree but the worktree repository itself does not exist then it means that the "entry" is stale and it could just be removed.

So, in case there's a "worktree entry" but not "worktree direectory" then just remove the 'stale' entry.

Signed-off-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
---

Hello Duy,

I noticed that your remove command could be enhanced for a particular case. So, I made up an ad-hoc patch "illustrating" how it could be done. I may have broken something by quieting out 'validate_worktree()' function, so take note of it.

You might add this as a separate commit or just incorporate it into one of your commits if you re-roll your 'nd/worktree-move' branch.

Thanks, Kaartic

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

diff --git a/builtin/worktree.c b/builtin/worktree.c
index b5afba164..f70bc0bd8 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -605,6 +605,22 @@ 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,9 +650,17 @@ static int remove_worktree(int ac, const char **av, const char *prefix)
 			die(_("already locked, reason: %s"), reason);
 		die(_("already locked, no reason"));
 	}
-	if (validate_worktree(wt, 0))
-		return -1;
-
+	if (validate_worktree(wt, 1)) {
+		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;
+		} else {
+			return -1;
+		}
+	}
 	if (!force) {
 		struct argv_array child_env = ARGV_ARRAY_INIT;
 		struct child_process cp;
@@ -670,13 +694,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