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

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 22, 2013, 19:15 UTC
Message-ID: <7v38wodusp.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20130222182654.GA18934@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 26 quoted lines

> On Fri, Feb 22, 2013 at 10:06:56AM -0800, Junio C Hamano wrote:
>
>> 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.

That does not match my recollection ($gmane/188427). In fact, I had those two patches you quoted earlier in mind when I wrote the "limit it and it will become easier" response.

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

That sounds like a sensible yardstick.

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

Well, "upload-pack" under the hood operates on a bare 40-hex. If you mean to do "split name at first colon, run get_sha1() on the LHS" in step 1., I would agree this is a good direction to go.

>      Right we just say "is it a ref". It should be:

With s/should/could optionally/, I would agree.

> That leaves the only inaccessible thing as direct-sha1s of trees and
> blobs that are reachable from commits.

Yes (with s/are reachable/are only reachable/).