{"thread":{"id":"48334","subject":"[PATCH 0/3] rebase -i: avoid stale \"# This is a combination of\" in commit messages","startedAt":"2018-04-20T12:17:58Z","lastAt":"2018-04-27T21:36:52Z","messageCount":15,"participants":["Johannes Schindelin","Stefan Beller","Eric Sunshine","Phillip Wood"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"345215","messageId":"cover.1524226637.git.johannes.schindelin@gmx.de","threadId":"48334","inReplyTo":null,"subject":"[PATCH 0/3] rebase -i: avoid stale \"# This is a combination of\" in commit messages","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-04-20T12:17:33Z","receivedAt":"2018-04-20T12:17:58Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Eric Sunshine pointed out that I had such a commit message in\nhttps://public-inbox.org/git/CAPig+cRrS0_nYJJY=O6cboV630sNQHPV5QGrQdD8MW-sYzNFGQ@mail.gmail.com/\nand I went on a hunt to figure out how the heck this happened.\n\nTurns out that if there is a fixup/squash chain where the *last* command\nfails with merge conflicts, and we either --skip ahead or resolve the\nconflict to a clean tree and then --continue, our code does not do a\nfinal cleanup.\n\nContrary to my initial gut feeling, this bug was not introduced by my\nrewrite in C of the core parts of rebase -i, but it looks to me as if\nthat bug was with us for a very long time (at least the --skip part).\n\nThe developer (read: user of rebase -i) in me says that we would want to\nfast-track this, but the author of rebase -i in me says that we should\nbe cautious and cook this in `next` for a while.\n\n\nJohannes Schindelin (3):\n rebase -i: demonstrate bug with fixup!/squash! commit messages\n sequencer: leave a tell-tale when a fixup/squash failed\n rebase --skip: clean up commit message after a failed fixup/squash\n\n sequencer.c | 57 ++++++++++++++++++++++++++++++++------\n t/t3418-rebase-continue.sh | 21 ++++++++++++++\n 2 files changed, 69 insertions(+), 9 deletions(-)\n\n\nbase-commit: fe0a9eaf31dd0c349ae4308498c33a5c3794b293\nPublished-As: https://github.com/dscho/git/releases/tag/clean-msg-after-fixup-continue-v1\nFetch-It-Via: git fetch https://github.com/dscho/git clean-msg-after-fixup-continue-v1\n-- \n2.17.0.windows.1.15.gaa56ade3205\n\n"},{"id":"345217","messageId":"ad2a1b2c60341d692e94b1cb997df05dd7cded88.1524226637.git.johannes.schindelin@gmx.de","threadId":"48334","inReplyTo":"cover.1524226637.git.johannes.schindelin@gmx.de","subject":"[PATCH 1/3] rebase -i: demonstrate bug with fixup!/squash! commit messages","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-04-20T12:17:53Z","receivedAt":"2018-04-20T12:18:16Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"When multiple fixup/squash commands are processed and the last one\ncauses merge conflicts and is skipped, we leave the \"This is a\ncombination of ...\" comments in the commit message.\n\nNoticed by Eric Sunshine.\n\nSigned-off-by: Johannes Schindelin \n---\n t/t3418-rebase-continue.sh | 21 +++++++++++++++++++++\n 1 file changed, 21 insertions(+)\n\ndiff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh\nindex 9214d0bb511..b177baee322 100755\n--- a/t/t3418-rebase-continue.sh\n+++ b/t/t3418-rebase-continue.sh\n@@ -88,6 +88,27 @@ test_expect_success 'rebase passes merge strategy options correctly' '\n \tgit rebase --continue\n '\n \n+test_expect_failure '--continue after failed fixup cleans commit message' '\n+\tgit checkout -b with-conflicting-fixup &&\n+\ttest_commit wants-fixup &&\n+\ttest_commit \"fixup! wants-fixup\" wants-fixup.t 1 wants-fixup-1 &&\n+\ttest_commit \"fixup! wants-fixup\" wants-fixup.t 2 wants-fixup-2 &&\n+\ttest_commit \"fixup! wants-fixup\" wants-fixup.t 3 wants-fixup-3 &&\n+\ttest_must_fail env FAKE_LINES=\"1 fixup 2 fixup 4\" \\\n+\t\tgit rebase -i HEAD~4 &&\n+\n+\t: now there is a conflict, and comments in the commit message &&\n+\tgit show HEAD >out &&\n+\tgrep \"This is a combination of\" out &&\n+\n+\t: skip and continue &&\n+\tgit rebase --skip &&\n+\n+\t: now the comments in the commit message should have been cleaned up &&\n+\tgit show HEAD >out &&\n+\t! grep \"This is a combination of\" out\n+'\n+\n test_expect_success 'setup rerere database' '\n \trm -fr .git/rebase-* &&\n \tgit reset --hard commit-new-file-F3-on-topic-branch &&\n-- \n2.17.0.windows.1.15.gaa56ade3205\n\n\n"},{"id":"345218","messageId":"4f87b8ea2c47b45107e2ba1cdd6d04a66fea0d16.1524226637.git.johannes.schindelin@gmx.de","threadId":"48334","inReplyTo":"cover.1524226637.git.johannes.schindelin@gmx.de","subject":"[PATCH 2/3] sequencer: leave a tell-tale when a fixup/squash failed","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-04-20T12:18:13Z","receivedAt":"2018-04-20T12:18:34Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"In the upcoming patch to clean up fixup/squash commit messages even when\nskipping a final fixup/squash that failed with merge conflicts, we will\nneed to have some indicator what happened.\n\nAs we need to remove the message-fixup and message-squash files upon\nfailure, we cannot use those. So let's just write an explicit amend-type\nfile, containing either `fixup` or `squash`. The absence of that file\nindicates that the we were not in the middle of a fixup or squash when\nmerge conflicts were happening.\n\nSigned-off-by: Johannes Schindelin \n---\n sequencer.c | 21 ++++++++++++++++++++-\n 1 file changed, 20 insertions(+), 1 deletion(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 667f35ebdff..a9c3bc26f84 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -106,6 +106,13 @@ static GIT_PATH_FUNC(rebase_path_author_script, \"rebase-merge/author-script\")\n * command is processed, this file is deleted.\n */\n static GIT_PATH_FUNC(rebase_path_amend, \"rebase-merge/amend\")\n+/*\n+ * If there was a merge conflict in a fixup/squash series, we need to\n+ * record the type so that a `git rebase --skip` can clean up the commit\n+ * message as appropriate. This file will contain that type (`fixup` or\n+ * `squash`), and not exist otherwise.\n+ */\n+static GIT_PATH_FUNC(rebase_path_amend_type, \"rebase-merge/amend-type\")\n /*\n * When we stop at a given patch via the \"edit\" command, this file contains\n * the abbreviated commit name of the corresponding patch.\n@@ -2392,10 +2399,20 @@ static int error_with_patch(struct commit *commit,\n static int error_failed_squash(struct commit *commit,\n \tstruct replay_opts *opts, int subject_len, const char *subject)\n {\n+\tconst char *amend_type = \"squash\";\n+\n+\tif (file_exists(rebase_path_fixup_msg())) {\n+\t\tunlink(rebase_path_fixup_msg());\n+\t\tamend_type = \"fixup\";\n+\t}\n+\tif (write_message(amend_type, strlen(amend_type),\n+\t\t rebase_path_amend_type(), 0))\n+\t\treturn error(_(\"could not write '%s'\"),\n+\t\t\t rebase_path_amend_type());\n+\n \tif (rename(rebase_path_squash_msg(), rebase_path_message()))\n \t\treturn error(_(\"could not rename '%s' to '%s'\"),\n \t\t\trebase_path_squash_msg(), rebase_path_message());\n-\tunlink(rebase_path_fixup_msg());\n \tunlink(git_path_merge_msg());\n \tif (copy_file(git_path_merge_msg(), rebase_path_message(), 0666))\n \t\treturn error(_(\"could not copy '%s' to '%s'\"),\n@@ -2572,6 +2589,7 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)\n \t\t\tunlink(rebase_path_author_script());\n \t\t\tunlink(rebase_path_stopped_sha());\n \t\t\tunlink(rebase_path_amend());\n+\t\t\tunlink(rebase_path_amend_type());\n \t\t\tdelete_ref(NULL, \"REBASE_HEAD\", NULL, REF_NO_DEREF);\n \t\t}\n \t\tif (item->command <= TODO_SQUASH) {\n@@ -2799,6 +2817,7 @@ static int commit_staged_changes(struct replay_opts *opts)\n \tif (run_git_commit(rebase_path_message(), opts, flags))\n \t\treturn error(_(\"could not commit staged changes.\"));\n \tunlink(rebase_path_amend());\n+\tunlink(rebase_path_amend_type());\n \treturn 0;\n }\n \n-- \n2.17.0.windows.1.15.gaa56ade3205\n\n\n"},{"id":"345219","messageId":"6d9f6ba1e73d2297cef3619a89ce69122438368d.1524226637.git.johannes.schindelin@gmx.de","threadId":"48334","inReplyTo":"cover.1524226637.git.johannes.schindelin@gmx.de","subject":"[PATCH 3/3] rebase --skip: clean up commit message after a failed fixup/squash","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-04-20T12:18:32Z","receivedAt":"2018-04-20T12:18:55Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"During a series of fixup/squash commands, the interactive rebase builds\nup a commit message with comments. This will be presented to the user in\nthe editor if at least one of those commands was a `squash`.\n\nHowever, if the last of these fixup/squash commands fails with merge\nconflicts, and if the user then decides to skip it (or resolve it to a\nclean worktree and then continue the rebase), the current code fails to\nclean up the commit message.\n\nThis commit fixes that behavior.\n\nThe diff is best viewed with --color-moved.\n\nSigned-off-by: Johannes Schindelin \n---\n sequencer.c | 36 ++++++++++++++++++++++++++++--------\n t/t3418-rebase-continue.sh | 2 +-\n 2 files changed, 29 insertions(+), 9 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex a9c3bc26f84..f067b7b24c5 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2781,17 +2781,12 @@ static int continue_single_pick(void)\n \n static int commit_staged_changes(struct replay_opts *opts)\n {\n-\tunsigned int flags = ALLOW_EMPTY | EDIT_MSG;\n+\tunsigned int flags = ALLOW_EMPTY | EDIT_MSG, is_fixup = 0, is_clean;\n \n \tif (has_unstaged_changes(1))\n \t\treturn error(_(\"cannot rebase: You have unstaged changes.\"));\n-\tif (!has_uncommitted_changes(0)) {\n-\t\tconst char *cherry_pick_head = git_path_cherry_pick_head();\n \n-\t\tif (file_exists(cherry_pick_head) && unlink(cherry_pick_head))\n-\t\t\treturn error(_(\"could not remove CHERRY_PICK_HEAD\"));\n-\t\treturn 0;\n-\t}\n+\tis_clean = !has_uncommitted_changes(0);\n \n \tif (file_exists(rebase_path_amend())) {\n \t\tstruct strbuf rev = STRBUF_INIT;\n@@ -2804,16 +2799,41 @@ static int commit_staged_changes(struct replay_opts *opts)\n \t\tif (get_oid_hex(rev.buf, &to_amend))\n \t\t\treturn error(_(\"invalid contents: '%s'\"),\n \t\t\t\trebase_path_amend());\n-\t\tif (oidcmp(&head, &to_amend))\n+\t\tif (!is_clean && oidcmp(&head, &to_amend))\n \t\t\treturn error(_(\"\\nYou have uncommitted changes in your \"\n \t\t\t\t \"working tree. Please, commit them\\n\"\n \t\t\t\t \"first and then run 'git rebase \"\n \t\t\t\t \"--continue' again.\"));\n+\t\tif (is_clean && !oidcmp(&head, &to_amend)) {\n+\t\t\tstrbuf_reset(&rev);\n+\t\t\t/*\n+\t\t\t * Clean tree, but we may need to finalize a\n+\t\t\t * fixup/squash chain. A failed fixup/squash leaves the\n+\t\t\t * file amend-type in rebase-merge/; It is okay if that\n+\t\t\t * file is missing, in which case there is no such\n+\t\t\t * chain to finalize.\n+\t\t\t */\n+\t\t\tread_oneliner(&rev, rebase_path_amend_type(), 0);\n+\t\t\tif (!strcmp(\"squash\", rev.buf))\n+\t\t\t\tis_fixup = TODO_SQUASH;\n+\t\t\telse if (!strcmp(\"fixup\", rev.buf)) {\n+\t\t\t\tis_fixup = TODO_FIXUP;\n+\t\t\t\tflags = (flags & ~EDIT_MSG) | CLEANUP_MSG;\n+\t\t\t}\n+\t\t}\n \n \t\tstrbuf_release(&rev);\n \t\tflags |= AMEND_MSG;\n \t}\n \n+\tif (is_clean && !is_fixup) {\n+\t\tconst char *cherry_pick_head = git_path_cherry_pick_head();\n+\n+\t\tif (file_exists(cherry_pick_head) && unlink(cherry_pick_head))\n+\t\t\treturn error(_(\"could not remove CHERRY_PICK_HEAD\"));\n+\t\treturn 0;\n+\t}\n+\n \tif (run_git_commit(rebase_path_message(), opts, flags))\n \t\treturn error(_(\"could not commit staged changes.\"));\n \tunlink(rebase_path_amend());\ndiff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh\nindex b177baee322..4880bff82ff 100755\n--- a/t/t3418-rebase-continue.sh\n+++ b/t/t3418-rebase-continue.sh\n@@ -88,7 +88,7 @@ test_expect_success 'rebase passes merge strategy options correctly' '\n \tgit rebase --continue\n '\n \n-test_expect_failure '--continue after failed fixup cleans commit message' '\n+test_expect_success '--continue after failed fixup cleans commit message' '\n \tgit checkout -b with-conflicting-fixup &&\n \ttest_commit wants-fixup &&\n \ttest_commit \"fixup! wants-fixup\" wants-fixup.t 1 wants-fixup-1 &&\n-- \n2.17.0.windows.1.15.gaa56ade3205\n"},{"id":"345232","messageId":"CAGZ79kZONQ_xuDYJ5Z8cUnF0odwz3+Sh+ST6g+5Tefav-QXEBw@mail.gmail.com","threadId":"48334","inReplyTo":"ad2a1b2c60341d692e94b1cb997df05dd7cded88.1524226637.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH 1/3] rebase -i: demonstrate bug with fixup!/squash! commit messages","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-04-20T16:40:46Z","receivedAt":"2018-04-20T16:40:51Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Apr 20, 2018 at 5:17 AM, Johannes Schindelin\n wrote:\n> When multiple fixup/squash commands are processed and the last one\n> causes merge conflicts and is skipped, we leave the \"This is a\n> combination of ...\" comments in the commit message.\n>\n> Noticed by Eric Sunshine.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> t/t3418-rebase-continue.sh | 21 +++++++++++++++++++++\n> 1 file changed, 21 insertions(+)\n>\n> diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh\n> index 9214d0bb511..b177baee322 100755\n> --- a/t/t3418-rebase-continue.sh\n> +++ b/t/t3418-rebase-continue.sh\n> @@ -88,6 +88,27 @@ test_expect_success 'rebase passes merge strategy options correctly' '\n> git rebase --continue\n> '\n>\n> +test_expect_failure '--continue after failed fixup cleans commit message' '\n> + git checkout -b with-conflicting-fixup &&\n> + test_commit wants-fixup &&\n> + test_commit \"fixup! wants-fixup\" wants-fixup.t 1 wants-fixup-1 &&\n> + test_commit \"fixup! wants-fixup\" wants-fixup.t 2 wants-fixup-2 &&\n> + test_commit \"fixup! wants-fixup\" wants-fixup.t 3 wants-fixup-3 &&\n> + test_must_fail env FAKE_LINES=\"1 fixup 2 fixup 4\" \\\n> + git rebase -i HEAD~4 &&\n> +\n> + : now there is a conflict, and comments in the commit message &&\n> + git show HEAD >out &&\n> + grep \"This is a combination of\" out &&\n\ntest_i18n_grep ?\n\n> +\n> + : skip and continue &&\n> + git rebase --skip &&\n> +\n> + : now the comments in the commit message should have been cleaned up &&\n> + git show HEAD >out &&\n> + ! grep \"This is a combination of\" out\n\nsame\n\nThanks,\nStefan\n"},{"id":"345253","messageId":"CAPig+cSb33EG2nATjCem4-2dp87SKOXm_SCWU=fCXJCBRZzgeg@mail.gmail.com","threadId":"48334","inReplyTo":"ad2a1b2c60341d692e94b1cb997df05dd7cded88.1524226637.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH 1/3] rebase -i: demonstrate bug with fixup!/squash! commit messages","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-04-20T19:29:34Z","receivedAt":"2018-04-20T19:29:39Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Apr 20, 2018 at 8:17 AM, Johannes Schindelin\n wrote:\n> When multiple fixup/squash commands are processed and the last one\n> causes merge conflicts and is skipped, we leave the \"This is a\n> combination of ...\" comments in the commit message.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh\n> @@ -88,6 +88,27 @@ test_expect_success 'rebase passes merge strategy options correctly' '\n> +test_expect_failure '--continue after failed fixup cleans commit message' '\n> + git checkout -b with-conflicting-fixup &&\n> + test_commit wants-fixup &&\n> + test_commit \"fixup! wants-fixup\" wants-fixup.t 1 wants-fixup-1 &&\n> + test_commit \"fixup! wants-fixup\" wants-fixup.t 2 wants-fixup-2 &&\n> + test_commit \"fixup! wants-fixup\" wants-fixup.t 3 wants-fixup-3 &&\n> + test_must_fail env FAKE_LINES=\"1 fixup 2 fixup 4\" \\\n> + git rebase -i HEAD~4 &&\n> +\n> + : now there is a conflict, and comments in the commit message &&\n> + git show HEAD >out &&\n> + grep \"This is a combination of\" out &&\n> +\n> + : skip and continue &&\n> + git rebase --skip &&\n\nI see that this test script doesn't utilize it, but do you want a\n\n test_when_finished \"reset_rebase\" &&\n\nbefore starting the rebase to clean up in case something before \"git\nrebase --skip\" fails?\n\n> + : now the comments in the commit message should have been cleaned up &&\n> + git show HEAD >out &&\n> + ! grep \"This is a combination of\" out\n> +'\n"},{"id":"345254","messageId":"CAPig+cRO2+NJ0-DiuHXA=mWjPnivqH3PiwM9R6OGj2Rr3eLauA@mail.gmail.com","threadId":"48334","inReplyTo":"CAPig+cSb33EG2nATjCem4-2dp87SKOXm_SCWU=fCXJCBRZzgeg@mail.gmail.com","subject":"Re: [PATCH 1/3] rebase -i: demonstrate bug with fixup!/squash! commit messages","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-04-20T19:31:11Z","receivedAt":"2018-04-20T19:31:16Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Apr 20, 2018 at 3:29 PM, Eric Sunshine wrote:\n> On Fri, Apr 20, 2018 at 8:17 AM, Johannes Schindelin\n> wrote:\n>> + test_must_fail env FAKE_LINES=\"1 fixup 2 fixup 4\" \\\n>> + git rebase -i HEAD~4 &&\n>> +\n>> + : now there is a conflict, and comments in the commit message &&\n>> + git show HEAD >out &&\n>> + grep \"This is a combination of\" out &&\n>> +\n>> + : skip and continue &&\n>> + git rebase --skip &&\n>\n> I see that this test script doesn't utilize it, but do you want a\n>\n> test_when_finished \"reset_rebase\" &&\n>\n> before starting the rebase to clean up in case something before \"git\n> rebase --skip\" fails?\n\nStated less ambiguously:\n\n ... in case something fails between \"git rebase -i ...\"\n and \"git rebase --skip\"?\n"},{"id":"345256","messageId":"CAPig+cQSbNVJgxa1VPnzOjQ2DkMAG36VVqnjg53CmC4wU_Q8AA@mail.gmail.com","threadId":"48334","inReplyTo":"4f87b8ea2c47b45107e2ba1cdd6d04a66fea0d16.1524226637.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH 2/3] sequencer: leave a tell-tale when a fixup/squash failed","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-04-20T19:34:16Z","receivedAt":"2018-04-20T19:34:20Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Apr 20, 2018 at 8:18 AM, Johannes Schindelin\n wrote:\n> In the upcoming patch to clean up fixup/squash commit messages even when\n> skipping a final fixup/squash that failed with merge conflicts, we will\n> need to have some indicator what happened.\n>\n> As we need to remove the message-fixup and message-squash files upon\n> failure, we cannot use those. So let's just write an explicit amend-type\n> file, containing either `fixup` or `squash`. The absence of that file\n> indicates that the we were not in the middle of a fixup or squash when\n\ns/the we/we/\n\n> merge conflicts were happening.\n>\n> Signed-off-by: Johannes Schindelin \n"},{"id":"345259","messageId":"nycvar.QRO.7.76.6.1804202149260.4241@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","threadId":"48334","inReplyTo":"CAGZ79kZONQ_xuDYJ5Z8cUnF0odwz3+Sh+ST6g+5Tefav-QXEBw@mail.gmail.com","subject":"Re: [PATCH 1/3] rebase -i: demonstrate bug with fixup!/squash! commit messages","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-04-20T20:04:35Z","receivedAt":"2018-04-20T20:05:10Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stefan,\n\nOn Fri, 20 Apr 2018, Stefan Beller wrote:\n\n> On Fri, Apr 20, 2018 at 5:17 AM, Johannes Schindelin\n> wrote:\n> > When multiple fixup/squash commands are processed and the last one\n> > causes merge conflicts and is skipped, we leave the \"This is a\n> > combination of ...\" comments in the commit message.\n> >\n> > Noticed by Eric Sunshine.\n> >\n> > Signed-off-by: Johannes Schindelin \n> > ---\n> > t/t3418-rebase-continue.sh | 21 +++++++++++++++++++++\n> > 1 file changed, 21 insertions(+)\n> >\n> > diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh\n> > index 9214d0bb511..b177baee322 100755\n> > --- a/t/t3418-rebase-continue.sh\n> > +++ b/t/t3418-rebase-continue.sh\n> > @@ -88,6 +88,27 @@ test_expect_success 'rebase passes merge strategy options correctly' '\n> > git rebase --continue\n> > '\n> >\n> > +test_expect_failure '--continue after failed fixup cleans commit message' '\n> > + git checkout -b with-conflicting-fixup &&\n> > + test_commit wants-fixup &&\n> > + test_commit \"fixup! wants-fixup\" wants-fixup.t 1 wants-fixup-1 &&\n> > + test_commit \"fixup! wants-fixup\" wants-fixup.t 2 wants-fixup-2 &&\n> > + test_commit \"fixup! wants-fixup\" wants-fixup.t 3 wants-fixup-3 &&\n> > + test_must_fail env FAKE_LINES=\"1 fixup 2 fixup 4\" \\\n> > + git rebase -i HEAD~4 &&\n> > +\n> > + : now there is a conflict, and comments in the commit message &&\n> > + git show HEAD >out &&\n> > + grep \"This is a combination of\" out &&\n> \n> test_i18n_grep ?\n\nFunny thing is: I tested this with GETTEXT_POISON=1, and it succeeded. So\nI dug deeper why: I never understood that this is a *build* option. I\nalways thought that would be a test-time option... Oh well ;-)\n\n> > +\n> > + : skip and continue &&\n> > + git rebase --skip &&\n> > +\n> > + : now the comments in the commit message should have been cleaned up &&\n> > + git show HEAD >out &&\n> > + ! grep \"This is a combination of\" out\n> \n> same\n\nWill work on a fix. A brief test shows, however, that it is not quite as\neasy as s/grep/test_i18ngrep/, something more seems to be broken.\n\nWill keep you posted,\nDscho\n"},{"id":"345260","messageId":"nycvar.QRO.7.76.6.1804202232360.4241@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","threadId":"48334","inReplyTo":"nycvar.QRO.7.76.6.1804202149260.4241@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","subject":"Re: [PATCH 1/3] rebase -i: demonstrate bug with fixup!/squash! commit messages","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-04-20T20:38:05Z","receivedAt":"2018-04-20T20:38:32Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stefan,\n\nOn Fri, 20 Apr 2018, Johannes Schindelin wrote:\n\n> A brief test shows, however, that it is not quite as easy as\n> s/grep/test_i18ngrep/, something more seems to be broken.\n\nIt seems that this week is my Rabbit Hole Week.\n\nTurns out that we have a really, really long-standing bug in our rebase -i\nwhere we construct the commit messages for fixup/squash chains.\n\nBackground: when having multiple fixup!/squash! commits for the same\noriginal commit, the intermediate commits have messages starting with the\nmessage\n\n\t# This is a combination of commits.\n\nand then every fixup/squash command increments that and adds a header\n\n\t# This is the commit message #:\n\nbefore writing the respective commit message.\n\nThe problem arises from the fact that we deduce from parsing the first\nnumber in ASCII encoding on the first line.\n\nThat breaks e.g. when compiling with GETTEXT_POISON, and it is probably\nnot true in general, either.\n\nSo I introduced a patch that handles the absence of an ASCII-encoded\nnumber gracefully, and now the test passes with and without\nGETTEXT_POISON.\n\nThanks for the review that let me find and fix this bug!\nDscho\n"},{"id":"345262","messageId":"CAGZ79kbQCQPx01uFacxMWLdfJurn4jVcc+nvdnWHNVJNE3resw@mail.gmail.com","threadId":"48334","inReplyTo":"nycvar.QRO.7.76.6.1804202149260.4241@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","subject":"Re: [PATCH 1/3] rebase -i: demonstrate bug with fixup!/squash! commit messages","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-04-20T20:44:38Z","receivedAt":"2018-04-20T20:44:42Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Hi Johannes,\n\n> Funny thing is: I tested this with GETTEXT_POISON=1, and it succeeded.\n\nI actually wanted to review the code leading to this commit, and to find\nwhere to start reviewing I had 'git grep \"This is a combination of\"' which\nlead me to the translation files.\n\ns/grep/test_i18ngrep/ doesn't work as the syntax is slightly off,\ns/ ! grep/ test_i18n_grep !/ would work, just pointing out the obvious.\n"},{"id":"345263","messageId":"nycvar.QRO.7.76.6.1804202250070.4241@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","threadId":"48334","inReplyTo":"CAPig+cRO2+NJ0-DiuHXA=mWjPnivqH3PiwM9R6OGj2Rr3eLauA@mail.gmail.com","subject":"Re: [PATCH 1/3] rebase -i: demonstrate bug with fixup!/squash! commit messages","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-04-20T20:52:25Z","receivedAt":"2018-04-20T20:52:50Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Eric,\n\nOn Fri, 20 Apr 2018, Eric Sunshine wrote:\n\n> On Fri, Apr 20, 2018 at 3:29 PM, Eric Sunshine wrote:\n> > On Fri, Apr 20, 2018 at 8:17 AM, Johannes Schindelin\n> > wrote:\n> >> + test_must_fail env FAKE_LINES=\"1 fixup 2 fixup 4\" \\\n> >> + git rebase -i HEAD~4 &&\n> >> +\n> >> + : now there is a conflict, and comments in the commit message &&\n> >> + git show HEAD >out &&\n> >> + grep \"This is a combination of\" out &&\n> >> +\n> >> + : skip and continue &&\n> >> + git rebase --skip &&\n> >\n> > I see that this test script doesn't utilize it, but do you want a\n> >\n> > test_when_finished \"reset_rebase\" &&\n> >\n> > before starting the rebase to clean up in case something before \"git\n> > rebase --skip\" fails?\n\nNo, because then one of the next test cases fails:\n\n\tnot ok 15 - rebase -i --continue remembers --rerere-autoupdate\n\n;-)\n\nI'll use\n\n\ttest_when_finished \"test_might_fail git rebase --abort\"\n\ninstead, okay? ;-)\n\nCiao,\nDscho\n"},{"id":"345265","messageId":"nycvar.QRO.7.76.6.1804202304210.4241@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","threadId":"48334","inReplyTo":"CAGZ79kbQCQPx01uFacxMWLdfJurn4jVcc+nvdnWHNVJNE3resw@mail.gmail.com","subject":"Re: [PATCH 1/3] rebase -i: demonstrate bug with fixup!/squash! commit messages","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-04-20T21:05:42Z","receivedAt":"2018-04-20T21:06:07Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stefan,\n\nOn Fri, 20 Apr 2018, Stefan Beller wrote:\n\n> > Funny thing is: I tested this with GETTEXT_POISON=1, and it succeeded.\n> \n> I actually wanted to review the code leading to this commit, and to find\n> where to start reviewing I had 'git grep \"This is a combination of\"' which\n> lead me to the translation files.\n\nHeh...\n\n> s/grep/test_i18ngrep/ doesn't work as the syntax is slightly off,\n> s/ ! grep/ test_i18n_grep !/ would work, just pointing out the obvious.\n\nYes, I actually knew that, but my usage of a s/// as a shorthand for the\nidea to replace `grep` with `test_i18ngrep` was indeed misleading. Sorry\nabout that, and thank you for helping me getting this right.\n\nCiao,\nDscho\n"},{"id":"345367","messageId":"b6512eae-e214-9699-4d69-77117a0daec3@talktalk.net","threadId":"48334","inReplyTo":"6d9f6ba1e73d2297cef3619a89ce69122438368d.1524226637.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH 3/3] rebase --skip: clean up commit message after a failedfixup/squash","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-04-21T15:42:12Z","receivedAt":"2018-04-21T15:42:19Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 20/04/18 13:18, Johannes Schindelin wrote:\n> \n> During a series of fixup/squash commands, the interactive rebase builds\n> up a commit message with comments. This will be presented to the user in\n> the editor if at least one of those commands was a `squash`.\n> \n> However, if the last of these fixup/squash commands fails with merge\n> conflicts, and if the user then decides to skip it (or resolve it to a\n> clean worktree and then continue the rebase), the current code fails to\n> clean up the commit message.\n\nThanks for taking the time to track this down and fix it.\n\n> \n> This commit fixes that behavior.\n> \n> The diff is best viewed with --color-moved.\n> \n> Signed-off-by: Johannes Schindelin \n> ---\n> sequencer.c | 36 ++++++++++++++++++++++++++++--------\n> t/t3418-rebase-continue.sh | 2 +-\n> 2 files changed, 29 insertions(+), 9 deletions(-)\n> \n> diff --git a/sequencer.c b/sequencer.c\n> index a9c3bc26f84..f067b7b24c5 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -2781,17 +2781,12 @@ static int continue_single_pick(void)\n> \n> static int commit_staged_changes(struct replay_opts *opts)\n> {\n> -\tunsigned int flags = ALLOW_EMPTY | EDIT_MSG;\n> +\tunsigned int flags = ALLOW_EMPTY | EDIT_MSG, is_fixup = 0, is_clean;\n> \n> \tif (has_unstaged_changes(1))\n> \t\treturn error(_(\"cannot rebase: You have unstaged changes.\"));\n> -\tif (!has_uncommitted_changes(0)) {\n> -\t\tconst char *cherry_pick_head = git_path_cherry_pick_head();\n> \n> -\t\tif (file_exists(cherry_pick_head) && unlink(cherry_pick_head))\n> -\t\t\treturn error(_(\"could not remove CHERRY_PICK_HEAD\"));\n> -\t\treturn 0;\n> -\t}\n> +\tis_clean = !has_uncommitted_changes(0);\n> \n> \tif (file_exists(rebase_path_amend())) {\n> \t\tstruct strbuf rev = STRBUF_INIT;\n> @@ -2804,16 +2799,41 @@ static int commit_staged_changes(struct replay_opts *opts)\n> \t\tif (get_oid_hex(rev.buf, &to_amend))\n> \t\t\treturn error(_(\"invalid contents: '%s'\"),\n> \t\t\t\trebase_path_amend());\n> -\t\tif (oidcmp(&head, &to_amend))\n> +\t\tif (!is_clean && oidcmp(&head, &to_amend))\n> \t\t\treturn error(_(\"\\nYou have uncommitted changes in your \"\n> \t\t\t\t \"working tree. Please, commit them\\n\"\n> \t\t\t\t \"first and then run 'git rebase \"\n> \t\t\t\t \"--continue' again.\"));\n> +\t\tif (is_clean && !oidcmp(&head, &to_amend)) {\n\nLooking at pick_commits() it only writes to rebase_path_amend() if there \nare conflicts, not if the command has been rescheduled so this is safe.\n\n> +\t\t\tstrbuf_reset(&rev);\n> +\t\t\t/*\n> +\t\t\t * Clean tree, but we may need to finalize a\n> +\t\t\t * fixup/squash chain. A failed fixup/squash leaves the\n> +\t\t\t * file amend-type in rebase-merge/; It is okay if that\n> +\t\t\t * file is missing, in which case there is no such\n> +\t\t\t * chain to finalize.\n> +\t\t\t */\n> +\t\t\tread_oneliner(&rev, rebase_path_amend_type(), 0);\n> +\t\t\tif (!strcmp(\"squash\", rev.buf))\n> +\t\t\t\tis_fixup = TODO_SQUASH;\n> +\t\t\telse if (!strcmp(\"fixup\", rev.buf)) {\n> +\t\t\t\tis_fixup = TODO_FIXUP;\n> +\t\t\t\tflags = (flags & ~EDIT_MSG) | CLEANUP_MSG;\n\nI was going to say this should probably be (flags & ~(EDIT_MSG | \nVERIFY_MSG)) but for some reason VERIFY_MSG isn't set here - I wonder if \nit should be as I think it's set elsewhere when we edit the message.\n\n> +\t\t\t}\n> +\t\t}\n> \n> \t\tstrbuf_release(&rev);\n> \t\tflags |= AMEND_MSG;\n> \t}\n> \n> +\tif (is_clean && !is_fixup) {\n> +\t\tconst char *cherry_pick_head = git_path_cherry_pick_head();\n> +\n> +\t\tif (file_exists(cherry_pick_head) && unlink(cherry_pick_head))\n> +\t\t\treturn error(_(\"could not remove CHERRY_PICK_HEAD\"));\n> +\t\treturn 0;\n> +\t}\n> +\n> \tif (run_git_commit(rebase_path_message(), opts, flags))\n\nIf a squash command has been skipped, then rebase_path_message() still \ncontains the message of the skipped commit. If it passed NULL instead \nthen the user would get to edit the previous version of the squash \nmessage without the skipped commit message in it.\n\nAlso I think we only want to re-commit if the skipped squash/fixup was \npreceded by another squash/fixup. If the user skips the first \nsquash/fixup in a chain then HEAD has the commit message from the \noriginal pick so does not need amending. The first patch could perhaps \navoid writing rebase_path_amend_type() in that case by reading the \nsquash message and checking the message count is greater than two.\n\nBest Wishes\n\nPhillip\n\n> \t\treturn error(_(\"could not commit staged changes.\"));\n> \tunlink(rebase_path_amend());\n> diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh\n> index b177baee322..4880bff82ff 100755\n> --- a/t/t3418-rebase-continue.sh\n> +++ b/t/t3418-rebase-continue.sh\n> @@ -88,7 +88,7 @@ test_expect_success 'rebase passes merge strategy options correctly' '\n> \tgit rebase --continue\n> '\n> \n> -test_expect_failure '--continue after failed fixup cleans commit message' '\n> +test_expect_success '--continue after failed fixup cleans commit message' '\n> \tgit checkout -b with-conflicting-fixup &&\n> \ttest_commit wants-fixup &&\n> \ttest_commit \"fixup! wants-fixup\" wants-fixup.t 1 wants-fixup-1 &&\n> \n\n"},{"id":"345996","messageId":"nycvar.QRO.7.76.6.1804272304100.72@tvgsbejvaqbjf.bet","threadId":"48334","inReplyTo":"b6512eae-e214-9699-4d69-77117a0daec3@talktalk.net","subject":"Re: [PATCH 3/3] rebase --skip: clean up commit message after a failedfixup/squash","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-04-27T21:36:24Z","receivedAt":"2018-04-27T21:36:52Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Phillip,\n\nOn Sat, 21 Apr 2018, Phillip Wood wrote:\n\n> On 20/04/18 13:18, Johannes Schindelin wrote:\n> > \n> > During a series of fixup/squash commands, the interactive rebase builds\n> > up a commit message with comments. This will be presented to the user in\n> > the editor if at least one of those commands was a `squash`.\n> > \n> > However, if the last of these fixup/squash commands fails with merge\n> > conflicts, and if the user then decides to skip it (or resolve it to a\n> > clean worktree and then continue the rebase), the current code fails to\n> > clean up the commit message.\n> \n> Thanks for taking the time to track this down and fix it.\n\nIt was on my mind, but since I got caught twice by this bug within a week,\nI figured it was about time.\n\n> > diff --git a/sequencer.c b/sequencer.c\n> > index a9c3bc26f84..f067b7b24c5 100644\n> > --- a/sequencer.c\n> > +++ b/sequencer.c\n> > @@ -2781,17 +2781,12 @@ static int continue_single_pick(void)\n> > \n> > static int commit_staged_changes(struct replay_opts *opts)\n> > {\n> > -\tunsigned int flags = ALLOW_EMPTY | EDIT_MSG;\n> > +\tunsigned int flags = ALLOW_EMPTY | EDIT_MSG, is_fixup = 0, is_clean;\n> > \n> > if (has_unstaged_changes(1))\n> > \t\treturn error(_(\"cannot rebase: You have unstaged changes.\"));\n> > -\tif (!has_uncommitted_changes(0)) {\n> > -\t\tconst char *cherry_pick_head = git_path_cherry_pick_head();\n> > -\t\tif (file_exists(cherry_pick_head) && unlink(cherry_pick_head))\n> > -\t\t\treturn error(_(\"could not remove CHERRY_PICK_HEAD\"));\n> > -\t\treturn 0;\n> > -\t}\n> > +\tis_clean = !has_uncommitted_changes(0);\n> > \n> > if (file_exists(rebase_path_amend())) {\n> > \t\tstruct strbuf rev = STRBUF_INIT;\n> > @@ -2804,16 +2799,41 @@ static int commit_staged_changes(struct replay_opts\n> > *opts)\n> > if (get_oid_hex(rev.buf, &to_amend))\n> > return error(_(\"invalid contents: '%s'\"),\n> > \t\t\t\trebase_path_amend());\n> > -\t\tif (oidcmp(&head, &to_amend))\n> > +\t\tif (!is_clean && oidcmp(&head, &to_amend))\n> > return error(_(\"\\nYou have uncommitted changes in your \"\n> > \"working tree. Please, commit them\\n\"\n> > \"first and then run 'git rebase \"\n> > \"--continue' again.\"));\n> > +\t\tif (is_clean && !oidcmp(&head, &to_amend)) {\n> \n> Looking at pick_commits() it only writes to rebase_path_amend() if there are\n> conflicts, not if the command has been rescheduled so this is safe.\n\nThis is indeed the intent of that file.\n\n> > +\t\t\tstrbuf_reset(&rev);\n> > +\t\t\t/*\n> > +\t\t\t * Clean tree, but we may need to finalize a\n> > +\t\t\t * fixup/squash chain. A failed fixup/squash leaves\n> > the\n> > +\t\t\t * file amend-type in rebase-merge/; It is okay if\n> > that\n> > +\t\t\t * file is missing, in which case there is no such\n> > +\t\t\t * chain to finalize.\n> > +\t\t\t */\n> > +\t\t\tread_oneliner(&rev, rebase_path_amend_type(), 0);\n> > +\t\t\tif (!strcmp(\"squash\", rev.buf))\n> > +\t\t\t\tis_fixup = TODO_SQUASH;\n> > +\t\t\telse if (!strcmp(\"fixup\", rev.buf)) {\n> > +\t\t\t\tis_fixup = TODO_FIXUP;\n> > +\t\t\t\tflags = (flags & ~EDIT_MSG) | CLEANUP_MSG;\n> \n> I was going to say this should probably be (flags & ~(EDIT_MSG | VERIFY_MSG))\n> but for some reason VERIFY_MSG isn't set here - I wonder if it should be as I\n> think it's set elsewhere when we edit the message.\n\nAs this patch series is purely about the bug fix where interrupted\nfixup/squash series can lead to incorrect commit messages, I would say\nthat if this is a bug, it should be fixed in a separate patch series.\n\nThe name of that option is actually a little bit unfortunate: it bypasses\nthe pre-commit/commit-msg hooks. I am not sure why they are bypassed in\nthe commit_staged_changes() function.\n\n*clickety-click*\n\nIt would appear that I simply copied this from\n\nhttps://github.com/git/git/blob/v2.17.0/git-rebase--interactive.sh#L794-L808\n\nSo where does that come from? Let's use `git log -L\n794,808:git-rebase--interactive.sh v2.17.0` to find out.\n\n*clickety-click*\n\nFrom that log, it looks as if this was added in 2147f844ed1 (rebase -i: handle\nfixup of root commit correctly, 2012-07-24). But that is incorrect: the\n--no-verify invocation was only split into two by said commit, and moved\ninto the conditional. So we need to look a little further, with a larger\nline range (I extended it to 810 for the purpose of this analysis).\n\n*clickety-click*\n\nSo it goes all the way back to c5b09feb786 (Avoid update hook during\ngit-rebase --interactive, 2007-12-19). From that commit message, you can\nsee that the rationale for the --no-verify flag was as following: the `git\ncommit` might fail when continuing with staged changes, due to a check in\na hook, and since there is inadequate error checking in the\ngit-rebase--interactive.sh script, it would continue and squash the\nchanges into the next commit.\n\nFrom that description, it would appear that the proper fix would have been\nto 1) introduce proper error checking, and 2) offer a mode to bypass the\nhooks for *all* of the interactive rebase.\n\nIt is funny that 1) was addressed apparently 4 minutes later, in\ndbedf9729bd (Catch and handle git-commit failures in git-rebase\n--interactive, 2007-12-19).\n\nAs to 2): it seems to have been implemented in c44276563f9 (rebase\n--no-verify, 2008-10-06), but by that time it was probably safely\nforgotten that the --no-verify option was only introduced as a work-around\nfor the absence of the `git rebase -i --no-verify` mode.\n\nSo I guess it would be time to undo c5b09feb786...\n\nBut as I said, I'd rather really not taint this here patch series by an\nunrelated bug fix. It has gone to the length of four iterations already\neven without such distractions.\n\n> > +\t\t\t}\n> > +\t\t}\n> > \n> > strbuf_release(&rev);\n> > flags |= AMEND_MSG;\n> > }\n> > +\tif (is_clean && !is_fixup) {\n> > +\t\tconst char *cherry_pick_head = git_path_cherry_pick_head();\n> > +\n> > +\t\tif (file_exists(cherry_pick_head) && unlink(cherry_pick_head))\n> > +\t\t\treturn error(_(\"could not remove CHERRY_PICK_HEAD\"));\n> > +\t\treturn 0;\n> > +\t}\n> > +\n> > if (run_git_commit(rebase_path_message(), opts, flags))\n> \n> If a squash command has been skipped, then rebase_path_message() still\n> contains the message of the skipped commit. If it passed NULL instead\n> then the user would get to edit the previous version of the squash\n> message without the skipped commit message in it.\n\nTrue. One problem that I only caught late in my work on v4 is that\nrun_git_commit() did not handle the absence of defmsg *and* EDIT_MSG well:\nin that case, it always opened the editor. I addressed that in the new\n3/4. (The old 3/4 is no longer needed: it wrote the amend-type file whose\npurpose is now folded into current-fixups, introduced in the new 2/4.)\n\n> Also I think we only want to re-commit if the skipped squash/fixup was\n> preceded by another squash/fixup.\n\nI thought so, too, at first. But I think that is still not quite the\ncorrect thing: we really only want to re-commit if we are in the middle of\nthe final fixup/squash of the fixup/squash chain. And only if that final\nfixup/squash was not the only one in that chain.\n\n> If the user skips the first squash/fixup in a chain then HEAD has the\n> commit message from the original pick so does not need amending. The\n> first patch could perhaps avoid writing rebase_path_amend_type() in that\n> case by reading the squash message and checking the message count is\n> greater than two.\n\nAs I mentioned in another reply to you, I redid the whole shebang so that\nwe always write a `current-fixups` file as soon as we encounter a fixup or\nsquash. This file will build up the fixup/squash chain.\n\nOne thing that I had missed when I wrote that reply is that we sometimes\ndo *not* skip a failed fixup/squash, but instead resolve the merge\nconflicts, stage the changes and call `git rebase --continue`. In this\ncase, `git rebase` will commit the changes, *opening the commit message in\nan editor*. Therefore, the fixup/squash chain is broken at that point, and\nwe have to treat the current fixup/squash as if it were the final in the\ncurrent chain.\n\nI imagine that it will take a while to review v4 in depth ;-)\n\nCiao,\nDscho\n"}]}