upload-pack: No longer use hidden-refs as exclude_patterns #1866
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
There are issues in commit e188fe8: |
|
There are issues in commit 13ebc46: |
|
There are issues in commit 28f388f: |
Signed-off-by: SURA <[email protected]>
|
/submit |
|
Submitted as [email protected] To fetch this version into To fetch this version to local tag |
|
On the Git mailing list, Patrick Steinhardt wrote (reply to this): On Thu, Feb 27, 2025 at 12:46:11PM +0000, SURA via GitGitGadget wrote:
> From: SURA <[email protected]>
>
> Signed-off-by: SURA <[email protected]>
> ---
> 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 |
|
User |
Oh, so sorry, I should merge email messages The following is the content of the email Hello everyone OS: Linux Mint 22 I found that packed refs are excluded by the transfer.hideRefs front When the server uses git, after setting transfer.hideRefs, the It seems that 59c35fa accidentally damaged upload-pack when optimizing It seems that there is no simple fix except rolling back this commit? |
|
Oh, so sorry, I should merge email messages
See this: ***@***.***/T/#u
The following is the content of the email
---
Hello everyone
OS: Linux Mint 22
git version: v2.48.1
I found that packed refs are excluded by the transfer.hideRefs front
match, while loose refs use full match (when transfer.hideRefs ends
with '/', it is prefix match, which is normal)
When the server uses git, after setting transfer.hideRefs, the
references that the client can see before and after server repo gc are
different
It seems that 59c35fa accidentally damaged upload-pack when optimizing
git for-each-ref
It seems that there is no simple fix except rolling back this commit?
________________________________________
发件人: gitgitgadget[bot] ***@***.***>
发送时间: 2025年2月27日 22:57
收件人: gitgitgadget/git ***@***.***>
抄送: SURA ***@***.***>; Author ***@***.***>
主题: Re: [gitgitgadget/git] upload-pack: No longer use hidden-refs as exclude_patterns (PR #1866)
gitgitgadget[bot] left a comment (gitgitgadget/git#1866)
On the Git mailing list, Patrick Steinhardt wrote (reply to this):
On Thu, Feb 27, 2025 at 12:46:11PM +0000, SURA via GitGitGadget wrote:
From: SURA ***@***.***>
Signed-off-by: SURA ***@***.***>
---
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: #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
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you authored the thread.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
cc: Patrick Steinhardt [email protected]