Re: Cloning empty repository uses locally configured default branch name

From: Jonathan Tan <jonathantanmy@google.com>
To: gitster@pobox.com
Cc: jonathantanmy@google.com, git@vger.kernel.org
Subject: Re: Cloning empty repository uses locally configured default branch name
Date: Tue,  8 Dec 2020 10:55:38 -0800	[thread overview]
Message-ID: <20201208185538.973044-1-jonathantanmy@google.com> (raw)
In-Reply-To: <xmqq7dpt82l4.fsf@gitster.c.googlers.com>

> Jonathan Tan <jonathantanmy@google.com> writes:
> 
> > Has anyone looked into solutions for this? Both protocol v0 and v2 do
> > not send symref information about unborn branches (v0 because, as
> > protocol-capabilities.txt says, "servers SHOULD include this capability
> > for the HEAD symref if it is one of the refs being sent"; v2 because
> > a symref is included only if it refers to one of the refs being sent).
> > In protocol v2, this could be done by adding a capability to ls-remote
> > (maybe, "unborn"), and in protocol v0, this could be done either by
> > updating the existing "symref" capability to be written even when the
> > target branch is unborn (which is potentially backwards incompatible) or
> > introducing a new capability which is like "symref".
> 
> Thanks for looking into this (I think this came up again today
> during my reviews of some topic).
> 
> It would be a backward incompatible change to add to v0, but at this
> point shouldn't we be leaving v0 as-is and move everybody to v2?

That makes sense.

> If it is a simple and safe enough change, though, saying "why not"
> is very tempting, though.

I'll look into how simple and safe it is.

> > A small issue is that upload-pack protocol v0 doesn't even write the
> > blank ref line ("000...000 capabilities^{}") if HEAD points to an unborn
> > branch, but that can be fixed as in the patch below.
> 
> I think the codepaths we have today in process_capabilities() and
> process_dummy_ref() (both in connect.c) would do the right thing
> when it sees a blank ref line even when nothing gets transported,
> but I smell that the rewrite of this state machine is fairly recent
> (say in the past few years) and I do not offhand know if clients
> before the rewrite of the state machine (say in v2.18.0) would be OK
> with the change.  It should be easy to check, though.

Yes - I backported my patch to v2.17.0 and it works:

  $ GIT_TRACE_PACKET=1 ~/git/bin-wrappers/git ls-remote "file://$(pwd)/empty"
  10:49:33.474111 pkt-line.c:80           packet:  upload-pack> 0000000000000000000000000000000000000000 capabilities^{}\0multi_ack thin-pack side-band side-band-64k ofs-delta shallow deepen-since deepen-not deepen-relative no-progress include-tag multi_ack_detailed agent=git/2.17.0.dirty
  10:49:33.474182 pkt-line.c:80           packet:  upload-pack> 0000
  10:49:33.474243 pkt-line.c:80           packet:          git< 0000000000000000000000000000000000000000 capabilities^{}\0multi_ack thin-pack side-band side-band-64k ofs-delta shallow deepen-since deepen-not deepen-relative no-progress include-tag multi_ack_detailed agent=git/2.17.0.dirty
  10:49:33.474315 pkt-line.c:80           packet:          git< 0000
  10:49:33.474320 pkt-line.c:80           packet:          git> 0000
  10:49:33.474358 pkt-line.c:80           packet:  upload-pack< 0000