git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / Atom feed
From: Jeff King <peff@peff.net>
To: Stefan Beller <sbeller@google.com>
Cc: "git@vger.kernel.org" <git@vger.kernel.org>,
	Duy Nguyen <pclouds@gmail.com>,
	Junio C Hamano <gitster@pobox.com>
Subject: Re: [RFC/WIP PATCH 04/11] upload-pack-2: Implement the version 2 of upload-pack
Date: Wed, 27 May 2015 16:34:52 -0400
Message-ID: <20150527203451.GD14309@peff.net> (raw)
In-Reply-To: <CAGZ79kYaoViDrz9TKXWebif4mfyUjzJ6b3id8ozTqtwsmjAC1A@mail.gmail.com>

On Wed, May 27, 2015 at 10:40:37AM -0700, Stefan Beller wrote:

> > If we are upload-pack-2, should we advertise that in the capabilities? I
> > think it may make things easier later if we try to provide some
> > opportunistic out-of-band data. E.g., if see tell git-daemon:
> >
> >   git-upload-pack repo\0host=whatever\0\0version=2
> >
> > how do we know whether version=2 was understood and kicked us into v2
> > mode, versus an old server that ignored it?
> 
> So in my vision we would call git-upload-pack-2 instead of having a "version=2".
> and if git-upload-pack-2 doesn't exist, the whole conversation is
> over, the client
> it is left to make up some good error message or retry version 1.

I'd like for that to be a starting point for us, and then to be able to
add-on "hints" to ease the transition path in whatever way we want. We
may even not do that in the long run, but I want to leave the door open
if we can.

> But I think advertising both which versions the server could deal
> with, as well as
> the currently expected version is a good thing.
> 
> capability: can_speak=1,2
> capability: speaking_now=2

I was thinking just "speaking_now=2", but it probably makes sense to do
both. I do not think using it to "downgrade" will ever be particularly
useful (certainly not from v2 to v1, since to understand the flag both
sides must be v2 in the first place). But advertising that via the v1
conversation will be a good way to tell the other side that upgrade is
possible.

> > Also, do we need the capability noise-word?
> 
> I thought it opens up a new possible door in the future.
> As we ignore anything not starting with "capability" for now, you
> could introduce
> your foo and bar ping pong easily and still be version 2 compatible.
> 
> S: capability: thin
> S: capability: another-capability
> S: ping-pong foo
> S: done
> C: (not having understood ping-pong) just answering with capability: thin
> C: done, let's proceed to refs advertisement
> 
> The alternative client would do:
> 
> C: ping-pong: foo-data1a
> S: ping-pong: foo-data1b
> C: ping-pong: foo-data2a
> C: capability: thin
> ...

Right, but I think (and please correct me if there's a case I'm missing)
that the behavior is the same whether it is spelled "ping-pong" or
"capability:ping-pong". That is, the rule for "capability:" is "if you
do not understand it, ignore it and do not mention it in your
capabilities; the server is required to assume you were written before
that capability was invented". But that is _also_ the rule for
ping-pong, I think.

> > Eric mentioned the underflow problems here, but I wonder even more:
> > what's wrong with the global ends_with() that we already provide?
> 
> I was missing knowledge we have that, and apparently I was thinking it's
> faster to come up with my own version than to look for it. :)

Makes sense. :)

