Re: [PATCH] use child_process_init() to initialize struct child_process variables

[Jeff King <peff@peff.net>]

On Tue, Nov 04, 2014 at 01:56:15PM -0800, Junio C Hamano wrote:

> >>   2. Including two lines, like:
> >>
> >>         $sha1 HEAD\0symref=refs/heads/master
> >>         $sha1 HEAD
> >>
> >>      which JGit does the right thing with (and git.git seems to, as
> >>      well).
> >
> > Sounds sensible, even though it looks ugly X-<.
> 
> I have a mild preference for a syntax that is more similar to the
> on-wire protocol, so that connect.c::parse_feature_value() can be
> reused to parse it and possibly annotate_refs_with_symref_info() can
> also be reused by calling it from transport.c::get_refs_from_bundle().

Yeah, what I wrote above was the simplest thing that could work, and
does not need to be the final form.  I know that you already know what
I'm about to describe below, Junio, but I want to expand on the
situation for the benefit of onlookers (and potential implementers like
Philip).

The online protocol is hampered by the "if you see something after a
NUL, it is a capabilities string, and you must throw out the previous
capabilities string and replace it with this one" historical rule. And
that's why we cannot do:

  $sha1 refs/heads/master\0thin-pack side-band etc
  $sha1 HEAD\0symref=refs/heads/master

as it would throw out "thin-pack", "side-band", etc. Instead we do it
more like:

  $sha1 refs/heads/master\0thin-pack side-band etc symref=HEAD:refs/heads/master
  $sha1 HEAD

to shove _all_ of the symref mappings into the capability string, rather
than letting them ride along with their respective refs. The downside is
that we are bounded in the number of symref mappings we can send (by the
maximum length for a single pkt-line), and therefore send only the value
of HEAD.

The bundle code is not bound by this historical legacy, and could do it
in a different (and more efficient and flexible) way. But it is probably
saner to just keep them identical. It makes the code simpler, and having
bundle as the only transport which has the extra flexibility does not
really buy us much (and probably just invites confusion).

-Peff