On 02/21, Jonathan Tan wrote:
> On Tue, 6 Feb 2018 17:13:05 -0800
> Brandon Williams <bmw...@google.com> wrote:
>
> > Introduce the transport-helper capability 'stateless-connect'. This
> > capability indicates that the transport-helper can be requested to run
> > the 'stateless-connect' command which should attempt to make a
> > stateless connection with a remote end. Once established, the
> > connection can be used by the git client to communicate with
> > the remote end natively in a stateless-rpc manner as supported by
> > protocol v2. This means that the client must send everything the server
> > needs in a single request as the client must not assume any
> > state-storing on the part of the server or transport.
>
> Maybe it's worth mentioning that support in the actual remote helpers
> will be added in a subsequent patch.
I can mention that.
>
> > If a stateless connection cannot be established then the remote-helper
> > will respond in the same manner as the 'connect' command indicating that
> > the client should fallback to using the dumb remote-helper commands.
>
> This makes sense, but there doesn't seem to be any code in this patch
> that implements this.
>
> > @@ -612,6 +615,11 @@ static int process_connect_service(struct transport
> > *transport,
> > if (data->connect) {
> > strbuf_addf(&cmdbuf, "connect %s\n", name);
> > ret = run_connect(transport, &cmdbuf);
> > + } else if (data->stateless_connect) {
> > + strbuf_addf(&cmdbuf, "stateless-connect %s\n", name);
> > + ret = run_connect(transport, &cmdbuf);
> > + if (ret)
> > + transport->stateless_rpc = 1;
>
> Why is process_connect_service() falling back to stateless_connect if
> connect doesn't work? I don't think this fallback would work, as a
> client that needs "connect" might need its full capabilities.
Right now there isn't really a notion of "needing" connect since if
connect fails then you need to fallback to doing the dumb thing. Also
note that there isn't all fallback from connect to stateless-connect
here. If the remote helper advertises connect, only connect will be
tried even if stateless-connect is advertised. So this only really
works in the case where stateless-connect is advertised and connect
isn't, as is with our http remote-helper.
--
Brandon Williams
