commit: refuse to amend during conflict resolution

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 <[email protected]>
      
     @@ 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.

gitgitgadget