{"thread":{"id":"63013","subject":"[PATCH] upload-pack: no longer use hidden-refs as exclude_patterns","startedAt":"2025-02-27T12:46:15Z","lastAt":"2025-02-27T15:51:44Z","messageCount":3,"participants":["SURA via GitGitGadget","Patrick Steinhardt","SURA"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"513144","messageId":"pull.1866.git.1740660371583.gitgitgadget@gmail.com","threadId":"63013","inReplyTo":null,"subject":"[PATCH] upload-pack: no longer use hidden-refs as exclude_patterns","fromName":"SURA via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-02-27T12:46:11Z","receivedAt":"2025-02-27T12:46:15Z","isPatch":true,"sender":{"key":"surak8806@gmail.com","avatar":null},"body":"From: SURA \n\nSigned-off-by: SURA \n---\n upload-pack: No longer use hidden-refs as exclude_patterns\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1866%2FSURA907%2Fmaster-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1866/SURA907/master-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1866\n\n upload-pack.c | 15 +++------------\n 1 file changed, 3 insertions(+), 12 deletions(-)\n\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 728b2477fcc..9ae42a463a3 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -609,21 +609,12 @@ static int allow_hidden_refs(enum allow_uor allow_uor)\n static void for_each_namespaced_ref_1(each_ref_fn fn,\n \t\t\t\t struct upload_pack_data *data)\n {\n-\tconst char **excludes = NULL;\n \t/*\n-\t * If `data->allow_uor` allows fetching hidden refs, we need to\n-\t * mark all references (including hidden ones), to check in\n-\t * `is_our_ref()` below.\n-\t *\n-\t * Otherwise, we only care about whether each reference's object\n-\t * has the OUR_REF bit set or not, so do not need to visit\n-\t * hidden references.\n+\t * config transfer.hideRefs of upload-pack is diffient from arg exclude of for-each-ref,\n+\t * We should not set exclude_patterns here\n \t */\n-\tif (allow_hidden_refs(data->allow_uor))\n-\t\texcludes = hidden_refs_to_excludes(&data->hidden_refs);\n-\n \trefs_for_each_namespaced_ref(get_main_ref_store(the_repository),\n-\t\t\t\t excludes, fn, data);\n+\t\t\t\t NULL, fn, data);\n }\n \n \n\nbase-commit: 08bdfd453584e489d5a551aecbdcb77584e1b958\n-- \ngitgitgadget\n"},{"id":"513149","messageId":"Z8B8u_d37IFOpSln@pks.im","threadId":"63013","inReplyTo":"pull.1866.git.1740660371583.gitgitgadget@gmail.com","subject":"Re: [PATCH] upload-pack: no longer use hidden-refs as exclude_patterns","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-02-27T14:54:51Z","receivedAt":"2025-02-27T14:54:56Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Feb 27, 2025 at 12:46:11PM +0000, SURA via GitGitGadget wrote:\n> From: SURA \n> \n> Signed-off-by: SURA \n> ---\n> upload-pack: No longer use hidden-refs as exclude_patterns\n> \n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1866%2FSURA907%2Fmaster-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1866/SURA907/master-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1866\n> \n> upload-pack.c | 15 +++------------\n> 1 file changed, 3 insertions(+), 12 deletions(-)\n> \n> diff --git a/upload-pack.c b/upload-pack.c\n> index 728b2477fcc..9ae42a463a3 100644\n> --- a/upload-pack.c\n> +++ b/upload-pack.c\n> @@ -609,21 +609,12 @@ static int allow_hidden_refs(enum allow_uor allow_uor)\n> static void for_each_namespaced_ref_1(each_ref_fn fn,\n> \t\t\t\t struct upload_pack_data *data)\n> {\n> -\tconst char **excludes = NULL;\n> \t/*\n> -\t * If `data->allow_uor` allows fetching hidden refs, we need to\n> -\t * mark all references (including hidden ones), to check in\n> -\t * `is_our_ref()` below.\n> -\t *\n> -\t * Otherwise, we only care about whether each reference's object\n> -\t * has the OUR_REF bit set or not, so do not need to visit\n> -\t * hidden references.\n> +\t * config transfer.hideRefs of upload-pack is diffient from arg exclude of for-each-ref,\n> +\t * We should not set exclude_patterns here\n> \t */\n> -\tif (allow_hidden_refs(data->allow_uor))\n> -\t\texcludes = hidden_refs_to_excludes(&data->hidden_refs);\n> -\n> \trefs_for_each_namespaced_ref(get_main_ref_store(the_repository),\n> -\t\t\t\t excludes, fn, data);\n> +\t\t\t\t NULL, fn, data);\n> }\n\nThis message is missing any context _why_ we want to do this. For\nbackground: setting up these exclude patterns for hidden references is\nquite an important performance optimization in large repositories, so\ndisabling it just like that is not an option without a good reason to do\nso.\n\nSo what is the issue that you see and why is this fix the solution for\nthat issue?\n\nPatrick\n"},{"id":"513153","messageId":"MA0P287MB06412DF70BCDA0D99641129FE4CD2@MA0P287MB0641.INDP287.PROD.OUTLOOK.COM","threadId":"63013","inReplyTo":"Z8B8u_d37IFOpSln@pks.im","subject":"Re: [PATCH] upload-pack: no longer use hidden-refs as exclude_patterns","fromName":"SURA","fromEmail":"sura907@hotmail.com","sentAt":"2025-02-27T15:51:31Z","receivedAt":"2025-02-27T15:51:44Z","isPatch":true,"sender":{"key":"sura907@hotmail.com","avatar":null},"body":"\n在 2025/2/27 22:54, Patrick Steinhardt 写道:\n> On Thu, Feb 27, 2025 at 12:46:11PM +0000, SURA via GitGitGadget wrote:\n>> From: SURA \n>>\n>> Signed-off-by: SURA \n>> ---\n>> upload-pack: No longer use hidden-refs as exclude_patterns\n>>\n>> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1866%2FSURA907%2Fmaster-v1\n>> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1866/SURA907/master-v1\n>> Pull-Request: https://github.com/gitgitgadget/git/pull/1866\n>>\n>> upload-pack.c | 15 +++------------\n>> 1 file changed, 3 insertions(+), 12 deletions(-)\n>>\n>> diff --git a/upload-pack.c b/upload-pack.c\n>> index 728b2477fcc..9ae42a463a3 100644\n>> --- a/upload-pack.c\n>> +++ b/upload-pack.c\n>> @@ -609,21 +609,12 @@ static int allow_hidden_refs(enum allow_uor allow_uor)\n>> static void for_each_namespaced_ref_1(each_ref_fn fn,\n>> \t\t\t\t struct upload_pack_data *data)\n>> {\n>> -\tconst char **excludes = NULL;\n>> \t/*\n>> -\t * If `data->allow_uor` allows fetching hidden refs, we need to\n>> -\t * mark all references (including hidden ones), to check in\n>> -\t * `is_our_ref()` below.\n>> -\t *\n>> -\t * Otherwise, we only care about whether each reference's object\n>> -\t * has the OUR_REF bit set or not, so do not need to visit\n>> -\t * hidden references.\n>> +\t * config transfer.hideRefs of upload-pack is diffient from arg exclude of for-each-ref,\n>> +\t * We should not set exclude_patterns here\n>> \t */\n>> -\tif (allow_hidden_refs(data->allow_uor))\n>> -\t\texcludes = hidden_refs_to_excludes(&data->hidden_refs);\n>> -\n>> \trefs_for_each_namespaced_ref(get_main_ref_store(the_repository),\n>> -\t\t\t\t excludes, fn, data);\n>> +\t\t\t\t NULL, fn, data);\n>> }\n> This message is missing any context _why_ we want to do this. For\n> background: setting up these exclude patterns for hidden references is\n> quite an important performance optimization in large repositories, so\n> disabling it just like that is not an option without a good reason to do\n> so.\n>\n> So what is the issue that you see and why is this fix the solution for\n> that issue?\n>\n> Patrick\n\n\nOh, so sorry, I should merge mail message\n\nSee: \nhttps://lore.kernel.org/git/CAD6AYr-ZC32VNfUfMB63H-rQRfTdV=VQfBm67i2mG+6GDCNxkQ@mail.gmail.com/T/#u\n\nCopy message here\n\n---\n\nHello everyone\n\nOS: Linux Mint 22\ngit version: v2.48.1\n\nI found that packed refs are excluded by the transfer.hideRefs front\nmatch, while loose refs use full match (when transfer.hideRefs ends\nwith '/', it is prefix match, which is normal)\n\nWhen the server uses git, after setting transfer.hideRefs, the\nreferences that the client can see before and after server repo gc are\ndifferent\n\nIt seems that 59c35fa accidentally damaged upload-pack when optimizing\ngit for-each-ref\n\nIt seems that there is no simple fix except rolling back this commit?\n\n"}]}