{"thread":{"id":"56115","subject":"[PATCH 0/2] Normalize newlines in merge & interpret-trailer","startedAt":"2021-07-16T07:43:41Z","lastAt":"2021-08-26T18:40:16Z","messageCount":17,"participants":["Luca Weiss via GitGitGadget","Phillip Wood","Luca Weiss","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"430304","messageId":"pull.1048.git.git.1626421416.gitgitgadget@gmail.com","threadId":"56115","inReplyTo":null,"subject":"[PATCH 0/2] Normalize newlines in merge & interpret-trailer","fromName":"Luca Weiss via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-07-16T07:43:34Z","receivedAt":"2021-07-16T07:43:41Z","isPatch":true,"sender":{"key":"luca@z3ntu.xyz","avatar":null},"body":"These two patches fix a problem where the trailer would be appended to the\ncommit message without an empty line, so parsing the trailers again\nafterwards would fail.\n\nIn practice either one of the patches fixes the exact behavior I see but in\nboth cases it makes sense to normalize the newlines.\n\nThe exact use case where this issue was found is a \"git merge --no-edit\"\nwith a commit-msg hook that adds a trailer immediately afterwards. The input\nthe commit-msg script gets is not terminated by a newline (which is fixed by\nthe second commit) while the first one makes interpret-trailer capable of\nhandling such input without a final newline.\n\nLuca Weiss (2):\n trailer: handle input without trailing newline\n merge: make sure to terminate message with newline\n\n builtin/merge.c | 1 +\n t/t7513-interpret-trailers.sh | 7 +++++++\n trailer.c | 3 +++\n 3 files changed, 11 insertions(+)\n\n\nbase-commit: 75ae10bc75336db031ee58d13c5037b929235912\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1048%2Fz3ntu%2Fmaster-newline-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1048/z3ntu/master-newline-v1\nPull-Request: https://github.com/git/git/pull/1048\n-- \ngitgitgadget\n"},{"id":"430305","messageId":"c17ff7ac483ad3990680a3bb1b67898564eb9379.1626421416.git.gitgitgadget@gmail.com","threadId":"56115","inReplyTo":"pull.1048.git.git.1626421416.gitgitgadget@gmail.com","subject":"[PATCH 1/2] trailer: handle input without trailing newline","fromName":"Luca Weiss via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-07-16T07:43:35Z","receivedAt":"2021-07-16T07:43:42Z","isPatch":true,"sender":{"key":"luca@z3ntu.xyz","avatar":null},"body":"From: Luca Weiss \n\nAdd a corresponding test case for this as well\n\nSigned-off-by: Luca Weiss \n---\n t/t7513-interpret-trailers.sh | 7 +++++++\n trailer.c | 3 +++\n 2 files changed, 10 insertions(+)\n\ndiff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh\nindex 04885d0a5e5..ff5f1724ad0 100755\n--- a/t/t7513-interpret-trailers.sh\n+++ b/t/t7513-interpret-trailers.sh\n@@ -17,6 +17,7 @@ test_expect_success 'setup' '\n \n \t\tbody\n \tEOF\n+\tprintf \"subject\\n\\nbody\" > basic_message_no_eol &&\n \tcat >complex_message_body <<-\\EOF &&\n \t\tmy subject\n \n@@ -676,6 +677,12 @@ test_expect_success 'with message that has an old style conflict block' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'with message without trailing newline twice' '\n+\tgit interpret-trailers --trailer \"Cc: Peff\" basic_message_no_eol > intermediary &&\n+\tgit interpret-trailers --trailer \"Cc: Peff\" intermediary > actual &&\n+\ttest_cmp intermediary actual\n+'\n+\n test_expect_success 'with commit complex message and trailer args' '\n \tcat complex_message_body >expected &&\n \tsed -e \"s/ Z\\$/ /\" >>expected <<-\\EOF &&\ndiff --git a/trailer.c b/trailer.c\nindex 7c7cb61a945..f53cf7d07d4 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -765,6 +765,9 @@ static void read_input_file(struct strbuf *sb, const char *file)\n \t\tif (strbuf_read(sb, fileno(stdin), 0) < 0)\n \t\t\tdie_errno(_(\"could not read from stdin\"));\n \t}\n+\n+\t/* Make sure the input ends with a newline */\n+\tstrbuf_complete_line(sb);\n }\n \n static const char *next_line(const char *str)\n-- \ngitgitgadget\n\n"},{"id":"430306","messageId":"31371c25743e1001b4fac89e80e7206ff477ac8a.1626421416.git.gitgitgadget@gmail.com","threadId":"56115","inReplyTo":"pull.1048.git.git.1626421416.gitgitgadget@gmail.com","subject":"[PATCH 2/2] merge: make sure to terminate message with newline","fromName":"Luca Weiss via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-07-16T07:43:36Z","receivedAt":"2021-07-16T07:43:43Z","isPatch":true,"sender":{"key":"luca@z3ntu.xyz","avatar":null},"body":"From: Luca Weiss \n\nSigned-off-by: Luca Weiss \n---\n builtin/merge.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex a8a843b1f54..646bb49367f 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -867,6 +867,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)\n \t}\n \tif (signoff)\n \t\tappend_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0);\n+\tstrbuf_complete_line(&msg);\n \twrite_merge_heads(remoteheads);\n \twrite_file_buf(git_path_merge_msg(the_repository), msg.buf, msg.len);\n \tif (run_commit_hook(0 < option_edit, get_index_file(), \"prepare-commit-msg\",\n-- \ngitgitgadget\n"},{"id":"430316","messageId":"8678772b-dd5d-9188-8b63-98d05cedb323@gmail.com","threadId":"56115","inReplyTo":"31371c25743e1001b4fac89e80e7206ff477ac8a.1626421416.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] merge: make sure to terminate message with newline","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2021-07-16T10:23:06Z","receivedAt":"2021-07-16T10:23:20Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"\nHi Luca\n\nThanks for your patches. It would be very helpful to have the \nexplanation from the cover letter in the commit messages for both \ncommits to explain why this change is being made, otherwise that \ninformation will not appear in the history.\n\nThe cover letter says this happened when using '--no-edit', but unless \nI've missed something 'git merge --no-edit' creates its message using \nfmt_merge_msg() which calls strbuf_complete_line() just before it \nreturns. append_signoff() and 'merge -m' always terminate the message \nwith a newline. The only path I found that does not ensure the message \nends with a newline before calling the prepare-commit-msg hook is when \nusing '-F' and I suspect that may have been a deliberate decision but it \ncould be an oversight. In any case we would want to make sure that 'git \ncommit -F' and 'git merge -F' to behave the same which I think they do \nat the moment.\n\nBest Wishes\n\nPhillip\n\nOn 16/07/2021 08:43, Luca Weiss via GitGitGadget wrote:\n> From: Luca Weiss \n> \n> Signed-off-by: Luca Weiss \n> ---\n> builtin/merge.c | 1 +\n> 1 file changed, 1 insertion(+)\n> \n> diff --git a/builtin/merge.c b/builtin/merge.c\n> index a8a843b1f54..646bb49367f 100644\n> --- a/builtin/merge.c\n> +++ b/builtin/merge.c\n> @@ -867,6 +867,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)\n> \t}\n> \tif (signoff)\n> \t\tappend_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0);\n> +\tstrbuf_complete_line(&msg);\n> \twrite_merge_heads(remoteheads);\n> \twrite_file_buf(git_path_merge_msg(the_repository), msg.buf, msg.len);\n> \tif (run_commit_hook(0 < option_edit, get_index_file(), \"prepare-commit-msg\",\n> \n\n"},{"id":"430317","messageId":"AB048897-F70A-4388-B2A6-56BFEA40B303@z3ntu.xyz","threadId":"56115","inReplyTo":"8678772b-dd5d-9188-8b63-98d05cedb323@gmail.com","subject":"Re: [PATCH 2/2] merge: make sure to terminate message with newline","fromName":"Luca Weiss","fromEmail":"luca@z3ntu.xyz","sentAt":"2021-07-16T12:37:48Z","receivedAt":"2021-07-16T12:38:05Z","isPatch":true,"sender":{"key":"luca@z3ntu.xyz","avatar":null},"body":"Hi Phillip,\n\nSo the behavior that I have observed is the following:\n\nI've added a usleep for ~10 seconds instead of the line I added and\n.git/MERGE_MSG was not terminated with a newline.\nThis didn't change when using --log\nIt was properly handled with --signoff and the trailer was added\ncorrectly.\n\nI have a simple reproducer here:\n\nmkdir /tmp/test\ncd /tmp/test\ngit init\necho 'dest=\"$1.tmp\"; git interpret-trailers --trailer \"Foo: Bar\" < \"$1\" > \"${dest}\"; mv \"${dest}\" \"$1\"' > .git/hooks/commit-msg\nchmod +x .git/hooks/commit-msg\ngit commit --allow-empty -m \"Initial commit\"\nsleep 1\ngit switch -c foobar\ngit commit --allow-empty -m \"Foo1\"\nsleep 1\ngit commit --allow-empty -m \"Foo2\"\ngit switch master\ngit merge --no-ff --no-edit foobar\n# look at merge commit message now\n\nWith my patch(es) this works properly.\n\nIf you have any other ideas on how to fix this, I am open for\nsuggestions :)\n\nOtherwise I'll try to add more detail to the individual commit\nmessages (I deliberately kept the \"unnecessary\" detail out of the actual\ncommit messages before).\n\nRegards\nLuca\n\np.s. sorry for replying off-list, and if this is wrongly bottom/top posted, email client troubles ;)\n\nOn July 16, 2021 12:23:06 PM GMT+02:00, Phillip Wood wrote:\n>\n>Hi Luca\n>\n>Thanks for your patches. It would be very helpful to have the \n>explanation from the cover letter in the commit messages for both \n>commits to explain why this change is being made, otherwise that \n>information will not appear in the history.\n>\n>The cover letter says this happened when using '--no-edit', but unless \n>I've missed something 'git merge --no-edit' creates its message using \n>fmt_merge_msg() which calls strbuf_complete_line() just before it \n>returns. append_signoff() and 'merge -m' always terminate the message \n>with a newline. The only path I found that does not ensure the message \n>ends with a newline before calling the prepare-commit-msg hook is when \n>using '-F' and I suspect that may have been a deliberate decision but it \n>could be an oversight. In any case we would want to make sure that 'git \n>commit -F' and 'git merge -F' to behave the same which I think they do \n>at the moment.\n>\n>Best Wishes\n>\n>Phillip\n>\n>On 16/07/2021 08:43, Luca Weiss via GitGitGadget wrote:\n>> From: Luca Weiss \n>> \n>> Signed-off-by: Luca Weiss \n>> ---\n>> builtin/merge.c | 1 +\n>> 1 file changed, 1 insertion(+)\n>> \n>> diff --git a/builtin/merge.c b/builtin/merge.c\n>> index a8a843b1f54..646bb49367f 100644\n>> --- a/builtin/merge.c\n>> +++ b/builtin/merge.c\n>> @@ -867,6 +867,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)\n>> \t}\n>> \tif (signoff)\n>> \t\tappend_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0);\n>> +\tstrbuf_complete_line(&msg);\n>> \twrite_merge_heads(remoteheads);\n>> \twrite_file_buf(git_path_merge_msg(the_repository), msg.buf, msg.len);\n>> \tif (run_commit_hook(0 < option_edit, get_index_file(), \"prepare-commit-msg\",\n>> \n>\n"},{"id":"430361","messageId":"16229b1d-e4a6-7a8d-8ea0-ae7c3f13075d@gmail.com","threadId":"56115","inReplyTo":"AB048897-F70A-4388-B2A6-56BFEA40B303@z3ntu.xyz","subject":"Re: [PATCH 2/2] merge: make sure to terminate message with newline","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2021-07-16T17:30:50Z","receivedAt":"2021-07-16T17:30:55Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Luca\n\nOn 16/07/2021 13:37, Luca Weiss wrote:\n> Hi Phillip,\n> \n> So the behavior that I have observed is the following:\n> \n> I've added a usleep for ~10 seconds instead of the line I added and\n> .git/MERGE_MSG was not terminated with a newline.\n> This didn't change when using --log\n> It was properly handled with --signoff and the trailer was added\n> correctly.\n> \n> I have a simple reproducer here:\n> \n> mkdir /tmp/test\n> cd /tmp/test\n> git init\n> echo 'dest=\"$1.tmp\"; git interpret-trailers --trailer \"Foo: Bar\" < \"$1\" > \"${dest}\"; mv \"${dest}\" \"$1\"' > .git/hooks/commit-msg\n> chmod +x .git/hooks/commit-msg\n> git commit --allow-empty -m \"Initial commit\"\n> sleep 1\n> git switch -c foobar\n> git commit --allow-empty -m \"Foo1\"\n> sleep 1\n> git commit --allow-empty -m \"Foo2\"\n> git switch master\n> git merge --no-ff --no-edit foobar\n> # look at merge commit message now\n\nThanks for the reproducer, I can confirm it shows the bug for me. What I \nmissed this morning was that we promptly chop the '\\n' off the end of \nthe message we get back from fmt_merge_msg(). I've looked through the \nhistory and this behavior dates back to the beginning of the builtin \nmerge added in 1c7b76be7d (\"Build in merge\", 2008-07-07). Back then we \nadded a newline to the end of the message before writing .git/MERGE_MSG \nor committing in finish_automerge() but merge_trivial() did not add a \nnew line before committing. Commit 66f4b98ad9 (\"Teach merge the \n'[-e|--edit]' option\", 2011-10-08) added prepare_to_commit() which added \nthe newline and was called by both finish_automerge() and \nmerge_trivial(). This behavior was changed in d540b70c85 (\"merge: \ncleanup messages like commit\", 2019-04-17) after which we only added a \nnewline if the message was going to be edited. I've cc'd Denton to see \nif he remembers if this was intentional or not.\n\nI suspect the best way to fix this is to stop stripping the newline that \nis added by fmt_merge_msg() and remove the line in prepare_to_commit() \nthat adds the newline when editing. That would leave '-F' untouched so \nit would still not add missing newline in that case - I'm not sure if \nthat is desirable or not but I think it matches what 'git commit -F' does.\n\n> With my patch(es) this works properly.\n> \n> If you have any other ideas on how to fix this, I am open for\n> suggestions :)\n> \n> Otherwise I'll try to add more detail to the individual commit\n> messages (I deliberately kept the \"unnecessary\" detail out of the actual\n> commit messages before).\n\nThe commit message should explain why you're making the change - that is \nnot unnecessary detail but essential context to help others reading the \nhistory in the future to understand the reason for the change.\n\n> Regards\n> Luca\n> \n> p.s. sorry for replying off-list, and if this is wrongly bottom/top posted, email client troubles ;)\n\nThis message seems to have made it onto the list. We normally reply in \nline as I've done here but don't worry if you're having problems with \nyour mail client.\n\nI've not had time to look at the first patch properly but from a quick \nglance it seems to be a sensible approach\n\nBest Wishes\n\nPhillip\n\n> On July 16, 2021 12:23:06 PM GMT+02:00, Phillip Wood wrote:\n>>\n>> Hi Luca\n>>\n>> Thanks for your patches. It would be very helpful to have the\n>> explanation from the cover letter in the commit messages for both\n>> commits to explain why this change is being made, otherwise that\n>> information will not appear in the history.\n>>\n>> The cover letter says this happened when using '--no-edit', but unless\n>> I've missed something 'git merge --no-edit' creates its message using\n>> fmt_merge_msg() which calls strbuf_complete_line() just before it\n>> returns. append_signoff() and 'merge -m' always terminate the message\n>> with a newline. The only path I found that does not ensure the message\n>> ends with a newline before calling the prepare-commit-msg hook is when\n>> using '-F' and I suspect that may have been a deliberate decision but it\n>> could be an oversight. In any case we would want to make sure that 'git\n>> commit -F' and 'git merge -F' to behave the same which I think they do\n>> at the moment.\n>>\n>> Best Wishes\n>>\n>> Phillip\n>>\n>> On 16/07/2021 08:43, Luca Weiss via GitGitGadget wrote:\n>>> From: Luca Weiss \n>>>\n>>> Signed-off-by: Luca Weiss \n>>> ---\n>>> builtin/merge.c | 1 +\n>>> 1 file changed, 1 insertion(+)\n>>>\n>>> diff --git a/builtin/merge.c b/builtin/merge.c\n>>> index a8a843b1f54..646bb49367f 100644\n>>> --- a/builtin/merge.c\n>>> +++ b/builtin/merge.c\n>>> @@ -867,6 +867,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)\n>>> \t}\n>>> \tif (signoff)\n>>> \t\tappend_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0);\n>>> +\tstrbuf_complete_line(&msg);\n>>> \twrite_merge_heads(remoteheads);\n>>> \twrite_file_buf(git_path_merge_msg(the_repository), msg.buf, msg.len);\n>>> \tif (run_commit_hook(0 < option_edit, get_index_file(), \"prepare-commit-msg\",\n>>>\n>>\n"},{"id":"430382","messageId":"YPHe/W7+Oh63NpB0@coredump.intra.peff.net","threadId":"56115","inReplyTo":"16229b1d-e4a6-7a8d-8ea0-ae7c3f13075d@gmail.com","subject":"Re: [PATCH 2/2] merge: make sure to terminate message with newline","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-07-16T19:33:17Z","receivedAt":"2021-07-16T19:33:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 16, 2021 at 06:30:50PM +0100, Phillip Wood wrote:\n\n> > I have a simple reproducer here:\n> > \n> > mkdir /tmp/test\n> > cd /tmp/test\n> > git init\n> > echo 'dest=\"$1.tmp\"; git interpret-trailers --trailer \"Foo: Bar\" < \"$1\" > \"${dest}\"; mv \"${dest}\" \"$1\"' > .git/hooks/commit-msg\n> > chmod +x .git/hooks/commit-msg\n> > git commit --allow-empty -m \"Initial commit\"\n> > sleep 1\n> > git switch -c foobar\n> > git commit --allow-empty -m \"Foo1\"\n> > sleep 1\n> > git commit --allow-empty -m \"Foo2\"\n> > git switch master\n> > git merge --no-ff --no-edit foobar\n> > # look at merge commit message now\n> \n> Thanks for the reproducer, I can confirm it shows the bug for me. What I\n> missed this morning was that we promptly chop the '\\n' off the end of the\n> message we get back from fmt_merge_msg(). I've looked through the history\n> and this behavior dates back to the beginning of the builtin merge added in\n> 1c7b76be7d (\"Build in merge\", 2008-07-07). Back then we added a newline to\n> the end of the message before writing .git/MERGE_MSG or committing in\n> finish_automerge() but merge_trivial() did not add a new line before\n> committing. Commit 66f4b98ad9 (\"Teach merge the '[-e|--edit]' option\",\n> 2011-10-08) added prepare_to_commit() which added the newline and was called\n> by both finish_automerge() and merge_trivial(). This behavior was changed in\n> d540b70c85 (\"merge: cleanup messages like commit\", 2019-04-17) after which\n> we only added a newline if the message was going to be edited. I've cc'd\n> Denton to see if he remembers if this was intentional or not.\n\nI think we still end up calling cleanup_message() on the result before\nusing it as the final message, and that will fix any missing newline.\nBut we feed the intermediate state before then to the hooks (which is\nexactly where one might expect to use interpret-trailers).\n\nI'm not sure if we should be doing a preemptive call to\ncleanup_message() before feeding the hooks (we'd still need to do the\nfinal one, to clean up whatever the hooks return to us). I guess\nprobably not, because I think that would remove comments, as well. So\nadding in just the missing newline is probably better.\n\n> I suspect the best way to fix this is to stop stripping the newline that is\n> added by fmt_merge_msg() and remove the line in prepare_to_commit() that\n> adds the newline when editing. That would leave '-F' untouched so it would\n> still not add missing newline in that case - I'm not sure if that is\n> desirable or not but I think it matches what 'git commit -F' does.\n\nSince it will usually be added back in by cleanup_message() anyway, I\nthink it's OK to just add it preemptively. The exception would be if the\nuser asked for no cleanup at all. So making it conditional on\ncleanup_mode would work, whether -F or not.\n\nI suppose that does mean people turning off cleanup mode would get a\nmessage without a newline from fmt_merge_msg(), though, which is perhaps\nunexpected.\n\nSo maybe just keeping the newline there, as you suggest, is the best\nway.\n\n> The commit message should explain why you're making the change - that is not\n> unnecessary detail but essential context to help others reading the history\n> in the future to understand the reason for the change.\n\nYes. A summary of the problem, the reasoning, and the discussion here\nwould be appropriate for the commit message.\n\n-Peff\n"},{"id":"430383","messageId":"YPHfgyR5ZS2pPr0P@coredump.intra.peff.net","threadId":"56115","inReplyTo":"c17ff7ac483ad3990680a3bb1b67898564eb9379.1626421416.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] trailer: handle input without trailing newline","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-07-16T19:35:31Z","receivedAt":"2021-07-16T19:35:42Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 16, 2021 at 07:43:35AM +0000, Luca Weiss via GitGitGadget wrote:\n\n> From: Luca Weiss \n> \n> Add a corresponding test case for this as well\n\nThe details you put in the cover letter won't make it into the Git\nhistory. This would be a good place to put them.\n\n> t/t7513-interpret-trailers.sh | 7 +++++++\n> trailer.c | 3 +++\n> 2 files changed, 10 insertions(+)\n\nThat patch itself looks good to me, with one minor style nit:\n\n> diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh\n> index 04885d0a5e5..ff5f1724ad0 100755\n> --- a/t/t7513-interpret-trailers.sh\n> +++ b/t/t7513-interpret-trailers.sh\n> @@ -17,6 +17,7 @@ test_expect_success 'setup' '\n> \n> \t\tbody\n> \tEOF\n> +\tprintf \"subject\\n\\nbody\" > basic_message_no_eol &&\n\nOur preferred style is to omit whitespace between redirection operators\nand filenames (i.e., \">basic_message_no_eol\").\n\n> +test_expect_success 'with message without trailing newline twice' '\n> +\tgit interpret-trailers --trailer \"Cc: Peff\" basic_message_no_eol > intermediary &&\n> +\tgit interpret-trailers --trailer \"Cc: Peff\" intermediary > actual &&\n> +\ttest_cmp intermediary actual\n> +'\n\nDitto here.\n\n-Peff\n"},{"id":"430389","messageId":"xmqqeebyqadi.fsf@gitster.g","threadId":"56115","inReplyTo":"31371c25743e1001b4fac89e80e7206ff477ac8a.1626421416.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] merge: make sure to terminate message with newline","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-07-16T20:20:09Z","receivedAt":"2021-07-16T20:20:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Luca Weiss via GitGitGadget\" writes:\n\n> From: Luca Weiss \n\nPlease explain what you are trying to fix here. The title hints\nthat this change is to deal with an message that ends in an\nincomplete line, but where does that incomplete line come?\n\nI find it somewhat important to know that details especially because\nin this patch ...\n\n> Signed-off-by: Luca Weiss \n> ---\n> builtin/merge.c | 1 +\n> 1 file changed, 1 insertion(+)\n>\n> diff --git a/builtin/merge.c b/builtin/merge.c\n> index a8a843b1f54..646bb49367f 100644\n> --- a/builtin/merge.c\n> +++ b/builtin/merge.c\n> @@ -867,6 +867,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)\n> \t}\n> \tif (signoff)\n> \t\tappend_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0);\n> +\tstrbuf_complete_line(&msg);\n\n... the placement of this call looks dubious.\n\nBefore the context of this hunk, we \n\n - started with an empty msg at the beginning of this function;\n\n - added merge_msg to msg, where merge_msg came from options like -F\n and -m;\n\n - optionally added to msg comments to be shown in the editor; and\n\n - optionally called append_signoff() to add s-o-b.\n\nAt which step do we make msg end with an incomplete line and under\nwhat condition?\n\nIf it is after processing -F/-m and if we did not call the editor,\nfor example, we \"append\" the sign-off after that incomplete line and\nyour new call to strbuf_complete_line() would be too late if that is\nthe case, wouldn't it? I do not know if that is, since your commit\nlog message lacks meaningful details, and that is exactly why people\nwant you to explain what breakage you observed and what your fix is.\n\nThanks.\n\n> \twrite_merge_heads(remoteheads);\n> \twrite_file_buf(git_path_merge_msg(the_repository), msg.buf, msg.len);\n> \tif (run_commit_hook(0 < option_edit, get_index_file(), \"prepare-commit-msg\",\n"},{"id":"430390","messageId":"xmqq8s26q9ot.fsf@gitster.g","threadId":"56115","inReplyTo":"YPHe/W7+Oh63NpB0@coredump.intra.peff.net","subject":"Re: [PATCH 2/2] merge: make sure to terminate message with newline","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-07-16T20:34:58Z","receivedAt":"2021-07-16T20:35:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I think we still end up calling cleanup_message() on the result before\n> using it as the final message, and that will fix any missing newline.\n> But we feed the intermediate state before then to the hooks (which is\n> exactly where one might expect to use interpret-trailers).\n>\n> I'm not sure if we should be doing a preemptive call to\n> cleanup_message() before feeding the hooks (we'd still need to do the\n> final one, to clean up whatever the hooks return to us). I guess\n> probably not, because I think that would remove comments, as well. So\n> adding in just the missing newline is probably better.\n\nTo be quite honest, I think this patch is a half-way solution and I\nam not sure if it is better than either of the two \"purist\"\nextremes:\n\n * If the hooks want to see messages with as little loss of\n information from the original as possible, we should give them\n without clean-up (which you pointed out above) *and* without this\n patch.\n\n * If the hooks want to see messages as canonicalized as people\n would see in normal \"git log\" output, we should be passing the\n full clean-up to lose even comments and in such a case there is\n no need for this \"always terminate\" patch (we'd instead do far\n more).\n\nBetween the two approaches, both are purists' view, I'd prefer the\nformer, but from that stance, the conclusion would become that there\nis no need to do anything, which may be a bit unsatisfactory.\n\n> Since it will usually be added back in by cleanup_message() anyway, I\n> think it's OK to just add it preemptively. The exception would be if the\n> user asked for no cleanup at all. So making it conditional on\n> cleanup_mode would work, whether -F or not.\n>\n> I suppose that does mean people turning off cleanup mode would get a\n> message without a newline from fmt_merge_msg(), though, which is perhaps\n> unexpected.\n>\n> So maybe just keeping the newline there, as you suggest, is the best\n> way.\n\nHmph.\n\nIf the user tells us \"refrain from touching my message as much as\npossible\" and feeds us a proposed log message that ends with an\nincomplete line, I would think they expect us not to turn it into a\ncomplete line, and I would think doing this change only when cleanup\nis in effect would make sense. This is especially true for users\nwho do not let any hooks to interfere. They used to get their\nincomplete lines intact, now their incomplete lines will\nunconditionally get completed. I am not sure if I would want to\ndefend this change from their complaints.\n"},{"id":"430391","messageId":"YPH1qKMPOqhCzp4Y@coredump.intra.peff.net","threadId":"56115","inReplyTo":"xmqq8s26q9ot.fsf@gitster.g","subject":"Re: [PATCH 2/2] merge: make sure to terminate message with newline","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-07-16T21:10:00Z","receivedAt":"2021-07-16T21:10:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 16, 2021 at 01:34:58PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > I think we still end up calling cleanup_message() on the result before\n> > using it as the final message, and that will fix any missing newline.\n> > But we feed the intermediate state before then to the hooks (which is\n> > exactly where one might expect to use interpret-trailers).\n> >\n> > I'm not sure if we should be doing a preemptive call to\n> > cleanup_message() before feeding the hooks (we'd still need to do the\n> > final one, to clean up whatever the hooks return to us). I guess\n> > probably not, because I think that would remove comments, as well. So\n> > adding in just the missing newline is probably better.\n> \n> To be quite honest, I think this patch is a half-way solution and I\n> am not sure if it is better than either of the two \"purist\"\n> extremes:\n> \n> * If the hooks want to see messages with as little loss of\n> information from the original as possible, we should give them\n> without clean-up (which you pointed out above) *and* without this\n> patch.\n> \n> * If the hooks want to see messages as canonicalized as people\n> would see in normal \"git log\" output, we should be passing the\n> full clean-up to lose even comments and in such a case there is\n> no need for this \"always terminate\" patch (we'd instead do far\n> more).\n> \n> Between the two approaches, both are purists' view, I'd prefer the\n> former, but from that stance, the conclusion would become that there\n> is no need to do anything, which may be a bit unsatisfactory.\n\nYes, I agree with all of that (including the \"as little loss of\ninformation as possible\" preference).\n\n> > Since it will usually be added back in by cleanup_message() anyway, I\n> > think it's OK to just add it preemptively. The exception would be if the\n> > user asked for no cleanup at all. So making it conditional on\n> > cleanup_mode would work, whether -F or not.\n> >\n> > I suppose that does mean people turning off cleanup mode would get a\n> > message without a newline from fmt_merge_msg(), though, which is perhaps\n> > unexpected.\n> >\n> > So maybe just keeping the newline there, as you suggest, is the best\n> > way.\n> \n> Hmph.\n> \n> If the user tells us \"refrain from touching my message as much as\n> possible\" and feeds us a proposed log message that ends with an\n> incomplete line, I would think they expect us not to turn it into a\n> complete line, and I would think doing this change only when cleanup\n> is in effect would make sense. This is especially true for users\n> who do not let any hooks to interfere. They used to get their\n> incomplete lines intact, now their incomplete lines will\n> unconditionally get completed. I am not sure if I would want to\n> defend this change from their complaints.\n\nRight, what I was suggesting was:\n\n if (cleanup_mode != COMMIT_MSG_CLEANUP_NONE)\n\tstrbuf_complete(&msg);\n\nwhich would cover that case. But Phillip mentioned that our own\nfmt_merge_msg() does not include a newline. So it would not be the user\nfeeding us an incomplete line, but rather Git feeding it. And that was\nwhat I suggested should be corrected (which I suppose would be in\naddition to correcting lines from the user).\n\nHowever, I see a call to strbuf_complete_line() at the end of\nfmt_merge_msg(), so I am puzzled about what he meant.\n\n-Peff\n"},{"id":"430399","messageId":"xmqqsg0dq59v.fsf@gitster.g","threadId":"56115","inReplyTo":"pull.1048.git.git.1626421416.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/2] Normalize newlines in merge & interpret-trailer","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-07-16T22:10:20Z","receivedAt":"2021-07-16T22:10:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Luca Weiss via GitGitGadget\" writes:\n\n> These two patches fix a problem where the trailer would be appended to the\n> commit message without an empty line, so parsing the trailers again\n> afterwards would fail.\n>\n> In practice either one of the patches fixes the exact behavior I see but in\n> both cases it makes sense to normalize the newlines.\n>\n> The exact use case where this issue was found is a \"git merge --no-edit\"\n> with a commit-msg hook that adds a trailer immediately afterwards. The input\n> the commit-msg script gets is not terminated by a newline (which is fixed by\n> the second commit) while the first one makes interpret-trailer capable of\n> handling such input without a final newline.\n\nWhen you fold some of what you wrote in the above into the proposed\ncommit log message proper when you send an updated version of the\nseries, please pay special attention to the phrases like \"empty\nline\", \"normalize newline\" and \"terminated by a newline\".\n\n - As there are some folks who use Git on Windows on this list, when\n we say \"normalize the newlines\", they will think of CRLF vs LF,\n but I do not think that is what you are talking about here.\n\n - As I asked in my review of one of your patches, please explain\n where the incomplete line comes from (e.g. saying \"if the user\n ends the edited log message with an incomplete line\" would make\n it clear how we missed such an incomplete line to come into the\n system).\n\n - I am guessing that \"without an empty line\" is because we usually\n append trailers with one newline after the last line of the log\n message, with the expectation that the existing log message ends\n with a complete line, but an incomplete line at the end of the\n log message absorbs the newline and makes it as part of the last\n line that is now a complete line? And a trailer block that is\n not separated with a blank line from the last paragraph of the\n message body is not taken as a trailer block, causing the later\n parsing to fail, but from your description it was unclear how\n the trailer block is added without the paragraph break.\n\nThanks.\n"},{"id":"430400","messageId":"xmqqo8b1q547.fsf@gitster.g","threadId":"56115","inReplyTo":"YPH1qKMPOqhCzp4Y@coredump.intra.peff.net","subject":"Re: [PATCH 2/2] merge: make sure to terminate message with newline","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-07-16T22:13:44Z","receivedAt":"2021-07-16T22:13:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> which would cover that case. But Phillip mentioned that our own\n> fmt_merge_msg() does not include a newline. So it would not be the user\n> feeding us an incomplete line, but rather Git feeding it.\n>\n> And that was\n> what I suggested should be corrected (which I suppose would be in\n> addition to correcting lines from the user).\n>\n> However, I see a call to strbuf_complete_line() at the end of\n> fmt_merge_msg(), so I am puzzled about what he meant.\n\nWhew. You got me worried. Yeah, if fmt_merge_msg() is not\nterminating its output, we should fix that, but I do not see such a\nbreakage in the current code, either.\n\nThanks.\n"},{"id":"430418","messageId":"693954a7-af64-67c5-41b9-b648a9fe3ef2@gmail.com","threadId":"56115","inReplyTo":"YPH1qKMPOqhCzp4Y@coredump.intra.peff.net","subject":"Re: [PATCH 2/2] merge: make sure to terminate message with newline","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2021-07-17T13:40:55Z","receivedAt":"2021-07-17T13:41:00Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 16/07/2021 22:10, Jeff King wrote:\n> On Fri, Jul 16, 2021 at 01:34:58PM -0700, Junio C Hamano wrote:\n> \n>> Jeff King writes:\n>>\n>>> I think we still end up calling cleanup_message() on the result before\n>>> using it as the final message, and that will fix any missing newline.\n>>> But we feed the intermediate state before then to the hooks (which is\n>>> exactly where one might expect to use interpret-trailers).\n>>>\n>>> I'm not sure if we should be doing a preemptive call to\n>>> cleanup_message() before feeding the hooks (we'd still need to do the\n>>> final one, to clean up whatever the hooks return to us). I guess\n>>> probably not, because I think that would remove comments, as well. So\n>>> adding in just the missing newline is probably better.\n>>\n>> To be quite honest, I think this patch is a half-way solution and I\n>> am not sure if it is better than either of the two \"purist\"\n>> extremes:\n>>\n>> * If the hooks want to see messages with as little loss of\n>> information from the original as possible, we should give them\n>> without clean-up (which you pointed out above) *and* without this\n>> patch.\n>>\n>> * If the hooks want to see messages as canonicalized as people\n>> would see in normal \"git log\" output, we should be passing the\n>> full clean-up to lose even comments and in such a case there is\n>> no need for this \"always terminate\" patch (we'd instead do far\n>> more).\n>>\n>> Between the two approaches, both are purists' view, I'd prefer the\n>> former, but from that stance, the conclusion would become that there\n>> is no need to do anything, which may be a bit unsatisfactory.\n> \n> Yes, I agree with all of that (including the \"as little loss of\n> information as possible\" preference).\n> \n>>> Since it will usually be added back in by cleanup_message() anyway, I\n>>> think it's OK to just add it preemptively. The exception would be if the\n>>> user asked for no cleanup at all. So making it conditional on\n>>> cleanup_mode would work, whether -F or not.\n>>>\n>>> I suppose that does mean people turning off cleanup mode would get a\n>>> message without a newline from fmt_merge_msg(), though, which is perhaps\n>>> unexpected.\n>>>\n>>> So maybe just keeping the newline there, as you suggest, is the best\n>>> way.\n>>\n>> Hmph.\n>>\n>> If the user tells us \"refrain from touching my message as much as\n>> possible\" and feeds us a proposed log message that ends with an\n>> incomplete line, I would think they expect us not to turn it into a\n>> complete line, and I would think doing this change only when cleanup\n>> is in effect would make sense. This is especially true for users\n>> who do not let any hooks to interfere. They used to get their\n>> incomplete lines intact, now their incomplete lines will\n>> unconditionally get completed. I am not sure if I would want to\n>> defend this change from their complaints.\n> \n> Right, what I was suggesting was:\n> \n> if (cleanup_mode != COMMIT_MSG_CLEANUP_NONE)\n> \tstrbuf_complete(&msg);\n> \n> which would cover that case. But Phillip mentioned that our own\n> fmt_merge_msg() does not include a newline.\n\nI mentioned that we remove the newline that is added by fmt_merge_msg(), \nnot that there is no newline added by fmt_merge_msg() - maybe I wasn't \nclear enough. In builtin/merge.c:prepare_merge_message() we do\n\n\tfmt_merge_msg(merge_names, merge_msg, &opts);\n\tif (merge_msg->len)\n\t\tstrbuf_setlen(merge_msg, merge_msg->len - 1);\n\nThis has been the case since the beginning of the builtin merge[1]. I \nassume it was trying to emulate the result of a command substitution in \nthe shell version.\n\nBest Wishes\n\nPhillip\n\n[1] See \nhttps://lore.kernel.org16229b1d-e4a6-7a8d-8ea0-ae7c3f13075d@gmail.com/ \nfor more details of my archaeology on this.\n\n> So it would not be the user\n> feeding us an incomplete line, but rather Git feeding it. And that was\n> what I suggested should be corrected (which I suppose would be in\n> addition to correcting lines from the user).\n> \n> However, I see a call to strbuf_complete_line() at the end of\n> fmt_merge_msg(), so I am puzzled about what he meant.\n\n\n\n> \n> -Peff\n> \n"},{"id":"430433","messageId":"YPMXqLESKApq2uPI@coredump.intra.peff.net","threadId":"56115","inReplyTo":"693954a7-af64-67c5-41b9-b648a9fe3ef2@gmail.com","subject":"Re: [PATCH 2/2] merge: make sure to terminate message with newline","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-07-17T17:47:20Z","receivedAt":"2021-07-17T17:47:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jul 17, 2021 at 02:40:55PM +0100, Phillip Wood wrote:\n\n> > which would cover that case. But Phillip mentioned that our own\n> > fmt_merge_msg() does not include a newline.\n> \n> I mentioned that we remove the newline that is added by fmt_merge_msg(), not\n> that there is no newline added by fmt_merge_msg() - maybe I wasn't clear\n> enough. In builtin/merge.c:prepare_merge_message() we do\n> \n> \tfmt_merge_msg(merge_names, merge_msg, &opts);\n> \tif (merge_msg->len)\n> \t\tstrbuf_setlen(merge_msg, merge_msg->len - 1);\n\nOf maybe I didn't read carefully enough. :)\n\nEither way, thanks for clarifying. Doing something like:\n\n cat >.git/hooks/commit-msg <<\\EOF\n #!/bin/sh\n xxd \"$1\"\n EOF\n chmod +x .git/hooks/commit-msg\n\n git merge --no-edit ...\n\nshows off the problem; the hook sees that intermediate state.\n\nLikewise if we do:\n\n git merge -m \"foo\" ...\n\nwhich similarly suppresses the editor. There are actually two\ninteresting cases here:\n\n - if merge.log is not set, then we'd see \"foo\" with no newline\n\n - if it is set, we'll get a newline after \"foo\", but with no newline\n after the log data\n\nLikewise for:\n\n printf foo >no-newline\n git merge -F no-newline ...\n\nSo I think we'd probably want to see a 3-patch series:\n\n 1. Make interpret-trailers handle input missing the final newline.\n This isn't strictly necessary after patches 2 and 3, but it makes\n sense to be more robust with unexpected input.\n\n 2. Drop the newline-stripping from prepare_merge_message(). The\n examples above show some ways we could cover this in the tests.\n This will help --no-edit case, but also using merge.log with \"-m\"\n or \"-F\".\n\n 3. Teach prepare_to_commit() to add the extra newline before letting\n hooks see the message. This should probably be done only when\n cleanup_mode != COMMIT_MSG_CLEANUP_NONE.\n\nLuca, do you want to try revising your series in that direction?\n\n-Peff\n"},{"id":"430762","messageId":"7464462.Hysq1jLez0@g550jk","threadId":"56115","inReplyTo":"YPMXqLESKApq2uPI@coredump.intra.peff.net","subject":"Re: [PATCH 2/2] merge: make sure to terminate message with newline","fromName":"Luca Weiss","fromEmail":"luca@z3ntu.xyz","sentAt":"2021-07-21T10:41:44Z","receivedAt":"2021-07-21T11:27:31Z","isPatch":true,"sender":{"key":"luca@z3ntu.xyz","avatar":null},"body":"On Samstag, 17. Juli 2021 19:47:20 CEST Jeff King wrote:\n> On Sat, Jul 17, 2021 at 02:40:55PM +0100, Phillip Wood wrote:\n> > > which would cover that case. But Phillip mentioned that our own\n> > > fmt_merge_msg() does not include a newline.\n> > \n> > I mentioned that we remove the newline that is added by fmt_merge_msg(),\n> > not that there is no newline added by fmt_merge_msg() - maybe I wasn't\n> > clear enough. In builtin/merge.c:prepare_merge_message() we do\n> > \n> > \tfmt_merge_msg(merge_names, merge_msg, &opts);\n> > \tif (merge_msg->len)\n> > \t\n> > \t\tstrbuf_setlen(merge_msg, merge_msg->len - 1);\n> \n> Of maybe I didn't read carefully enough. :)\n> \n> Either way, thanks for clarifying. Doing something like:\n> \n> cat >.git/hooks/commit-msg <<\\EOF\n> #!/bin/sh\n> xxd \"$1\"\n> EOF\n> chmod +x .git/hooks/commit-msg\n> \n> git merge --no-edit ...\n> \n> shows off the problem; the hook sees that intermediate state.\n> \n> Likewise if we do:\n> \n> git merge -m \"foo\" ...\n> \n> which similarly suppresses the editor. There are actually two\n> interesting cases here:\n> \n> - if merge.log is not set, then we'd see \"foo\" with no newline\n> \n> - if it is set, we'll get a newline after \"foo\", but with no newline\n> after the log data\n> \n> Likewise for:\n> \n> printf foo >no-newline\n> git merge -F no-newline ...\n> \n> So I think we'd probably want to see a 3-patch series:\n> \n> 1. Make interpret-trailers handle input missing the final newline.\n> This isn't strictly necessary after patches 2 and 3, but it makes\n> sense to be more robust with unexpected input.\n> \n> 2. Drop the newline-stripping from prepare_merge_message(). The\n> examples above show some ways we could cover this in the tests.\n> This will help --no-edit case, but also using merge.log with \"-m\"\n> or \"-F\".\n> \n> 3. Teach prepare_to_commit() to add the extra newline before letting\n> hooks see the message. This should probably be done only when\n> cleanup_mode != COMMIT_MSG_CLEANUP_NONE.\n> \n> Luca, do you want to try revising your series in that direction?\n> \n> -Peff\n\nHi Peff,\n\nif you have a good idea on how to create these patches, feel free to do so.\nIf not, I can take a shot at it this or next week.\n\nRegards\nLuca\n\n\n"},{"id":"433857","messageId":"2333170.ZUCfCSpfzX@g550jk","threadId":"56115","inReplyTo":"YPMXqLESKApq2uPI@coredump.intra.peff.net","subject":"Re: [PATCH 2/2] merge: make sure to terminate message with newline","fromName":"Luca Weiss","fromEmail":"luca@z3ntu.xyz","sentAt":"2021-08-26T18:32:57Z","receivedAt":"2021-08-26T18:40:16Z","isPatch":true,"sender":{"key":"luca@z3ntu.xyz","avatar":null},"body":"Hi Peff and others,\n\nOn Samstag, 17. Juli 2021 19:47:20 CEST Jeff King wrote:\n> [cut]\n> So I think we'd probably want to see a 3-patch series:\n> \n> 1. Make interpret-trailers handle input missing the final newline.\n> This isn't strictly necessary after patches 2 and 3, but it makes\n> sense to be more robust with unexpected input.\n> \n> 2. Drop the newline-stripping from prepare_merge_message(). The\n> examples above show some ways we could cover this in the tests.\n> This will help --no-edit case, but also using merge.log with \"-m\"\n> or \"-F\".\n> \n> 3. Teach prepare_to_commit() to add the extra newline before letting\n> hooks see the message. This should probably be done only when\n> cleanup_mode != COMMIT_MSG_CLEANUP_NONE.\n> \n> Luca, do you want to try revising your series in that direction?\n> \n> -Peff\n\nI haven't found time to revisit the patches yet but I have found another \nunexpected behavior with git merge that is shown with these commands:\n\n# git needs to be set up to sign commits with gpg\nmkdir /tmp/test\ncd /tmp/test\ngit init\ngit commit --allow-empty -m \"Foo1\"\ngit commit --allow-empty -m \"Foo2\"\ngit tag -s tag_foo -m \"foooo!\"\ngit checkout HEAD~1\ngit merge --no-ff --no-edit --log -m \"Merge tag_foo\" tag_foo\n# git show\n\nThere are two problems with the resulting commit message:\n* The newline between -m message and tag message is missing\n* There's a big pgp signature (BEGIN PGP SIGNATURE) block included in the \ncommit message. When using an editor this would be removed because the text \nstarts with comments.\n\nI don't think I have enough insight into where to fix this so I'd appreciate if \nsomebody else could fix it :)\n\nRegards\nLuca\n\n\n"}]}