Re: [PATCH 0/2] Reorganize read-tree

[Daniel Barkalow <barkalow@iabervon.org>]

On Sun, 4 Sep 2005, Junio C Hamano wrote:

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > I got mostly done with this before Linus mentioned the possibility of
> > having multiple index entries in the same stage for a single path. I
> > finished it anyway, but I'm not sure that we won't want to know which of
> > the common ancestors contributed which, and, if some of them don't have a
> > path, we wouldn't be able to tell. The other advantages I see to this
> > approach are:
> 
> I've finished reading your patch, after beating it reasonably
> heavily by feeding combinations of nonsense trees to make sure
> it produces the same result as the original implementation.  I
> have not found any regression from the read-tree in "master"
> branch, after you fixed the path ordering issues.

Good.

> > There are various potential refinements, plus removing a bunch of memory
> > leaks, still to do, but I think this is sufficiently close to review.
> 
> I am not so worried about the leaks right now; they are
> something that could be fixed before it hits the "master"
> branch.

Right.

> I like your approach of reading the input trees, along with the
> existing index contents, and re-populating the index one path at
> a time.  It probably is more readable.
> 
> I further think that you can get the best of both worlds, by
> inventing a convention that mode=0 entry means 'this path does
> not exist in this tree'. This would allow you to have multiple
> entries at the same stage and still tell which one came from
> which tree.  Instead of calling fn in unpack_trees(), you could
> make it only unpack the tree into the index, and then after
> unpacking is done, call fn() repeatedly to resolve the resulting
> index. 

I think that almost all of the benefit actually comes from calling fn() in 
unpack_trees() and not putting anything in the index before merging. 
Without that, you need the complex index management and the complicated 
search for DF conflicts. The main point of not reading everything into the 
index before calling fn() on stuff is that the index is actually really 
difficult to deal with in this situation (because you are simultaneously 
moving through it, removing and modifying entries, and searching it for 
conflicts). The improvement in readability comes from not doing this.

	-Daniel
*This .sig left intentionally blank*