Thread (9 messages) 9 messages, 4 authors, 2025-03-06

Re: The transfer.hideRefs of the upload-pack process does not work properly

From: SURA <hidden>
Date: 2025-02-28 02:32:13

Taylor Blau [off-list ref] 于2025年2月28日周五 08:12写道:
(There's a parallel discussion going on in [1], so it isn't entirely
clear which thread to respond to. Since it seems a little premature to
comment on the patch itself, I'll respond here.)
So sorry, I used another email address to submit the patch, it is
indeed a bit early
On Thu, Feb 27, 2025 at 03:24:07PM +0800, SURA wrote:
quoted
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's true that the low-level loose references iterator does not know how
to handle excluded patterns, and that is by design. In the packed-refs
case, we can skip over whole sections of the packed-refs file according
to which patterns are excluded.

But in the loose references case, we haven't implemented anything like
that to skip over, e.g. enumerating the contents of
"$GIT_DIR/refs/heads/foo" when "refs/heads/foo/" is excluded. (As an
aside, this is something that we could do, it just hasn't been
implemented yet).

So in practice today the only way to exclude loose references according
to some set of exclusion patterns would be to filter them out as we
iterate over them. But that is the caller's responsibility, as we see in
a handful of comments in refs.h which say "any pattern in
'exclude_patterns' [is] omitted on a best-effort basis".

So upload-pack / etc. will see all loose references, and it filters out
the ones which it's supposed to hide via:

    upload_pack() -> for_each_namespaced_ref_1() -> send_ref() ->
    write_v0_ref() -> mark_our_ref() -> ref_is_hidden()

, where mark_our_ref() tosses out references that the low-level refs
iterator gave back to it which match one of the excluded patterns.

And there we have ref_is_hidden() doing the following for each hidden
pattern:

    if (subject &&
        skip_prefix(subject, match, &p) &&
        (!*p || *p == '/'))
     return !neg;

So if the reference either matches the pattern exactly, or matches up to
a '/', then it is hidden and thus not advertised.

I have to imagine I'm missing something, but perhaps it would be useful
if you could provide a reproduction script that demonstrates what you're
seeing.
quoted
It seems that 59c35fa accidentally damaged upload-pack when optimizing
git for-each-ref
No. 59c35fac54 (refs/packed-backend.c: implement jump lists to avoid
excluded pattern(s), 2023-07-10) predates any behavior changes in
upload-pack, which were introduced later on in 18b6b1b5c5
(upload-pack.c: avoid enumerating hidden refs where possible,
2023-07-10).

Thanks,
Taylor

[1]: https://lore.kernel.org/git/MA0P287MB06412DF70BCDA0D99641129FE4CD2@MA0P287MB0641.INDP287.PROD.OUTLOOK.COM/T/#t (local)
I shouldn't have mentioned the '/' suffix, it's confusing

My previous description was not clear enough. The early hiding
according to exclude_patterns in packed_ref_iterator_begin seems to be
designed for git for-each-ref's exclude. It is different from the
ref_hidden matching rule used by upload-pack.

I provide a reproducible step to make it clear

------

# create git repo
$ mkdir sura-repo && cd sura-repo
$ git init

# create one commit
$ echo "hello" > file-001
$ git add . && git commit -m "init repo"

# create some refs
$ git checkout -b sura
$ git checkout -b sura-001
$ git checkout -b sura-002
$ git checkout -b sura-003

# show refs
$ git for-each-ref
d0205e0d0a0a7a6d1a712afb3734ad3e88eeda1c commit refs/heads/master
d0205e0d0a0a7a6d1a712afb3734ad3e88eeda1c commit refs/heads/sura
d0205e0d0a0a7a6d1a712afb3734ad3e88eeda1c commit refs/heads/sura-001
d0205e0d0a0a7a6d1a712afb3734ad3e88eeda1c commit refs/heads/sura-002
d0205e0d0a0a7a6d1a712afb3734ad3e88eeda1c commit refs/heads/sura-003

# upload-pack, normal, hide 'refs/heads/sura'
$ git -c transfer.hiderefs=refs/heads/sura upload-pack .git
0103d0205e0d0a0a7a6d1a712afb3734ad3e88eeda1c HEADmulti_ack thin-pack
side-band side-band-64k ofs-delta shallow deepen-since deepen-not
deepen-relative no-progress include-tag multi_ack_detailed
symref=HEAD:refs/heads/sura object-format=sha1 agent=git/2.46.0
003fd0205e0d0a0a7a6d1a712afb3734ad3e88eeda1c refs/heads/master
0041d0205e0d0a0a7a6d1a712afb3734ad3e88eeda1c refs/heads/sura-001
0041d0205e0d0a0a7a6d1a712afb3734ad3e88eeda1c refs/heads/sura-002
0041d0205e0d0a0a7a6d1a712afb3734ad3e88eeda1c refs/heads/sura-003
0000

# gc make loose refs to packed refs
$ git gc

# then upload-pack
$ git -c transfer.hiderefs=refs/heads/sura upload-pack .git
0103d0205e0d0a0a7a6d1a712afb3734ad3e88eeda1c HEADmulti_ack thin-pack
side-band side-band-64k ofs-delta shallow deepen-since deepen-not
deepen-relative no-progress include-tag multi_ack_detailed
symref=HEAD:refs/heads/sura object-format=sha1 agent=git/2.46.0
003fd0205e0d0a0a7a6d1a712afb3734ad3e88eeda1c refs/heads/master
0000
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help