Revert "Merge branch 'en/no-amend-during-conflicts'"

This reverts commit a334b7ec86,
reversing changes made to cee3798cb4,
as we found a regression and decided to give ourselves a chance to
fix it better after Git 2.56 final.

  cf. https://lore.kernel.org/git/xmqqik3vc1pc.fsf@gitster.g/
maint
Junio C Hamano 2026-09-23 10:34:33 -07:00
parent 3bc0341126
commit 0f8e75abeb
8 changed files with 27 additions and 305 deletions

View File

@ -867,12 +867,6 @@ Fixes since v2.55
Additionally, an obsolete variable used for retired Azure Pipelines Additionally, an obsolete variable used for retired Azure Pipelines
environments has been removed. environments has been removed.


* Teach 'am', 'revert', and 'rebase' that running 'commit --amend' or a
partial 'commit <paths>' makes no sense during operations that stop
and return control to the user to resolve conflicts left in the
working tree, just like 'cherry-pick' and 'merge' do.
(merge cc499d40e5 en/no-amend-during-conflicts later to maint).

* The error path in 'git submodule--helper' has been updated to plug a * The error path in 'git submodule--helper' has been updated to plug a
memory leak when a repository handle could not be obtained, memory leak when a repository handle could not be obtained,
leveraging an updated idempotent repo_clear(). leveraging an updated idempotent repo_clear().

View File

