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

From: Junio C Hamano <gitster@pobox.com>
To: Jonathan Tan <jonathantanmy@google.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v4 0/3] Cloning with remote unborn HEAD
Date: Tue, 22 Dec 2020 14:06:43 -0800	[thread overview]
Message-ID: <xmqqwnx9fqbw.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <cover.1608673963.git.jonathantanmy@google.com> (Jonathan Tan's message of "Tue, 22 Dec 2020 13:54:17 -0800")

Jonathan Tan <jonathantanmy@google.com> writes:

> Thanks Junio for informing me of the test failures. Turns out that it
> was partly because I didn't memset oid (and in some code paths, it gets
> read without being written to), and partly because I didn't set
> GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME. Here's an updated patch set with
> the fixes.
>
> Jonathan Tan (3):
>   ls-refs: report unborn targets of symrefs
>   connect, transport: add no-op arg for future patch
>   clone: respect remote unborn HEAD

Having to unset the GIT_TEST_* environment even when we have an
explicit "git -c init.defaultBranch=<val>" is a bit awkward and
frustrating, but hopefully this would futureproof the tests for
the current and future world ;-)

Will replace.

>
>  Documentation/config.txt                |  2 +
>  Documentation/config/init.txt           |  2 +-
>  Documentation/config/lsrefs.txt         |  3 ++
>  Documentation/technical/protocol-v2.txt | 10 ++++-
>  builtin/clone.c                         | 19 +++++++--
>  builtin/fetch-pack.c                    |  3 +-
>  builtin/fetch.c                         |  2 +-
>  builtin/ls-remote.c                     |  2 +-
>  builtin/remote.c                        |  2 +-
>  connect.c                               | 29 ++++++++++++--
>  ls-refs.c                               | 52 +++++++++++++++++++++++--
>  ls-refs.h                               |  1 +
>  remote.h                                |  3 +-
>  serve.c                                 |  2 +-
>  t/t5606-clone-options.sh                |  8 ++--
>  t/t5702-protocol-v2.sh                  | 11 ++++++
>  transport-helper.c                      |  7 +++-
>  transport-internal.h                    | 13 +++----
>  transport.c                             | 29 ++++++++------
>  transport.h                             |  7 +++-
>  20 files changed, 164 insertions(+), 43 deletions(-)
>  create mode 100644 Documentation/config/lsrefs.txt
>
> Range-diff against v3:
> 1:  7d20ec323a ! 1:  a66e50626e ls-refs: report unborn targets of symrefs
>     @@ ls-refs.c: static int send_ref(const char *refname, const struct object_id *oid,
>      +	int flag;
>      +	int oid_is_null;
>      +
>     ++	memset(&oid, 0, sizeof(oid));
>      +	strbuf_addf(&namespaced, "%sHEAD", get_git_namespace());
>      +	resolve_ref_unsafe(namespaced.buf, 0, &oid, &flag);
>      +	oid_is_null = is_null_oid(&oid);
> 2:  b5a78857eb = 2:  14f3962adc connect, transport: add no-op arg for future patch
> 3:  c2303dc976 ! 3:  e770fc46eb clone: respect remote unborn HEAD
>     @@ t/t5606-clone-options.sh: test_expect_success 'redirected clone -v does show pro
>       
>       test_expect_success 'chooses correct default initial branch name' '
>      -	git init --bare empty &&
>     ++	GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME= \
>      +	git -c init.defaultBranch=foo init --bare empty &&
>      +	test_config -C empty lsrefs.allowUnborn true &&
>       	GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME= \
>     @@ t/t5702-protocol-v2.sh: test_expect_success 'clone with file:// using protocol v
>       '
>       
>      +test_expect_success 'clone of empty repo propagates name of default branch' '
>     ++	GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME= \
>      +	git -c init.defaultBranch=mydefaultbranch init file_empty_parent &&
>      +	test_config -C file_empty_parent lsrefs.allowUnborn true &&
>      +
>     ++	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