Re: [PATCH] fetch-pack: always allow fetching of literal SHA1s
- From
Shawn Pearce <spearce@spearce.org>
- Date
- May 10, 2017, 04:57 UTC
- Message-ID
- <CAJo=hJuaPhxQwD+DdMcP_21j61KR_GY=P+tHrYScgzZFrtemEA@mail.gmail.com>
- In-Reply-To
- <20170510043343.mgb7heqzu2etcgvf@sigill.intra.peff.net>
On Tue, May 9, 2017 at 9:33 PM, Jeff King <peff@peff.net> wrote:
Show 40 quoted lines
> 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. :)