Re: [PATCH 1/3] upload-pack: send the HEAD information

From: Jeff King <peff@peff.net>
Date: Sep 8, 2013, 07:13 UTC
Message-ID: <20130908071359.GJ14019@sigill.intra.peff.net>
In-Reply-To: <xmqqsixhyhan.fsf@gitster.dls.corp.google.com>

On Fri, Sep 06, 2013 at 10:46:24AM -0700, Junio C Hamano wrote:

> I think it is perfectly fine to expose _only_ HEAD now, and wait
> until we find a good reason that we should send this information for
> other symbolic refs in the repository.

Yeah, I agree with that.

Show 17 quoted lines

> However, because we already anticipate that we may find such a good
> reason later, on-the-wire format should be prepared to support such
> later enhancement.  I think sending
> 
> 	symref=HEAD:refs/heads/master
> 
> is probably one good way to do so, as Peff suggested in that old
> thread ($gmane/102070; note that back then this wasn't suggested as
> a proper capability so the exact format he suggests in the message
> is different).  Then we could later add advertisements for other
> symbolic refs if we find it necessary to do so, e.g.
> 
> 	symref=HEAD:refs/heads/master
>         symref=refs/remotes/origin/HEAD:refs/remotes/origin/master
> 
> (all on one line together with other capabilities separated with a
> SP in between).

It somehow feels a little weird to me that we would output the information about refs/foo on the HEAD line. A few possible issues (and I am playing devil's advocate to some degree here):

  1. What if we have a large number of symrefs? Would we run afoul of
     pkt-line length limits?

  2. What's the impact of having to display all symrefs on the first
     line, before we output other refs? Right now we can just stream out
     refs as we read them, but we would have to make two passes (and/or
     cache them all) to find all of the symrefs before we start
     outputting. Will the extra latency ever matter?

What do you think about teaching git to read extra data after "\0" for _every_ ref line? And then ref advertisement might look something like:

  <sha1> HEAD\0multi_ack thin-pack ... symref=refs/heads/master\n
  <sha1> refs/heads/master\n
  <sha1> refs/heads/my-alias\0symref=refs/heads/master

That would leave us future room for more ref annotations if we should want them, and I think (but haven't tested) that existing receivers should ignore everything after the NUL.

-Peff