All of lore.kernel.org
 help / color / mirror / Atom feed
From: Phillip Wood <phillip.wood123@gmail.com>
To: "SZEDER Gábor" <szeder.dev@gmail.com>, git@vger.kernel.org
Subject: Re: [BUG] 'git rebase -i ---rebase-merges' can leave MERGE_MSG behind
Date: Sun, 1 Aug 2021 10:56:17 +0100	[thread overview]
Message-ID: <80afcf4f-fc5a-7722-1a4d-c3e1f1de1e09@gmail.com> (raw)
In-Reply-To: <20210731062322.GB23408@szeder.dev>

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.

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
> 

  reply	other threads:[~2021-08-01  9:57 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-31  6:23 [BUG] 'git rebase -i ---rebase-merges' can leave MERGE_MSG behind SZEDER Gábor
2021-08-01  9:56 ` Phillip Wood [this message]
2021-08-05 12:58   ` Phillip Wood

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=80afcf4f-fc5a-7722-1a4d-c3e1f1de1e09@gmail.com \
    --to=phillip.wood123@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=szeder.dev@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.