Subject: Re: [PATCH v3 28/35] transport-helper: introduce stateless-connect

From: Brandon Williams <[email-masked]>
Date: Thu, 22 Feb 2018 10:53:53 -0800

On 02/21, Jonathan Tan wrote:
> On Tue,  6 Feb 2018 17:13:05 -0800
> Brandon Williams <[email-masked]> 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