Re: [PATCH] merge-tree: sometimes, d/f conflict is not an issue

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi,

On Sat, 7 Jul 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > @@ -643,6 +643,20 @@ int threeway_merge(struct cache_entry **stages,
> >  	index = stages[0];
> >  	head = stages[o->head_idx];
> >  
> > +	/*
> > +	 * Special case: do not care about a dir/file conflict, when
> > +	 * the entries have not been touched.
> > +	 * IOW if the ancestors are identical to the remote, and the
> > +	 * index is the same as head, just take head.
> > +	 */
> 
> Suppose paths "A" and "A/B" are involved, and you resolved with
> this logic to have "A" as a blob (so your HEAD does not have
> "A/B").  If the remote adds "A/B", what prevents the resulting
> index to have both "A" and "A/B" resolved at stage #0?

Hmm.

> A logic to do "if it is unchanged on one and changed in another,
> take changed one" already exists in later part of the code; your
> patch just circumvents D/F checks built into threeway_merge for
> this one case, and only because this one case happens to have
> reported.  It doesn't feel right.

Well, for me the code in threeway_merge does not feel right. There is a 
table in technical/trivial-merge.txt (which I not fully understand, since 
nowhere in the table, there is a mention of the index, and nowhere is 
explained what "ALT" is supposed to mean), but threeway_merge only 
references the numbers. The code is not obvious to me.

I spent some hours staring on, and trying to make sense of it. Alas, I am 
an idiot or something, since my brain feels like a mashed potato, and I 
still do not understand the code. For example, it is not apparent to me 
why the variable "head" should be set to NULL, when it was the 
df_conflict_entry.

So yeah, this patch was marked as "PATCH", but the subject line is not 
long enough for the proper prefix, which would have started like 
"[This PATCH works for me, and I do not know how to make it better, since 
I do not understand the code in threeway_merge(), and that does not make 
me happy, but that is the way things are, and maybe someone more 
intelligent than me recognizes what is meant by my little patch, and can 
fix it up, ...]".

> IOW, don't make unpack-trees to make policy decisions on final 
> resolution, unless it is operating under aggressive rule (where the 
> caller explicitly allows it to make more than the "trivial" decisions).  
> The caller (in this case, merge-recursive) should see A at stage #2 with 
> A/B at stages #1 and #3 and decide what to do.

Okay, so you're saying that merge-recursive should use the aggressive 
strategy?

Ciao,
Dscho