git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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

  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).