From: Phillip Wood <phillip.wood123@gmail.com>
Date: Mon, 24 Mar 2025 09:31:53 GMT
Subject: Re: [PATCH v2 1/3] remote: allow `guess_remote_head()` to suppress advice
Message-ID: <106cecbf-6a7c-46bd-8115-2b5fb06cc9af@gmail.com>
In-Reply-To: <20250321231639.180762-2-jltobler@gmail.com>

Hi Justin

On 21/03/2025 23:16, Justin Tobler wrote:
> The `repo_default_branch_name()` invoked through `guess_remote_head()`
> is configured to always display the default branch advice message.
> 
> Adapt `guess_remote_head()` to accept flags and convert the `all`
> parameter to a flag. 

This makes sense if we want to convert the boolean argument to a flags 
argument. However the only callers that do not set `all` when calling 
this function are in git clone where we don't want to emit the warning. 
It is probably not worth a re-roll but I wonder if we really need to do 
this conversion or if we should just hard code guess_remote_head() to be 
quiet when it calls repo_default_branch_name()

Best Wishes

Phillip

> Add the `REMOTE_GUESS_HEAD_QUIET` flag to to enable
> suppression of advice messages. Call sites are updated accordingly.
> > Signed-off-by: Justin Tobler <jltobler@gmail.com>
> ---
>   builtin/fetch.c  |  2 +-
>   builtin/remote.c |  2 +-
>   remote.c         | 10 ++++++----
>   remote.h         | 11 +++++++----
>   4 files changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 95fd0018b9..763314bfcb 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -1638,7 +1638,7 @@ static int set_head(const struct ref *remote_refs, struct remote *remote)
>   
>   	get_fetch_map(remote_refs, &refspec, &fetch_map_tail, 0);
>   	matches = guess_remote_head(find_ref_by_name(remote_refs, "HEAD"),
> -				    fetch_map, 1);
> +				    fetch_map, REMOTE_GUESS_HEAD_ALL);
>   	for (ref = matches; ref; ref = ref->next) {
>   		string_list_append(&heads, strip_refshead(ref->name));
>   	}
> diff --git a/builtin/remote.c b/builtin/remote.c
> index 1b7aad8838..d2aeb5ba1f 100644
> --- a/builtin/remote.c
> +++ b/builtin/remote.c
> @@ -511,7 +511,7 @@ static int get_head_names(const struct ref *remote_refs, struct ref_states *stat
>   
>   	get_fetch_map(remote_refs, &refspec, &fetch_map_tail, 0);
>   	matches = guess_remote_head(find_ref_by_name(remote_refs, "HEAD"),
> -				    fetch_map, 1);
> +				    fetch_map, REMOTE_GUESS_HEAD_ALL);
>   	for (ref = matches; ref; ref = ref->next)
>   		string_list_append(&states->heads, abbrev_branch(ref->name));
>   
> diff --git a/remote.c b/remote.c
> index e609cf5c56..1db88beaf3 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -2297,7 +2297,7 @@ struct ref *get_local_heads(void)
>   
>   struct ref *guess_remote_head(const struct ref *head,
>   			      const struct ref *refs,
> -			      int all)
> +			      unsigned flags)
>   {
>   	const struct ref *r;
>   	struct ref *list = NULL;
> @@ -2315,8 +2315,10 @@ struct ref *guess_remote_head(const struct ref *head,
>   		return copy_ref(find_ref_by_name(refs, head->symref));
>   
>   	/* If a remote branch exists with the default branch name, let's use it. */
> -	if (!all) {
> -		char *default_branch = repo_default_branch_name(the_repository, 0);
> +	if (!(flags & REMOTE_GUESS_HEAD_ALL)) {
> +		char *default_branch =
> +			repo_default_branch_name(the_repository,
> +						 flags & REMOTE_GUESS_HEAD_QUIET);
>   		char *ref = xstrfmt("refs/heads/%s", default_branch);
>   
>   		r = find_ref_by_name(refs, ref);
> @@ -2339,7 +2341,7 @@ struct ref *guess_remote_head(const struct ref *head,
>   		    oideq(&r->old_oid, &head->old_oid)) {
>   			*tail = copy_ref(r);
>   			tail = &((*tail)->next);
> -			if (!all)
> +			if (!(flags & REMOTE_GUESS_HEAD_ALL))
>   				break;
>   		}
>   	}
> diff --git a/remote.h b/remote.h
> index 6be5031f64..7e4943ae3a 100644
> --- a/remote.h
> +++ b/remote.h
> @@ -387,15 +387,18 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb,
>   			 int show_divergence_advice);
>   
>   struct ref *get_local_heads(void);
> +
>   /*
>    * Find refs from a list which are likely to be pointed to by the given HEAD
> - * ref. If 'all' is false, returns the most likely ref; otherwise, returns a
> - * list of all candidate refs. If no match is found (or 'head' is NULL),
> - * returns NULL. All returns are newly allocated and should be freed.
> + * ref. If REMOTE_GUESS_HEAD_ALL is set, return a list of all candidate refs;
> + * otherwise, return the most likely ref. If no match is found (or 'head' is
> + * NULL), returns NULL. All returns are newly allocated and should be freed.
>    */
> +#define REMOTE_GUESS_HEAD_ALL	(1 << 0)
> +#define REMOTE_GUESS_HEAD_QUIET (1 << 1)
>   struct ref *guess_remote_head(const struct ref *head,
>   			      const struct ref *refs,
> -			      int all);
> +			      unsigned flags);
>   
>   /* Return refs which no longer exist on remote */
>   struct ref *get_stale_heads(struct refspec *rs, struct ref *fetch_map);