> From: Elijah Newren <newren@gmail.com>
> 
> Running `git commit --amend` during conflict resolution is an ugly
> foot-gun.  For many years, we have rejected amending during conflict
> resolution in the middle of
>    - a merge
>    - a cherry-pick
> However, this was never extended to other operations that can also
> produce conflicts:
>    - an `am` operation
>    - a revert
>    - a rebase
> 
> Extend it to handle these other cases now.

> Extending to `am`, revert, and the apply backend of rebase are fairly
> straightforward.  However, with the merge backend of rebase we have to
> be more careful, since it powers interactive rebases and
>    - the interactive machinery internally uses `git commit --amend` for
>      `squash` and `reword` directives
>    - users are expected to `git commit --amend` after hitting an `edit`
>      or `break` directive
> So, we need to be careful with rebase to only reject amending when doing
> conflict resolution.
> 
> A few files under the rebase-merge/ directory provide us the necessary
> information:
> 
>    - stopped-sha is written only when the rebase stops and hands control
>      back to the user, so its presence marks a genuine stop -- as opposed
>      to the sequencer's own internal `git commit --amend` while applying
>      a squash, fixup, or reword, during which no stopped-sha exists.
> 
>    - amend is written only when the rebase stops with HEAD already
>      pointing at the commit the user is meant to amend: a clean `edit`,
>      or a fast-forward `reword`. 

>      Its absence at a stop therefore means
>      the commit did not apply, so HEAD is the previously-applied commit
>      rather than the one being rebased -- exactly the case we refuse.
> 
> So for the merge backend we die when stopped-sha exists and amend does
> not.  This covers a plain conflicted pick as well as a conflicted `edit`
> (both leave HEAD on the previously-applied commit), while still allowing
> a clean `edit` or `reword` stop and a `break` stop (no stopped-sha).
> stopped-sha is unlinked at the start of the resume loop, so a resumed
> squash's internal amend is unaffected.
> 
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>      commit: refuse to amend during conflict resolution
> 
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2389%2Fnewren%2Frefuse-amend-during-conflicts-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2389/newren/refuse-amend-during-conflicts-v1
> Pull-Request: https://github.com/git/git/pull/2389
> 
>   builtin/commit.c                | 41 ++++++++++++++++
>   t/t3404-rebase-interactive.sh   | 87 +++++++++++++++++++++++++++++++++
>   t/t3507-cherry-pick-conflict.sh | 11 +++++
>   t/t4151-am-abort.sh             | 11 +++++
>   4 files changed, 150 insertions(+)
> 
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 28f6174503..a9fd04366e 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -30,6 +30,7 @@
>   #include "path.h"
>   #include "preload-index.h"
>   #include "read-cache.h"
> +#include "refs.h"
>   #include "repository.h"
>   #include "string-list.h"
>   #include "rerere.h"
> @@ -1336,6 +1337,46 @@ static int parse_and_validate_options(int argc, const char *argv[],
>   		else if (whence == FROM_REBASE_PICK)
>   			die(_("You are in the middle of a rebase -- cannot amend."));
>   	}
> +	if (amend && whence == FROM_COMMIT) {
> +		char *applying, *apply_dir, *stopped_sha, *amend_marker;
> +		int in_am, conflicted_stop;
> +
> +		/* Check middle of revert */
> +		if (refs_ref_exists(get_main_ref_store(the_repository),
> +				    "REVERT_HEAD"))
> +			die(_("You are in the middle of a revert -- cannot amend."));

> +		/* Check middle of `am` */
> +		applying = repo_git_path(the_repository,
> +					 "rebase-apply/applying");
> +		in_am = file_exists(applying);
> +
> +		free(applying);
> +		if (in_am)
> +			die(_("You are in the middle of an am session -- cannot amend."));
> +
> +		/* Check middle of rebase specifically stopped for conflicts */
> +		apply_dir = repo_git_path(the_repository,
> +					  "rebase-apply");
> +		stopped_sha = repo_git_path(the_repository,
> +					    "rebase-merge/stopped-sha");
> +		amend_marker = repo_git_path(the_repository,
> +					     "rebase-merge/amend");
> +		/*
> +		 * The apply backend only ever stops for conflicts; the
> +		 * merge backend writes stopped-sha but omits `amend`,
> +		 * which it writes only at a clean edit/reword stop.
> +		 */
> +		conflicted_stop =
> +			file_exists(apply_dir) ||
> +			(file_exists(stopped_sha) && !file_exists(amend_marker));
> +
> +		free(apply_dir);
> +		free(stopped_sha);
> +		free(amend_marker);
> +		if (conflicted_stop)
> +			die(_("You are resolving conflicts during a rebase -- cannot amend."));
> +	}
>   	if (fixup_message && squash_message)
>   		die(_("options '%s' and '%s' cannot be used together"), "--squash", "--fixup");
>   	die_for_incompatible_opt4(!!use_message, "-C",
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index ff11abb2f2..01d4735b3b 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -1884,6 +1884,93 @@ test_expect_success 'correct error message for commit --amend after empty pick'
>   	test_grep "middle of a rebase -- cannot amend." err
>   '
>   
> +test_expect_success 'commit --amend 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
> +	) &&
> +	test_path_is_file .git/rebase-merge/patch &&
> +	test_path_is_missing .git/rebase-merge/amend &&
> +	echo resolved >conflict &&
> +	git add conflict &&
> +	test_must_fail git commit --amend --no-edit 2>err &&
> +	test_grep "You are resolving conflicts during a rebase -- cannot amend" err
> +'
> +
> +test_expect_success 'commit --amend is refused when an "edit" pick conflicts' '
> +	test_when_finished "git rebase --abort" &&
> +	git checkout --detach conflict-branch &&
> +	(
> +		set_fake_editor &&
> +		FAKE_LINES="1 edit 3" &&
> +		export FAKE_LINES &&
> +		test_must_fail git rebase -i A
> +	) &&
> +	test_path_is_file .git/rebase-merge/patch &&
> +	test_path_is_missing .git/rebase-merge/amend &&
> +	echo resolved >conflict &&
> +	git add conflict &&
> +	test_must_fail git commit --amend --no-edit 2>err &&
> +	test_grep "You are resolving conflicts during a rebase -- cannot amend" err
> +'
> +
> +test_expect_success 'commit --amend is allowed at a rebase edit stop' '
> +	test_when_finished "git rebase --abort" &&
> +	git checkout --detach no-conflict-branch &&
> +	(
> +		set_fake_editor &&
> +		FAKE_LINES="edit 1 2 3 4" &&
> +		export FAKE_LINES &&
> +		git rebase -i A
> +	) &&
> +	test_path_is_file .git/rebase-merge/amend &&
> +	echo tweak >fileJ &&
> +	git add fileJ &&
> +	git commit --amend --no-edit
> +'
> +
> +test_expect_success 'commit --amend is allowed at a rebase break stop' '
> +	test_when_finished "git rebase --abort" &&
> +	git checkout --detach no-conflict-branch &&
> +	(
> +		set_fake_editor &&
> +		FAKE_LINES="break 1 2 3 4" &&
> +		export FAKE_LINES &&
> +		git rebase -i A
> +	) &&
> +	test_must_fail git rev-parse --verify REBASE_HEAD &&
> +	echo tweak >fileJ &&
> +	git add fileJ &&
> +	git commit --amend --no-edit
> +'
> +
> +test_expect_success 'commit --amend 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 &&
> +		# the apply backend only ever stops for conflicts, and
> +		# leaves HEAD on the previously-applied commit
> +		test_path_is_dir .git/rebase-apply &&
> +		test_path_is_missing .git/rebase-apply/applying &&
> +		echo resolved >file &&
> +		git add file &&
> +		test_must_fail git commit --amend --no-edit 2>err &&
> +		test_grep "You are resolving conflicts during a rebase -- cannot amend" 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 44596cb1e8..42de398f76 100755
> --- a/t/t3507-cherry-pick-conflict.sh
> +++ b/t/t3507-cherry-pick-conflict.sh
> @@ -364,6 +364,17 @@ test_expect_success 'failed revert sets REVERT_HEAD' '
>   	test_cmp_rev picked REVERT_HEAD
>   '
>   
> +test_expect_success 'commit --amend of revert fails' '
> +	pristine_detach initial &&
> +
> +	test_must_fail git revert picked &&
> +	echo resolved >foo &&
> +	git add foo &&
> +	test_must_fail git commit --amend 2>err &&
> +
> +	test_grep "in the middle of a revert -- cannot amend." 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 8e1ecf8a68..9313a074b2 100755
> --- a/t/t4151-am-abort.sh
> +++ b/t/t4151-am-abort.sh
> @@ -63,6 +63,17 @@ do
>   
>   done
>   
> +test_expect_success 'commit --amend 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 --amend 2>err &&
> +	test_grep "in the middle of an am session -- cannot amend." 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 &&
> 
> base-commit: 2c3adbb2c475981e340c79fdc5e7f4f9b5d9054e

> ...  However, with the merge backend of rebase we have to
> be more careful, since it powers interactive rebases and
>   - the interactive machinery internally uses `git commit --amend` for
>     `squash` and `reword` directives
>   - users are expected to `git commit --amend` after hitting an `edit`
>     or `break` directive
> So, we need to be careful with rebase to only reject amending when doing
> conflict resolution.

