[PATCH v4 5/5] commit: refuse partial commits during conflict resolution

["Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com>]

From: Elijah Newren <newren@gmail.com>

Similar to the previous commit, just as `git commit --amend` is a
foot-gun during conflict resolution, so is a partial commit (`git commit
<paths>`).  Recording a conflict resolution is about capturing the state
of the entire tree on top of HEAD, not a subset of paths.  For many years
we have rejected partial commits in the middle of
  - a merge
  - a cherry-pick

but, just like amending, this was never extended to the other operations
that can also leave conflicts to resolve:
  - an `am` operation
  - a revert
  - a rebase that stopped for conflict resolution

Reuse sequencer_ongoing_operation(), introduced for the analogous
`--amend` check, to detect these and refuse the partial commit.  A rebase
that stopped because a pick became empty is not conflict resolution and,
as an earlier patch established, is deliberately left permitted.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/commit.c                | 24 ++++++++++++++++++-----
 sequencer.h                     |  5 +++--
 t/t3404-rebase-interactive.sh   | 34 +++++++++++++++++++++++++++++++++
 t/t3507-cherry-pick-conflict.sh | 11 +++++++++++
 t/t4151-am-abort.sh             | 11 +++++++++++
 5 files changed, 78 insertions(+), 7 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 284fc7fdc6..4e0fd58f0a 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -515,11 +515,25 @@ static const char *prepare_index(const char **argv, const char *prefix,
 	 */
 	commit_style = COMMIT_PARTIAL;
 
-	if (whence != FROM_COMMIT) {
-		if (whence == FROM_MERGE)
-			die(_("cannot do a partial commit during a merge."));
-		else if (is_from_cherry_pick(whence))
-			die(_("cannot do a partial commit during a cherry-pick."));
+	switch (sequencer_ongoing_operation(the_repository, whence)) {
+	case ONGOING_NONE:
+		break;
+	case ONGOING_MERGE:
+		die(_("cannot do a partial commit during a merge."));
+	case ONGOING_CHERRY_PICK:
+		die(_("cannot do a partial commit during a cherry-pick."));
+	case ONGOING_REBASE_NOW_EMPTY:
+		/*
+		 * A pick that became empty is not a conflict, and creating
+		 * a new commit (partial or not) poses no problem.
+		 */
+		break;
+	case ONGOING_REVERT:
+		die(_("cannot do a partial commit during a revert."));
+	case ONGOING_AM:
+		die(_("cannot do a partial commit during an am session."));
+	case ONGOING_REBASE_CONFLICT:
+		die(_("cannot do a partial commit while resolving conflicts during a rebase."));
 	}
 
 	if (list_paths(&partial, !current_head ? NULL : "HEAD", &pathspec))
diff --git a/sequencer.h b/sequencer.h
index fb4a744208..61ebc2ca40 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -279,8 +279,9 @@ int sequencer_determine_whence(struct repository *r, enum commit_whence *whence)
 
 /*
  * An in-progress operation that records its result (often a conflict
- * resolution) as a new commit on top of HEAD, during which amending
- * HEAD via "git commit --amend" is almost always a mistake.
+ * resolution) as a new commit on top of HEAD.  Some ways of invoking
+ * "git commit" -- amending HEAD, or a partial commit -- are almost
+ * always a mistake during such an operation.
  */
 enum ongoing_operation {
 	ONGOING_NONE = 0,
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 4e6c3e2f19..8c63682b7f 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1970,6 +1970,40 @@ test_expect_success 'commit --amend is refused at an apply-backend conflict stop
 	)
 '
 
+test_expect_success 'partial commit is refused at a rebase conflict stop' '
+	test_when_finished "git rebase --abort" &&
+	git checkout --detach conflict-branch &&
+	(
+		set_fake_editor &&
+		FAKE_LINES="1 3" &&
+		export FAKE_LINES &&
+		test_must_fail git rebase -i A
+	) &&
+	echo resolved >conflict &&
+	git add conflict &&
+	test_must_fail git commit conflict 2>err &&
+	test_grep "cannot do a partial commit while resolving conflicts during a rebase." err
+'
+
+test_expect_success 'partial commit is refused at an apply-backend conflict stop' '
+	test_when_finished "rm -rf apply-backend" &&
+	test_create_repo apply-backend &&
+	(
+		cd apply-backend &&
+		test_commit base file &&
+		git branch -M mainline &&
+		test_commit upstream file upstream &&
+		git checkout -b side mainline~1 &&
+		test_commit conflicting file side &&
+		test_commit unrelated other &&
+		test_must_fail git rebase --apply mainline &&
+		echo resolved >file &&
+		git add file &&
+		test_must_fail git commit file 2>err &&
+		test_grep "cannot do a partial commit while resolving conflicts during a rebase." err
+	)
+'
+
 test_expect_success 'todo has correct onto hash' '
 	GIT_SEQUENCE_EDITOR=cat git rebase -i no-conflict-branch~4 no-conflict-branch >actual &&
 	onto=$(git rev-parse --short HEAD~4) &&
diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
index 42de398f76..c3d024c97f 100755
--- a/t/t3507-cherry-pick-conflict.sh
+++ b/t/t3507-cherry-pick-conflict.sh
@@ -375,6 +375,17 @@ test_expect_success 'commit --amend of revert fails' '
 	test_grep "in the middle of a revert -- cannot amend." err
 '
 
+test_expect_success 'partial commit during a revert fails' '
+	pristine_detach initial &&
+
+	test_must_fail git revert picked &&
+	echo resolved >foo &&
+	git add foo &&
+	test_must_fail git commit foo 2>err &&
+
+	test_grep "cannot do a partial commit during a revert." err
+'
+
 test_expect_success 'successful revert does not set REVERT_HEAD' '
 	pristine_detach base &&
 	git revert base &&
diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh
index 9313a074b2..c80269e015 100755
--- a/t/t4151-am-abort.sh
+++ b/t/t4151-am-abort.sh
@@ -74,6 +74,17 @@ test_expect_success 'commit --amend during a failed am fails' '
 	git am --abort
 '
 
+test_expect_success 'partial commit during a failed am fails' '
+	git reset --hard initial &&
+	cp file-2-expect file-2 &&
+	test_must_fail git am 000[1245]-*.patch &&
+	echo resolved >file-1 &&
+	git add file-1 &&
+	test_must_fail git commit file-1 2>err &&
+	test_grep "cannot do a partial commit during an am session." err &&
+	git am --abort
+'
+
 test_expect_success 'am -3 --skip removes otherfile-4' '
 	git reset --hard initial &&
 	test_must_fail git am -3 0003-*.patch &&
-- 
gitgitgadget