Re: [PATCH] cache-tree: populate cache-tree on successful merge

[David Turner <dturner@twopensource.com>]

On Tue, 2015-07-28 at 13:47 -0700, Junio C Hamano wrote:
> David Turner <dturner@twopensource.com> writes:
> 
> > When we unpack trees into an existing index, we discard the old index
> > and replace it with the new, merged index.  Ensure that this index has
> > its cache-tree populated.  This will make subsequent git status and
> > commit commands faster.
> >
> > Signed-off-by: David Turner <dturner@twopensource.com>
> > Signed-off-by: Brian Degenhardt <bmd@bmdhacks.com>
> > ---
> >
> > This patch is by my colleague, Brian Degenhardt (as part of his work
> > on git at Twitter).  I'm sending it with his and Twitter's approval.
> 
> I'd need to tweak the From:/Author: line then, and flip the order of
> the sign-off, as Brian wrote and signed off then David relayed (as
> attached).

Where do I put an Author: line? In the commit message above the
signoffs?  As an email header?  I didn't see an option to git send-email
that would do this.  I don't want to use the From: header because I want
to be the point-of-contact for these patches.

> > diff --git a/unpack-trees.c b/unpack-trees.c
> > index 2927660..befc247 100644
> > --- a/unpack-trees.c
> > +++ b/unpack-trees.c
> > @@ -1156,6 +1156,13 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
> >  	o->src_index = NULL;
> >  	ret = check_updates(o) ? (-2) : 0;
> >  	if (o->dst_index) {
> > +		if (!o->result.cache_tree)
> > +			o->result.cache_tree = cache_tree();
> > +
> > +		if (!cache_tree_fully_valid(o->result.cache_tree)) {
> > +			cache_tree_update(&o->result, WRITE_TREE_SILENT | WRITE_TREE_REPAIR);
> > +		}
> 
> This does the cache-tree thing unconditionally, not "on successful
> merge".  cache_tree_update() would refuse when it sees an unmerged
> entry, but somehow the discrepancy between the title and the code
> bothers me.
> 
> By the way, I wonder if we can lose/revert aecf567c (cache-tree:
> create/update cache-tree on checkout, 2014-07-05), now the
> underlying unpack_trees() does the necessary cache_tree_update()
> when a branch is checked out.

Well, the tests still pass, so I guess so. That is, we still need the
WRITE_TREE_REPAIR bit, but not the update check.

Will re-roll once I hear back on the author line.