From: Junio C Hamano <gitster@pobox.com>
To: Alex Riesen <raa.lkml@gmail.com>
Cc: "Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
"Anders Melchiorsen" <mail@cup.kalibalik.dk>,
git@vger.kernel.org, "Samuel Tardieu" <sam@rfc1149.net>,
"Linus Torvalds" <torvalds@linux-foundation.org>,
"Junio C Hamano" <gitster@pobox.com>,
"SZEDER Gábor" <szeder@ira.uka.de>
Subject: Re: [PATCH] Fix for a merge where a branch has an F->D transition
Date: Tue, 12 May 2009 23:19:47 -0700 [thread overview]
Message-ID: <7v63g57ce4.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <20090511192536.GA1485@blimp.localdomain> (Alex Riesen's message of "Mon\, 11 May 2009 21\:25\:36 +0200")
Alex Riesen <raa.lkml@gmail.com> writes:
> Frankly, I'm not really sure. The solution came largely ... empirical
> way. IOW, I tried more or less random things which looked like they
> should fix the problem. So a review is very much appreciated. Please.
I've always thought that D/F conflict logic in merge-recursive is placed
at the wrong processing phase. IIRC, it enumerates potential D/F
conflicting paths before even attempting to process renames, and it is
easy to miss a path that was previously file going away as the result of a
clean merge (in which case it is ok to have a directory there as the
result of a merge for other paths). This breakage could be a small
example of it.
Regardless, I think your patch is a reasonable fix to go to the
maintenance track. Thanks for looking into it.
> merge-recursive.c | 5 +++--
> 1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/merge-recursive.c b/merge-recursive.c
> index a3721ef..3c5420b 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -980,14 +980,15 @@ static int process_renames(struct merge_options *o,
>
> if (mfi.clean &&
> sha_eq(mfi.sha, ren1->pair->two->sha1) &&
> - mfi.mode == ren1->pair->two->mode)
> + mfi.mode == ren1->pair->two->mode) {
> /*
> * This messaged is part of
> * t6022 test. If you change
> * it update the test too.
> */
> output(o, 3, "Skipped %s (merged same as existing)", ren1_dst);
> - else {
> + ren1->dst_entry->processed = 0;
> + } else {
> if (mfi.merge || !mfi.clean)
> output(o, 1, "Renaming %s => %s", ren1_src, ren1_dst);
> if (mfi.merge)
> --
> 1.6.3.28.ga852b
next prev parent reply other threads:[~2009-05-13 6:19 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-05-11 9:42 [PATCH] Add a reminder test case for a merge with F/D transition Alex Riesen
2009-05-11 19:25 ` [PATCH] Fix for a merge where a branch has an F->D transition Alex Riesen
2009-05-13 6:19 ` Junio C Hamano [this message]
2009-05-13 6:38 ` Alex Riesen
2009-05-13 9:59 ` Johannes Schindelin
2009-05-13 11:33 ` Alex Riesen
2009-05-20 10:34 ` Johannes Schindelin
2009-05-20 22:09 ` Alex Riesen
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=7v63g57ce4.fsf@alter.siamese.dyndns.org \
--to=gitster@pobox.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=mail@cup.kalibalik.dk \
--cc=raa.lkml@gmail.com \
--cc=sam@rfc1149.net \
--cc=szeder@ira.uka.de \
--cc=torvalds@linux-foundation.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).