Re: [PATCH v5 0/3] Cloning with remote unborn HEAD

Jonathan Tan [off-list ref] writes:

Thanks, Peff, for your review. I have addressed your comments (through
replies to your emails and here in this v5 patch set).

Jonathan Tan (3):
  ls-refs: report unborn targets of symrefs
  connect, transport: encapsulate arg in struct
  clone: respect remote unborn HEAD

Applying this alone to 'master' seems to pass all tests, but
the topic seems to have funny interactions with another topic
in flight, jk/peel-iterated-oid

There is textual conflict whose resolution seems trivial, but with
that resolved ...

diff --cc builtin/clone.c
index e335734b4c,77fdc61f4d..0000000000

--- i/builtin/clone.c
+++ w/builtin/clone.c

@@@ -1326,10 -1330,21 +1330,21 @@@ int cmd_clone(int argc, const char **ar
  		remote_head = NULL;
  		option_no_checkout = 1;
  		if (!option_bare) {
- 			const char *branch = git_default_branch_name(0);
- 			char *ref = xstrfmt("refs/heads/%s", branch);
+ 			const char *branch;
+ 			char *ref;
+ 
+ 			if (transport_ls_refs_options.unborn_head_target &&
+ 			    skip_prefix(transport_ls_refs_options.unborn_head_target,
+ 					"refs/heads/", &branch)) {
+ 				ref = transport_ls_refs_options.unborn_head_target;
+ 				transport_ls_refs_options.unborn_head_target = NULL;
+ 			} else {
 -				branch = git_default_branch_name();
++				branch = git_default_branch_name(0);
+ 				ref = xstrfmt("refs/heads/%s", branch);
+ 			}
  
  			install_branch_config(0, branch, remote_name, ref);
+ 			create_symref("HEAD", ref, "");
  			free(ref);
  		}
  	}


... numerous tests fail.

For example, t5702 dies like so:

expecting success of 5702.15 'clone of empty repo propagates name of default branch':
        test_when_finished "rm -rf file_empty_parent file_empty_child" &&

        GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME= \
        git -c init.defaultBranch=mydefaultbranch init file_empty_parent &&

        GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME= \
        git -c init.defaultBranch=main -c protocol.version=2 \
                clone "file://$(pwd)/file_empty_parent" file_empty_child &&
        grep "refs/heads/mydefaultbranch" file_empty_child/.git/HEAD

Initialized empty Git repository in /usr/local/google/home/jch/w/git.git/t/trash directory.t5702-protocol-v2/file_empty_parent/.git/
Cloning into 'file_empty_child'...
fatal: expected flush after ref listing
not ok 15 - clone of empty repo propagates name of default branch

On Tue, Jan 26, 2021 at 05:11:42PM -0800, Junio C Hamano wrote:

Jonathan Tan [off-list ref] writes:

quoted

Thanks, Peff, for your review. I have addressed your comments (through
replies to your emails and here in this v5 patch set).

Jonathan Tan (3):
  ls-refs: report unborn targets of symrefs
  connect, transport: encapsulate arg in struct
  clone: respect remote unborn HEAD

Applying this alone to 'master' seems to pass all tests, but
the topic seems to have funny interactions with another topic
in flight, jk/peel-iterated-oid

I was worried at first I really screwed up something subtle, but it is
indeed just a funny local interaction.

Here's a fix which can be applied on top of jt/clone-unborn-head. It
could equally well be applied as part of the merge (with a minor
adjustment in the context), but I think it ought to be squashed into
Jonathan's patch 1 anyway.

The conflict you had to resolve was a red herring (it wasn't part of
jk/peel-iterated-oid at all, but rather other commits that got pulled in
because my topic is based on a more recent master).

-- >8 --
Subject: [PATCH] ls-refs: don't peel NULL oid

When the "unborn" feature is enabled, upload-pack serving an ls-refs
command will pass a NULL oid into send_ref(). In this case, there is no
point trying to peel the ref, since we know it points to nothing.

For now this is a harmless waste of cycles (we re-resolve HEAD and find
out that indeed, it points to nothing). But after merging with another
topic that contains 36a317929b (refs: switch peel_ref() to
peel_iterated_oid(), 2021-01-20), we'd actually end up passing NULL to
peel_object(), which segfaults!

Signed-off-by: Jeff King <redacted>
---
 ls-refs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ls-refs.c b/ls-refs.c
index 4077adeb6a..bc91f03653 100644
--- a/ls-refs.c
+++ b/ls-refs.c

@@ -66,7 +66,7 @@ static int send_ref(const char *refname, const struct object_id *oid,
 			    strip_namespace(symref_target));
 	}
 
-	if (data->peel) {
+	if (data->peel && oid) {
 		struct object_id peeled;
 		if (!peel_ref(refname, &peeled))
 			strbuf_addf(&refline, " peeled:%s", oid_to_hex(&peeled));

-- 
2.30.0.724.gc858251c49