Re: libreoffice merge issue ...

From: Jeff King <peff@peff.net>
Date: Apr 29, 2011, 12:55 UTC
Message-ID: <20110429125505.GA4540@sigill.intra.peff.net>
In-Reply-To: <20110421140132.GA6696@brouette>

On Thu, Apr 21, 2011 at 04:01:32PM +0200, Damien Wyart wrote:

Show 21 quoted lines

> * Junio C Hamano <gitster@pobox.com> [2011-02-16 13:30]:
> > Yeah, the reverted 83c9031 (unpack_trees(): skip trees that are the
> > same in all input, 2010-12-22) also seems to have seriously broken
> > intermediate merge merge-recursive makes. I actually recall scratching
> > my head when I made 00e6ee7 (Merge branch 'maint', 2011-02-11) that
> > was causing add/add conflict when it shouldn't. It turns out that
> > quite a lot of entries were missing in contrib/ area from the virtual
> > common ancestry tree synthesized by merge-recursive that called into
> > the botched unpack_trees()---it of course would result in add/add
> > conflict if a merge is done using such a tree as the common.
> 
> > No, I haven't had a chance nor energy to dig further than what
> > I reported above.
> 
> Out of curiosity, I would like to know if digging further into this
> issue is still on your TODO list. I feel understanding exactly what was
> wrong in 83c9031 would be interesting ; having just the revert is a bit
> frustrating.
> 
> The initial optimization in 83c9031 seemed right at first glance, so
> I would be interesting in having a more final answer to this.

I didn't dig further, but looking at 83c9031 with a fresh set of eyes, I think that merge-recursive falls under the "exceptions" that Junio listed in the commit message. That is, he indicates correctly that we cannot use this optimization for "reset --hard" or for checking out for the first time.

Similarly, merge-recursive is going to do many 3-way merges between trees into an empty index (when merging virtual ancestors), and we need to unpack those subtrees even if they are all the same. But given the conditions added by 83c9031 in unpack-trees.c:fast_forward_merge, I don't see anything preventing the optimization in how merge-recursive calls into unpack-trees.

We do a discard_cache() right before doing the 3-way merge for virtual ancestors. So we will see that there are no entries in our current index for that directory. That may be a clue that the optimization should not be used.

-Peff