-Peff

  reply index

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-26 22:01 [RFC/WIP PATCH 00/11] Protocol version 2, again! Stefan Beller
2015-05-26 22:01 ` [RFC/WIP PATCH 01/11] upload-pack: make client capability parsing code a separate function Stefan Beller
2015-05-26 22:01 ` [RFC/WIP PATCH 02/11] upload-pack: only accept capabilities on the first "want" line Stefan Beller
2015-05-26 22:17   ` Junio C Hamano
2015-05-26 22:20     ` Stefan Beller
2015-05-26 22:01 ` [RFC/WIP PATCH 03/11] upload-pack: move capabilities out of send_ref Stefan Beller
2015-05-26 22:01 ` [RFC/WIP PATCH 04/11] upload-pack-2: Implement the version 2 of upload-pack Stefan Beller
2015-05-27  2:30   ` Eric Sunshine
2015-05-27  6:35   ` Jeff King
2015-05-27 17:30     ` Eric Sunshine
2015-05-27 20:14       ` Jeff King
2015-05-27 17:40     ` Stefan Beller
2015-05-27 20:34       ` Jeff King [this message]
2015-05-27 20:45         ` Stefan Beller
2015-05-27 21:46           ` Jeff King
2015-05-26 22:01 ` [RFC/WIP PATCH 05/11] transport: add infrastructure to support a protocol version number Stefan Beller
2015-05-27  6:39   ` Jeff King
2015-05-27 19:01     ` Stefan Beller
2015-05-27 20:17       ` Jeff King
2015-05-27 19:10     ` Junio C Hamano
2015-05-26 22:01 ` [RFC/WIP PATCH 06/11] remote.h: add get_remote_capabilities, request_capabilities Stefan Beller
2015-05-27  3:25   ` Eric Sunshine
2015-05-27  6:50     ` Jeff King
2015-05-27 17:19       ` Eric Sunshine
2015-05-27 20:09         ` Jeff King
2015-05-27  6:45   ` Jeff King
2015-05-29 19:39     ` Stefan Beller
2015-05-29 22:08       ` Jeff King
2015-05-26 22:01 ` [RFC/WIP PATCH 07/11] fetch-pack: use the configured transport protocol Stefan Beller
2015-05-26 22:19   ` Junio C Hamano
2015-05-26 22:23     ` Stefan Beller
2015-05-27  6:53   ` Jeff King
2015-05-26 22:01 ` [RFC/WIP PATCH 08/11] transport: connect_setup appends protocol version number Stefan Beller
2015-05-26 22:21   ` Junio C Hamano
2015-05-26 22:31     ` Stefan Beller
2015-05-27  5:09       ` Junio C Hamano
2015-05-27  6:56         ` Jeff King
2015-05-27  3:33   ` Eric Sunshine
2015-05-27  7:02   ` Jeff King
2015-05-26 22:01 ` [RFC/WIP PATCH 09/11] transport: get_refs_via_connect exchanges capabilities before refs Stefan Beller
2015-05-27  5:37   ` Eric Sunshine
2015-05-27  7:06   ` Jeff King
2015-05-26 22:01 ` [RFC/WIP PATCH 10/11] t5544: add a test case for the new protocol Stefan Beller
2015-05-27  5:34   ` Eric Sunshine
2015-05-27  7:12   ` Jeff King
2015-05-26 22:01 ` [RFC/WIP PATCH 11/11] Document protocol version 2 Stefan Beller
2015-05-29 20:35   ` Junio C Hamano
2015-05-29 21:36     ` Stefan Beller
2015-05-29 21:52       ` Junio C Hamano
2015-05-29 22:21         ` Jeff King
2015-06-01 23:14           ` Stefan Beller
2015-06-01 23:40             ` Stefan Beller
2015-06-04 13:18               ` Jeff King
2015-06-04 17:01                 ` Junio C Hamano
2015-06-02 17:06             ` Junio C Hamano
2015-05-27  6:18 ` [RFC/WIP PATCH 00/11] Protocol version 2, again! Jeff King
2015-05-27  7:08   ` Jeff King
2015-06-01 17:49     ` Stefan Beller
2015-06-02 10:10       ` Duy Nguyen
2015-06-04 13:09       ` Jeff King
2015-06-04 16:44         ` Stefan Beller

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20150527203451.GD14309@peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.com \
    --cc=sbeller@google.com \
    /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

git@vger.kernel.org list mirror (unofficial, one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Example config snippet for mirrors

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git