Re: [PATCH] archive: let remote clients get reachable commits

From: Jeff King <peff@peff.net>
Date: Feb 22, 2013, 18:26 UTC
Message-ID: <20130222182654.GA18934@sigill.intra.peff.net>
In-Reply-To: <7vfw0odxz3.fsf@alter.siamese.dyndns.org>

On Fri, Feb 22, 2013 at 10:06:56AM -0800, Junio C Hamano wrote:

Show 7 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > How are you proposing to verify master~12 in that example? Because
> > during parsing, it starts with "master", and we remember that?
> 
> By not cheating (i.e. using get_sha1()), but making sure you can
> parse "master" and the adornment on it "~12" is something sane.

So, like these patches:

  http://article.gmane.org/gmane.comp.version-control.git/188386

  http://article.gmane.org/gmane.comp.version-control.git/188387

? They do not allow arbitrary sha1s that happen to point to branch tips, but I am not sure whether that is something people care about or not.

> That is why I said "this is harder than one would naively think, but
> limiting will make it significantly easier".  I didn't say that it
> would become "trivial", did I?

I'm not implying it would be trivial. It was an honest question, since you did not seem to want to do the pass-more-information-out-of-get-sha1 approach last time this came up.

Even though those patches above are from me, I've come to the conclusion that the best thing to do is to harmonize with upload-pack. Then you never have the "well, but I could fetch it, so why won't upload-archive let me get it" argument. Something like:

  1. split name at first colon (like we already do)

  2. make sure the left-hand side is reachable according to the same
     rules that upload-pack uses. Right we just say "is it a ref". It
     should be:

      2a. if it is a commit-ish, is it reachable from a ref?

      2b. otherwise, is it pointed to directly by a ref?

  3. Abort if it's not reachable. Abort if it's not a tree-ish. No
     checks necessary on the right-hand side, because a path lookup in a
     tree-ish is always reachable from the tree-ish. I.e., the same rule
     we have now.

I did not check if upload-pack will respect a "want" line for an object accessible only by peeling a tag. But an obvious 2c could be "is it accessible by peeling the refs?"

That leaves the only inaccessible thing as direct-sha1s of trees and blobs that are reachable from commits. But you also cannot ask for those directly via upload-pack, and I do not think it's worth it to do the much more expensive reachability check to verify those (OTOH, it is no more expensive than the current "counting objects" for a clone, and we could do it only as a fallback when cheaper checks do not work).

-Peff