Re: [PATCH] check_everything_connected: assume alternate ref tips are valid

From: Jeff King <peff@peff.net>
Date: Jun 29, 2019, 07:43 UTC
Message-ID: <20190629074348.GA5080@sigill.intra.peff.net>
In-Reply-To: <601d8561-6e24-559c-6fbb-fa25a7389fa0@gmail.com>

On Fri, Jun 28, 2019 at 08:51:04AM -0400, Derrick Stolee wrote:

Show 18 quoted lines

> On 6/28/2019 6:11 AM, Jeff King wrote:
> > When we receive a remote ref update to sha1 "X", we want to check that
> > we have all of the objects needed by "X". We can assume that our
> > repository is not currently corrupted, and therefore if we have a ref
> > pointing at "Y", we have all of its objects. So we can stop our
> > traversal from "X" as soon as we hit "Y".
> > 
> > If we make the same non-corruption assumption about any repositories we
> > use to store alternates, then we can also use their ref tips to shorten
> > the traversal.
> 
> I was confused by this paragraph, because I didn't know about
> for_each_alternate_ref() and how refs_From_alternate_cb() will
> strip the "/objects" and append "/refs" to check refs if they
> exist. All of that logic is in transport.c but used by
> fetch-pack.c and builtin/receive-pack.c. But now we are adding
> to revision.c, so the restriction to "this helps data transfer"
> is getting murkier.

Using it for data transfer is still the main thing for our internal calls, but I think it's worth exposing it for general use via rev-list. I imagine it would mostly be for poking around and debugging, but it should allow things like:

  # what do we have that our alternate does not
  git rev-list --all --not --alternate-refs

> Is this something that should be extracted to the object-store
> layer? Or is it so tricky to use that we shouldn't make it too
> easy to fall into a bad pattern?

I'm not sure what you have in mind, exactly. If you are asking whether there are more places that alternate refs could be used, I can't think of any. If you are asking whether this is in the wrong place, no, I think it's the right place. :)

-Peff