Re: [PATCH] fix simple deepening of a repo

> On Sun, 23 Aug 2009, Junio C Hamano wrote:
> > Nicolas Pitre <nico@cam.org> writes:
> > > If all refs sent by the remote repo during a fetch are reachable
> > > locally, then no further conversation is performed with the
> > > remote. This check is skipped when the --depth argument is
> > > provided to allow the deepening of a shallow clone which
> > > corresponding remote repo has no changed.
> > >
> > > However, some additional filtering was added in commit c29727d5
> > > to remove those refs which are equal on both sides.  If the
> > > remote repo has not changed, then the list of refs to give the
> > > remote process becomes empty and simply attempting to deepen a
> > > shallow repo always fails.
> > >
> > > Let's stop being smart in that case and simply send the whole
> > > list over when that condition is met.  The remote will do the
> > > right thing anyways.
> > >
> > > Test cases for this issue are also provided.
> > >
> > > Signed-off-by: Nicolas Pitre <nico@cam.org>
> > > ---
> >
> > Thanks.  The fix looks correct (as usual with patches from you).
> >
> > But it makes me wonder if this logic to filter refs is buying us
> > anything.
> >
> > >  	for (rm = refs; rm; rm = rm->next) {
> > > +		nr_refs++;
> > >  		if (rm->peer_ref &&
> > >  		    !hashcmp(rm->peer_ref->old_sha1, rm->old_sha1))
> > >  			continue;
> >
> > 		ALLOC_GROW(heads, nr_heads + 1, nr_alloc);
> > 		heads[nr_heads++] = rm;
> > 	}
> >
> > What is the point of not asking for the refs that we know are the
> > same?
>
> I could see the advantage if the number of refs is really huge. 
> Wasn't some converted repositories producing a lot of branches and/or
> tags significantly slowing down a fetch operation?  Granted that was
> long ago when that issue got "fixed" so the details are fuzzy to me.