[PATCH v4 1/4] worktree remove: refuse a worktree in the middle of an operation

[Jasper Mayone <me@jaspermayone.com>]

"git worktree remove" refuses to remove a worktree with modified or
untracked files unless --force is given, but it removes a worktree
that is in the middle of a rebase, a "git am" session, a bisect, a
cherry-pick or a revert when its files happen to be clean. The state
of that operation lives in the worktree's own $GIT_DIR, so it is lost
with the worktree. Examples of such a clean worktree are a rebase
stopped at an "edit" or "break" line or after a failed "exec", a
cherry-pick sequence after a conflict was resolved and committed, and
a single cherry-pick or revert whose conflict was resolved to the
current contents, which leaves CHERRY_PICK_HEAD or REVERT_HEAD behind.

Treat such a worktree as unclean: refuse to remove it unless --force
is given, and name the operation in the message.

A worktree whose directory is already gone is still removed without
--force, as before: the check is part of the check for a clean
worktree, which is only done when the directory exists.

Reported-by: D. Ben Knoble <ben.knoble@gmail.com>
Signed-off-by: Jasper Mayone <me@jaspermayone.com>
---
 Documentation/git-worktree.adoc | 17 ++++--
 builtin/worktree.c              | 37 ++++++++++++-
 t/t2403-worktree-move.sh        | 94 +++++++++++++++++++++++++++++++++
 3 files changed, 142 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-worktree.adoc b/Documentation/git-worktree.adoc
index e6e77252ab..4a7dc47dd8 100644
--- a/Documentation/git-worktree.adoc
+++ b/Documentation/git-worktree.adoc
@@ -141,10 +141,16 @@ the new working tree location.
 
 `remove`::
 
-Remove a worktree. Only clean worktrees (no untracked files and no
-modification in tracked files) can be removed. Unclean worktrees or ones
-with submodules can be removed with `--force`. The main worktree cannot be
-removed.
+Remove a worktree. The main worktree cannot be removed. Without
+`--force`, a worktree is not removed when:
++
+--
+* it has untracked files or modifications in tracked files,
+* it has submodules, or
+* it is in the middle of a rebase, a `git am` session, a bisect, a
+  cherry-pick or a revert, because removing it loses the state of that
+  operation.
+--
 
 `repair [<path>...]`::
 
@@ -189,7 +195,8 @@ twice. If the destination is already assigned to some other worktree but is
 missing (for instance, if _<new-path>_ was deleted manually), then `--force`
 allows the move to proceed; use `--force` twice if the destination is locked.
 +
-`remove` refuses to remove an unclean worktree unless `--force` is used.
+`remove` refuses to remove a worktree in the cases listed under
+`remove` above unless `--force` is used.
 To remove a locked worktree, specify `--force` twice.
 
 `-b <new-branch>`::
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 77ecd0f71f..7f53215cda 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -26,6 +26,7 @@
 #include "submodule.h"
 #include "utf8.h"
 #include "worktree.h"
+#include "wt-status.h"
 #include "quote.h"
 
 #define BUILTIN_WORKTREE_ADD_USAGE \
@@ -1408,6 +1409,38 @@ static void check_clean_worktree(struct worktree *wt,
 			  original_path, ret);
 }
 
