Re: [PATCH] builtin-clone: fix for new unpack_trees() semantics

[Daniel Barkalow <barkalow@iabervon.org>]

On Mon, 10 Mar 2008, Johannes Schindelin wrote:

> Hi,
> 
> On Sun, 9 Mar 2008, Daniel Barkalow wrote:
> 
> > On Sun, 9 Mar 2008, Johannes Schindelin wrote:
> > 
> > > On Sun, 9 Mar 2008, Daniel Barkalow wrote:
> > > 
> > > > On Sun, 9 Mar 2008, Johannes Schindelin wrote:
> > > > 
> > > > > In git.git's "next" branch, unpack_trees() must specify source and target 
> > > > > index.
> > > > > 
> > > > > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > > > > ---
> > > > > 
> > > > > 	To be squashed into 10/11
> > > > > 
> > > > >  builtin-clone.c |    2 ++
> > > > >  1 files changed, 2 insertions(+), 0 deletions(-)
> > > > > 
> > > > > diff --git a/builtin-clone.c b/builtin-clone.c
> > > > > index e4047ed..3890e12 100644
> > > > > --- a/builtin-clone.c
> > > > > +++ b/builtin-clone.c
> > > > > @@ -534,6 +534,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
> > > > >  		opts.verbose_update = !option_quiet;
> > > > >  		opts.merge = 1;
> > > > >  		opts.fn = twoway_merge;
> > > > > +		opts.src_index = &the_index;
> > > > > +		opts.dst_index = &the_index;
> > > > 
> > > > Actually, I think the sensible thing is to just not do a merge here, since 
> > > > we know there's no index beforehand and the two trees are the same.
> > > > 
> > > > I think the odd twoway merge of two copies of HEAD is just an artifact of 
> > > > clone originally just doing "git checkout HEAD", and that got translated 
> > > > various times failing to notice the special cases.
> > > 
> > > Okay, but would oneway_merge not want to write the index, too (rightfully 
> > > so)?
> > 
> > I'm thinking:
> > 
> >   memset(&opts, 0, sizeof opts);
> >   opts.update = 1;
> >   opts.verbose_update = !option_quiet;
> >   opts.dst_index = &the_index;
> > 
> >   init_tree_desc(&t[0], tree->buffer, tree->size);
> >   unpack_trees(1, t, &opts);
> > 
> > That is, write it, but not read it, and only have one tree.
> 
> Yes.  And this sets dst_index (what I tried to hint at with my patch).

Oh, yes. Your patch is what prompted me to fix that part (and to find that 
Linus's unpack_trees was in next now). But seeing src_index in your patch 
made me wonder what it wanted a source index for anyway, since this is the 
first index we've had in this repo.

	-Daniel
*This .sig left intentionally blank*