Re: [PATCH v4 0/5] commit: refuse to amend during conflict resolution
From: Phillip Wood <hidden>
Date: 2026-09-02 15:41:39
Hi Elijah The changes from v2 look good, I've got mixed feelings about the new patch 2 but I'm happy with everything else. Thanks for working on it Phillip On 01/09/2026 23:24, Elijah Newren via GitGitGadget wrote:
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 [off-list ref]
@@ 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.