{"thread":{"id":"66376","subject":"[PATCH REGRESSION] builtin/rebase: allow user to amend committed conflicts again","startedAt":"2026-09-23T13:16:52Z","lastAt":"2026-09-27T08:04:31Z","messageCount":13,"participants":["Patrick Steinhardt","Phillip Wood","Junio C Hamano","Elijah Newren","Johannes Sixt","Jiang Xin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"553058","messageId":"20260923-pks-rebase-conflict-bug-v1-1-3d3ccf5022bc@pks.im","threadId":"66376","inReplyTo":null,"subject":"[PATCH REGRESSION] builtin/rebase: allow user to amend committed conflicts again","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-23T13:16:43Z","receivedAt":"2026-09-23T13:16:52Z","isPatch":true,"body":"In 6257588252 (commit: refuse to amend during conflict resolution,\n2026-09-01), we have introduced logic to git-commit(1) that makes it\nrefuse creating a commit in some cases. This was done to remove a set of\ncommon foot guns.\n\nOne of these foot guns is when the user is performing an interactive\nrebase that stops at a conflict. Most of the time when we stop at a\nspecific commit we want the user to amend the HEAD commit, so they have\nbeen trained to use `git commit --amend`. But when there's a conflict,\nthey are instead supposed to commit it directly without amending the\nHEAD commit. So to remove that common pit fall, git-commit(1) now\nrefuses amending in that situation.\n\nThe logic that detects this scenario checks whether the file\n\"rebase-merge/stopped-sha\" exists, while \"rebase-merge/amend\" doesn't.\nAnd this is exactly the case when git-rebase(1) has stopped at such a\nconflicting commit.\n\nBut there's one problem here: this state persists even after the user\nhas already committed the resolved conflict, and consequently they still\ncannot amend after they have done so. This is overly restrictive though,\nas it's quite likely that a user may want to change the resolved commit\nonce again.\n\nIdeally, we'd be able to easily check whether HEAD has already been\nupdated to have the resolved conflict. But it seems like we do not have\nsufficient information to determine the original state of HEAD when the\ninteractive rebase has stopped, so this is not a workable solution.\n\nInstead, use the existence of \"MERGE_MSG\" to figure out whether the user\nhas already resolved and committed the conflict. It feels somewhat fishy\nto base our decisions on the existence of that particular file, as it\nreally is only a proxy for what we are actually after. But the whole way\nthat we track rebase state is somewhat iffy in the first place.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\nHi,\n\nthis is a regression caused by 6257588252 (commit: refuse to amend\nduring conflict resolution, 2026-09-01). Ideally, we should probably fix\nit before we release Git 2.56.\n\nI'm not particularly happy with the proposed fix -- it feels quite fishy\nto use the existence of MERGE_MSG as a proxy for whether or not the user\nhas already committed the resolved conflict. I couldn't come up with a\nbetter proxy though, so if you have one please let me know.\n\nThanks!\n\nPatrick\n---\n sequencer.c                   |  4 ++++\n t/t3404-rebase-interactive.sh | 34 ++++++++++++++++++++++++++++++++++\n 2 files changed, 38 insertions(+)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex e25ef5eb61..0f718c1d38 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -7045,9 +7045,13 @@ enum ongoing_operation sequencer_ongoing_operation(struct repository *r,\n \t * `amend` unless it stopped with HEAD already pointing at the commit\n \t * to be amended (a clean edit/reword stop); its absence therefore\n \t * marks a conflicted stop.\n+\t *\n+\t * Note that we also check for MERGE_MSG. This is to catch the case\n+\t * where the user has already resolved and committed the conflict.\n \t */\n \tif (file_exists(apply_dir()) ||\n \t    (file_exists(rebase_path_stopped_sha()) &&\n+\t     file_exists(git_path_merge_msg(r)) &&\n \t     !file_exists(rebase_path_amend())))\n \t\treturn ONGOING_REBASE_CONFLICT;\n \ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 8c63682b7f..d55afaa113 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -2486,6 +2486,40 @@ test_expect_success 'non-merge commands reject merge commits' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'can amend after committing a conflict' '\n+\ttest_when_finished rm -rf repo &&\n+\tgit init repo &&\n+\t(\n+\t\tcd repo &&\n+\n+\t\ttest_commit original file &&\n+\t\ttest_commit modified file &&\n+\t\tcat >todo <<-EOF &&\n+\t\tbreak\n+\t\tedit $(git rev-parse HEAD)\n+\t\tEOF\n+\t\tset_replace_editor todo &&\n+\t\tgit rebase -i HEAD~ &&\n+\n+\t\t# Modify \"file\" to cause a conflict.\n+\t\techo conflict >file &&\n+\t\tgit commit -a --message conflict &&\n+\t\ttest_must_fail git rebase --continue 2>err &&\n+\t\ttest_grep \"Resolve all conflicts manually\" err &&\n+\n+\t\t# Resolve the conflict.\n+\t\techo resolved >file &&\n+\t\tgit add file &&\n+\t\tgit commit --message resolve &&\n+\n+\t\t# And now try to amend to the conflict. This operation should\n+\t\t# succeed.\n+\t\techo change >file &&\n+\t\tgit commit --amend -a --no-edit &&\n+\t\tgit rebase --continue\n+\t)\n+'\n+\n # This must be the last test in this file\n test_expect_success '$EDITOR and friends are unchanged' '\n \ttest_editor_unchanged\n\n---\nbase-commit: 3bc0341126508f78f5869cbfc0005e987efdf0c7\nchange-id: 20260923-pks-rebase-conflict-bug-176e325ad079\n\n"},{"id":"553061","messageId":"c12d2ac3-5263-4301-aa64-a311a343dd40@gmail.com","threadId":"66376","inReplyTo":"20260923-pks-rebase-conflict-bug-v1-1-3d3ccf5022bc@pks.im","subject":"Re: [PATCH REGRESSION] builtin/rebase: allow user to amend committed conflicts again","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-09-23T14:02:21Z","receivedAt":"2026-09-23T14:02:26Z","isPatch":true,"body":"Hi Patrick\n\nOn 23/09/2026 14:16, Patrick Steinhardt wrote:\n> In 6257588252 (commit: refuse to amend during conflict resolution,\n> 2026-09-01), we have introduced logic to git-commit(1) that makes it\n> refuse creating a commit in some cases. This was done to remove a set of\n> common foot guns.\n> \n> One of these foot guns is when the user is performing an interactive\n> rebase that stops at a conflict. Most of the time when we stop at a\n> specific commit we want the user to amend the HEAD commit, so they have\n> been trained to use `git commit --amend`. But when there's a conflict,\n> they are instead supposed to commit it directly without amending the\n> HEAD commit. So to remove that common pit fall, git-commit(1) now\n> refuses amending in that situation.\n> \n> The logic that detects this scenario checks whether the file\n> \"rebase-merge/stopped-sha\" exists, while \"rebase-merge/amend\" doesn't.\n> And this is exactly the case when git-rebase(1) has stopped at such a\n> conflicting commit.\n> \n> But there's one problem here: this state persists even after the user\n> has already committed the resolved conflict, and consequently they still\n> cannot amend after they have done so. This is overly restrictive though,\n> as it's quite likely that a user may want to change the resolved commit\n> once again.\n> \n> Ideally, we'd be able to easily check whether HEAD has already been\n> updated to have the resolved conflict. But it seems like we do not have\n> sufficient information to determine the original state of HEAD when the\n> interactive rebase has stopped, so this is not a workable solution.\n\nYes, that's unfortunate - I think there is an argument that rebase \nshould be writing \".git/rebase-merge/stopped-head\" when it stops. That \nwould make it easy to detect if the user has committed since the rebase \nstopped. At the moment \"git rebase --continue\" will happily commit any \nstaged changes with the message from the commit that was being picked \nwhen the rebase stopped, even if the user has already committed a \nconflict resolution. Fixing that is definitely not -rc2 material.\n\nIn general we should be discouraging users from committing conflict \nresolutions themselves as it is a hang-over from the way \"git merge\" \noriginally worked that is error prone and loses the original authorship \nwhen applied to \"git rebase\"\n\n> Instead, use the existence of \"MERGE_MSG\" to figure out whether the user\n> has already resolved and committed the conflict. It feels somewhat fishy\n> to base our decisions on the existence of that particular file, as it\n> really is only a proxy for what we are actually after. \n\nI think that's probably the best we can do. If, after committing a \nconflict resolution from \"git rebase\", the user runs a \nmerge/cherry-pick/revert that has conflicts, then \"MERGE_MSG\" will also \nexist, but we don't want them to amend that case either so it should be \nfine.\n\nThe code changes look good, but I'm not convinced by the test\n\n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> index 8c63682b7f..d55afaa113 100755\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -2486,6 +2486,40 @@ test_expect_success 'non-merge commands reject merge commits' '\n>   \ttest_cmp expect actual\n>   '\n>   \n> +test_expect_success 'can amend after committing a conflict' '\n> +\ttest_when_finished rm -rf repo &&\n> +\tgit init repo &&\n\nThis test file is one of the slowest already, surely we don't need a \nwhole new repository and commit setup - can't we just add\n\n\tgit commit -F .git/MERGE_MSG &&\n\tgit commit --amend -m amended\n\nto the end of 'commit --amend is refused at a rebase conflict stop' \nwhich was added by 6257588252. That would also check that committing a \nconflict resolution works as well.\n\nThanks\n\nPhillip\n> +\t(\n> +\t\tcd repo &&\n> +\n> +\t\ttest_commit original file &&\n> +\t\ttest_commit modified file &&\n> +\t\tcat >todo <<-EOF &&\n> +\t\tbreak\n> +\t\tedit $(git rev-parse HEAD)\n> +\t\tEOF\n> +\t\tset_replace_editor todo &&\n> +\t\tgit rebase -i HEAD~ &&\n> +\n> +\t\t# Modify \"file\" to cause a conflict.\n> +\t\techo conflict >file &&\n> +\t\tgit commit -a --message conflict &&\n> +\t\ttest_must_fail git rebase --continue 2>err &&\n> +\t\ttest_grep \"Resolve all conflicts manually\" err &&\n> +\n> +\t\t# Resolve the conflict.\n> +\t\techo resolved >file &&\n> +\t\tgit add file &&\n> +\t\tgit commit --message resolve &&\n> +\n> +\t\t# And now try to amend to the conflict. This operation should\n> +\t\t# succeed.\n> +\t\techo change >file &&\n> +\t\tgit commit --amend -a --no-edit &&\n> +\t\tgit rebase --continue\n> +\t)\n> +'\n> +\n>   # This must be the last test in this file\n>   test_expect_success '$EDITOR and friends are unchanged' '\n>   \ttest_editor_unchanged\n> \n> ---\n> base-commit: 3bc0341126508f78f5869cbfc0005e987efdf0c7\n> change-id: 20260923-pks-rebase-conflict-bug-176e325ad079\n\n"},{"id":"553062","messageId":"24cc4bcc-1d26-46f5-a502-ba673713f4f0@gmail.com","threadId":"66376","inReplyTo":"c12d2ac3-5263-4301-aa64-a311a343dd40@gmail.com","subject":"Re: [PATCH REGRESSION] builtin/rebase: allow user to amend committed conflicts again","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-09-23T14:22:32Z","receivedAt":"2026-09-23T14:22:36Z","isPatch":true,"body":"On 23/09/2026 15:02, Phillip Wood wrote:\n> On 23/09/2026 14:16, Patrick Steinhardt wrote:\n>> Instead, use the existence of \"MERGE_MSG\" to figure out whether the user\n>> has already resolved and committed the conflict. It feels somewhat fishy\n>> to base our decisions on the existence of that particular file, as it\n>> really is only a proxy for what we are actually after. \n> \n> I think that's probably the best we can do. If, after committing a \n> conflict resolution from \"git rebase\", the user runs a merge/cherry- \n> pick/revert that has conflicts, then \"MERGE_MSG\" will also exist, but we \n> don't want them to amend that case either so it should be fine.\n> \n> The code changes look good,\n\nLet me rephrase that. The code changes look good for \"git rebase\", but \ndo we have a similar problem with \"cherry-pick\", \"merge\" and \"revert\"?\n\nThanks\n\nPhillip\n"},{"id":"553085","messageId":"xmqqik3vc1pc.fsf@gitster.g","threadId":"66376","inReplyTo":"24cc4bcc-1d26-46f5-a502-ba673713f4f0@gmail.com","subject":"Re: [PATCH REGRESSION] builtin/rebase: allow user to amend committed conflicts again","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-23T17:33:19Z","receivedAt":"2026-09-23T17:33:22Z","isPatch":true,"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n> On 23/09/2026 15:02, Phillip Wood wrote:\n>> On 23/09/2026 14:16, Patrick Steinhardt wrote:\n>>> Instead, use the existence of \"MERGE_MSG\" to figure out whether the user\n>>> has already resolved and committed the conflict. It feels somewhat fishy\n>>> to base our decisions on the existence of that particular file, as it\n>>> really is only a proxy for what we are actually after. \n>> \n>> I think that's probably the best we can do. If, after committing a \n>> conflict resolution from \"git rebase\", the user runs a merge/cherry- \n>> pick/revert that has conflicts, then \"MERGE_MSG\" will also exist, but we \n>> don't want them to amend that case either so it should be fine.\n>> \n>> The code changes look good,\n>\n> Let me rephrase that. The code changes look good for \"git rebase\", but \n> do we have a similar problem with \"cherry-pick\", \"merge\" and \"revert\"?\n>\n> Thanks\n\nNow, would it be a -rc2 material to just revert the regressing\nchange out of the release and restart the effort post release?\n"},{"id":"553089","messageId":"CABPp-BFadjqtOB_9cYkrs9UBgTp0hQxu4oiV_yqzYOuiu6g45w@mail.gmail.com","threadId":"66376","inReplyTo":"20260923-pks-rebase-conflict-bug-v1-1-3d3ccf5022bc@pks.im","subject":"Re: [PATCH REGRESSION] builtin/rebase: allow user to amend committed conflicts again","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2026-09-23T17:48:14Z","receivedAt":"2026-09-23T17:48:26Z","isPatch":true,"body":"Hi Patrick,\n\nOn Wed, Sep 23, 2026 at 6:16 AM Patrick Steinhardt <ps@pks.im> wrote:\n>\n> In 6257588252 (commit: refuse to amend during conflict resolution,\n> 2026-09-01), we have introduced logic to git-commit(1) that makes it\n> refuse creating a commit in some cases. This was done to remove a set of\n> common foot guns.\n>\n> One of these foot guns is when the user is performing an interactive\n> rebase that stops at a conflict. Most of the time when we stop at a\n> specific commit we want the user to amend the HEAD commit, so they have\n> been trained to use `git commit --amend`. But when there's a conflict,\n> they are instead supposed to commit it directly without amending the\n> HEAD commit. So to remove that common pit fall, git-commit(1) now\n> refuses amending in that situation.\n\nAre they supposed to commit it directly?  The conflict advice tells\nthem to stage the resolution and run \"git rebase --continue\".  In\nfact, there appear to be a number of problems with using a plain \"git\ncommit\"; more on that below.\n\n> The logic that detects this scenario checks whether the file\n> \"rebase-merge/stopped-sha\" exists, while \"rebase-merge/amend\" doesn't.\n> And this is exactly the case when git-rebase(1) has stopped at such a\n> conflicting commit.\n>\n> But there's one problem here: this state persists even after the user\n> has already committed the resolved conflict, and consequently they still\n\nAfter reading ahead, should this be \"...has already committed the\nresolved conflict via a plain 'git commit'\"?  Resolving it via \"git\nrebase --continue\" doesn't have this problem.\n\n> cannot amend after they have done so. This is overly restrictive though,\n> as it's quite likely that a user may want to change the resolved commit\n> once again.\n\nOof.  Thanks for finding and reporting this.\n\n> Ideally, we'd be able to easily check whether HEAD has already been\n> updated to have the resolved conflict. But it seems like we do not have\n> sufficient information to determine the original state of HEAD when the\n> interactive rebase has stopped, so this is not a workable solution.\n>\n> Instead, use the existence of \"MERGE_MSG\" to figure out whether the user\n> has already resolved and committed the conflict. It feels somewhat fishy\n> to base our decisions on the existence of that particular file, as it\n> really is only a proxy for what we are actually after. But the whole way\n> that we track rebase state is somewhat iffy in the first place.\n>\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n> Hi,\n>\n> this is a regression caused by 6257588252 (commit: refuse to amend\n> during conflict resolution, 2026-09-01). Ideally, we should probably fix\n> it before we release Git 2.56.\n>\n> I'm not particularly happy with the proposed fix -- it feels quite fishy\n> to use the existence of MERGE_MSG as a proxy for whether or not the user\n> has already committed the resolved conflict. I couldn't come up with a\n> better proxy though, so if you have one please let me know.\n\nYeah, I'm also a bit worried about using MERGE_MSG here.  In particular,\n\n    git reset\n\nremoves MERGE_MSG without moving HEAD.  With this patch, a subsequent\n\n    git commit --amend -a\n\nis therefore allowed while the conflict resolution is still\nuncommitted, bringing back the foot-gun that 6257588252 was trying to\nprevent.\n\nFor the short-term 2.56, we could either revert that series (it's a\nlong-standing bug after all) and try again after the release.\nAlternatively, we could record HEAD when the sequencer stops, perhaps\nin rebase-merge/stopped-head, and then reject the amend while HEAD\nstill equals stopped-head and allow it once a plain commit has\nadvanced HEAD.  stopped-sha would remain until rebase --continue,\nsince it is needed for the rewritten-commit mapping and fixup/squash\nbookkeeping.\n\nLonger term, I wonder whether plain \"git commit\" should be rejected\nwhile resolving conflicts for rebase, am, cherry-pick, and revert,\nwith users directed to the corresponding \"--continue\" command.  Plain\ncommit has a surprising collection of behaviors:\n\n  * During am or an apply-backend rebase, it ignores final-commit and\nauthor-script, losing the original message, author, and author date.\nThe corresponding --continue will report \"No changes - did you forget\nto use 'git add'?\" even though the user already added and committed\nthe resolution.  Amid the generic recovery advice, the user must infer\nthat the corresponding \"--skip\" is now needed to bypass the patch that\ntheir manual commit already handled.\n\n  * During a merge-backend rebase, it reads MERGE_MSG, so the message\nsurvives, but the original author and author date do not.\n\n  * It may bypass sequencer options such as explicit signing and\ndate-handling options.\n\n  * --abort behavior then varies by operation: rebase returns to the\noriginal commit (orig-head), `am` leaves you at the manual commit, and\ncherry-pick and revert refuse to rewind because HEAD moved.\n\nHaving the operation own both the commit and its state transition\nseems much easier to reason about.  I would leave \"git merge\" as an\nexception, given the very long-standing \"resolve, add, commit\"\nworkflow, but I think plain \"git commit\" should eventually be\ndisallowed as a way to resolve conflicts for other commands.\n\nThat's post-2.56 work.  For now I think either reverting (and trying\nagain after the release), or recording HEAD in stopped-head seems\npreferable to relying on MERGE_MSG.\n\nThoughts?\n"},{"id":"553090","messageId":"CABPp-BENMwiHh=y_RtfY3Y+uyjRvbpPSTRE9sCGvOtpeeMgapw@mail.gmail.com","threadId":"66376","inReplyTo":"xmqqik3vc1pc.fsf@gitster.g","subject":"Re: [PATCH REGRESSION] builtin/rebase: allow user to amend committed conflicts again","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2026-09-23T17:49:53Z","receivedAt":"2026-09-23T17:50:05Z","isPatch":true,"body":"On Wed, Sep 23, 2026 at 10:33 AM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Phillip Wood <phillip.wood123@gmail.com> writes:\n>\n> > On 23/09/2026 15:02, Phillip Wood wrote:\n> >> On 23/09/2026 14:16, Patrick Steinhardt wrote:\n> >>> Instead, use the existence of \"MERGE_MSG\" to figure out whether the user\n> >>> has already resolved and committed the conflict. It feels somewhat fishy\n> >>> to base our decisions on the existence of that particular file, as it\n> >>> really is only a proxy for what we are actually after.\n> >>\n> >> I think that's probably the best we can do. If, after committing a\n> >> conflict resolution from \"git rebase\", the user runs a merge/cherry-\n> >> pick/revert that has conflicts, then \"MERGE_MSG\" will also exist, but we\n> >> don't want them to amend that case either so it should be fine.\n> >>\n> >> The code changes look good,\n> >\n> > Let me rephrase that. The code changes look good for \"git rebase\", but\n> > do we have a similar problem with \"cherry-pick\", \"merge\" and \"revert\"?\n> >\n> > Thanks\n>\n> Now, would it be a -rc2 material to just revert the regressing\n> change out of the release and restart the effort post release?\n\nYeah, reverting and retrying after the release probably makes sense\ngiven how close we are to 2.56.\n"},{"id":"553091","messageId":"xmqq7bkbc0h5.fsf@gitster.g","threadId":"66376","inReplyTo":"CABPp-BFadjqtOB_9cYkrs9UBgTp0hQxu4oiV_yqzYOuiu6g45w@mail.gmail.com","subject":"Re: [PATCH REGRESSION] builtin/rebase: allow user to amend committed conflicts again","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-23T17:59:50Z","receivedAt":"2026-09-23T17:59:52Z","isPatch":true,"body":"Elijah Newren <newren@gmail.com> writes:\n\n> Longer term, I wonder whether plain \"git commit\" should be rejected\n> while resolving conflicts for rebase, am, cherry-pick, and revert,\n> with users directed to the corresponding \"--continue\" command.  Plain\n> commit has a surprising collection of behaviors:\n> ...\n> workflow, but I think plain \"git commit\" should eventually be\n> disallowed as a way to resolve conflicts for other commands.\n>\n> That's post-2.56 work.\n\nI would say castrating \"git commit\" so that it can only do a plain\nvanilla committing, while it may be a very good move from everything\nyou said above, is post-3.0, not post-2.56, work ;-).\n\n> For now I think either reverting (and trying\n> again after the release), or recording HEAD in stopped-head seems\n> preferable to relying on MERGE_MSG.\n\nBetween the two I'd say giving us a chance for a clean start is far\nmore preferrable than repeating \"Patrick thought of MERGE_MSG and\nafter a few hours Phillip and Elijah thought of a more robust new\nmechanism.  Let's hope there is no more holes found in the newly\nproposed mechanism in another few hours\" after -rc2 got tagged.\n\n"},{"id":"553092","messageId":"arQZDXxf0139omx5@pks.im","threadId":"66376","inReplyTo":"CABPp-BFadjqtOB_9cYkrs9UBgTp0hQxu4oiV_yqzYOuiu6g45w@mail.gmail.com","subject":"Re: [PATCH REGRESSION] builtin/rebase: allow user to amend committed conflicts again","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-23T18:23:09Z","receivedAt":"2026-09-23T18:23:21Z","isPatch":true,"body":"On Wed, Sep 23, 2026 at 10:48:14AM -0700, Elijah Newren wrote:\n> Hi Patrick,\n> \n> On Wed, Sep 23, 2026 at 6:16 AM Patrick Steinhardt <ps@pks.im> wrote:\n> >\n> > In 6257588252 (commit: refuse to amend during conflict resolution,\n> > 2026-09-01), we have introduced logic to git-commit(1) that makes it\n> > refuse creating a commit in some cases. This was done to remove a set of\n> > common foot guns.\n> >\n> > One of these foot guns is when the user is performing an interactive\n> > rebase that stops at a conflict. Most of the time when we stop at a\n> > specific commit we want the user to amend the HEAD commit, so they have\n> > been trained to use `git commit --amend`. But when there's a conflict,\n> > they are instead supposed to commit it directly without amending the\n> > HEAD commit. So to remove that common pit fall, git-commit(1) now\n> > refuses amending in that situation.\n> \n> Are they supposed to commit it directly?  The conflict advice tells\n> them to stage the resolution and run \"git rebase --continue\".  In\n> fact, there appear to be a number of problems with using a plain \"git\n> commit\"; more on that below.\n\nI dunno. All I can say is that I've always been committing directly\nmyself. So it's certainly a workflow that used to work alright. And...\n\n> > The logic that detects this scenario checks whether the file\n> > \"rebase-merge/stopped-sha\" exists, while \"rebase-merge/amend\" doesn't.\n> > And this is exactly the case when git-rebase(1) has stopped at such a\n> > conflicting commit.\n> >\n> > But there's one problem here: this state persists even after the user\n> > has already committed the resolved conflict, and consequently they still\n> \n> After reading ahead, should this be \"...has already committed the\n> resolved conflict via a plain 'git commit'\"?  Resolving it via \"git\n> rebase --continue\" doesn't have this problem.\n\n... honestly I don't think I even had it in my mind that you can just\ncontinue the rebase and that does everything for you. Thing is, I also\nlike to verify the result of the merge, and committing myself allows me\nto do that immediately.\n\n[snip]\n> > I'm not particularly happy with the proposed fix -- it feels quite fishy\n> > to use the existence of MERGE_MSG as a proxy for whether or not the user\n> > has already committed the resolved conflict. I couldn't come up with a\n> > better proxy though, so if you have one please let me know.\n> \n> Yeah, I'm also a bit worried about using MERGE_MSG here.  In particular,\n> \n>     git reset\n> \n> removes MERGE_MSG without moving HEAD.  With this patch, a subsequent\n> \n>     git commit --amend -a\n> \n> is therefore allowed while the conflict resolution is still\n> uncommitted, bringing back the foot-gun that 6257588252 was trying to\n> prevent.\n> \n> For the short-term 2.56, we could either revert that series (it's a\n> long-standing bug after all) and try again after the release.\n> Alternatively, we could record HEAD when the sequencer stops, perhaps\n> in rebase-merge/stopped-head, and then reject the amend while HEAD\n> still equals stopped-head and allow it once a plain commit has\n> advanced HEAD.  stopped-sha would remain until rebase --continue,\n> since it is needed for the rewritten-commit mapping and fixup/squash\n> bookkeeping.\n> \n> Longer term, I wonder whether plain \"git commit\" should be rejected\n> while resolving conflicts for rebase, am, cherry-pick, and revert,\n> with users directed to the corresponding \"--continue\" command.  Plain\n> commit has a surprising collection of behaviors:\n> \n>   * During am or an apply-backend rebase, it ignores final-commit and\n> author-script, losing the original message, author, and author date.\n> The corresponding --continue will report \"No changes - did you forget\n> to use 'git add'?\" even though the user already added and committed\n> the resolution.  Amid the generic recovery advice, the user must infer\n> that the corresponding \"--skip\" is now needed to bypass the patch that\n> their manual commit already handled.\n\nTrue, that's an issue I've been hitting a bunch of times.\n\n>   * During a merge-backend rebase, it reads MERGE_MSG, so the message\n> survives, but the original author and author date do not.\n> \n>   * It may bypass sequencer options such as explicit signing and\n> date-handling options.\n> \n>   * --abort behavior then varies by operation: rebase returns to the\n> original commit (orig-head), `am` leaves you at the manual commit, and\n> cherry-pick and revert refuse to rewind because HEAD moved.\n> \n> Having the operation own both the commit and its state transition\n> seems much easier to reason about.  I would leave \"git merge\" as an\n> exception, given the very long-standing \"resolve, add, commit\"\n> workflow, but I think plain \"git commit\" should eventually be\n> disallowed as a way to resolve conflicts for other commands.\n\nIt certainly is much easier to reason about, true. But it's definitely\na breaking change for something that mostly works alright and that does\nhave some benefits over the \"sanctioned\" way of doing this via\ngit-rebase(1).\n\n> That's post-2.56 work.  For now I think either reverting (and trying\n> again after the release), or recording HEAD in stopped-head seems\n> preferable to relying on MERGE_MSG.\n> \n> Thoughts?\n\nI think reverting is probably the safest change for now, and we can then\ndiscuss how to properly handle this. I'm not a fan myself of refusing\nthe commit outright as that would break my own workflow. And I'd assume\nthat I'm probably not the only person using that workflow, also because\nit does let you inspect the result before you move on.\n\nIt makes me wonder whether we can instead fix git-commit(1) itself to\nmaybe not reset authorship information. But that's probably a much\nharder change to do, and probably it would make the mess that we have\nwith the \".git/rebase-merge\" state directory even bigger.\n\nThanks!\n\nPatrick\n"},{"id":"553093","messageId":"xmqqzex7akcv.fsf@gitster.g","threadId":"66376","inReplyTo":"arQZDXxf0139omx5@pks.im","subject":"Re: [PATCH REGRESSION] builtin/rebase: allow user to amend committed conflicts again","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-23T18:33:20Z","receivedAt":"2026-09-23T18:33:23Z","isPatch":true,"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> I think reverting is probably the safest change for now, and we can then\n> discuss how to properly handle this. I'm not a fan myself of refusing\n> the commit outright as that would break my own workflow. And I'd assume\n> that I'm probably not the only person using that workflow, also because\n> it does let you inspect the result before you move on.\n\nYup, splitting a commit into multiple pieces and other manipulation\nis easier to do if we are allowed to \"git commit\" in the middle of a\n\"rebase -i\" session, and if \"git commit\" is to be allowed, \"git\ncommit --amend\" needs to be allowed immediately following that \"git\ncommit\", if only to reword a misspelt log message.\n\n> It makes me wonder whether we can instead fix git-commit(1) itself to\n> maybe not reset authorship information. But that's probably a much\n> harder change to do, and probably it would make the mess that we have\n> with the \".git/rebase-merge\" state directory even bigger.\n\nI do not think I understand what you mean by \"fix git-commit\".  Make\nit pay attention to some file in .git/ directory and override the\nauthorship information over what it usually uses, and make sure it\nremoves that file after it consumed it, or something like that?\n\n"},{"id":"553096","messageId":"arQcwhuMtcZNoAmI@pks.im","threadId":"66376","inReplyTo":"xmqqzex7akcv.fsf@gitster.g","subject":"Re: [PATCH REGRESSION] builtin/rebase: allow user to amend committed conflicts again","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-23T18:38:58Z","receivedAt":"2026-09-23T18:39:06Z","isPatch":true,"body":"On Wed, Sep 23, 2026 at 11:33:20AM -0700, Junio C Hamano wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> > It makes me wonder whether we can instead fix git-commit(1) itself to\n> > maybe not reset authorship information. But that's probably a much\n> > harder change to do, and probably it would make the mess that we have\n> > with the \".git/rebase-merge\" state directory even bigger.\n> \n> I do not think I understand what you mean by \"fix git-commit\".  Make\n> it pay attention to some file in .git/ directory and override the\n> authorship information over what it usually uses, and make sure it\n> removes that file after it consumed it, or something like that?\n\nYeah, exactly. The fact that it resets authorship information of a\nconflicting commit is probably quite surprising overall as an outcome.\nIt's arguable whether this even qualifies as \"fix\", as in the end\ngit-commit(1) simply does what it always does. But it probably doesn't\nmatch the expected outcome in many cases.\n\nPatrick\n"},{"id":"553100","messageId":"CABPp-BEQSx4m3BcT28CpVGCtsH75+x3gmv4OJz_ecLVLx+kBWg@mail.gmail.com","threadId":"66376","inReplyTo":"xmqqzex7akcv.fsf@gitster.g","subject":"Re: [PATCH REGRESSION] builtin/rebase: allow user to amend committed conflicts again","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2026-09-23T19:11:35Z","receivedAt":"2026-09-23T19:11:48Z","isPatch":true,"body":"On Wed, Sep 23, 2026 at 11:33 AM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Patrick Steinhardt <ps@pks.im> writes:\n>\n> > I think reverting is probably the safest change for now, and we can then\n> > discuss how to properly handle this. I'm not a fan myself of refusing\n> > the commit outright as that would break my own workflow. And I'd assume\n> > that I'm probably not the only person using that workflow, also because\n> > it does let you inspect the result before you move on.\n>\n> Yup, splitting a commit into multiple pieces and other manipulation\n> is easier to do if we are allowed to \"git commit\" in the middle of a\n> \"rebase -i\" session, and if \"git commit\" is to be allowed, \"git\n> commit --amend\" needs to be allowed immediately following that \"git\n> commit\", if only to reword a misspelt log message.\n\nMakes sense.\n\n> > It makes me wonder whether we can instead fix git-commit(1) itself to\n> > maybe not reset authorship information. But that's probably a much\n> > harder change to do, and probably it would make the mess that we have\n> > with the \".git/rebase-merge\" state directory even bigger.\n>\n> I do not think I understand what you mean by \"fix git-commit\".  Make\n> it pay attention to some file in .git/ directory and override the\n> authorship information over what it usually uses, and make sure it\n> removes that file after it consumed it, or something like that?\n\nYes, `git commit` already does something analogous with\nCHERRY_PICK_HEAD: it uses the referenced commit as the source of\nauthor information via read_commit_message(\"CHERRY_PICK_HEAD\"), reads\nthe proposed log message from MERGE_MSG, and consumes CHERRY_PICK_HEAD\nafter a successful commit in sequencer_post_commit_cleanup().\n\nTeaching git commit to consume REBASE_HEAD in the same way seems promising.\n\n\"git am\" is harder; it doesn't have a specific pseudoref so we'd have\nto dig it out of the author-script and final-commit state files.\n"},{"id":"553141","messageId":"6ff9d1ac-ff06-439c-bb0a-ce57742e8ff9@kdbg.org","threadId":"66376","inReplyTo":"CABPp-BENMwiHh=y_RtfY3Y+uyjRvbpPSTRE9sCGvOtpeeMgapw@mail.gmail.com","subject":"Re: [PATCH REGRESSION] builtin/rebase: allow user to amend committed conflicts again","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2026-09-24T06:10:24Z","receivedAt":"2026-09-24T06:10:34Z","isPatch":true,"body":"[Cc: Jiang Xin]\n\nAm 23.09.26 um 19:49 schrieb Elijah Newren:\n> Yeah, reverting and retrying after the release probably makes sense\n> given how close we are to 2.56.\n\nThe reverted series (0f8e75abebff) re-introduced one translatable string\n(\"You are in the middle of a rebase -- cannot amend.\"), which tranlators\nmay have removed from *.po files by now.\n\n-- Hannes\n\n"},{"id":"553370","messageId":"CANYiYbF0yD-6CrhzZ3H83ZkynuMOdi1h5z20uLzwqNtQQG0ydA@mail.gmail.com","threadId":"66376","inReplyTo":"6ff9d1ac-ff06-439c-bb0a-ce57742e8ff9@kdbg.org","subject":"Re: [PATCH REGRESSION] builtin/rebase: allow user to amend committed conflicts again","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2026-09-27T08:04:19Z","receivedAt":"2026-09-27T08:04:31Z","isPatch":true,"body":"On Thu, Sep 24, 2026 at 2:10 PM Johannes Sixt <j6t@kdbg.org> wrote:\n>\n> [Cc: Jiang Xin]\n>\n> Am 23.09.26 um 19:49 schrieb Elijah Newren:\n> > Yeah, reverting and retrying after the release probably makes sense\n> > given how close we are to 2.56.\n>\n> The reverted series (0f8e75abebff) re-introduced one translatable string\n> (\"You are in the middle of a rebase -- cannot amend.\"), which tranlators\n> may have removed from *.po files by now.\n\nThanks for the heads-up. I noticed the revert commit upstream. I will\nrebase all l10n commits onto the new upstream branch to prevent the\nreverted commits from being reintroduced, and will try to restore the\nreverted l10n entries.\n\n>\n> -- Hannes\n>\n"}]}