Re: [PATCH 2/2] merge: make sure to terminate message with newline

From: Luca Weiss <luca@z3ntu.xyz>
Date: Jul 16, 2021, 12:37 UTC
Message-ID: <AB048897-F70A-4388-B2A6-56BFEA40B303@z3ntu.xyz>
In-Reply-To: <8678772b-dd5d-9188-8b63-98d05cedb323@gmail.com>

Hi Phillip,

So the behavior that I have observed is the following:

I've added a usleep for ~10 seconds instead of the line I added and .git/MERGE_MSG was not terminated with a newline. This didn't change when using --log It was properly handled with --signoff and the trailer was added correctly.

I have a simple reproducer here:

mkdir /tmp/test cd /tmp/test git init echo 'dest="$1.tmp"; git interpret-trailers --trailer "Foo: Bar" < "$1" > "${dest}"; mv "${dest}" "$1"' > .git/hooks/commit-msg chmod +x .git/hooks/commit-msg git commit --allow-empty -m "Initial commit" sleep 1 git switch -c foobar git commit --allow-empty -m "Foo1" sleep 1 git commit --allow-empty -m "Foo2" git switch master git merge --no-ff --no-edit foobar # look at merge commit message now

With my patch(es) this works properly.

If you have any other ideas on how to fix this, I am open for suggestions :)

Otherwise I'll try to add more detail to the individual commit messages (I deliberately kept the "unnecessary" detail out of the actual commit messages before).

Regards Luca

p.s. sorry for replying off-list, and if this is wrongly bottom/top posted, email client troubles ;)

On July 16, 2021 12:23:06 PM GMT+02:00, Phillip Wood <phillip.wood123@gmail.com> wrote:

Show 45 quoted lines

>
>Hi Luca
>
>Thanks for your patches. It would be very helpful to have the 
>explanation from the cover letter in the commit messages for both 
>commits to explain why this change is being made, otherwise that 
>information will not appear in the history.
>
>The cover letter says this happened when using '--no-edit', but unless 
>I've missed something 'git merge --no-edit' creates its message using 
>fmt_merge_msg() which calls strbuf_complete_line() just before it 
>returns. append_signoff() and 'merge -m' always terminate the message 
>with a newline. The only path I found that does not ensure the message 
>ends with a newline before calling the prepare-commit-msg hook is when 
>using '-F' and I suspect that may have been a deliberate decision but it 
>could be an oversight. In any case we would want to make sure that 'git 
>commit -F' and 'git merge -F' to behave the same which I think they do 
>at the moment.
>
>Best Wishes
>
>Phillip
>
>On 16/07/2021 08:43, Luca Weiss via GitGitGadget wrote:
>> From: Luca Weiss <luca@z3ntu.xyz>
>> 
>> Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
>> ---
>>   builtin/merge.c | 1 +
>>   1 file changed, 1 insertion(+)
>> 
>> diff --git a/builtin/merge.c b/builtin/merge.c
>> index a8a843b1f54..646bb49367f 100644
>> --- a/builtin/merge.c
>> +++ b/builtin/merge.c
>> @@ -867,6 +867,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)
>>   	}
>>   	if (signoff)
>>   		append_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0);
>> +	strbuf_complete_line(&msg);
>>   	write_merge_heads(remoteheads);
>>   	write_file_buf(git_path_merge_msg(the_repository), msg.buf, msg.len);
>>   	if (run_commit_hook(0 < option_edit, get_index_file(), "prepare-commit-msg",
>> 
>