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

From: Jonathan Tan <jonathantanmy@google.com>
Date: Feb 22, 2018, 00:01 UTC
Message-ID: <20180221160116.1619fcc4ffa7e9c13a5ab9ba@google.com>
In-Reply-To: <20180207011312.189834-29-bmwill@google.com>

On Tue, 6 Feb 2018 17:13:05 -0800 Brandon Williams <bmwill@google.com> wrote:

Show 9 quoted lines

> 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.

> 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.

Show 9 quoted lines

> @@ -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.