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