Re: [PATCH v5] fetch-pack: always allow fetching of literal SHA1s

[Jeff King <peff@peff.net>]

On Fri, May 12, 2017 at 11:00:36AM -0700, Jonathan Tan wrote:

> > @@ -617,7 +632,8 @@ static void filter_refs(struct fetch_pack_args *args,
> >                                 else if (cmp == 0) {
> >                                         keep = 1; /* definitely have it */
> >                                         sought[i]->match_status = REF_MATCHED;
> > -                               }
> > +                               } else if (is_literal_sha1(sought[i]))
> > +                                       seeking_raw_oid = 1;
> 
> As far as I can tell, this seems to coincidentally work because SHA-1s
> as strings compare less than any ref name (HEAD or refs/...) - if this
> weren't the case, the "break" statement above might cause this line to
> never be executed. I'm not sure if we want to rely on that.

Right, I was thinking we'd hit all of the sought refs in this loop, but
we don't necessarily (if we run out of remote refs to compare to).
Finishing it off with:

  /* scan the remainder of the requests */
  for (; i < nr_sought; i++)
	if (is_literal_sha1(...))

though it would probably be less confusing to just do a separate loop
after the original is done.

Anyway, it looks like your v6 just keeps the lazy approach, so this is
all moot.

-Peff