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

From: Jeff King <peff@peff.net>
Date: May 13, 2017, 08:30 UTC
Message-ID: <20170513083012.lcs47tz3xb7avl45@sigill.intra.peff.net>
In-Reply-To: <CAGf8dgLcCeMYGPF1PSPy5M5zLvj2hb_EfpDBPbcNe+96c9YpRA@mail.gmail.com>

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

Show 12 quoted lines

> > @@ -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