Re: [PATCH] Fix for a merge where a branch has an F->D transition

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: May 20, 2009, 10:34 UTC
Message-ID: <alpine.DEB.1.00.0905201228040.16461@intel-tinevez-2-302>
In-Reply-To: <20090511192536.GA1485@blimp.localdomain>

Hi,

On Mon, 11 May 2009, Alex Riesen wrote:

Show 8 quoted lines

> Some path names which transitioned from file to a directory were not
> updated in the final part of the merge (loop around unmerged entries in
> merge_trees), because the branch in process_renames which filtered out
> updates for the files with the same content ("merged same as existing")
> has left the rename entry in processed state. In this case, the
> processing cannot be finished at the process_renames phase (because
> the old file still blocks creation of directory where new files should
> appear), and must be postponed until the update_entry phase.

I know that as a German, I am supposed to like long sentences and crowded paragraphs. Maybe I am not that German after all.

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

This does not give me the cozy feeling I need to review a patch. After all, I do not want to do all the work that should be done by the patch author, but I just want to put in my knowledge to verify that the patch is correct.

But as you asked me explicitely...

Show 19 quoted lines

> 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 {

So basically, you say that a dst_entry has not been processed, when it _has_ been? That cannot be correct...

Ciao, Dscho