Re: [PATCH v4 2/3] connect, transport: add no-op arg for future patch

[Jeff King <peff@peff.net>]

On Tue, Dec 22, 2020 at 01:54:19PM -0800, Jonathan Tan wrote:

> In a future patch we plan to return the name of an unborn current branch
> from deep in the callchain to a caller via a new pointer parameter that
> points at a variable in the caller when the caller calls
> get_remote_refs() and transport_get_remote_refs(). Add the parameter to
> functions involved in the callchain, but no caller passes an actual
> argument yet in this step. Thus, the future patch only needs to concern
> itself with new logic.

OK. Since the call stack is so deep, it's nice to get all of this diff
noise out of the way of the third patch.

It does make me wonder if we should be passing a struct like:

  struct transport_fetch_options {
	struct strvec ref_prefixes;
	char **unborn_head;
  }
  #define TRANSPORT_FETCH_OPTIONS_INIT = { STRVEC_INIT }

which would solve this problem once for any future options.

> @@ -455,7 +455,8 @@ struct ref **get_remote_refs(int fd_out, struct packet_reader *reader,
>  			     struct ref **list, int for_push,
>  			     const struct strvec *ref_prefixes,
>  			     const struct string_list *server_options,
> -			     int stateless_rpc)
> +			     int stateless_rpc,
> +			     char **unborn_head_target)

Is a single string enough? The way the protocol is defined, I think the
server is free to tell us about other unborn symrefs, too (but of course
our implementation does not). And I'm not sure what we'd do with such
values (in a "--mirror" clone, I guess we could make local copies of
them).

Should we be prepared for that at the transport layer, or is it
over-engineering?

-Peff