On 6/29/2019 3:43 AM, Jeff King wrote: > On Fri, Jun 28, 2019 at 08:51:04AM -0400, Derrick Stolee wrote: > >> 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 So this is an example where the alternate refs are being used without any network activity. >> 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. :) Just double-checking that it is appropriate for revision.c to take dependence on transport.h instead of moving the alternate ref stuff into a different header file. I trust your opinion. -Stolee
