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

From: Jeff King <peff@peff.net>
Date: Nov 3, 2014, 22:04 UTC
Message-ID: <20141103220408.GA12462@peff.net>
In-Reply-To: <xmqqmw88rvh3.fsf@gitster.dls.corp.google.com>

On Mon, Nov 03, 2014 at 10:26:48AM -0800, Junio C Hamano wrote:

Show 14 quoted lines

> "Philip Oakley" <philipoakley@iee.org> writes:
> 
> > This certainly looks the way to go. The one extra question would be
> > whether the symref should be included by default when HEAD is present,
> > or only if there was possible ambiguity between the other listed
> > refs.
> 
> Just include the "\0symref=..." for any symbolic ref you mention,
> and the ref in question does not even have to be "HEAD", I would
> say.
> 
> The mechanism chosen should be something that will be transparently
> ignored by existing implementations, there is no need to make the
> data format conditional.

One thing I glossed over in my suggestion of the NUL trick: it works on git.git, but no clue about elsewhere. I can imagine that other non-C implementations might treat the whole thing (NUL and extra data included) as the refname. Back when we did the NUL trick to the online protocol, git.git was the only serious implementation. But nowadays we should at least consider the impact on JGit, libgit2, and/or dulwich (breaking them is not necessarily a showstopper IMHO, but we should at least know what we are breaking).

I peeked at libgit2 and I think it does not support bundles at all yet, so that is safe. Grepping for "bundle" in dulwich turns up no hits, either.

Looks like JGit does support them. I did a very brief test, and it seems to silently ignore a HEAD ref that has the NUL (I guess maybe it just rejects it as a malformed refname).

We could make JGit happier either by:

  1. Only including the symref magic in ambiguous cases, so that regular
     ones Just Work as usual.

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

-Peff