Re: [PATCH v2 3/3] commit: refuse partial commits during conflict resolution

> 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
>    - a rebase that stopped at a 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 all of these and refuse the partial commit.

> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>   builtin/commit.c                | 22 ++++++++++++++-------
>   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, 74 insertions(+), 9 deletions(-)
> 
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 4a6054aae0..9da3f1191b 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -515,13 +515,21 @@ 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."));
> -		else if (is_from_rebase(whence))
> -			die(_("cannot do a partial commit while resolving a commit that became empty."));
> +	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_EMPTY:
> +		die(_("cannot do a partial commit while resolving a commit that became empty."));
> +	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 3a4bd97db1..634d1ddcb3 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 7cf06e5f9a..1314b0fd05 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -1971,6 +1971,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 &&