Re: [PATCH] upload-pack: no longer use hidden-refs as exclude_patterns

[Patrick Steinhardt <ps@pks.im>]

On Thu, Feb 27, 2025 at 12:46:11PM +0000, SURA via GitGitGadget wrote:
> From: SURA <sura907@hotmail.com>
> 
> Signed-off-by: SURA <sura907@hotmail.com>
> ---
>     upload-pack: No longer use hidden-refs as exclude_patterns
> 
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1866%2FSURA907%2Fmaster-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1866/SURA907/master-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/1866
> 
>  upload-pack.c | 15 +++------------
>  1 file changed, 3 insertions(+), 12 deletions(-)
> 
> diff --git a/upload-pack.c b/upload-pack.c
> index 728b2477fcc..9ae42a463a3 100644
> --- a/upload-pack.c
> +++ b/upload-pack.c
> @@ -609,21 +609,12 @@ static int allow_hidden_refs(enum allow_uor allow_uor)
>  static void for_each_namespaced_ref_1(each_ref_fn fn,
>  				      struct upload_pack_data *data)
>  {
> -	const char **excludes = NULL;
>  	/*
> -	 * If `data->allow_uor` allows fetching hidden refs, we need to
> -	 * mark all references (including hidden ones), to check in
> -	 * `is_our_ref()` below.
> -	 *
> -	 * Otherwise, we only care about whether each reference's object
> -	 * has the OUR_REF bit set or not, so do not need to visit
> -	 * hidden references.
> +	 * config transfer.hideRefs of upload-pack is diffient from arg exclude of for-each-ref,
> +	 * We should not set exclude_patterns here
>  	 */
> -	if (allow_hidden_refs(data->allow_uor))
> -		excludes = hidden_refs_to_excludes(&data->hidden_refs);
> -
>  	refs_for_each_namespaced_ref(get_main_ref_store(the_repository),
> -				     excludes, fn, data);
> +				     NULL, fn, data);
>  }

This message is missing any context _why_ we want to do this. For
background: setting up these exclude patterns for hidden references is
quite an important performance optimization in large repositories, so
disabling it just like that is not an option without a good reason to do
so.

So what is the issue that you see and why is this fix the solution for
that issue?

Patrick