Re: [PATCH] RFC/Add documentation for version protocol 2

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 22, 2015, 19:19 UTC
Message-ID: <xmqqd22wdl88.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1429658342-5295-1-git-send-email-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

> This adds the design document for protocol version 2.
> It's better to rewrite the design document instead of trying to
> squash it into the existing pack-protocol.txt and then differentiating
> between version 1 and 2 all the time.

Just a handful of random thoughts, without expressing agreement or objection at this point.

> diff --git a/Documentation/technical/pack-protocol-2.txt

I wonder, if we are really revamping, if we want this to be limited to "pack" protocol (see more below).

Show 25 quoted lines

> +General structure
> +=================
> +
> +There are four phases involved in the protocol, which are described below:
> +
> +1) capability negotiation
> +2) goal annoncement
> +3) reference advertisement
> +4) pack transfer
> +
> +
> +1) Capability negotiation
> +-------------------------
> +
> +In this phase both client and server send their capabilities to the other side
> +using the following protocol:
> +
> +---
> +list-of-capabilities = *capability flush-pkt
> +capability           =  PKT-LINE(1*(LC_ALPHA / DIGIT / "-" / "_"))
> +----
> +
> +The capabilities itself are described in protocol-capabilities.txt
> +Sending the capabilities to the other side MAY happen concurrently or
> +one after another. There is no order who sends first.

Doesn't that set us up for an easy deadlock (i.e. both sides fill their outgoing pipe because they are not listening)?

Show 12 quoted lines

> +2) Goal annoncement
> +-------------------
> +
> +The goal of this phase is for the client to tell the server what
> +outcome it expects from this communication, such as pushing or
> +pulling data from the server.
> +
> +---
> +list-of-goals    = *goal
> +goal             = PKT-LINE(action-line)
> +action-line      = action *(SP action-parameter)
> +action           = "noop" / "ls-remote" / "fetch" / "push" / "fetch-shallow"

This is interesting, in that it implies that you can connect to a service and after learning what is running on the other hand then decide you would be fetching or pushing. Which is *never* be possible with v1 where you first connect to a specific service that knows how to handle "fetch".

If we are going in this "in-protocol message switches the service" route, we should also support "archive" as one of the actions, no? Yes, I know you named the document "pack-protocol" and "archive" does not give you packs, but "ls-remote" does not transfer pack data, either.

And when we add "archive" (and later "refer to bundle on CDN" to help initial clone), it would become clear that steps #3 and #4 are optional components that are shared by majority of the protocol users (i.e. fetch, push, ls-remote uses #3, fetch, push uses #4.), and other services that also use the protocol need their own equivalents for steps #3 and #4.

Of course, we do not have to do it this way and stick to "one 'goal' per service is pre-arranged before the protocol exchange happens, either via git-daemon or ssh shell command line interactiosn" way of doing things we have always done in v1 protocol.

I have to wonder what role, if any, should "git daemon" (and its equivalent, "the shell command line", if the transport is over ssh) play in this new world order.

Note again that I am not objecting. I am trying to fathom the ramifications of what you wrote here.

Show 5 quoted lines

> +3) Ref advertisement
> +--------------------
> +3) and 4) are highly dependant on the mode for fetch and push. As currently
> +only mode "version1" is supported, the these phases follow the ref advertisement
> +in pack protocol version 1 without capabilities on the first line of the refs.