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 differentIt'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-refNo. 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