Re: [PATCH] Fix for a merge where a branch has an F->D transition
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 13, 2009, 06:19 UTC
- Message-ID
- <7v63g57ce4.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <20090511192536.GA1485@blimp.localdomain>
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.
Show 27 quoted lines
> 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