{"thread":{"id":"64166","subject":"[PATCH 0/2] rebase -i: respect commit.cleanup when picking fixups","startedAt":"2025-09-18T09:00:49Z","lastAt":"2025-09-18T15:29:08Z","messageCount":4,"participants":["Phillip Wood","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"526694","messageId":"cover.1758186038.git.phillip.wood@dunelm.org.uk","threadId":"64166","inReplyTo":null,"subject":"[PATCH 0/2] rebase -i: respect commit.cleanup when picking fixups","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-09-18T09:00:37Z","receivedAt":"2025-09-18T09:00:49Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nThis series fixes a recent bug report[2] where \"git rebase\n--autosquash\" left comments in the commit message when squashing fixup\ncommits. The first patch fixes the bug and the second removes some\ncode that becomes unused after the changes in the first patch. Thanks\nto Simon for reporting this.\n\n[1] \n\nBase-Commit: 92c87bdc406e5bcf1b516a861d609b03d99c23b7\nPublished-As: https://github.com/phillipwood/git/releases/tag/pw%2Frebase-commit-msg-cleanup%2Fv1\nView-Changes-At: https://github.com/phillipwood/git/compare/92c87bdc4...9c7f4e39c\nFetch-It-Via: git fetch https://github.com/phillipwood/git pw/rebase-commit-msg-cleanup/v1\n\n\nPhillip Wood (2):\n rebase -i: respect commit.cleanup when picking fixups\n sequencer: remove VERBATIM_MSG flag\n\n sequencer.c | 12 ------------\n t/t3415-rebase-autosquash.sh | 28 ++++++++++++++++++++++------\n 2 files changed, 22 insertions(+), 18 deletions(-)\n\n-- \n2.51.0.595.g259a2096d9c\n\n"},{"id":"526695","messageId":"8fb1b4b4a5f4857a3d7d98d029b22904751ec144.1758186038.git.phillip.wood@dunelm.org.uk","threadId":"64166","inReplyTo":"cover.1758186038.git.phillip.wood@dunelm.org.uk","subject":"[PATCH 1/2] rebase -i: respect commit.cleanup when picking fixups","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-09-18T09:00:38Z","receivedAt":"2025-09-18T09:00:50Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nIf the user uses a prepare-commit-msg hook to add comments to the\ncommit message template and sets commit.cleanup to remove them when the\ncommit is created then the comments will not be removed when rebase\ncommits the final command in a chain of \"fixup\" commands[1]. This\nhappens because f7d42ceec52 (rebase -i: do leave commit message intact\nin fixup! chains, 2021-01-28) started passing the VERBATIM_MSG flag\nwhen committing the final command in a chain of \"fixup\" commands. That\nchange was added in response to a bug report[2] where the commit\nmessage was being cleaned up when it should not be. The cause of that\nbug was that before f7d42ceec52 the sequencer passed CLEANUP_MSG\nwhen committing the final fixup. That commit should have simply\nremoved the CLEANUP_MSG flag, not changed it to VERBATIM_MSG. Using\nVERBATIM_MSG ignores the user's commit.cleanup config when committing\nthe final fixup which means it behaves differently to an ordinary\n\"pick\" command which respects commit.cleanup.\n\nFix this by not setting an explicit cleanup flag when committing the\nfinal fixup which matches the way \"pick\" commands behave. The test\nadded in f7d42ceec52 is replaced with one that checks that \"fixup\"\nand \"pick\" commands do not clean up the message when commit.cleanup\nis not set and do clean up the message when it is set.\n\n[1] https://lore.kernel.org/git/CA+itcS3DxbgpFy2aPRvHQvTAYE=dU0kfeDdidVwWLU=rBAWR4w@mail.gmail.com\n[2] https://lore.kernel.org/git/CANVGpwZGbzYLMeMze64e_OU9p3bjyEgzC5thmNBr6LttBt+YGw@mail.gmail.com\n\nReported-by: Simon Cheng \nSigned-off-by: Phillip Wood \n---\n sequencer.c | 1 -\n t/t3415-rebase-autosquash.sh | 28 ++++++++++++++++++++++------\n 2 files changed, 22 insertions(+), 7 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 9ae40a91b24..47c57c2ea3c 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2436,7 +2436,6 @@ static int do_pick_commit(struct repository *r,\n \t\tif (!final_fixup)\n \t\t\tmsg_file = rebase_path_squash_msg();\n \t\telse if (file_exists(rebase_path_fixup_msg())) {\n-\t\t\tflags |= VERBATIM_MSG;\n \t\t\tmsg_file = rebase_path_fixup_msg();\n \t\t} else {\n \t\t\tconst char *dest = git_path_squash_msg(r);\ndiff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh\nindex 5d093e3a7ab..5033411a431 100755\n--- a/t/t3415-rebase-autosquash.sh\n+++ b/t/t3415-rebase-autosquash.sh\n@@ -486,12 +486,28 @@ test_expect_success 'fixup a fixup' '\n \ttest XZWY = $(git show | tr -cd W-Z)\n '\n \n-test_expect_success 'fixup does not clean up commit message' '\n-\toneline=\"#818\" &&\n-\tgit commit --allow-empty -m \"$oneline\" &&\n-\tgit commit --fixup HEAD --allow-empty &&\n-\tgit -c commit.cleanup=strip rebase -ki --autosquash HEAD~2 &&\n-\ttest \"$oneline\" = \"$(git show -s --format=%s)\"\n+test_expect_success 'pick and fixup respect commit.cleanup' '\n+\tgit reset --hard base &&\n+\ttest_commit --no-tag \"fixup! second commit\" file1 fixup &&\n+\ttest_commit something &&\n+\twrite_script .git/hooks/prepare-commit-msg <<-\\EOF &&\n+\tprintf \"\\n# Prepared\\n\" >> \"$1\"\n+\tEOF\n+\tgit rebase -i --autosquash HEAD~3 &&\n+\ttest_commit_message HEAD~1 <<-\\EOF &&\n+\tsecond commit\n+\n+\t# Prepared\n+\tEOF\n+\ttest_commit_message HEAD <<-\\EOF &&\n+\tsomething\n+\n+\t# Prepared\n+\tEOF\n+\tgit reset --hard something &&\n+\tgit -c commit.cleanup=strip rebase -i --autosquash HEAD~3 &&\n+\ttest_commit_message HEAD~1 -m \"second commit\" &&\n+\ttest_commit_message HEAD -m \"something\"\n '\n \n test_done\n-- \n2.51.0.595.g259a2096d9c\n\n"},{"id":"526696","messageId":"9c7f4e39c93a4ae09c291296cdc37d2d235b1ac3.1758186038.git.phillip.wood@dunelm.org.uk","threadId":"64166","inReplyTo":"cover.1758186038.git.phillip.wood@dunelm.org.uk","subject":"[PATCH 2/2] sequencer: remove VERBATIM_MSG flag","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-09-18T09:00:39Z","receivedAt":"2025-09-18T09:00:51Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nAs the last commit deleted the only user of VERBATIM_MSG remove\nit. This reverts remaining parts of commit f7d42ceec52 (rebase -i:\ndo leave commit message intact in fixup! chains, 2021-01-28) that\nwere not deleted by the last commit.\n\nSigned-off-by: Phillip Wood \n---\n sequencer.c | 11 -----------\n 1 file changed, 11 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 47c57c2ea3c..6d29a938aa5 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -1087,7 +1087,6 @@ N_(\"you have staged changes in your working tree\\n\"\n #define CLEANUP_MSG (1<<3)\n #define VERIFY_MSG (1<<4)\n #define CREATE_ROOT_COMMIT (1<<5)\n-#define VERBATIM_MSG (1<<6)\n \n static int run_command_silent_on_success(struct child_process *cmd)\n {\n@@ -1125,9 +1124,6 @@ static int run_git_commit(const char *defmsg,\n {\n \tstruct child_process cmd = CHILD_PROCESS_INIT;\n \n-\tif ((flags & CLEANUP_MSG) && (flags & VERBATIM_MSG))\n-\t\tBUG(\"CLEANUP_MSG and VERBATIM_MSG are mutually exclusive\");\n-\n \tcmd.git_cmd = 1;\n \n \tif (is_rebase_i(opts) &&\n@@ -1166,8 +1162,6 @@ static int run_git_commit(const char *defmsg,\n \t\tstrvec_pushl(&cmd.args, \"-C\", \"HEAD\", NULL);\n \tif ((flags & CLEANUP_MSG))\n \t\tstrvec_push(&cmd.args, \"--cleanup=strip\");\n-\tif ((flags & VERBATIM_MSG))\n-\t\tstrvec_push(&cmd.args, \"--cleanup=verbatim\");\n \tif ((flags & EDIT_MSG))\n \t\tstrvec_push(&cmd.args, \"-e\");\n \telse if (!(flags & CLEANUP_MSG) &&\n@@ -1540,9 +1534,6 @@ static int try_to_commit(struct repository *r,\n \tenum commit_msg_cleanup_mode cleanup;\n \tint res = 0;\n \n-\tif ((flags & CLEANUP_MSG) && (flags & VERBATIM_MSG))\n-\t\tBUG(\"CLEANUP_MSG and VERBATIM_MSG are mutually exclusive\");\n-\n \tif (parse_head(r, ¤t_head))\n \t\treturn -1;\n \n@@ -1618,8 +1609,6 @@ static int try_to_commit(struct repository *r,\n \n \tif (flags & CLEANUP_MSG)\n \t\tcleanup = COMMIT_MSG_CLEANUP_ALL;\n-\telse if (flags & VERBATIM_MSG)\n-\t\tcleanup = COMMIT_MSG_CLEANUP_NONE;\n \telse if ((opts->signoff || opts->record_origin) &&\n \t\t !opts->explicit_cleanup)\n \t\tcleanup = COMMIT_MSG_CLEANUP_SPACE;\n-- \n2.51.0.595.g259a2096d9c\n\n"},{"id":"526707","messageId":"xmqqtt0z93ul.fsf@gitster.g","threadId":"64166","inReplyTo":"8fb1b4b4a5f4857a3d7d98d029b22904751ec144.1758186038.git.phillip.wood@dunelm.org.uk","subject":"Re: [PATCH 1/2] rebase -i: respect commit.cleanup when picking fixups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-09-18T15:29:06Z","receivedAt":"2025-09-18T15:29:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> ... The cause of that\n> bug was that before f7d42ceec52 the sequencer passed CLEANUP_MSG\n> when committing the final fixup. That commit should have simply\n> removed the CLEANUP_MSG flag, not changed it to VERBATIM_MSG. Using\n> VERBATIM_MSG ignores the user's commit.cleanup config when committing\n> the final fixup which means it behaves differently to an ordinary\n> \"pick\" command which respects commit.cleanup.\n\nSo cleanly analysed and clearly described. \n\n> diff --git a/sequencer.c b/sequencer.c\n> index 9ae40a91b24..47c57c2ea3c 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -2436,7 +2436,6 @@ static int do_pick_commit(struct repository *r,\n> \t\tif (!final_fixup)\n> \t\t\tmsg_file = rebase_path_squash_msg();\n> \t\telse if (file_exists(rebase_path_fixup_msg())) {\n> -\t\t\tflags |= VERBATIM_MSG;\n> \t\t\tmsg_file = rebase_path_fixup_msg();\n> \t\t} else {\n> \t\t\tconst char *dest = git_path_squash_msg(r);\n\nAnd this is the location the earlier commit replaced the code that\nsets CLEANUP_MSG, which matches the fix described in the proposed\nlog message. Makes sense.\n\nThe commit at the source of the bug is what introduced VERBATIM_MSG,\nand I think it can be removed (this was the only place it was set).\n\nAnd you removed it in a separate step ;-). Well done.\n\nThanks.\n"}]}