> A few files under the rebase-merge/ directory provide us the necessary
> information:
>
>   - stopped-sha is written only when the rebase stops and hands control
>     back to the user, so its presence marks a genuine stop -- as opposed
>     to the sequencer's own internal `git commit --amend` while applying
>     a squash, fixup, or reword, during which no stopped-sha exists.
>
>   - amend is written only when the rebase stops with HEAD already
>     pointing at the commit the user is meant to amend: a clean `edit`,
>     or a fast-forward `reword`.  Its absence at a stop therefore means
>     the commit did not apply, so HEAD is the previously-applied commit
>     rather than the one being rebased -- exactly the case we refuse.
>
> So for the merge backend we die when stopped-sha exists and amend does
> not.  This covers a plain conflicted pick as well as a conflicted `edit`
> (both leave HEAD on the previously-applied commit), while still allowing
> a clean `edit` or `reword` stop and a `break` stop (no stopped-sha).
> stopped-sha is unlinked at the start of the resume loop, so a resumed
> squash's internal amend is unaffected.

> +	if (amend && whence == FROM_COMMIT) {
> +		char *applying, *apply_dir, *stopped_sha, *amend_marker;
> +		int in_am, conflicted_stop;
> +
> +		/* Check middle of revert */
> +		if (refs_ref_exists(get_main_ref_store(the_repository),
> +				    "REVERT_HEAD"))
> +			die(_("You are in the middle of a revert -- cannot amend."));

> > @@ -1336,6 +1337,46 @@ static int parse_and_validate_options(int argc, const char *argv[],
> >               else if (whence == FROM_REBASE_PICK)
> >                       die(_("You are in the middle of a rebase -- cannot amend."));
> >       }
> > +     if (amend && whence == FROM_COMMIT) {
> > +             char *applying, *apply_dir, *stopped_sha, *amend_marker;
> > +             int in_am, conflicted_stop;
> > +
> > +             /* Check middle of revert */
> > +             if (refs_ref_exists(get_main_ref_store(the_repository),
> > +                                 "REVERT_HEAD"))
> > +                     die(_("You are in the middle of a revert -- cannot amend."));
>
> I think it would be much cleaner to move this check and sequencer
> related ones below into sequencer_determine_whence() so that we don't
> have to hard code the paths here. It might be worth checking for "am"
> and the "apply" based rebase in that function as well.

>
> "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > ...  However, with the merge backend of rebase we have to
> > be more careful, since it powers interactive rebases and
> >   - the interactive machinery internally uses `git commit --amend` for
> >     `squash` and `reword` directives
> >   - users are expected to `git commit --amend` after hitting an `edit`
> >     or `break` directive
> > So, we need to be careful with rebase to only reject amending when doing
> > conflict resolution.
>
> True.
>
> In addition, in any and all of these scenarios that lets the user
> deal with conflicts in his or her working tree files and record the
> result of conflict resolution in a commit, we should reject not only
> "git commit --amend" but also "git commit <paths>", shouldn't we?
>
> It may probably be better done in a separate topic, as the guiding
> principle is slightly different (i.e., "recording the conflict
> resolution is about recording the state on top of the current HEAD
> and never about updating the state recorded in the current HEAD" is
> the theme of the current topic.  "recording the conflict resolution
> is always about the entire tree" is the other topic), so we may want
> to leave a #leftoverbits marker here.

>
> "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > @@ -1336,6 +1337,46 @@ static int parse_and_validate_options(int argc, const char *argv[],
> >               else if (whence == FROM_REBASE_PICK)
> >                       die(_("You are in the middle of a rebase -- cannot amend."));
> >       }
>
> Let's make a mental note that the function receives these parameters:
>
> static int parse_and_validate_options(int argc, const char *argv[],
>                                       const struct option *options,
>                                       const char * const usage[],
>                                       const char *prefix,
>                                       struct commit *current_head,
>                                       struct wt_status *s)
>
> > +     if (amend && whence == FROM_COMMIT) {
> > +             char *applying, *apply_dir, *stopped_sha, *amend_marker;
> > +             int in_am, conflicted_stop;
> > +
> > +             /* Check middle of revert */
> > +             if (refs_ref_exists(get_main_ref_store(the_repository),
> > +                                 "REVERT_HEAD"))
> > +                     die(_("You are in the middle of a revert -- cannot amend."));
>
> "the_repository" can become "s->repo".  The same comment for other
> checks in this block.

> From: Elijah Newren <newren@gmail.com>
> 
> When a rebase applies a commit that becomes empty, it stops and asks the
> user to decide whether to keep it or drop it.  HEAD still points at the
> previously-applied commit at that point, so either amending or creating
> a partial commit is refused, with one of the following messages:
> 
>      You are in the middle of a rebase -- cannot amend.
>      cannot do a partial commit during a rebase.
> 
> Neither message hints that the real problem is a commit that became
> empty, and "during a rebase" is overly broad besides -- amending and
> partial commits are fine at an `edit` or `break` stop.  Reword both to
> describe the actual situation.
> 
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>   builtin/commit.c              | 4 ++--
>   t/t3404-rebase-interactive.sh | 4 ++--
>   2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 28f6174503..0d908d72bb 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -521,7 +521,7 @@ static const char *prepare_index(const char **argv, const char *prefix,
>   		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 during a rebase."));
> +			die(_("cannot do a partial commit while resolving a commit that became empty."));

>   	}
>   
>   	if (list_paths(&partial, !current_head ? NULL : "HEAD", &pathspec))
> @@ -1334,7 +1334,7 @@ static int parse_and_validate_options(int argc, const char *argv[],
>   		else if (is_from_cherry_pick(whence))
>   			die(_("You are in the middle of a cherry-pick -- cannot amend."));
>   		else if (whence == FROM_REBASE_PICK)
> -			die(_("You are in the middle of a rebase -- cannot amend."));
> +			die(_("You are resolving a commit that became empty -- cannot amend."));
>   	}
>   	if (fixup_message && squash_message)
>   		die(_("options '%s' and '%s' cannot be used together"), "--squash", "--fixup");
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index ff11abb2f2..1e78dbfd90 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -1868,7 +1868,7 @@ test_expect_success 'correct error message for partial commit after empty pick'
>   	) &&
>   	echo x >file1 &&
>   	test_must_fail git commit file1 2>err &&
> -	test_grep "cannot do a partial commit during a rebase." err
> +	test_grep "cannot do a partial commit while resolving a commit that became empty." err
>   '
>   
>   test_expect_success 'correct error message for commit --amend after empty pick' '
> @@ -1881,7 +1881,7 @@ test_expect_success 'correct error message for commit --amend after empty pick'
>   	) &&
>   	echo x>file1 &&
>   	test_must_fail git commit -a --amend 2>err &&
> -	test_grep "middle of a rebase -- cannot amend." err
> +	test_grep "resolving a commit that became empty -- cannot amend." err
>   '
>   
>   test_expect_success 'todo has correct onto hash' '

> From: Elijah Newren <newren@gmail.com>
> 
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 0d908d72bb..4a6054aae0 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1326,15 +1326,30 @@ static int parse_and_validate_options(int argc, const char *argv[],
>   		use_editor = 0;
>   
>   	/* Sanity check options */
> -	if (amend && !current_head)
> -		die(_("You have nothing to amend."));
> -	if (amend && whence != FROM_COMMIT) {
> -		if (whence == FROM_MERGE)
> +	if (amend) {
> +		if (!current_head)
> +			die(_("You have nothing to amend."));
> +		/*
> +		 * Refuse to amend in the middle of any operation that is
> +		 * meant to record its result as a new commit on top of HEAD
> +		 * rather than by rewriting HEAD.
> +		 */
> +		switch (sequencer_ongoing_operation(s->repo, whence)) {
> +		case ONGOING_NONE:
> +			break;
> +		case ONGOING_MERGE:
>   			die(_("You are in the middle of a merge -- cannot amend."));
> -		else if (is_from_cherry_pick(whence))
> +		case ONGOING_CHERRY_PICK:
>   			die(_("You are in the middle of a cherry-pick -- cannot amend."));

> diff --git a/sequencer.c b/sequencer.c
> index 65afd100d9..bd4a724410 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -6966,6 +6966,71 @@ int sequencer_determine_whence(struct repository *r, enum commit_whence *whence)
>   	return 0;
>   }
>   
> +enum ongoing_operation sequencer_ongoing_operation(struct repository *r,
> +						   enum commit_whence whence)
> +{
> [...]
> +	/*
> +	 * In the middle of a rebase that stopped for conflict resolution?
> +	 * The apply backend only ever stops for conflicts, so the presence
> +	 * of its state directory is enough.  The merge backend writes
> +	 * stopped-sha whenever it hands control back to the user, but omits
> +	 * `amend` unless it stopped with HEAD already pointing at the commit
> +	 * to be amended (a clean edit/reword stop); its absence therefore
> +	 * marks a conflicted stop.
> +	 */
> +	path = repo_git_path(r, "rebase-apply");
> +	found = file_exists(path);
> +	free(path);
> +	if (!found) {
> +		char *stopped_sha = repo_git_path(r, "rebase-merge/stopped-sha");
> +		char *amend_marker = repo_git_path(r, "rebase-merge/amend");
> +
> +		found = file_exists(stopped_sha) && !file_exists(amend_marker);

> +		free(stopped_sha);
> +		free(amend_marker);
> +	}
> +	if (found)
> +		return ONGOING_REBASE_CONFLICT;
> +
> +	return ONGOING_NONE;
> +}
> +
>   int sequencer_get_update_refs_state(const char *wt_dir,
>   				    struct string_list *refs)
>   {
> diff --git a/sequencer.h b/sequencer.h
> index 64a9c7fb1b..3a4bd97db1 100644
> --- a/sequencer.h
> +++ b/sequencer.h
> @@ -277,6 +277,29 @@ int sequencer_get_last_command(struct repository* r,
>   			       enum replay_action *action);
>   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.
> + */
> +enum ongoing_operation {
> +	ONGOING_NONE = 0,
> +	ONGOING_MERGE,
> +	ONGOING_CHERRY_PICK,
> +	ONGOING_REBASE_EMPTY,
> +	ONGOING_REVERT,
> +	ONGOING_AM,
> +	ONGOING_REBASE_CONFLICT
> +};
> +
> +/*
> + * Return which in-progress operation, if any, is underway; see enum
> + * ongoing_operation.  'whence' is the origin already computed for the
> + * pending commit.
> + */
> +enum ongoing_operation sequencer_ongoing_operation(struct repository *r,
> +						   enum commit_whence whence);
> +
>   /**
>    * Append the set of ref-OID pairs that are currently stored for the 'git
>    * rebase --update-refs' feature if such a rebase is currently happening.
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index 1e78dbfd90..7cf06e5f9a 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -1884,6 +1884,93 @@ test_expect_success 'correct error message for commit --amend after empty pick'
>   	test_grep "resolving a commit that became empty -- cannot amend." err
>   '
>   
> +test_expect_success 'commit --amend 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
> +	) &&
> +	test_path_is_file .git/rebase-merge/patch &&
> +	test_path_is_missing .git/rebase-merge/amend &&
> +	echo resolved >conflict &&
> +	git add conflict &&
> +	test_must_fail git commit --amend --no-edit 2>err &&
> +	test_grep "You are resolving conflicts during a rebase -- cannot amend" err
> +'
> +
> +test_expect_success 'commit --amend is refused when an "edit" pick conflicts' '
> +	test_when_finished "git rebase --abort" &&
> +	git checkout --detach conflict-branch &&
> +	(
> +		set_fake_editor &&
> +		FAKE_LINES="1 edit 3" &&
> +		export FAKE_LINES &&
> +		test_must_fail git rebase -i A
> +	) &&
> +	test_path_is_file .git/rebase-merge/patch &&
> +	test_path_is_missing .git/rebase-merge/amend &&
> +	echo resolved >conflict &&
> +	git add conflict &&
> +	test_must_fail git commit --amend --no-edit 2>err &&
> +	test_grep "You are resolving conflicts during a rebase -- cannot amend" err
> +'
> +
> +test_expect_success 'commit --amend is allowed at a rebase edit stop' '
> +	test_when_finished "git rebase --abort" &&
> +	git checkout --detach no-conflict-branch &&
> +	(
> +		set_fake_editor &&
> +		FAKE_LINES="edit 1 2 3 4" &&
> +		export FAKE_LINES &&
> +		git rebase -i A
> +	) &&
> +	test_path_is_file .git/rebase-merge/amend &&
> +	echo tweak >fileJ &&
> +	git add fileJ &&
> +	git commit --amend --no-edit
> +'
> +
> +test_expect_success 'commit --amend is allowed at a rebase break stop' '
> +	test_when_finished "git rebase --abort" &&
> +	git checkout --detach no-conflict-branch &&
> +	(
> +		set_fake_editor &&
> +		FAKE_LINES="break 1 2 3 4" &&
> +		export FAKE_LINES &&
> +		git rebase -i A
> +	) &&
> +	test_must_fail git rev-parse --verify REBASE_HEAD &&
> +	echo tweak >fileJ &&
> +	git add fileJ &&
> +	git commit --amend --no-edit
> +'
> +
> +test_expect_success 'commit --amend 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 &&
> +		# the apply backend only ever stops for conflicts, and
> +		# leaves HEAD on the previously-applied commit
> +		test_path_is_dir .git/rebase-apply &&
> +		test_path_is_missing .git/rebase-apply/applying &&
> +		echo resolved >file &&
> +		git add file &&
> +		test_must_fail git commit --amend --no-edit 2>err &&
> +		test_grep "You are resolving conflicts during a rebase -- cannot amend" 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 44596cb1e8..42de398f76 100755
> --- a/t/t3507-cherry-pick-conflict.sh
> +++ b/t/t3507-cherry-pick-conflict.sh
> @@ -364,6 +364,17 @@ test_expect_success 'failed revert sets REVERT_HEAD' '
>   	test_cmp_rev picked REVERT_HEAD
>   '
>   
> +test_expect_success 'commit --amend of revert fails' '
> +	pristine_detach initial &&
> +
> +	test_must_fail git revert picked &&
> +	echo resolved >foo &&
> +	git add foo &&
> +	test_must_fail git commit --amend 2>err &&
> +
> +	test_grep "in the middle of a revert -- cannot amend." 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 8e1ecf8a68..9313a074b2 100755
> --- a/t/t4151-am-abort.sh
> +++ b/t/t4151-am-abort.sh
> @@ -63,6 +63,17 @@ do
>   
>   done
>   
> +test_expect_success 'commit --amend 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 --amend 2>err &&
> +	test_grep "in the middle of an am session -- cannot amend." 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 &&

> Both git commit --amend and a partial commit (git commit <paths>) are
> foot-guns while the user is in the middle of an operation that resolves
> conflicts on top of HEAD: recording a conflict resolution is about capturing
> the state of the whole tree as a new commit, not about rewriting HEAD or
> committing a subset of paths.
> 
> Historically we only rejected these during a merge or a cherry-pick or when
> resolving an empty pick during a rebase. The same hazard exists for am,
> revert, and rebase conflict stops, none of which were covered. This series
> extends the refusal to all of them.
> 
> The three patches:
> 
>   1. reword the two pre-existing "empty commit" rebase messages, which were
>      misleadingly generic
>   2. refuse git commit --amend during these additional operations
>   3. refuse partial commits during the same operations.

> Elijah Newren (3):
>    commit: reword the empty-commit rebase errors
>    commit: refuse to amend during conflict resolution
>    commit: refuse partial commits during conflict resolution
> 
>   builtin/commit.c                |  51 +++++++++----
>   sequencer.c                     |  65 +++++++++++++++++
>   sequencer.h                     |  24 ++++++
>   t/t3404-rebase-interactive.sh   | 125 +++++++++++++++++++++++++++++++-
>   t/t3507-cherry-pick-conflict.sh |  22 ++++++
>   t/t4151-am-abort.sh             |  22 ++++++
>   6 files changed, 293 insertions(+), 16 deletions(-)
> 
> 
> base-commit: 2c3adbb2c475981e340c79fdc5e7f4f9b5d9054e
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2389%2Fnewren%2Frefuse-amend-during-conflicts-v2
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2389/newren/refuse-amend-during-conflicts-v2
> Pull-Request: https://github.com/git/git/pull/2389
> 
> Range-diff vs v1:
> 
>   -:  ---------- > 1:  65c48ed3cb commit: reword the empty-commit rebase errors
>   1:  a3d6b059c6 ! 2:  4a1461e527 commit: refuse to amend during conflict resolution
>       @@ Commit message
>            Signed-off-by: Elijah Newren <newren@gmail.com>
>        
>         ## builtin/commit.c ##
>       -@@
>       - #include "path.h"
>       - #include "preload-index.h"
>       - #include "read-cache.h"
>       -+#include "refs.h"
>       - #include "repository.h"
>       - #include "string-list.h"
>       - #include "rerere.h"
>        @@ builtin/commit.c: static int parse_and_validate_options(int argc, const char *argv[],
>       - 		else if (whence == FROM_REBASE_PICK)
>       - 			die(_("You are in the middle of a rebase -- cannot amend."));
>       + 		use_editor = 0;
>       +
>       + 	/* Sanity check options */
>       +-	if (amend && !current_head)
>       +-		die(_("You have nothing to amend."));
>       +-	if (amend && whence != FROM_COMMIT) {
>       +-		if (whence == FROM_MERGE)
>       ++	if (amend) {
>       ++		if (!current_head)
>       ++			die(_("You have nothing to amend."));
>       ++		/*
>       ++		 * Refuse to amend in the middle of any operation that is
>       ++		 * meant to record its result as a new commit on top of HEAD
>       ++		 * rather than by rewriting HEAD.
>       ++		 */
>       ++		switch (sequencer_ongoing_operation(s->repo, whence)) {
>       ++		case ONGOING_NONE:
>       ++			break;
>       ++		case ONGOING_MERGE:
>       + 			die(_("You are in the middle of a merge -- cannot amend."));
>       +-		else if (is_from_cherry_pick(whence))
>       ++		case ONGOING_CHERRY_PICK:
>       + 			die(_("You are in the middle of a cherry-pick -- cannot amend."));
>       +-		else if (whence == FROM_REBASE_PICK)
>       ++		case ONGOING_REBASE_EMPTY:
>       + 			die(_("You are resolving a commit that became empty -- cannot amend."));
>       ++		case ONGOING_REVERT:
>       ++			die(_("You are in the middle of a revert -- cannot amend."));
>       ++		case ONGOING_AM:
>       ++			die(_("You are in the middle of an am session -- cannot amend."));
>       ++		case ONGOING_REBASE_CONFLICT:
>       ++			die(_("You are resolving conflicts during a rebase -- cannot amend."));
>       ++		}
>         	}
>       -+	if (amend && whence == FROM_COMMIT) {
>       -+		char *applying, *apply_dir, *stopped_sha, *amend_marker;
>       -+		int in_am, conflicted_stop;
>       + 	if (fixup_message && squash_message)
>       + 		die(_("options '%s' and '%s' cannot be used together"), "--squash", "--fixup");
>       +
>       + ## sequencer.c ##
>       +@@ sequencer.c: int sequencer_determine_whence(struct repository *r, enum commit_whence *whence)
>       + 	return 0;
>       + }
>       +
>       ++enum ongoing_operation sequencer_ongoing_operation(struct repository *r,
>       ++						   enum commit_whence whence)
>       ++{
>       ++	char *path;
>       ++	int found;
>        +
>       -+		/* Check middle of revert */
>       -+		if (refs_ref_exists(get_main_ref_store(the_repository),
>       -+				    "REVERT_HEAD"))
>       -+			die(_("You are in the middle of a revert -- cannot amend."));
>       ++	/*
>       ++	 * The merge, cherry-pick, and (empty) rebase-pick stops are already
>       ++	 * distinguished by 'whence'.
>       ++	 */
>       ++	switch (whence) {
>       ++	case FROM_MERGE:
>       ++		return ONGOING_MERGE;
>       ++	case FROM_CHERRY_PICK_SINGLE:
>       ++	case FROM_CHERRY_PICK_MULTI:
>       ++		return ONGOING_CHERRY_PICK;
>       ++	case FROM_REBASE_PICK:
>       ++		return ONGOING_REBASE_EMPTY;
>       ++	case FROM_COMMIT:
>       ++		break;
>       ++	}
>        +
>       -+		/* Check middle of `am` */
>       -+		applying = repo_git_path(the_repository,
>       -+					 "rebase-apply/applying");
>       -+		in_am = file_exists(applying);
>       ++	/*
>       ++	 * 'whence' is FROM_COMMIT, but we may still be in the middle of an
>       ++	 * operation that records its result on top of HEAD; detect those
>       ++	 * from their on-disk state.
>       ++	 */
>        +
>       -+		free(applying);
>       -+		if (in_am)
>       -+			die(_("You are in the middle of an am session -- cannot amend."));
>       ++	/* In the middle of a revert? */
>       ++	if (refs_ref_exists(get_main_ref_store(r), "REVERT_HEAD"))
>       ++		return ONGOING_REVERT;
>        +
>       -+		/* Check middle of rebase specifically stopped for conflicts */
>       -+		apply_dir = repo_git_path(the_repository,
>       -+					  "rebase-apply");
>       -+		stopped_sha = repo_git_path(the_repository,
>       -+					    "rebase-merge/stopped-sha");
>       -+		amend_marker = repo_git_path(the_repository,
>       -+					     "rebase-merge/amend");
>       -+		/*
>       -+		 * The apply backend only ever stops for conflicts; the
>       -+		 * merge backend writes stopped-sha but omits `amend`,
>       -+		 * which it writes only at a clean edit/reword stop.
>       -+		 */
>       -+		conflicted_stop =
>       -+			file_exists(apply_dir) ||
>       -+			(file_exists(stopped_sha) && !file_exists(amend_marker));
>       ++	/* In the middle of an `am`? */
>       ++	path = repo_git_path(r, "rebase-apply/applying");
>       ++	found = file_exists(path);
>       ++	free(path);
>       ++	if (found)
>       ++		return ONGOING_AM;
>       ++
>       ++	/*
>       ++	 * In the middle of a rebase that stopped for conflict resolution?
>       ++	 * The apply backend only ever stops for conflicts, so the presence
>       ++	 * of its state directory is enough.  The merge backend writes
>       ++	 * stopped-sha whenever it hands control back to the user, but omits
>       ++	 * `amend` unless it stopped with HEAD already pointing at the commit
>       ++	 * to be amended (a clean edit/reword stop); its absence therefore
>       ++	 * marks a conflicted stop.
>       ++	 */
>       ++	path = repo_git_path(r, "rebase-apply");
>       ++	found = file_exists(path);
>       ++	free(path);
>       ++	if (!found) {
>       ++		char *stopped_sha = repo_git_path(r, "rebase-merge/stopped-sha");
>       ++		char *amend_marker = repo_git_path(r, "rebase-merge/amend");
>        +
>       -+		free(apply_dir);
>       ++		found = file_exists(stopped_sha) && !file_exists(amend_marker);
>        +		free(stopped_sha);
>        +		free(amend_marker);
>       -+		if (conflicted_stop)
>       -+			die(_("You are resolving conflicts during a rebase -- cannot amend."));
>        +	}
>       - 	if (fixup_message && squash_message)
>       - 		die(_("options '%s' and '%s' cannot be used together"), "--squash", "--fixup");
>       - 	die_for_incompatible_opt4(!!use_message, "-C",
>       ++	if (found)
>       ++		return ONGOING_REBASE_CONFLICT;
>       ++
>       ++	return ONGOING_NONE;
>       ++}
>       ++
>       + int sequencer_get_update_refs_state(const char *wt_dir,
>       + 				    struct string_list *refs)
>       + {
>       +
>       + ## sequencer.h ##
>       +@@ sequencer.h: int sequencer_get_last_command(struct repository* r,
>       + 			       enum replay_action *action);
>       + 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.
>       ++ */
>       ++enum ongoing_operation {
>       ++	ONGOING_NONE = 0,
>       ++	ONGOING_MERGE,
>       ++	ONGOING_CHERRY_PICK,
>       ++	ONGOING_REBASE_EMPTY,
>       ++	ONGOING_REVERT,
>       ++	ONGOING_AM,
>       ++	ONGOING_REBASE_CONFLICT
>       ++};
>       ++
>       ++/*
>       ++ * Return which in-progress operation, if any, is underway; see enum
>       ++ * ongoing_operation.  'whence' is the origin already computed for the
>       ++ * pending commit.
>       ++ */
>       ++enum ongoing_operation sequencer_ongoing_operation(struct repository *r,
>       ++						   enum commit_whence whence);
>       ++
>       + /**
>       +  * Append the set of ref-OID pairs that are currently stored for the 'git
>       +  * rebase --update-refs' feature if such a rebase is currently happening.
>        
>         ## t/t3404-rebase-interactive.sh ##
>        @@ t/t3404-rebase-interactive.sh: test_expect_success 'correct error message for commit --amend after empty pick'
>       - 	test_grep "middle of a rebase -- cannot amend." err
>       + 	test_grep "resolving a commit that became empty -- cannot amend." err
>         '
>         
>        +test_expect_success 'commit --amend is refused at a rebase conflict stop' '
>   -:  ---------- > 3:  e0be8cdf63 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 &&

>
> > The three patches:
> >
> >   1. reword the two pre-existing "empty commit" rebase messages, which were
> >      misleadingly generic
> >   2. refuse git commit --amend during these additional operations
> >   3. refuse partial commits during the same operations.
>
> Thanks for working on this, it is a useful improvement to our UI. I've
> left a couple of comments but this all looks pretty sound to me.

> From: Elijah Newren <newren@gmail.com>
>
> When a rebase applies a commit that becomes empty, it stops and asks the
> user to decide whether to keep it or drop it.  HEAD still points at the
> previously-applied commit at that point, so either amending or creating
> a partial commit is refused, with one of the following messages:
>
>     You are in the middle of a rebase -- cannot amend.
>     cannot do a partial commit during a rebase.
>
> Neither message hints that the real problem is a commit that became
> empty, and "during a rebase" is overly broad besides -- amending and
> partial commits are fine at an `edit` or `break` stop.  Reword both to
> describe the actual situation.
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  builtin/commit.c              | 4 ++--
>  t/t3404-rebase-interactive.sh | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 28f6174503..0d908d72bb 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -521,7 +521,7 @@ static const char *prepare_index(const char **argv, const char *prefix,
>  		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 during a rebase."));
> +			die(_("cannot do a partial commit while resolving a commit that became empty."));

> @@ -1334,7 +1334,7 @@ static int parse_and_validate_options(int argc, const char *argv[],
>  		else if (is_from_cherry_pick(whence))
>  			die(_("You are in the middle of a cherry-pick -- cannot amend."));
>  		else if (whence == FROM_REBASE_PICK)
> -			die(_("You are in the middle of a rebase -- cannot amend."));
> +			die(_("You are resolving a commit that became empty -- cannot amend."));
>  	}

> More importantly, I am not sure if whence == FROM_REBASE_PICK at
> this point in the code flow is a sufficient sign to tell that we
> were not just in the middle of a rebase, not just a rebase stopped
> with _some_ conflict, but the way the rebase stopped was because a
> step in rebase resulted in a commit that is no-op relative to the
> previous commit.  What makes us certain that the rebase-pick is
> empty?

>> @@ -521,7 +521,7 @@ static const char *prepare_index(const char **argv, const char *prefix,
>>   		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 during a rebase."));
>> +			die(_("cannot do a partial commit while resolving a commit that became empty."));
>
> "while committing a commit that became empty" would be clearer to me, 
> but I what you have is definitely an improvement on the existing message.

>
> Junio C Hamano <gitster@pobox.com> writes:
>
> >> +                    die(_("cannot do a partial commit while resolving a commit that became empty."));
> >
> > That is a mouthful.  It also is awkward to say "while resolving a commit".
>
> This still stands, but I haven't come up with a better alternative yet.
>
> > More importantly, I am not sure if whence == FROM_REBASE_PICK at
> > this point in the code flow is a sufficient sign to tell that we
> > were not just in the middle of a rebase, not just a rebase stopped
> > with _some_ conflict, but the way the rebase stopped was because a
> > step in rebase resulted in a commit that is no-op relative to the
> > previous commit.  What makes us certain that the rebase-pick is
> > empty?
>
> This confusion was because FROM_REBASE_PICK is a misleading name.
>
> sequencer_determine_whence() is the only place that declares the
> whence is FROM_REBASE_PICK, and it specifically checks if the
> rebase-head and cherry-pick-head are identical before yielding that
> value, so by definition we are dealing with an empty-pick situation.
>
> This came from 430b75f720 (commit: give correct advice for empty
> commit during a rebase, 2019-12-06); interestingly, the name of
> FROM_REBASE_PICK and is_from_rebase() seem to have confused even the
> originating commit ;-)  The lines in question
>
> +               else if (is_from_rebase(whence))
> +                       die(_("cannot do a partial commit during a rebase."));
>
> are from that commit, which wanted to "give correct advice for empty
> commit during a rebase".
>
> We may want to
>
>  * change the code that does whence == FROM_REBASE_PICK to use
>    is_from_rebase(whence) everywhere (other than the implementation
>    of is_from_rebase() itself, of course).
>
>  * give FROM_REBASE_PICK and is_from_rebase() better names that
>    contain "empty" somewhere.
>
> to unconfuse me.

>
> Phillip Wood <phillip.wood123@gmail.com> writes:
>
> >> @@ -521,7 +521,7 @@ static const char *prepare_index(const char **argv, const char *prefix,
> >>              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 during a rebase."));
> >> +                    die(_("cannot do a partial commit while resolving a commit that became empty."));
> >
> > "while committing a commit that became empty" would be clearer to me,
> > but I what you have is definitely an improvement on the existing message.
>
> A stupid question, but wouldn't a partial commit of an empty commit
> still an empty commit?  IOW, why do we need to reject a partial
> commit while committing a commit that became empty?

> From: Elijah Newren <newren@gmail.com>
>
> Commit 430b75f7209c (commit: give correct advice for empty commit during
> a rebase, 2019-12-06) introduced a FROM_REBASE_PICK enum value and an
> is_from_rebase() function.  Those names failed to convey that they were
> specifically about hitting a commit that becomes empty when rebasing.
> Clarify their names now.

> From: Elijah Newren <newren@gmail.com>
>
> For years, we disallowed partial commits during merges or cherry-picks.
> In commit 430b75f7209c (commit: give correct advice for empty commit
> during a rebase, 2019-12-06) it was noted that the "cannot do a partial
> commit during a cherry-pick" message was also printed when rebasing a
> commit that became empty, and rather than drop the check in that case,
> that commit opted to make the message print the actual operation that
> was in progress.
>
> Since a commit that has become empty comes without conflicts, a new
> partial commit poses no problems; remove the error in that case.
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  builtin/commit.c              | 2 --
>  t/t3404-rebase-interactive.sh | 5 ++---
>  2 files changed, 2 insertions(+), 5 deletions(-)

>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 569e31fb60..610820c99f 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -520,8 +520,6 @@ static const char *prepare_index(const char **argv, const char *prefix,
>  			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_empty(whence))
> -			die(_("cannot do a partial commit during a rebase."));
>  	}
>  
>  	if (list_paths(&partial, !current_head ? NULL : "HEAD", &pathspec))
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index ff11abb2f2..3588e16543 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -1858,7 +1858,7 @@ test_expect_success 'post-commit hook is called' '
>  	test_cmp expect actual
>  '
>  
> -test_expect_success 'correct error message for partial commit after empty pick' '
> +test_expect_success 'partial commit is allowed when a rebase pick becomes empty' '
>  	test_when_finished "git rebase --abort" &&
>  	(
>  		set_fake_editor &&
> @@ -1867,8 +1867,7 @@ test_expect_success 'correct error message for partial commit after empty pick'
>  		test_must_fail git rebase -i A D
>  	) &&
>  	echo x >file1 &&
> -	test_must_fail git commit file1 2>err &&
> -	test_grep "cannot do a partial commit during a rebase." err
> +	git commit file1
>  '
>  
>  test_expect_success 'correct error message for commit --amend after empty pick' '

> From: Elijah Newren <newren@gmail.com>
>
> When a rebase applies a commit that becomes empty, it stops and asks the
> user to decide whether to keep it or drop it.  HEAD still points at the
> previously-applied commit at that point, so amending is refused, with:
>
>     You are in the middle of a rebase -- cannot amend.
>
> That message would suggest that amending is not allowed during an 'edit'
> or 'break' stop, which is misleading, plus it lacks the specificity that
> might help the user know why their particular case is a problem: the
> commit they intended to amend became empty and was dropped, so amending
> would affect the wrong commit.  Reword the error accordingly.

> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  builtin/commit.c              | 2 +-
>  t/t3404-rebase-interactive.sh | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 610820c99f..774fb8299d 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1332,7 +1332,7 @@ static int parse_and_validate_options(int argc, const char *argv[],
>  		else if (is_from_cherry_pick(whence))
>  			die(_("You are in the middle of a cherry-pick -- cannot amend."));
>  		else if (is_from_rebase_empty(whence))
> -			die(_("You are in the middle of a rebase -- cannot amend."));
> +			die(_("The now-empty commit has been dropped -- cannot amend."));

>  	}
>  	if (fixup_message && squash_message)
>  		die(_("options '%s' and '%s' cannot be used together"), "--squash", "--fixup");
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index 3588e16543..81f4844950 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -1880,7 +1880,7 @@ test_expect_success 'correct error message for commit --amend after empty pick'
>  	) &&
>  	echo x>file1 &&
>  	test_must_fail git commit -a --amend 2>err &&
> -	test_grep "middle of a rebase -- cannot amend." err
> +	test_grep "now-empty commit has been dropped -- cannot amend." err
>  '
>  
>  test_expect_success 'todo has correct onto hash' '

> -	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_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."));
>  	}

>
> "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Elijah Newren <newren@gmail.com>
> >
> > Commit 430b75f7209c (commit: give correct advice for empty commit during
> > a rebase, 2019-12-06) introduced a FROM_REBASE_PICK enum value and an
> > is_from_rebase() function.  Those names failed to convey that they were
> > specifically about hitting a commit that becomes empty when rebasing.
> > Clarify their names now.
>
> Becomes empty is different from picking an empty commit, right.  I
> am not sure if "is_from_rebase_empty()" conveys the difference and
> more importantly, I am afraid it hints the latter.  I have a feeling
> that EMPTY_REBASE (instead of REBASE_EMPTY) may match what we want
> to express slightly better, but not by a large margin to make a
> difference.  Perhaps Phillip has a better idea?

>
> > While at it, change `whence == FROM_REBASE_EMPTY` to use
> > `is_from_rebase_empty(whence)`.
>
> Very much appreciated.

> From: Elijah Newren <newren@gmail.com>
> 
> Commit 430b75f7209c (commit: give correct advice for empty commit during
> a rebase, 2019-12-06) introduced a FROM_REBASE_PICK enum value and an
> is_from_rebase() function.  Those names failed to convey that they were
> specifically about hitting a commit that becomes empty when rebasing.
> Clarify their names now.
> 
> While at it, change `whence == FROM_REBASE_NOW_EMPTY` to use
> `is_from_rebase_now_empty(whence)`.

> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>   builtin/commit.c | 14 +++++++-------
>   sequencer.c      |  2 +-
>   wt-status.h      |  6 +++---
>   3 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 28f6174503..17cc27e53e 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -520,7 +520,7 @@ static const char *prepare_index(const char **argv, const char *prefix,
>   			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))
> +		else if (is_from_rebase_now_empty(whence))
>   			die(_("cannot do a partial commit during a rebase."));
>   	}
>   
> @@ -893,7 +893,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>   	 */
>   	else if (whence == FROM_MERGE)
>   		hook_arg1 = "merge";
> -	else if (is_from_cherry_pick(whence) || whence == FROM_REBASE_PICK) {
> +	else if (is_from_cherry_pick(whence) || is_from_rebase_now_empty(whence)) {
>   		hook_arg1 = "commit";
>   		hook_arg2 = "CHERRY_PICK_HEAD";
>   	}
> @@ -1086,7 +1086,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>   		if (amend)
>   			fputs(_(empty_amend_advice), stderr);
>   		else if (is_from_cherry_pick(whence) ||
> -			 whence == FROM_REBASE_PICK) {
> +			 is_from_rebase_now_empty(whence)) {
>   			fputs(_(empty_cherry_pick_advice), stderr);
>   			if (whence == FROM_CHERRY_PICK_SINGLE)
>   				fputs(_(empty_cherry_pick_advice_single), stderr);
> @@ -1333,7 +1333,7 @@ static int parse_and_validate_options(int argc, const char *argv[],
>   			die(_("You are in the middle of a merge -- cannot amend."));
>   		else if (is_from_cherry_pick(whence))
>   			die(_("You are in the middle of a cherry-pick -- cannot amend."));
> -		else if (whence == FROM_REBASE_PICK)
> +		else if (is_from_rebase_now_empty(whence))
>   			die(_("You are in the middle of a rebase -- cannot amend."));
>   	}
>   	if (fixup_message && squash_message)
> @@ -1353,7 +1353,7 @@ static int parse_and_validate_options(int argc, const char *argv[],
>   	if (amend && !use_message && !fixup_message)
>   		use_message = "HEAD";
>   	if (!use_message && !is_from_cherry_pick(whence) &&
> -	    !is_from_rebase(whence) && renew_authorship)
> +	    !is_from_rebase_now_empty(whence) && renew_authorship)
>   		die(_("--reset-author can be used only with -C, -c or --amend."));
>   	if (use_message) {
>   		use_message_buffer = read_commit_message(use_message);
> @@ -1362,7 +1362,7 @@ static int parse_and_validate_options(int argc, const char *argv[],
>   			author_message_buffer = use_message_buffer;
>   		}
>   	}
> -	if ((is_from_cherry_pick(whence) || whence == FROM_REBASE_PICK) &&
> +	if ((is_from_cherry_pick(whence) || is_from_rebase_now_empty(whence)) &&
>   	    !renew_authorship) {
>   		author_message = "CHERRY_PICK_HEAD";
>   		author_message_buffer = read_commit_message(author_message);
> @@ -1887,7 +1887,7 @@ int cmd_commit(int argc,
>   		if (!reflog_msg)
>   			reflog_msg = is_from_cherry_pick(whence)
>   					? "commit (cherry-pick)"
> -					: is_from_rebase(whence)
> +					: is_from_rebase_now_empty(whence)
>   					? "commit (rebase)"
>   					: "commit";
>   		commit_list_insert(current_head, &parents);
> diff --git a/sequencer.c b/sequencer.c
> index 65afd100d9..d336c309ca 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -6956,7 +6956,7 @@ int sequencer_determine_whence(struct repository *r, enum commit_whence *whence)
>   		    !repo_get_oid(r, "REBASE_HEAD", &rebase_head) &&
>   		    !repo_get_oid(r, "CHERRY_PICK_HEAD", &cherry_pick_head) &&
>   		    oideq(&rebase_head, &cherry_pick_head))
> -			*whence = FROM_REBASE_PICK;
> +			*whence = FROM_REBASE_NOW_EMPTY;
>   		else
>   			*whence = FROM_CHERRY_PICK_SINGLE;
>   
> diff --git a/wt-status.h b/wt-status.h
> index e9fe32e98c..2143f50b49 100644
> --- a/wt-status.h
> +++ b/wt-status.h
> @@ -41,7 +41,7 @@ enum commit_whence {
>   	FROM_MERGE,      /* commit came from merge */
>   	FROM_CHERRY_PICK_SINGLE, /* commit came from cherry-pick */
>   	FROM_CHERRY_PICK_MULTI, /* commit came from a sequence of cherry-picks */
> -	FROM_REBASE_PICK /* commit came from a pick/reword/edit */
> +	FROM_REBASE_NOW_EMPTY /* rebase applied a pick that became empty */
>   };
>   
>   static inline int is_from_cherry_pick(enum commit_whence whence)
> @@ -50,9 +50,9 @@ static inline int is_from_cherry_pick(enum commit_whence whence)
>   		whence == FROM_CHERRY_PICK_MULTI;
>   }
>   
> -static inline int is_from_rebase(enum commit_whence whence)
> +static inline int is_from_rebase_now_empty(enum commit_whence whence)
>   {
> -	return whence == FROM_REBASE_PICK;
> +	return whence == FROM_REBASE_NOW_EMPTY;
>   }
>   
>   struct wt_status_change_data {

> From: Elijah Newren <newren@gmail.com>
> 
> For years, we disallowed partial commits during merges or cherry-picks.
> In commit 430b75f7209c (commit: give correct advice for empty commit
> during a rebase, 2019-12-06) it was noted that the "cannot do a partial
> commit during a cherry-pick" message was also printed when rebasing a
> commit that became empty, and rather than drop the check in that case,
> that commit opted to make the message print the actual operation that
> was in progress.
> 
> Since a commit that has become empty comes without conflicts, a new
> partial commit poses no problems; remove the error in that case.

> 
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>   builtin/commit.c              | 2 --
>   t/t3404-rebase-interactive.sh | 5 ++---
>   2 files changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 17cc27e53e..01b79185e7 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -520,8 +520,6 @@ static const char *prepare_index(const char **argv, const char *prefix,
>   			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_now_empty(whence))
> -			die(_("cannot do a partial commit during a rebase."));
>   	}
>   
>   	if (list_paths(&partial, !current_head ? NULL : "HEAD", &pathspec))
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index ff11abb2f2..3588e16543 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -1858,7 +1858,7 @@ test_expect_success 'post-commit hook is called' '
>   	test_cmp expect actual
>   '
>   
> -test_expect_success 'correct error message for partial commit after empty pick' '
> +test_expect_success 'partial commit is allowed when a rebase pick becomes empty' '
>   	test_when_finished "git rebase --abort" &&
>   	(
>   		set_fake_editor &&
> @@ -1867,8 +1867,7 @@ test_expect_success 'correct error message for partial commit after empty pick'
>   		test_must_fail git rebase -i A D
>   	) &&
>   	echo x >file1 &&
> -	test_must_fail git commit file1 2>err &&
> -	test_grep "cannot do a partial commit during a rebase." err
> +	git commit file1
>   '
>   
>   test_expect_success 'correct error message for commit --amend after empty pick' '

> From: Elijah Newren <newren@gmail.com>
> 
> When a rebase applies a commit that becomes empty, it stops and asks the
> user to decide whether to keep it or drop it.  HEAD still points at the
> previously-applied commit at that point, so amending is refused, with:
> 
>      You are in the middle of a rebase -- cannot amend.
> 
> That message would suggest that amending is not allowed during an 'edit'
> or 'break' stop, which is misleading, plus it lacks the specificity that
> might help the user know why their particular case is a problem: the
> commit they intended to amend became empty and was dropped, so amending
> would affect the wrong commit.  Reword the error accordingly.

> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>   builtin/commit.c              | 2 +-
>   t/t3404-rebase-interactive.sh | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 01b79185e7..9b6eaa3c72 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1332,7 +1332,7 @@ static int parse_and_validate_options(int argc, const char *argv[],
>   		else if (is_from_cherry_pick(whence))
>   			die(_("You are in the middle of a cherry-pick -- cannot amend."));
>   		else if (is_from_rebase_now_empty(whence))
> -			die(_("You are in the middle of a rebase -- cannot amend."));
> +			die(_("The now-empty commit has been dropped -- cannot amend."));
>   	}
>   	if (fixup_message && squash_message)
>   		die(_("options '%s' and '%s' cannot be used together"), "--squash", "--fixup");
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index 3588e16543..81f4844950 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -1880,7 +1880,7 @@ test_expect_success 'correct error message for commit --amend after empty pick'
>   	) &&
>   	echo x>file1 &&
>   	test_must_fail git commit -a --amend 2>err &&
> -	test_grep "middle of a rebase -- cannot amend." err
> +	test_grep "now-empty commit has been dropped -- cannot amend." err
>   '
>   
>   test_expect_success 'todo has correct onto hash' '

> From: Elijah Newren <newren@gmail.com>
> 
> Running `git commit --amend` during conflict resolution is an ugly
> foot-gun.  For many years, we have rejected amending during conflict
> resolution in the middle of
>    - a merge
>    - a cherry-pick
> However, this was never extended to other operations that can also
> produce conflicts:
>    - an `am` operation
>    - a revert
>    - a rebase
> 
> Extend it to handle these other cases now.
> 
> Extending to `am`, revert, and the apply backend of rebase are fairly
> straightforward.  However, with the merge backend of rebase we have to
> be more careful, since it powers interactive rebases and
>    - the interactive machinery internally uses `git commit --amend` for
>      `squash` and `reword` directives
>    - users are expected to `git commit --amend` after hitting an `edit`
>      or `break` directive
> So, we need to be careful with rebase to only reject amending when doing
> conflict resolution.
> 
> A few files under the rebase-merge/ directory provide us the necessary
> information:
> 
>    - stopped-sha is written only when the rebase stops and hands control
>      back to the user, so its presence marks a genuine stop -- as opposed
>      to the sequencer's own internal `git commit --amend` while applying
>      a squash, fixup, or reword, during which no stopped-sha exists.
> 
>    - amend is written only when the rebase stops with HEAD already
>      pointing at the commit the user is meant to amend: a clean `edit`,
>      or a fast-forward `reword`.  Its absence at a stop therefore means
>      the commit did not apply, so HEAD is the previously-applied commit
>      rather than the one being rebased -- exactly the case we refuse.
> 
> So for the merge backend we die when stopped-sha exists and amend does
> not.  This covers a plain conflicted pick as well as a conflicted `edit`
> (both leave HEAD on the previously-applied commit), while still allowing
> a clean `edit` or `reword` stop and a `break` stop (no stopped-sha).
> stopped-sha is unlinked at the start of the resume loop, so a resumed
> squash's internal amend is unaffected.

> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>   builtin/commit.c                | 27 +++++++---
>   sequencer.c                     | 57 +++++++++++++++++++++
>   sequencer.h                     | 23 +++++++++
>   t/t3404-rebase-interactive.sh   | 87 +++++++++++++++++++++++++++++++++
>   t/t3507-cherry-pick-conflict.sh | 11 +++++
>   t/t4151-am-abort.sh             | 11 +++++
>   6 files changed, 210 insertions(+), 6 deletions(-)
> 
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 9b6eaa3c72..284fc7fdc6 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1324,15 +1324,30 @@ static int parse_and_validate_options(int argc, const char *argv[],
>   		use_editor = 0;
>   
>   	/* Sanity check options */
> -	if (amend && !current_head)
> -		die(_("You have nothing to amend."));
> -	if (amend && whence != FROM_COMMIT) {
> -		if (whence == FROM_MERGE)
> +	if (amend) {
> +		if (!current_head)
> +			die(_("You have nothing to amend."));
> +		/*
> +		 * Refuse to amend in the middle of any operation that is
> +		 * meant to record its result as a new commit on top of HEAD
> +		 * rather than by rewriting HEAD.
> +		 */
> +		switch (sequencer_ongoing_operation(s->repo, whence)) {
> +		case ONGOING_NONE:
> +			break;
> +		case ONGOING_MERGE:
>   			die(_("You are in the middle of a merge -- cannot amend."));
> -		else if (is_from_cherry_pick(whence))
> +		case ONGOING_CHERRY_PICK:
>   			die(_("You are in the middle of a cherry-pick -- cannot amend."));
> -		else if (is_from_rebase_now_empty(whence))
> +		case ONGOING_REBASE_NOW_EMPTY:
>   			die(_("The now-empty commit has been dropped -- cannot amend."));
> +		case ONGOING_REVERT:
> +			die(_("You are in the middle of a revert -- cannot amend."));
> +		case ONGOING_AM:
> +			die(_("You are in the middle of an am session -- cannot amend."));
> +		case ONGOING_REBASE_CONFLICT:
> +			die(_("You are resolving conflicts during a rebase -- cannot amend."));
> +		}
>   	}
>   	if (fixup_message && squash_message)
>   		die(_("options '%s' and '%s' cannot be used together"), "--squash", "--fixup");
> diff --git a/sequencer.c b/sequencer.c
> index d336c309ca..6a28e0dd9a 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -142,6 +142,13 @@ static GIT_PATH_FUNC(rebase_path_author_script, "rebase-merge/author-script")
>    * command is processed, this file is deleted.
>    */
>   static GIT_PATH_FUNC(rebase_path_amend, "rebase-merge/amend")
> +/*
> + * The apply ("am") backend keeps its state in the rebase-apply directory;
> + * the "applying" file within it marks a plain `git am` (as opposed to an
> + * apply-based rebase).
> + */
> +static GIT_PATH_FUNC(apply_dir, "rebase-apply")
> +static GIT_PATH_FUNC(apply_path_applying, "rebase-apply/applying")
>   /*
>    * When we stop at a given patch via the "edit" command, this file contains
>    * the commit object name of the corresponding patch.
> @@ -6966,6 +6973,56 @@ int sequencer_determine_whence(struct repository *r, enum commit_whence *whence)
>   	return 0;
>   }
>   
> +enum ongoing_operation sequencer_ongoing_operation(struct repository *r,
> +						   enum commit_whence whence)
> +{
> +	/*
> +	 * The merge, cherry-pick, and (empty) rebase-pick stops are already
> +	 * distinguished by 'whence'.
> +	 */
> +	switch (whence) {
> +	case FROM_MERGE:
> +		return ONGOING_MERGE;
> +	case FROM_CHERRY_PICK_SINGLE:
> +	case FROM_CHERRY_PICK_MULTI:
> +		return ONGOING_CHERRY_PICK;
> +	case FROM_REBASE_NOW_EMPTY:
> +		return ONGOING_REBASE_NOW_EMPTY;
> +	case FROM_COMMIT:
> +		break;
> +	}
> +
> +	/*
> +	 * 'whence' is FROM_COMMIT, but we may still be in the middle of an
> +	 * operation that records its result on top of HEAD; detect those
> +	 * from their on-disk state.
> +	 */
> +
> +	/* In the middle of a revert? */
> +	if (refs_ref_exists(get_main_ref_store(r), "REVERT_HEAD"))
> +		return ONGOING_REVERT;
> +
> +	/* In the middle of an `am`? */
> +	if (file_exists(apply_path_applying()))
> +		return ONGOING_AM;
> +
> +	/*
> +	 * In the middle of a rebase that stopped for conflict resolution?
> +	 * The apply backend only ever stops for conflicts, so the presence
> +	 * of its state directory is enough.  The merge backend writes
> +	 * stopped-sha whenever it hands control back to the user, but omits
> +	 * `amend` unless it stopped with HEAD already pointing at the commit
> +	 * to be amended (a clean edit/reword stop); its absence therefore
> +	 * marks a conflicted stop.
> +	 */
> +	if (file_exists(apply_dir()) ||
> +	    (file_exists(rebase_path_stopped_sha()) &&
> +	     !file_exists(rebase_path_amend())))
> +		return ONGOING_REBASE_CONFLICT;
> +
> +	return ONGOING_NONE;
> +}
> +
>   int sequencer_get_update_refs_state(const char *wt_dir,
>   				    struct string_list *refs)
>   {
> diff --git a/sequencer.h b/sequencer.h
> index 64a9c7fb1b..fb4a744208 100644
> --- a/sequencer.h
> +++ b/sequencer.h
> @@ -277,6 +277,29 @@ int sequencer_get_last_command(struct repository* r,
>   			       enum replay_action *action);
>   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.
> + */
> +enum ongoing_operation {
> +	ONGOING_NONE = 0,
> +	ONGOING_MERGE,
> +	ONGOING_CHERRY_PICK,
> +	ONGOING_REBASE_NOW_EMPTY,
> +	ONGOING_REVERT,
> +	ONGOING_AM,
> +	ONGOING_REBASE_CONFLICT
> +};
> +
> +/*
> + * Return which in-progress operation, if any, is underway; see enum
> + * ongoing_operation.  'whence' is the origin already computed for the
> + * pending commit.
> + */
> +enum ongoing_operation sequencer_ongoing_operation(struct repository *r,
> +						   enum commit_whence whence);
> +
>   /**
>    * Append the set of ref-OID pairs that are currently stored for the 'git
>    * rebase --update-refs' feature if such a rebase is currently happening.
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index 81f4844950..4e6c3e2f19 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -1883,6 +1883,93 @@ test_expect_success 'correct error message for commit --amend after empty pick'
>   	test_grep "now-empty commit has been dropped -- cannot amend." err
>   '
>   
> +test_expect_success 'commit --amend 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
> +	) &&
> +	test_path_is_file .git/rebase-merge/patch &&
> +	test_path_is_missing .git/rebase-merge/amend &&
> +	echo resolved >conflict &&
> +	git add conflict &&
> +	test_must_fail git commit --amend --no-edit 2>err &&
> +	test_grep "You are resolving conflicts during a rebase -- cannot amend" err
> +'
> +
> +test_expect_success 'commit --amend is refused when an "edit" pick conflicts' '
> +	test_when_finished "git rebase --abort" &&
> +	git checkout --detach conflict-branch &&
> +	(
> +		set_fake_editor &&
> +		FAKE_LINES="1 edit 3" &&
> +		export FAKE_LINES &&
> +		test_must_fail git rebase -i A
> +	) &&
> +	test_path_is_file .git/rebase-merge/patch &&
> +	test_path_is_missing .git/rebase-merge/amend &&
> +	echo resolved >conflict &&
> +	git add conflict &&
> +	test_must_fail git commit --amend --no-edit 2>err &&
> +	test_grep "You are resolving conflicts during a rebase -- cannot amend" err
> +'
> +
> +test_expect_success 'commit --amend is allowed at a rebase edit stop' '
> +	test_when_finished "git rebase --abort" &&
> +	git checkout --detach no-conflict-branch &&
> +	(
> +		set_fake_editor &&
> +		FAKE_LINES="edit 1 2 3 4" &&
> +		export FAKE_LINES &&
> +		git rebase -i A
> +	) &&
> +	test_path_is_file .git/rebase-merge/amend &&
> +	echo tweak >fileJ &&
> +	git add fileJ &&
> +	git commit --amend --no-edit
> +'
> +
> +test_expect_success 'commit --amend is allowed at a rebase break stop' '
> +	test_when_finished "git rebase --abort" &&
> +	git checkout --detach no-conflict-branch &&
> +	(
> +		set_fake_editor &&
> +		FAKE_LINES="break 1 2 3 4" &&
> +		export FAKE_LINES &&
> +		git rebase -i A
> +	) &&
> +	test_must_fail git rev-parse --verify REBASE_HEAD &&
> +	echo tweak >fileJ &&
> +	git add fileJ &&
> +	git commit --amend --no-edit
> +'
> +
> +test_expect_success 'commit --amend 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 &&
> +		# the apply backend only ever stops for conflicts, and
> +		# leaves HEAD on the previously-applied commit
> +		test_path_is_dir .git/rebase-apply &&
> +		test_path_is_missing .git/rebase-apply/applying &&
> +		echo resolved >file &&
> +		git add file &&
> +		test_must_fail git commit --amend --no-edit 2>err &&
> +		test_grep "You are resolving conflicts during a rebase -- cannot amend" 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 44596cb1e8..42de398f76 100755
> --- a/t/t3507-cherry-pick-conflict.sh
> +++ b/t/t3507-cherry-pick-conflict.sh
> @@ -364,6 +364,17 @@ test_expect_success 'failed revert sets REVERT_HEAD' '
>   	test_cmp_rev picked REVERT_HEAD
>   '
>   
> +test_expect_success 'commit --amend of revert fails' '
> +	pristine_detach initial &&
> +
> +	test_must_fail git revert picked &&
> +	echo resolved >foo &&
> +	git add foo &&
> +	test_must_fail git commit --amend 2>err &&
> +
> +	test_grep "in the middle of a revert -- cannot amend." 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 8e1ecf8a68..9313a074b2 100755
> --- a/t/t4151-am-abort.sh
> +++ b/t/t4151-am-abort.sh
> @@ -63,6 +63,17 @@ do
>   
>   done
>   
> +test_expect_success 'commit --amend 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 --amend 2>err &&
> +	test_grep "in the middle of an am session -- cannot amend." 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 &&

> Changes since v3:
> 
>   * Just a clarifying rename, but applied in 3 places:
>     * FROM_REBASE_EMPTY -> FROM_REBASE_NOW_EMPTY
>     * is_from_rebase_empty() -> is_from_rebase_now_empty()
>     * ONGOING_REBASE_EMPTY -> ONGOING_REBASE_NOW_EMPTY
> 
> Changes since v2:
> 
>   * Two new preparatory patches:
>     * Rename FROM_REBASE_PICK and is_from_rebase() to point out they are
>       about empty commits
>     * Allow a partial commit when a rebase pick becomes empty
>   * Tweaked the error message for attempted amend on now-dropped empty commit
>     (suggestions for further improvements welcome)
>   * Used the path accessor functions within sequencer.c to simplify the new
>     helper function
> 
> Both git commit --amend and a partial commit (git commit <paths>) are
> foot-guns while the user is in the middle of an operation that resolves
> conflicts on top of HEAD: recording a conflict resolution is about capturing
> the state of the whole tree as a new commit, not about rewriting HEAD or
> committing a subset of paths.
> 
> Historically we only rejected these during a merge or a cherry-pick or when
> resolving an empty pick during a rebase. The same hazard exists for am,
> revert, and rebase conflict stops, none of which were covered. This series
> extends the refusal to all of them.
> 
> The three patches:
> 
>   1. reword the two pre-existing "empty commit" rebase messages, which were
>      misleadingly generic
>   2. refuse git commit --amend during these additional operations
>   3. refuse partial commits during the same operations.
> 
> Elijah Newren (5):
>    commit: clarify FROM_REBASE_PICK and is_from_rebase() names
>    commit: allow a partial commit when a rebase pick becomes empty
>    commit: reword the empty-commit rebase amend error
>    commit: refuse to amend during conflict resolution
>    commit: refuse partial commits during conflict resolution
> 
>   builtin/commit.c                |  65 +++++++++++-----
>   sequencer.c                     |  59 ++++++++++++++-
>   sequencer.h                     |  24 ++++++
>   t/t3404-rebase-interactive.sh   | 128 +++++++++++++++++++++++++++++++-
>   t/t3507-cherry-pick-conflict.sh |  22 ++++++
>   t/t4151-am-abort.sh             |  22 ++++++
>   wt-status.h                     |   6 +-
>   7 files changed, 299 insertions(+), 27 deletions(-)
> 
> 
> base-commit: 2c3adbb2c475981e340c79fdc5e7f4f9b5d9054e
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2389%2Fnewren%2Frefuse-amend-during-conflicts-v4
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2389/newren/refuse-amend-during-conflicts-v4
> Pull-Request: https://github.com/git/git/pull/2389
> 
> Range-diff vs v3:
> 
>   1:  7e198a20fa ! 1:  bd361679b9 commit: clarify FROM_REBASE_PICK and is_from_rebase() names
>       @@ Commit message
>            specifically about hitting a commit that becomes empty when rebasing.
>            Clarify their names now.
>        
>       -    While at it, change `whence == FROM_REBASE_EMPTY` to use
>       -    `is_from_rebase_empty(whence)`.
>       +    While at it, change `whence == FROM_REBASE_NOW_EMPTY` to use
>       +    `is_from_rebase_now_empty(whence)`.
>        
>            Signed-off-by: Elijah Newren <newren@gmail.com>
>        
>       @@ builtin/commit.c: static const char *prepare_index(const char **argv, const char
>         		else if (is_from_cherry_pick(whence))
>         			die(_("cannot do a partial commit during a cherry-pick."));
>        -		else if (is_from_rebase(whence))
>       -+		else if (is_from_rebase_empty(whence))
>       ++		else if (is_from_rebase_now_empty(whence))
>         			die(_("cannot do a partial commit during a rebase."));
>         	}
>         
>       @@ builtin/commit.c: static int prepare_to_commit(const char *index_file, const cha
>         	else if (whence == FROM_MERGE)
>         		hook_arg1 = "merge";
>        -	else if (is_from_cherry_pick(whence) || whence == FROM_REBASE_PICK) {
>       -+	else if (is_from_cherry_pick(whence) || is_from_rebase_empty(whence)) {
>       ++	else if (is_from_cherry_pick(whence) || is_from_rebase_now_empty(whence)) {
>         		hook_arg1 = "commit";
>         		hook_arg2 = "CHERRY_PICK_HEAD";
>         	}
>       @@ builtin/commit.c: static int prepare_to_commit(const char *index_file, const cha
>         			fputs(_(empty_amend_advice), stderr);
>         		else if (is_from_cherry_pick(whence) ||
>        -			 whence == FROM_REBASE_PICK) {
>       -+			 is_from_rebase_empty(whence)) {
>       ++			 is_from_rebase_now_empty(whence)) {
>         			fputs(_(empty_cherry_pick_advice), stderr);
>         			if (whence == FROM_CHERRY_PICK_SINGLE)
>         				fputs(_(empty_cherry_pick_advice_single), stderr);
>       @@ builtin/commit.c: static int parse_and_validate_options(int argc, const char *ar
>         		else if (is_from_cherry_pick(whence))
>         			die(_("You are in the middle of a cherry-pick -- cannot amend."));
>        -		else if (whence == FROM_REBASE_PICK)
>       -+		else if (is_from_rebase_empty(whence))
>       ++		else if (is_from_rebase_now_empty(whence))
>         			die(_("You are in the middle of a rebase -- cannot amend."));
>         	}
>         	if (fixup_message && squash_message)
>       @@ builtin/commit.c: static int parse_and_validate_options(int argc, const char *ar
>         		use_message = "HEAD";
>         	if (!use_message && !is_from_cherry_pick(whence) &&
>        -	    !is_from_rebase(whence) && renew_authorship)
>       -+	    !is_from_rebase_empty(whence) && renew_authorship)
>       ++	    !is_from_rebase_now_empty(whence) && renew_authorship)
>         		die(_("--reset-author can be used only with -C, -c or --amend."));
>         	if (use_message) {
>         		use_message_buffer = read_commit_message(use_message);
>       @@ builtin/commit.c: static int parse_and_validate_options(int argc, const char *ar
>         		}
>         	}
>        -	if ((is_from_cherry_pick(whence) || whence == FROM_REBASE_PICK) &&
>       -+	if ((is_from_cherry_pick(whence) || is_from_rebase_empty(whence)) &&
>       ++	if ((is_from_cherry_pick(whence) || is_from_rebase_now_empty(whence)) &&
>         	    !renew_authorship) {
>         		author_message = "CHERRY_PICK_HEAD";
>         		author_message_buffer = read_commit_message(author_message);
>       @@ builtin/commit.c: int cmd_commit(int argc,
>         			reflog_msg = is_from_cherry_pick(whence)
>         					? "commit (cherry-pick)"
>        -					: is_from_rebase(whence)
>       -+					: is_from_rebase_empty(whence)
>       ++					: is_from_rebase_now_empty(whence)
>         					? "commit (rebase)"
>         					: "commit";
>         		commit_list_insert(current_head, &parents);
>       @@ sequencer.c: int sequencer_determine_whence(struct repository *r, enum commit_wh
>         		    !repo_get_oid(r, "CHERRY_PICK_HEAD", &cherry_pick_head) &&
>         		    oideq(&rebase_head, &cherry_pick_head))
>        -			*whence = FROM_REBASE_PICK;
>       -+			*whence = FROM_REBASE_EMPTY;
>       ++			*whence = FROM_REBASE_NOW_EMPTY;
>         		else
>         			*whence = FROM_CHERRY_PICK_SINGLE;
>         
>       @@ wt-status.h: enum commit_whence {
>         	FROM_CHERRY_PICK_SINGLE, /* commit came from cherry-pick */
>         	FROM_CHERRY_PICK_MULTI, /* commit came from a sequence of cherry-picks */
>        -	FROM_REBASE_PICK /* commit came from a pick/reword/edit */
>       -+	FROM_REBASE_EMPTY /* rebase applied a pick that became empty */
>       ++	FROM_REBASE_NOW_EMPTY /* rebase applied a pick that became empty */
>         };
>         
>         static inline int is_from_cherry_pick(enum commit_whence whence)
>       @@ wt-status.h: static inline int is_from_cherry_pick(enum commit_whence whence)
>         }
>         
>        -static inline int is_from_rebase(enum commit_whence whence)
>       -+static inline int is_from_rebase_empty(enum commit_whence whence)
>       ++static inline int is_from_rebase_now_empty(enum commit_whence whence)
>         {
>        -	return whence == FROM_REBASE_PICK;
>       -+	return whence == FROM_REBASE_EMPTY;
>       ++	return whence == FROM_REBASE_NOW_EMPTY;
>         }
>         
>         struct wt_status_change_data {
>   2:  e169303619 ! 2:  a0b9900437 commit: allow a partial commit when a rebase pick becomes empty
>       @@ builtin/commit.c: static const char *prepare_index(const char **argv, const char
>         			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_empty(whence))
>       +-		else if (is_from_rebase_now_empty(whence))
>        -			die(_("cannot do a partial commit during a rebase."));
>         	}
>         
>   3:  0850a999da ! 3:  c4511a9887 commit: reword the empty-commit rebase amend error
>       @@ builtin/commit.c
>        @@ builtin/commit.c: static int parse_and_validate_options(int argc, const char *argv[],
>         		else if (is_from_cherry_pick(whence))
>         			die(_("You are in the middle of a cherry-pick -- cannot amend."));
>       - 		else if (is_from_rebase_empty(whence))
>       + 		else if (is_from_rebase_now_empty(whence))
>        -			die(_("You are in the middle of a rebase -- cannot amend."));
>        +			die(_("The now-empty commit has been dropped -- cannot amend."));
>         	}
>   4:  9f80d8a00d ! 4:  e77b34c1cf commit: refuse to amend during conflict resolution
>       @@ builtin/commit.c: static int parse_and_validate_options(int argc, const char *ar
>        -		else if (is_from_cherry_pick(whence))
>        +		case ONGOING_CHERRY_PICK:
>         			die(_("You are in the middle of a cherry-pick -- cannot amend."));
>       --		else if (is_from_rebase_empty(whence))
>       -+		case ONGOING_REBASE_EMPTY:
>       +-		else if (is_from_rebase_now_empty(whence))
>       ++		case ONGOING_REBASE_NOW_EMPTY:
>         			die(_("The now-empty commit has been dropped -- cannot amend."));
>        +		case ONGOING_REVERT:
>        +			die(_("You are in the middle of a revert -- cannot amend."));
>       @@ sequencer.c: int sequencer_determine_whence(struct repository *r, enum commit_wh
>        +	case FROM_CHERRY_PICK_SINGLE:
>        +	case FROM_CHERRY_PICK_MULTI:
>        +		return ONGOING_CHERRY_PICK;
>       -+	case FROM_REBASE_EMPTY:
>       -+		return ONGOING_REBASE_EMPTY;
>       ++	case FROM_REBASE_NOW_EMPTY:
>       ++		return ONGOING_REBASE_NOW_EMPTY;
>        +	case FROM_COMMIT:
>        +		break;
>        +	}
>       @@ sequencer.h: int sequencer_get_last_command(struct repository* r,
>        +	ONGOING_NONE = 0,
>        +	ONGOING_MERGE,
>        +	ONGOING_CHERRY_PICK,
>       -+	ONGOING_REBASE_EMPTY,
>       ++	ONGOING_REBASE_NOW_EMPTY,
>        +	ONGOING_REVERT,
>        +	ONGOING_AM,
>        +	ONGOING_REBASE_CONFLICT
>   5:  050b9e8a52 ! 5:  b93b26ed9f commit: refuse partial commits during conflict resolution
>       @@ builtin/commit.c: static const char *prepare_index(const char **argv, const char
>        +		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:
>       ++	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.
>