+/*
+ * A rebase, "git am", bisect, cherry-pick or revert keeps its state in
+ * the worktree's own $GIT_DIR, and removing the worktree throws that
+ * away even when "git status" says the files are clean.
+ */
+static void check_no_operation_in_progress(struct worktree *wt,
+					   const char *original_path)
+{
+	struct wt_status_state state = { 0 };
+
+	if (wt_status_check_rebase(wt, &state)) {
+		if (state.am_in_progress)
+			die(_("'%s' is in the middle of an am session, use --force to delete it"),
+			    original_path);
+		die(_("'%s' is in the middle of a rebase, use --force to delete it"),
+		    original_path);
+	}
+	if (wt_status_check_bisect(wt, &state))
+		die(_("'%s' is in the middle of a bisect, use --force to delete it"),
+		    original_path);
+	wt_status_state_free_buffers(&state);
+	if (refs_ref_exists(get_worktree_ref_store(wt), "CHERRY_PICK_HEAD"))
+		die(_("'%s' is in the middle of a cherry-pick, use --force to delete it"),
+		    original_path);
+	if (refs_ref_exists(get_worktree_ref_store(wt), "REVERT_HEAD"))
+		die(_("'%s' is in the middle of a revert, use --force to delete it"),
+		    original_path);
+	if (is_directory(worktree_git_path(wt, "sequencer")))
+		die(_("'%s' is in the middle of a cherry-pick or revert, use --force to delete it"),
+		    original_path);
+}
+
 static int delete_git_work_tree(struct worktree *wt)
 {
 	struct strbuf sb = STRBUF_INIT;
@@ -1461,8 +1494,10 @@ static int remove_worktree(int ac, const char **av, const char *prefix,
 	strbuf_release(&errmsg);
 
 	if (file_exists(wt->path)) {
-		if (!force)
+		if (!force) {
+			check_no_operation_in_progress(wt, av[0]);
 			check_clean_worktree(wt, av[0]);
+		}
 
 		ret |= delete_git_work_tree(wt);
 	}
diff --git a/t/t2403-worktree-move.sh b/t/t2403-worktree-move.sh
index 69768c1207..2e45cd77f9 100755
--- a/t/t2403-worktree-move.sh
+++ b/t/t2403-worktree-move.sh
@@ -271,4 +271,98 @@ test_expect_success 'move worktree with relative path to absolute path' '
 	test_cmp expect .git/worktrees/absolute/gitdir
 '
 
+test_expect_success 'setup commits that conflict for am, cherry-pick and revert' '
+	git worktree add --detach conflict-setup init &&
+	(
+		cd conflict-setup &&
+		test_commit base conflict.t base &&
+		git switch --detach init &&
+		test_commit side conflict.t side &&
+		test_commit side-two
+	) &&
+	git worktree remove conflict-setup
+'
+
+test_expect_success 'remove worktree in the middle of a rebase' '
+	test_when_finished "rm -rf mid-rebase; git worktree prune" &&
+	git worktree add --detach mid-rebase side &&
+	test_must_fail git -C mid-rebase rebase -f -x false init &&
+	test_must_fail git worktree remove mid-rebase 2>err &&
+	test_grep "in the middle of a rebase" err &&
+	test_path_is_dir mid-rebase &&
+	git worktree remove --force mid-rebase &&
+	test_path_is_missing mid-rebase
+'
+
+test_expect_success 'remove worktree in the middle of an am session' '
+	test_when_finished "rm -rf mid-am patch; git worktree prune" &&
+	git format-patch -1 --stdout side >patch &&
+	git worktree add --detach mid-am base &&
+	(
+		cd mid-am &&
+		test_must_fail git am ../patch &&
+		git reset --hard
+	) &&
+	test_must_fail git worktree remove mid-am 2>err &&
+	test_grep "in the middle of an am session" err &&
+	git worktree remove --force mid-am
+'
+
+test_expect_success 'remove worktree in the middle of a bisect' '
+	test_when_finished "rm -rf mid-bisect; git worktree prune" &&
+	git worktree add --detach mid-bisect side-two &&
+	git -C mid-bisect bisect start side-two init &&
+	test_must_fail git worktree remove mid-bisect 2>err &&
+	test_grep "in the middle of a bisect" err &&
+	git worktree remove --force mid-bisect
+'
+
+test_expect_success 'remove worktree in the middle of a cherry-pick' '
+	test_when_finished "rm -rf mid-pick; git worktree prune" &&
+	git worktree add --detach mid-pick base &&
+	(
+		cd mid-pick &&
+		test_must_fail git cherry-pick side &&
+		git checkout --ours conflict.t &&
+		git add conflict.t &&
+		git rev-parse --verify CHERRY_PICK_HEAD
+	) &&
+	test_path_is_missing .git/worktrees/mid-pick/sequencer &&
+	test_must_fail git worktree remove mid-pick 2>err &&
+	test_grep "in the middle of a cherry-pick," err &&
+	git worktree remove --force mid-pick
+'
+
+test_expect_success 'remove worktree in the middle of a revert' '
+	test_when_finished "rm -rf mid-revert; git worktree prune" &&
+	git worktree add --detach mid-revert base &&
+	(
+		cd mid-revert &&
+		test_must_fail git revert side &&
+		git checkout --ours conflict.t &&
+		git add conflict.t &&
+		git rev-parse --verify REVERT_HEAD
+	) &&
+	test_path_is_missing .git/worktrees/mid-revert/sequencer &&
+	test_must_fail git worktree remove mid-revert 2>err &&
+	test_grep "in the middle of a revert" err &&
+	git worktree remove --force mid-revert
+'
+
+test_expect_success 'remove worktree in the middle of a cherry-pick sequence' '
+	test_when_finished "rm -rf mid-pick-seq; git worktree prune" &&
+	git worktree add --detach mid-pick-seq base &&
+	(
+		cd mid-pick-seq &&
+		test_must_fail git cherry-pick side side-two &&
+		git checkout --theirs conflict.t &&
+		git add conflict.t &&
+		git commit --no-edit
+	) &&
+	test_path_is_dir .git/worktrees/mid-pick-seq/sequencer &&
+	test_must_fail git worktree remove mid-pick-seq 2>err &&
+	test_grep "in the middle of a cherry-pick or revert" err &&
+	git worktree remove --force mid-pick-seq
+'
+
 test_done
-- 
2.56.0