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

[Jonathan Tan <jonathantanmy@google.com>]

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

That's a good idea, and I've switched patch 2 to doing this. It also
makes it easier to explain (no "unborn_head" dummy variable that does
nothing, since I can just introduce "unborn_head" in patch 3).

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

With the struct, I think we're prepared for it.