From: Phillip Wood <phillip.wood123@gmail.com>
Date: Mon, 24 Mar 2025 09:32:18 GMT
Subject: Re: [PATCH v2 2/3] builtin/clone: suppress unexpected default branch advice
Message-ID: <966aa1cc-fd4a-42b7-a241-0537767021ed@gmail.com>
In-Reply-To: <20250321231639.180762-3-jltobler@gmail.com>

Hi Justin

On 21/03/2025 23:16, Justin Tobler wrote:
> In 199f44cb2ead (builtin/clone: allow remote helpers to detect repo,
> 2024-02-27), clones started partially initializing the refdb before
> executing the remote helpers by creating a HEAD file and "refs/"
> directory. This has resulted in some scenarios where git-clone(1) now
> prints the default branch name advice message where it previously did
> not.
> 
> A side-effect of the HEAD file already existing, is that computation of
> the default branch name is handled later in execution. This matters
> because prior to 97abaab5f6 (refs: drop `git_default_branch_name()`,
> 2024-05-17), the default branch value would be computed during its first
> execution and cached. Subsequent invocations would simply return the
> cached value. Since the next `git_default_branch_name()` call site,
> which is invoked through `guess_remote_head()`, is not configured to
> suppress the advice message, computing the default branch name results
> in the advice message being printed.
> 
> Configure `guess_remote_head()` to suppress the advice message,
> restoring the previous behavior.
> 
> Signed-off-by: Justin Tobler <jltobler@gmail.com>
> ---
>   builtin/clone.c         |  3 ++-
>   t/t5607-clone-bundle.sh | 12 ++++++++++++
>   2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/builtin/clone.c b/builtin/clone.c
> index f14229abf4..a4008715ec 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -1523,7 +1523,8 @@ int cmd_clone(int argc,
>   	}
>   
>   	remote_head = find_ref_by_name(refs, "HEAD");
> -	remote_head_points_at = guess_remote_head(remote_head, mapped_refs, 0);
> +	remote_head_points_at = guess_remote_head(remote_head, mapped_refs,
> +						  REMOTE_GUESS_HEAD_QUIET);

There is another caller of guess_remote_head() in wanted_peer_refs() 
which I think we probably want to be quiet as well. I don't see how it 
is helpful to the user to prompt them about setting the default branch 
name used when creating local repositories during a clone of a remote 
repository.

Best Wishes

Phillip

>   
>   	if (option_branch) {
>   		our_head_points_at = find_remote_branch(mapped_refs, option_branch);
> diff --git a/t/t5607-clone-bundle.sh b/t/t5607-clone-bundle.sh
> index 82e3621ec5..f526bb1c69 100755
> --- a/t/t5607-clone-bundle.sh
> +++ b/t/t5607-clone-bundle.sh
> @@ -211,4 +211,16 @@ test_expect_success 'git bundle v3 rejects unknown capabilities' '
>   	test_grep "unknown capability .unknown=silly." output
>   '
>   
> +test_expect_success 'cloning bundle suppresses default branch name advice' '
> +	test_when_finished "rm -rf bundle-repo clone-repo" &&
> +
> +	git init bundle-repo &&
> +	git -C bundle-repo commit --allow-empty -m init &&
> +	git -C bundle-repo bundle create repo.bundle --all &&
> +	GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME= \
> +		git clone bundle-repo/repo.bundle clone-repo 2>err &&
> +
> +	test_grep ! "hint: " err
> +'
> +
>   test_done