@ -515,25 +515,13 @@ static const char *prepare_index(const char **argv, const char *prefix,
*/ */
commit_style = COMMIT_PARTIAL; commit_style = COMMIT_PARTIAL;


switch (sequencer_ongoing_operation(the_repository, whence)) { if (whence != FROM_COMMIT) {
case ONGOING_NONE: if (whence == FROM_MERGE)
break; die(_("cannot do a partial commit during a merge."));
case ONGOING_MERGE: else if (is_from_cherry_pick(whence))
die(_("cannot do a partial commit during a merge.")); die(_("cannot do a partial commit during a cherry-pick."));
case ONGOING_CHERRY_PICK: else if (is_from_rebase(whence))
die(_("cannot do a partial commit during a cherry-pick.")); die(_("cannot do a partial commit during a rebase."));
case ONGOING_REBASE_NOW_EMPTY:
/*
* A pick that became empty is not a conflict, and creating
* a new commit (partial or not) poses no problem.
*/
break;
case ONGOING_REVERT:
die(_("cannot do a partial commit during a revert."));
case ONGOING_AM:
die(_("cannot do a partial commit during an am session."));
case ONGOING_REBASE_CONFLICT:
die(_("cannot do a partial commit while resolving conflicts during a rebase."));
} }


if (list_paths(&partial, !current_head ? NULL : "HEAD", &pathspec)) if (list_paths(&partial, !current_head ? NULL : "HEAD", &pathspec))
@ -905,7 +893,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
*/ */
else if (whence == FROM_MERGE) else if (whence == FROM_MERGE)
hook_arg1 = "merge"; hook_arg1 = "merge";
else if (is_from_cherry_pick(whence) || is_from_rebase_now_empty(whence)) { else if (is_from_cherry_pick(whence) || whence == FROM_REBASE_PICK) {
hook_arg1 = "commit"; hook_arg1 = "commit";
hook_arg2 = "CHERRY_PICK_HEAD"; hook_arg2 = "CHERRY_PICK_HEAD";
} }
@ -1098,7 +1086,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
if (amend) if (amend)
fputs(_(empty_amend_advice), stderr); fputs(_(empty_amend_advice), stderr);
else if (is_from_cherry_pick(whence) || else if (is_from_cherry_pick(whence) ||
is_from_rebase_now_empty(whence)) { whence == FROM_REBASE_PICK) {
fputs(_(empty_cherry_pick_advice), stderr); fputs(_(empty_cherry_pick_advice), stderr);
if (whence == FROM_CHERRY_PICK_SINGLE) if (whence == FROM_CHERRY_PICK_SINGLE)
fputs(_(empty_cherry_pick_advice_single), stderr); fputs(_(empty_cherry_pick_advice_single), stderr);
@ -1338,30 +1326,15 @@ static int parse_and_validate_options(int argc, const char *argv[],
use_editor = 0; use_editor = 0;


/* Sanity check options */ /* Sanity check options */
if (amend) { if (amend && !current_head)
if (!current_head) die(_("You have nothing to amend."));
die(_("You have nothing to amend.")); if (amend && whence != FROM_COMMIT) {
/* if (whence == FROM_MERGE)
* 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.")); die(_("You are in the middle of a merge -- cannot amend."));
case ONGOING_CHERRY_PICK: else if (is_from_cherry_pick(whence))
die(_("You are in the middle of a cherry-pick -- cannot amend.")); die(_("You are in the middle of a cherry-pick -- cannot amend."));
case ONGOING_REBASE_NOW_EMPTY: else if (whence == FROM_REBASE_PICK)
die(_("The now-empty commit has been dropped -- cannot amend.")); die(_("You are in the middle of a rebase -- 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) if (fixup_message && squash_message)
die(_("options '%s' and '%s' cannot be used together"), "--squash", "--fixup"); die(_("options '%s' and '%s' cannot be used together"), "--squash", "--fixup");
@ -1380,7 +1353,7 @@ static int parse_and_validate_options(int argc, const char *argv[],
if (amend && !use_message && !fixup_message) if (amend && !use_message && !fixup_message)
use_message = "HEAD"; use_message = "HEAD";
if (!use_message && !is_from_cherry_pick(whence) && if (!use_message && !is_from_cherry_pick(whence) &&
!is_from_rebase_now_empty(whence) && renew_authorship) !is_from_rebase(whence) && renew_authorship)
die(_("--reset-author can be used only with -C, -c or --amend.")); die(_("--reset-author can be used only with -C, -c or --amend."));
if (use_message) { if (use_message) {
use_message_buffer = read_commit_message(use_message); use_message_buffer = read_commit_message(use_message);
@ -1389,7 +1362,7 @@ static int parse_and_validate_options(int argc, const char *argv[],
author_message_buffer = use_message_buffer; author_message_buffer = use_message_buffer;
} }
} }
if ((is_from_cherry_pick(whence) || is_from_rebase_now_empty(whence)) && if ((is_from_cherry_pick(whence) || whence == FROM_REBASE_PICK) &&
!renew_authorship) { !renew_authorship) {
author_message = "CHERRY_PICK_HEAD"; author_message = "CHERRY_PICK_HEAD";
author_message_buffer = read_commit_message(author_message); author_message_buffer = read_commit_message(author_message);
@ -1914,7 +1887,7 @@ int cmd_commit(int argc,
if (!reflog_msg) if (!reflog_msg)
reflog_msg = is_from_cherry_pick(whence) reflog_msg = is_from_cherry_pick(whence)
? "commit (cherry-pick)" ? "commit (cherry-pick)"
: is_from_rebase_now_empty(whence) : is_from_rebase(whence)
? "commit (rebase)" ? "commit (rebase)"
: "commit"; : "commit";
commit_list_insert(current_head, &parents); commit_list_insert(current_head, &parents);

View File

@ -143,13 +143,6 @@ static GIT_PATH_FUNC(rebase_path_author_script, "rebase-merge/author-script")
* command is processed, this file is deleted. * command is processed, this file is deleted.
*/ */
static GIT_PATH_FUNC(rebase_path_amend, "rebase-merge/amend") 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 * When we stop at a given patch via the "edit" command, this file contains
* the commit object name of the corresponding patch. * the commit object name of the corresponding patch.
@ -6994,7 +6987,7 @@ int sequencer_determine_whence(struct repository *r, enum commit_whence *whence)
!repo_get_oid(r, "REBASE_HEAD", &rebase_head) && !repo_get_oid(r, "REBASE_HEAD", &rebase_head) &&
!repo_get_oid(r, "CHERRY_PICK_HEAD", &cherry_pick_head) && !repo_get_oid(r, "CHERRY_PICK_HEAD", &cherry_pick_head) &&
oideq(&rebase_head, &cherry_pick_head)) oideq(&rebase_head, &cherry_pick_head))
*whence = FROM_REBASE_NOW_EMPTY; *whence = FROM_REBASE_PICK;
else else
*whence = FROM_CHERRY_PICK_SINGLE; *whence = FROM_CHERRY_PICK_SINGLE;


@ -7004,56 +6997,6 @@ int sequencer_determine_whence(struct repository *r, enum commit_whence *whence)
return 0; 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, int sequencer_get_update_refs_state(const char *wt_dir,
struct string_list *refs) struct string_list *refs)
{ {

View File

@ -282,30 +282,6 @@ int sequencer_get_last_command(struct repository* r,
enum replay_action *action); enum replay_action *action);
int sequencer_determine_whence(struct repository *r, enum commit_whence *whence); 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. 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,
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 * 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. * rebase --update-refs' feature if such a rebase is currently happening.

View File

@ -1858,7 +1858,7 @@ test_expect_success 'post-commit hook is called' '
test_cmp expect actual test_cmp expect actual
' '


test_expect_success 'partial commit is allowed when a rebase pick becomes empty' ' test_expect_success 'correct error message for partial commit after empty pick' '
test_when_finished "git rebase --abort" && test_when_finished "git rebase --abort" &&
( (
set_fake_editor && set_fake_editor &&
@ -1867,7 +1867,8 @@ test_expect_success 'partial commit is allowed when a rebase pick becomes empty'
test_must_fail git rebase -i A D test_must_fail git rebase -i A D
) && ) &&
echo x >file1 && echo x >file1 &&
git commit file1 test_must_fail git commit file1 2>err &&
test_grep "cannot do a partial commit during a rebase." err
' '


test_expect_success 'correct error message for commit --amend after empty pick' ' test_expect_success 'correct error message for commit --amend after empty pick' '
@ -1880,128 +1881,7 @@ test_expect_success 'correct error message for commit --amend after empty pick'
) && ) &&
echo x>file1 && echo x>file1 &&
test_must_fail git commit -a --amend 2>err && test_must_fail git commit -a --amend 2>err &&
test_grep "now-empty commit has been dropped -- cannot amend." err 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 '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' ' test_expect_success 'todo has correct onto hash' '

View File

@ -365,28 +365,6 @@ test_expect_success 'failed revert sets REVERT_HEAD' '
test_cmp_rev picked 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 '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' ' test_expect_success 'successful revert does not set REVERT_HEAD' '
pristine_detach base && pristine_detach base &&
git revert base && git revert base &&

View File

@ -63,28 +63,6 @@ do


done 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 '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' ' test_expect_success 'am -3 --skip removes otherfile-4' '
git reset --hard initial && git reset --hard initial &&
test_must_fail git am -3 0003-*.patch && test_must_fail git am -3 0003-*.patch &&

View File

@ -41,7 +41,7 @@ enum commit_whence {
FROM_MERGE, /* commit came from merge */ FROM_MERGE, /* commit came from merge */
FROM_CHERRY_PICK_SINGLE, /* commit came from cherry-pick */ FROM_CHERRY_PICK_SINGLE, /* commit came from cherry-pick */
FROM_CHERRY_PICK_MULTI, /* commit came from a sequence of cherry-picks */ FROM_CHERRY_PICK_MULTI, /* commit came from a sequence of cherry-picks */
FROM_REBASE_NOW_EMPTY /* rebase applied a pick that became empty */ FROM_REBASE_PICK /* commit came from a pick/reword/edit */
}; };


static inline int is_from_cherry_pick(enum commit_whence whence) 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; whence == FROM_CHERRY_PICK_MULTI;
} }


static inline int is_from_rebase_now_empty(enum commit_whence whence) static inline int is_from_rebase(enum commit_whence whence)
{ {
return whence == FROM_REBASE_NOW_EMPTY; return whence == FROM_REBASE_PICK;
} }


struct wt_status_change_data { struct wt_status_change_data {