All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brandon Williams <bmwill@google.com>
To: Jeff King <peff@peff.net>
Cc: Jonathan Tan <jonathantanmy@google.com>,
	git@vger.kernel.org, jrnieder@gmail.com, spearce@spearce.org
Subject: Re: [PATCH v3] fetch-pack: always allow fetching of literal SHA1s
Date: Thu, 11 May 2017 10:00:50 -0700	[thread overview]
Message-ID: <20170511170050.GA83655@google.com> (raw)
In-Reply-To: <20170511100553.ptyazchujgdfxotl@sigill.intra.peff.net>

On 05/11, Jeff King wrote:
> On Wed, May 10, 2017 at 03:11:57PM -0700, Jonathan Tan wrote:
> 
> > fetch-pack, when fetching a literal SHA-1 from a server that is not
> > configured with uploadpack.allowtipsha1inwant (or similar), always
> > returns an error message of the form "Server does not allow request for
> > unadvertised object %s". However, it is sometimes the case that such
> > object is advertised. This situation would occur, for example, if a user
> > or a script was provided a SHA-1 instead of a branch or tag name for
> > fetching, and wanted to invoke "git fetch" or "git fetch-pack" using
> > that SHA-1.
> > 
> > Teach fetch-pack to also check the SHA-1s of the refs in the received
> > ref advertisement if a literal SHA-1 was given by the user.
> 
> Stepping back a bit, what does this mean for a world where we implement
> protocol extensions to let the client specify a set of refspecs to limit
> the advertisement?
> 
> If we give the server our usual set of fetch refspecs, then we might
> fail to fulfill a request that would have been advertised outside of
> that set. And the behavior is confusing and non-transparent to the user.
> I don't think that really makes sense, though; the advertisement we ask
> for from the server should include only the bits we're interested in for
> _this_ fetch.
> 
> If we tell the server "we are interested in abcd1234", then it's not
> going to find any matching ref by name, obviously. So should servers
> then treat 40-hex names in the incoming refspecs as a request to show
> any names which have a matching sha1? That works against any server-side
> optimizations to avoid looking at the complete set of refs, but it would
> only have to kick in when the user actually specified a single SHA-1
> (and even then only when allowAnySHA1 isn't on). So that's probably
> workable.
> 
> None of this is your problem now either way; the advertisement-limiting
> extension is still vaporware, albeit one we've discussed a lot. I just
> wanted to make sure we weren't painting ourselves into any corners. And
> I think this case could probably be handled.

I can't remember, is there any reason why it is still vaporware? As in
what is holding us back from doing the advertisement-limiting (or doing
away with the initial ref  advertisement)?

-- 
Brandon Williams

  reply	other threads:[~2017-05-11 17:01 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-09 18:20 [PATCH] fetch-pack: always allow fetching of literal SHA1s Jonathan Tan
2017-05-09 22:16 ` Jeff King
2017-05-10  4:22   ` Shawn Pearce
2017-05-10  4:33     ` Jeff King
2017-05-10  4:46       ` Mike Hommey
2017-05-10 17:50         ` Ævar Arnfjörð Bjarmason
2017-05-10 18:20           ` Jonathan Nieder
2017-05-10 18:48             ` Martin Fick
2017-05-10 18:54               ` Jonathan Nieder
2017-05-10  4:57       ` Shawn Pearce
2017-05-10 17:00       ` Jonathan Nieder
2017-05-10 18:55         ` Sebastian Schuberth
2017-05-11  9:59         ` Jeff King
2017-05-11 19:03           ` Jonathan Nieder
2017-05-11 21:04             ` Jeff King
2017-05-10 16:44 ` [PATCH v2] " Jonathan Tan
2017-05-10 18:01   ` Jonathan Nieder
2017-05-10 22:11 ` [PATCH v3] " Jonathan Tan
2017-05-10 23:22   ` Jonathan Nieder
2017-05-11  9:46   ` Jeff King
2017-05-11 17:51     ` Jonathan Tan
2017-05-11 20:52       ` Jeff King
2017-05-11 10:05   ` Jeff King
2017-05-11 17:00     ` Brandon Williams [this message]
2017-05-13  9:29       ` Jeff King
2017-05-11 21:14 ` [PATCH v4] " Jonathan Tan
2017-05-11 21:35   ` Jonathan Nieder
2017-05-11 21:59     ` Jeff King
2017-05-11 22:30 ` [PATCH v5] " Jonathan Tan
2017-05-11 22:46   ` Jonathan Nieder
2017-05-12  2:59     ` Jeff King
2017-05-12  6:01     ` Junio C Hamano
2017-05-12  7:59       ` Jeff King
2017-05-12  8:14         ` Jeff King
2017-05-12 18:00           ` Jonathan Tan
2017-05-13  8:30             ` Jeff King
2017-05-12 18:09         ` Jonathan Tan
2017-05-12 19:06           ` Jonathan Nieder
2017-05-12  3:06   ` Jeff King
2017-05-12 20:45 ` Jonathan Tan
2017-05-12 20:46 ` [PATCH v6] " Jonathan Tan
2017-05-12 22:28   ` Jonathan Nieder
2017-05-13  8:36   ` Jeff King
2017-05-15  1:26     ` Junio C Hamano
2017-05-15 17:32 ` [PATCH v7] " Jonathan Tan
2017-05-15 17:46   ` Jonathan Nieder
2017-05-15 22:10   ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170511170050.GA83655@google.com \
    --to=bmwill@google.com \
    --cc=git@vger.kernel.org \
    --cc=jonathantanmy@google.com \
    --cc=jrnieder@gmail.com \
    --cc=peff@peff.net \
    --cc=spearce@spearce.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.