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

[Taylor Blau <me@ttaylorr.com>]

(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.)

On Thu, Feb 27, 2025 at 03:24:07PM +0800, SURA wrote:
> 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.

> 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