Re: [PATCH] clone: send server options when using protocol v2

From: Jonathan Tan <jonathantanmy@google.com>
Date: Apr 9, 2019, 18:49 UTC
Message-ID: <20190409184949.207114-1-jonathantanmy@google.com>
In-Reply-To: <xmqqk1g342xi.fsf@gitster-ct.c.googlers.com>

Show 11 quoted lines

> Does the code behave sensibly when the --server-option=... option is
> given and
> 
>  (a) the given option is not understood by the other side that talks
>      protocol v2?  Or
> 
>  (b) it turns out that the other side does not talk protocol v2?
> 
> In the former case, I would expect that there would be a way to
> respond with a failure for the other side and "git clone" should
> notice and respond by die()-ing at the end.

Right now, the server ignores it (it collects them as "keys" in process_request() in serve.c and then passes them to the individual commands, but they don't do anything about it).

Right now it's documented as:

	-o <option>::
	--server-option=<option>::
		Transmit the given string to the server when communicating using
		protocol version 2.  The given string must not contain a NUL or LF
		character.
		When multiple `--server-option=<option>` are given, they are all
		sent to the other side in the order listed on the command line.

I'm inclined to add:

  The server's handling of server options, including unknown ones, is
  server-specific.

> In the latter case, I would expect at least that a warning about
> accepted but ignored server-option to be given, or "git clone" to
> fail with an error "cannot honor the server-option".

Right now there's no warning, but I'll add one in the next reroll to both "fetch" and "clone".

> If the code does not match the above expectations, at least that
> should be documented, I think.  If "git fetch" shares the same
> issue, this would be a good time to address it, too.