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

[Shawn Pearce <spearce@spearce.org>]

On Tue, May 9, 2017 at 9:33 PM, Jeff King <peff@peff.net> wrote:
> On Tue, May 09, 2017 at 09:22:11PM -0700, Shawn Pearce wrote:
>
>> > Hmm. That makes sense generally, as the request should succeed. But it
>> > seems like we're creating a client that will sometimes succeed and
>> > sometimes fail, and the reasoning will be somewhat opaque to the user.
>> > I have a feeling I'm missing some context on when you'd expect this to
>> > kick in.
>>
>> Specifically, someone I know was looking at building an application
>> that is passed only a SHA-1 for the tip of a ref on a popular hosting
>> site[1]. They wanted to run `git fetch URL SHA1`, but this failed
>> because the site doesn't have upload.allowtipsha1inwant enabled.
>> However the SHA1 was clearly in the output of git ls-remote.
>
> OK. So this is basically a case where we expect that the user knows what
> they're doing.
>
>> For various reasons they expected this to work, because it works
>> against other sites that do have upload.allowtipsha1inwant enabled.
>> And I personally just expected it to work because the fetch client
>> accepts SHA-1s, and the wire protocol uses "want SHA1" not "want ref",
>> and the SHA-1 passed on the command line was currently in the
>> advertisement when the connection opened, so its certainly something
>> the server is currently willing to serve.
>
> Right, makes sense.  I wondered if GitHub should be turning on
> allowTipSHA1InWant, but it really doesn't make sense to. We _do_ hide
> some internal refs[1], and they're things that users wouldn't want to
> fetch. The problem for your case really is just on the client side, and
> this patch fixes it.
>
> Some of this context might be useful in the commit message.
>
> -Peff
>
> [1] The reachability checks from upload-pack don't actually do much on
>     GitHub, because you can generally access the objects via the API or
>     the web site anyway. So I'm not really opposed to turning on
>     allowTipSHA1InWant if it would be useful for users, but after
>     Jonathan's patch I don't see how it would be.

Right, I agree. That is why we proposed this patch to the client,
rather than a support request with GitHub to change server
configuration. :)