On Sun, 23 Aug 2009, Junio C Hamano wrote:
> Nicolas Pitre <nico@xxxxxxx> 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@xxxxxxx>
> > ---
>
> 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.
> In other words, what breaks (not necessarily in the correctness sense, but
> also in the performance sense) if we get rid of this filtering altogether?
If you really want to get rid of that filtering, I'd still do it in a
separate patch. That way if any issue appears because of that then
bissection will point directly to that removal alone.
Nicolas
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html