Re: [BUG] 'git rebase -i ---rebase-merges' can leave MERGE_MSG behind

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Aug 5, 2021, 12:58 UTC
Message-ID: <3fde8317-e157-f6a9-5954-3623ce0215ab@gmail.com>
In-Reply-To: <80afcf4f-fc5a-7722-1a4d-c3e1f1de1e09@gmail.com>

On 01/08/2021 10:56, Phillip Wood wrote:

Show 21 quoted lines

> On 31/07/2021 07:23, SZEDER Gábor wrote:
>> Hi,
>>
>> 'git rebase -i --rebase-merges' leaves a stray '.git/MERGE_MSG' file
>> behind after it re-created an unchanged merge commit.  The test script
>> below demonstrates this issue (it doesn't fail, but look at its
>> verbose output to see what the issue and its consequences are).
>>
>> This issue is as old as --rebase-merges, I see the same output with
>> v2.18.0, the first release containing this feature.
>>
>> I suspect that fixing this is merely a matter of adding a missing
>> unlink(".git/MERGE_MSG") to the right place, but I won't dive into the
>> sequencer machinery to figure out where that right place might be :)
> 
> Looking at do_merge() in sequencer.c .git/MERGE_MSG is written before it 
> checks if it can fast-forward. If it does fast-forward then 'git commit' 
> is not run and .git/MERGE_MSG is not removed. I think the best way to 
> fix it would be to move the fast-forward code so it comes before the 
> code that writes .git/MERGE_MSG. That way we'll not write the message if 
> we can fast-forward.

I've spotted a couple of other small issues in do_merge(), I'll try and post some patches next week.

Best Wishes

Phillip

Show 54 quoted lines

> Best Wishes
> 
> Phillip
> 
>>
>>    ---  >8  ---
>>
>> #!/bin/sh
>>
>> test_description='test'
>>
>> . ./test-lib.sh
>> . "$TEST_DIRECTORY"/lib-rebase.sh
>>
>> test_expect_success '--rebase-merges leaves MERGE_MSG behind' '
>>     # A-B-M-D-E-F
>>     #  \ /
>>     #   C
>>     test_commit A &&
>>     test_commit B &&
>>     git checkout -b branch HEAD^ &&
>>     test_commit C &&
>>     git checkout master &&
>>     git merge --log branch &&
>>     test_commit D &&
>>     test_commit E &&
>>     test_commit F &&
>>
>>     # Rewrite a commit after the merge:
>>     write_script todo-editor <<-\EOF &&
>>     sed -i -e "/^pick .* E$/ s/^pick/edit/" "$1"
>>     EOF
>>
>>     # No MERGE_MSG present before starting the rebase, good:
>>     test_path_is_missing .git/MERGE_MSG &&
>>
>>     # Start rebasing before the merge, so the sequencer has to
>>     # re-create an identical merge commit.
>>     GIT_EDITOR=./todo-editor git rebase -i --rebase-merges A &&
>>
>>     # Just to make sure that the history so far is unchanged:
>>     test_cmp_rev E HEAD &&
>>
>>     # BUG: now there is a stray MERGE_MSG file:
>>     cat .git/MERGE_MSG &&
>>
>>     # And it interferes with the next "git commit", because its
>>     # content is included in the commit message template:
>>     echo foo >>E.t &&
>>     GIT_EDITOR=cat git commit -a
>> '
>>
>> test_done
>>