{"thread":{"id":"63011","subject":"The transfer.hideRefs of the upload-pack process does not work properly","startedAt":"2025-02-27T07:24:19Z","lastAt":"2025-03-06T01:21:12Z","messageCount":10,"participants":["SURA","Taylor Blau","Jeff King","Patrick Steinhardt","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"513135","messageId":"CAD6AYr-ZC32VNfUfMB63H-rQRfTdV=VQfBm67i2mG+6GDCNxkQ@mail.gmail.com","threadId":"63011","inReplyTo":null,"subject":"The transfer.hideRefs of the upload-pack process does not work properly","fromName":"SURA","fromEmail":"surak8806@gmail.com","sentAt":"2025-02-27T07:24:07Z","receivedAt":"2025-02-27T07:24:19Z","isPatch":false,"sender":{"key":"surak8806@gmail.com","avatar":null},"body":"Hello 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"},{"id":"513202","messageId":"Z8D/aiqN5e/aRSn7@nand.local","threadId":"63011","inReplyTo":"CAD6AYr-ZC32VNfUfMB63H-rQRfTdV=VQfBm67i2mG+6GDCNxkQ@mail.gmail.com","subject":"Re: The transfer.hideRefs of the upload-pack process does not work properly","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2025-02-28T00:12:26Z","receivedAt":"2025-02-28T00:12:29Z","isPatch":false,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"(There's a parallel discussion going on in [1], so it isn't entirely\nclear which thread to respond to. Since it seems a little premature to\ncomment on the patch itself, I'll respond here.)\n\nOn Thu, Feb 27, 2025 at 03:24:07PM +0800, SURA wrote:\n> I found that packed refs are excluded by the transfer.hideRefs front\n> match, while loose refs use full match (when transfer.hideRefs ends\n> with '/', it is prefix match, which is normal)\n>\n> When the server uses git, after setting transfer.hideRefs, the\n> references that the client can see before and after server repo gc are\n> different\n\nIt's true that the low-level loose references iterator does not know how\nto handle excluded patterns, and that is by design. In the packed-refs\ncase, we can skip over whole sections of the packed-refs file according\nto which patterns are excluded.\n\nBut in the loose references case, we haven't implemented anything like\nthat to skip over, e.g. enumerating the contents of\n\"$GIT_DIR/refs/heads/foo\" when \"refs/heads/foo/\" is excluded. (As an\naside, this is something that we could do, it just hasn't been\nimplemented yet).\n\nSo in practice today the only way to exclude loose references according\nto some set of exclusion patterns would be to filter them out as we\niterate over them. But that is the caller's responsibility, as we see in\na handful of comments in refs.h which say \"any pattern in\n'exclude_patterns' [is] omitted on a best-effort basis\".\n\nSo upload-pack / etc. will see all loose references, and it filters out\nthe ones which it's supposed to hide via:\n\n    upload_pack() -> for_each_namespaced_ref_1() -> send_ref() ->\n    write_v0_ref() -> mark_our_ref() -> ref_is_hidden()\n\n, where mark_our_ref() tosses out references that the low-level refs\niterator gave back to it which match one of the excluded patterns.\n\nAnd there we have ref_is_hidden() doing the following for each hidden\npattern:\n\n    if (subject &&\n        skip_prefix(subject, match, &p) &&\n        (!*p || *p == '/'))\n     return !neg;\n\nSo if the reference either matches the pattern exactly, or matches up to\na '/', then it is hidden and thus not advertised.\n\nI have to imagine I'm missing something, but perhaps it would be useful\nif you could provide a reproduction script that demonstrates what you're\nseeing.\n\n> It seems that 59c35fa accidentally damaged upload-pack when optimizing\n> git for-each-ref\n\nNo. 59c35fac54 (refs/packed-backend.c: implement jump lists to avoid\nexcluded pattern(s), 2023-07-10) predates any behavior changes in\nupload-pack, which were introduced later on in 18b6b1b5c5\n(upload-pack.c: avoid enumerating hidden refs where possible,\n2023-07-10).\n\nThanks,\nTaylor\n\n[1]: https://lore.kernel.org/git/MA0P287MB06412DF70BCDA0D99641129FE4CD2@MA0P287MB0641.INDP287.PROD.OUTLOOK.COM/T/#t\n"},{"id":"513217","messageId":"CAD6AYr84KuBb5N-LVBQo-6Gq2Ms3JKQCk0gyO8i=N8gp3whe_Q@mail.gmail.com","threadId":"63011","inReplyTo":"Z8D/aiqN5e/aRSn7@nand.local","subject":"Re: The transfer.hideRefs of the upload-pack process does not work properly","fromName":"SURA","fromEmail":"surak8806@gmail.com","sentAt":"2025-02-28T02:32:01Z","receivedAt":"2025-02-28T02:32:13Z","isPatch":false,"sender":{"key":"surak8806@gmail.com","avatar":null},"body":"Taylor Blau <me@ttaylorr.com> 于2025年2月28日周五 08:12写道：\n>\n> (There's a parallel discussion going on in [1], so it isn't entirely\n> clear which thread to respond to. Since it seems a little premature to\n> comment on the patch itself, I'll respond here.)\n\nSo sorry, I used another email address to submit the patch, it is\nindeed a bit early\n\n> On Thu, Feb 27, 2025 at 03:24:07PM +0800, SURA wrote:\n> > I found that packed refs are excluded by the transfer.hideRefs front\n> > match, while loose refs use full match (when transfer.hideRefs ends\n> > with '/', it is prefix match, which is normal)\n> >\n> > When the server uses git, after setting transfer.hideRefs, the\n> > references that the client can see before and after server repo gc are\n> > different\n>\n> It's true that the low-level loose references iterator does not know how\n> to handle excluded patterns, and that is by design. In the packed-refs\n> case, we can skip over whole sections of the packed-refs file according\n> to which patterns are excluded.\n>\n> But in the loose references case, we haven't implemented anything like\n> that to skip over, e.g. enumerating the contents of\n> \"$GIT_DIR/refs/heads/foo\" when \"refs/heads/foo/\" is excluded. (As an\n> aside, this is something that we could do, it just hasn't been\n> implemented yet).\n>\n> So in practice today the only way to exclude loose references according\n> to some set of exclusion patterns would be to filter them out as we\n> iterate over them. But that is the caller's responsibility, as we see in\n> a handful of comments in refs.h which say \"any pattern in\n> 'exclude_patterns' [is] omitted on a best-effort basis\".\n>\n> So upload-pack / etc. will see all loose references, and it filters out\n> the ones which it's supposed to hide via:\n>\n>     upload_pack() -> for_each_namespaced_ref_1() -> send_ref() ->\n>     write_v0_ref() -> mark_our_ref() -> ref_is_hidden()\n>\n> , where mark_our_ref() tosses out references that the low-level refs\n> iterator gave back to it which match one of the excluded patterns.\n>\n> And there we have ref_is_hidden() doing the following for each hidden\n> pattern:\n>\n>     if (subject &&\n>         skip_prefix(subject, match, &p) &&\n>         (!*p || *p == '/'))\n>      return !neg;\n>\n> So if the reference either matches the pattern exactly, or matches up to\n> a '/', then it is hidden and thus not advertised.\n>\n> I have to imagine I'm missing something, but perhaps it would be useful\n> if you could provide a reproduction script that demonstrates what you're\n> seeing.\n>\n> > It seems that 59c35fa accidentally damaged upload-pack when optimizing\n> > git for-each-ref\n>\n> No. 59c35fac54 (refs/packed-backend.c: implement jump lists to avoid\n> excluded pattern(s), 2023-07-10) predates any behavior changes in\n> upload-pack, which were introduced later on in 18b6b1b5c5\n> (upload-pack.c: avoid enumerating hidden refs where possible,\n> 2023-07-10).\n>\n> Thanks,\n> Taylor\n>\n> [1]: https://lore.kernel.org/git/MA0P287MB06412DF70BCDA0D99641129FE4CD2@MA0P287MB0641.INDP287.PROD.OUTLOOK.COM/T/#t\n\nI shouldn't have mentioned the '/' suffix, it's confusing\n\nMy previous description was not clear enough. The early hiding\naccording to exclude_patterns in packed_ref_iterator_begin seems to be\ndesigned for git for-each-ref's exclude. It is different from the\nref_hidden matching rule used by upload-pack.\n\nI provide a reproducible step to make it clear\n\n------\n\n# create git repo\n$ mkdir sura-repo && cd sura-repo\n$ git init\n\n# create one commit\n$ echo \"hello\" > file-001\n$ git add . && git commit -m \"init repo\"\n\n# create some refs\n$ git checkout -b sura\n$ git checkout -b sura-001\n$ git checkout -b sura-002\n$ git checkout -b sura-003\n\n# show refs\n$ git for-each-ref\nd0205e0d0a0a7a6d1a712afb3734ad3e88eeda1c commit refs/heads/master\nd0205e0d0a0a7a6d1a712afb3734ad3e88eeda1c commit refs/heads/sura\nd0205e0d0a0a7a6d1a712afb3734ad3e88eeda1c commit refs/heads/sura-001\nd0205e0d0a0a7a6d1a712afb3734ad3e88eeda1c commit refs/heads/sura-002\nd0205e0d0a0a7a6d1a712afb3734ad3e88eeda1c commit refs/heads/sura-003\n\n# upload-pack, normal, hide 'refs/heads/sura'\n$ git -c transfer.hiderefs=refs/heads/sura upload-pack .git\n0103d0205e0d0a0a7a6d1a712afb3734ad3e88eeda1c HEADmulti_ack thin-pack\nside-band side-band-64k ofs-delta shallow deepen-since deepen-not\ndeepen-relative no-progress include-tag multi_ack_detailed\nsymref=HEAD:refs/heads/sura object-format=sha1 agent=git/2.46.0\n003fd0205e0d0a0a7a6d1a712afb3734ad3e88eeda1c refs/heads/master\n0041d0205e0d0a0a7a6d1a712afb3734ad3e88eeda1c refs/heads/sura-001\n0041d0205e0d0a0a7a6d1a712afb3734ad3e88eeda1c refs/heads/sura-002\n0041d0205e0d0a0a7a6d1a712afb3734ad3e88eeda1c refs/heads/sura-003\n0000\n\n# gc make loose refs to packed refs\n$ git gc\n\n# then upload-pack\n$ git -c transfer.hiderefs=refs/heads/sura upload-pack .git\n0103d0205e0d0a0a7a6d1a712afb3734ad3e88eeda1c HEADmulti_ack thin-pack\nside-band side-band-64k ofs-delta shallow deepen-since deepen-not\ndeepen-relative no-progress include-tag multi_ack_detailed\nsymref=HEAD:refs/heads/sura object-format=sha1 agent=git/2.46.0\n003fd0205e0d0a0a7a6d1a712afb3734ad3e88eeda1c refs/heads/master\n0000\n"},{"id":"513504","messageId":"20250304075113.GD1283943@coredump.intra.peff.net","threadId":"63011","inReplyTo":"CAD6AYr84KuBb5N-LVBQo-6Gq2Ms3JKQCk0gyO8i=N8gp3whe_Q@mail.gmail.com","subject":"Re: The transfer.hideRefs of the upload-pack process does not work properly","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-03-04T07:51:13Z","receivedAt":"2025-03-04T07:51:15Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 28, 2025 at 10:32:01AM +0800, SURA wrote:\n\n> My previous description was not clear enough. The early hiding\n> according to exclude_patterns in packed_ref_iterator_begin seems to be\n> designed for git for-each-ref's exclude. It is different from the\n> ref_hidden matching rule used by upload-pack.\n\nFrom your reproduction, it looks like the issue is that for loose refs,\nasking for_each_ref() to exclude \"refs/heads/foo\" will not yield\n\"refs/heads/foo/bar\", but will yield \"refs/heads/foo-bar\".\n\nAnd that was true for packed-refs, too, before 59c35fac54\n(refs/packed-backend.c: implement jump lists to avoid excluded\npattern(s), 2023-07-10). After that, packed-refs exclude both.\n\nSo probably the solution is for the jump list in 59c35fac54 to be\npickier about finding its start/end points. It should insist on a\ntrailing \"/\" (I think end-of-string would also be valid, but it may be\neasier to ignore that, and it is OK to err on the side of inclusion,\nsince the caller is supposed to do their own filtering).\n\nProbably the logic needs to go into cmp_record_to_refname(), but I lack\nsufficient brain power at this time of night to even attempt a fix.\n\nThe smallest reproduction for me is:\n\n  git init\n  git commit --allow-empty -m foo\n  git pack-refs --all\n  git -c transfer.hiderefs=refs/he upload-pack .\n\nwhich shows \"refs/heads/main\" (or \"master\") before 59c35fac54, but not\nafter.\n\n-Peff\n"},{"id":"513505","messageId":"20250304075146.GA1297781@coredump.intra.peff.net","threadId":"63011","inReplyTo":"20250304075113.GD1283943@coredump.intra.peff.net","subject":"Re: The transfer.hideRefs of the upload-pack process does not work properly","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-03-04T07:51:46Z","receivedAt":"2025-03-04T07:51:47Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 04, 2025 at 02:51:14AM -0500, Jeff King wrote:\n\n> From your reproduction, it looks like the issue is that for loose refs,\n> asking for_each_ref() to exclude \"refs/heads/foo\" will not yield\n> \"refs/heads/foo/bar\", but will yield \"refs/heads/foo-bar\".\n> \n> And that was true for packed-refs, too, before 59c35fac54\n> (refs/packed-backend.c: implement jump lists to avoid excluded\n> pattern(s), 2023-07-10). After that, packed-refs exclude both.\n\nOh, and of course it would be interesting to know how reftables behave\nhere, too, as I think they recently learned about exclusions.\n\n-Peff\n"},{"id":"513528","messageId":"Z8bmSj_Ds7ePpzBM@pks.im","threadId":"63011","inReplyTo":"20250304075146.GA1297781@coredump.intra.peff.net","subject":"Re: The transfer.hideRefs of the upload-pack process does not work properly","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-04T11:38:50Z","receivedAt":"2025-03-04T11:38:59Z","isPatch":false,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Mar 04, 2025 at 02:51:46AM -0500, Jeff King wrote:\n> On Tue, Mar 04, 2025 at 02:51:14AM -0500, Jeff King wrote:\n> \n> > From your reproduction, it looks like the issue is that for loose refs,\n> > asking for_each_ref() to exclude \"refs/heads/foo\" will not yield\n> > \"refs/heads/foo/bar\", but will yield \"refs/heads/foo-bar\".\n> > \n> > And that was true for packed-refs, too, before 59c35fac54\n> > (refs/packed-backend.c: implement jump lists to avoid excluded\n> > pattern(s), 2023-07-10). After that, packed-refs exclude both.\n> \n> Oh, and of course it would be interesting to know how reftables behave\n> here, too, as I think they recently learned about exclusions.\n\nWell, we have tests that explicitly verify that prefixes cause us to\nexclude such refs in t1419, \"overlapping exclude regions\". So the\nreftable backend is bug-compatible :)\n\nGuess we'll need something similar to the following:\n\ndiff --git a/refs/reftable-backend.c b/refs/reftable-backend.c\nindex 7e90e13f745..45462f2ce6d 100644\n--- a/refs/reftable-backend.c\n+++ b/refs/reftable-backend.c\n@@ -594,6 +594,16 @@ static int should_exclude_current_ref(struct reftable_ref_iterator *iter)\n \t\tif (cmp < 0)\n \t\t\treturn 0;\n \n+\t\t/*\n+\t\t * The exclude pattern needs to either match exactly, or, if it\n+\t\t * is a prefix of the given reference, it must be a containing\n+\t\t * directory.\n+\t\t */\n+\t\tif (iter->ref.refname[iter->exclude_patterns_strlen] &&\n+\t\t    pattern[iter->exclude_patterns_strlen - 1] != '/' &&\n+\t\t    iter->ref.refname[iter->exclude_patterns_strlen] != '/')\n+\t\t\treturn 0;\n+\n \t\t/*\n \t\t * The reference shares a prefix with the exclude pattern and\n \t\t * shall thus be omitted. We skip all references that match the\n\nPatrick\n"},{"id":"513540","messageId":"Z8cs8Y4CVpP0QHNj@nand.local","threadId":"63011","inReplyTo":"Z8bmSj_Ds7ePpzBM@pks.im","subject":"Re: The transfer.hideRefs of the upload-pack process does not work properly","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2025-03-04T16:40:17Z","receivedAt":"2025-03-04T16:40:19Z","isPatch":false,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Mar 04, 2025 at 12:38:50PM +0100, Patrick Steinhardt wrote:\n> On Tue, Mar 04, 2025 at 02:51:46AM -0500, Jeff King wrote:\n> > On Tue, Mar 04, 2025 at 02:51:14AM -0500, Jeff King wrote:\n> >\n> > > From your reproduction, it looks like the issue is that for loose refs,\n> > > asking for_each_ref() to exclude \"refs/heads/foo\" will not yield\n> > > \"refs/heads/foo/bar\", but will yield \"refs/heads/foo-bar\".\n> > >\n> > > And that was true for packed-refs, too, before 59c35fac54\n> > > (refs/packed-backend.c: implement jump lists to avoid excluded\n> > > pattern(s), 2023-07-10). After that, packed-refs exclude both.\n> >\n> > Oh, and of course it would be interesting to know how reftables behave\n> > here, too, as I think they recently learned about exclusions.\n>\n> Well, we have tests that explicitly verify that prefixes cause us to\n> exclude such refs in t1419, \"overlapping exclude regions\". So the\n> reftable backend is bug-compatible :)\n>\n> Guess we'll need something similar to the following:\n>\n> diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c\n> index 7e90e13f745..45462f2ce6d 100644\n> --- a/refs/reftable-backend.c\n> +++ b/refs/reftable-backend.c\n> @@ -594,6 +594,16 @@ static int should_exclude_current_ref(struct reftable_ref_iterator *iter)\n>  \t\tif (cmp < 0)\n>  \t\t\treturn 0;\n>\n> +\t\t/*\n> +\t\t * The exclude pattern needs to either match exactly, or, if it\n> +\t\t * is a prefix of the given reference, it must be a containing\n> +\t\t * directory.\n> +\t\t */\n> +\t\tif (iter->ref.refname[iter->exclude_patterns_strlen] &&\n> +\t\t    pattern[iter->exclude_patterns_strlen - 1] != '/' &&\n> +\t\t    iter->ref.refname[iter->exclude_patterns_strlen] != '/')\n> +\t\t\treturn 0;\n> +\n>  \t\t/*\n>  \t\t * The reference shares a prefix with the exclude pattern and\n>  \t\t * shall thus be omitted. We skip all references that match the\n\nAh, makes sense. Thanks, both, for looking a little deeper. I can work\non a fix shortly...\n\nThanks,\nTaylor\n"},{"id":"513622","messageId":"Z8jadiyUj/U0TORF@nand.local","threadId":"63011","inReplyTo":"20250304075113.GD1283943@coredump.intra.peff.net","subject":"Re: The transfer.hideRefs of the upload-pack process does not work properly","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2025-03-05T23:12:54Z","receivedAt":"2025-03-05T23:12:57Z","isPatch":false,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Mar 04, 2025 at 02:51:13AM -0500, Jeff King wrote:\n> On Fri, Feb 28, 2025 at 10:32:01AM +0800, SURA wrote:\n>\n> > My previous description was not clear enough. The early hiding\n> > according to exclude_patterns in packed_ref_iterator_begin seems to be\n> > designed for git for-each-ref's exclude. It is different from the\n> > ref_hidden matching rule used by upload-pack.\n>\n> >From your reproduction, it looks like the issue is that for loose refs,\n> asking for_each_ref() to exclude \"refs/heads/foo\" will not yield\n> \"refs/heads/foo/bar\", but will yield \"refs/heads/foo-bar\".\n>\n> And that was true for packed-refs, too, before 59c35fac54\n> (refs/packed-backend.c: implement jump lists to avoid excluded\n> pattern(s), 2023-07-10). After that, packed-refs exclude both.\n\nThanks for the careful analysis. Since you and I co-wrote this feature\nin the first place, naturally I agree with what you wrote here ;-).\n\n> So probably the solution is for the jump list in 59c35fac54 to be\n> pickier about finding its start/end points. It should insist on a\n> trailing \"/\" (I think end-of-string would also be valid, but it may be\n> easier to ignore that, and it is OK to err on the side of inclusion,\n> since the caller is supposed to do their own filtering).\n>\n> Probably the logic needs to go into cmp_record_to_refname(), but I lack\n> sufficient brain power at this time of night to even attempt a fix.\n\nThat is definitely one way to fix the issue, and the fix would look\nsomething like the following:\n\n--- 8< ---\ndiff --git a/refs/packed-backend.c b/refs/packed-backend.c\nindex a7b6f74b6e..b137641f9d 100644\n--- a/refs/packed-backend.c\n+++ b/refs/packed-backend.c\n@@ -326,7 +326,8 @@ static int cmp_packed_ref_records(const void *v1, const void *v2,\n  * refname.\n  */\n static int cmp_record_to_refname(const char *rec, const char *refname,\n-                                int start, const struct snapshot *snapshot)\n+                                int start, int strict,\n+                                const struct snapshot *snapshot)\n {\n        const char *r1 = rec + snapshot_hexsz(snapshot) + 1;\n        const char *r2 = refname;\n@@ -334,8 +335,11 @@ static int cmp_record_to_refname(const char *rec, const char *refname,\n        while (1) {\n                if (*r1 == '\\n')\n                        return *r2 ? -1 : 0;\n-               if (!*r2)\n+               if (!*r2) {\n+                       if (strict && *r1 != '/')\n+                               return 1;\n                        return start ? 1 : -1;\n+               }\n                if (*r1 != *r2)\n                        return (unsigned char)*r1 < (unsigned char)*r2 ? -1 : +1;\n                r1++;\n--- >8 ---\n\nI'm eliding some plumbing here to pass the \"strict\" flag through the\ncallers eventually all the way down to cmp_record_to_refname().\n\nBut I think this is equivalent to pretending like the excluded patterns\nall end in a '/' character (if they weren't already like that to begin\nwith). So equivalently, you could do something like:\n\n--- 8< ---\ndiff --git a/refs/packed-backend.c b/refs/packed-backend.c\nindex a7b6f74b6e..e4569519a1 100644\n--- a/refs/packed-backend.c\n+++ b/refs/packed-backend.c\n@@ -1024,6 +1024,7 @@ static void populate_excluded_jump_list(struct packed_ref_iterator *iter,\n        size_t i, j;\n        const char **pattern;\n        struct jump_list_entry *last_disjoint;\n+       struct strbuf buf = STRBUF_INIT;\n\n        if (!excluded_patterns)\n                return;\n@@ -1043,8 +1044,13 @@ static void populate_excluded_jump_list(struct packed_ref_iterator *iter,\n                if (has_glob_special(*pattern))\n                        continue;\n\n-               start = find_reference_location(snapshot, *pattern, 0);\n-               end = find_reference_location_end(snapshot, *pattern, 0);\n+               strbuf_reset(&buf);\n+               strbuf_addstr(&buf, *pattern);\n+               if (buf.len && buf.buf[buf.len - 1] != '/')\n+                       strbuf_addch(&buf, '/');\n+\n+               start = find_reference_location(snapshot, buf.buf, 0);\n+               end = find_reference_location_end(snapshot, buf.buf, 0);\n\n                if (start == end)\n                        continue; /* nothing to jump over */\n@@ -1061,7 +1067,7 @@ static void populate_excluded_jump_list(struct packed_ref_iterator *iter,\n                 * Every entry in exclude_patterns has a meta-character,\n                 * nothing to do here.\n                 */\n-               return;\n+               goto out;\n        }\n\n        QSORT(iter->jump, iter->jump_nr, jump_list_entry_cmp);\n@@ -1095,6 +1101,9 @@ static void populate_excluded_jump_list(struct packed_ref_iterator *iter,\n\n        iter->jump_nr = j;\n        iter->jump_cur = 0;\n+\n+out:\n+       strbuf_release(&buf);\n }\n\n static struct ref_iterator *packed_ref_iterator_begin(\n--- >8 ---\n\nBut then we have to handle the reftable case too, which Patrick gave a\npotential fix to below. But equally fine I think would be to push this\n^^ logic up into refs.c::refs_ref_iterator_begin(), which would fix both\nat the same time.\n\n> The smallest reproduction for me is:\n>\n>   git init\n>   git commit --allow-empty -m foo\n>   git pack-refs --all\n>   git -c transfer.hiderefs=refs/he upload-pack .\n>\n> which shows \"refs/heads/main\" (or \"master\") before 59c35fac54, but not\n> after.\n\nThanks, this was a very clean reproduction that made it much easier to\ndiagnose what was going on here ;-).\n\nThanks,\nTaylor\n"},{"id":"513623","messageId":"xmqqwmd3jb3y.fsf@gitster.g","threadId":"63011","inReplyTo":"Z8jadiyUj/U0TORF@nand.local","subject":"Re: The transfer.hideRefs of the upload-pack process does not work properly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-05T23:45:05Z","receivedAt":"2025-03-05T23:45:09Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau <me@ttaylorr.com> writes:\n\n> But then we have to handle the reftable case too, which Patrick gave a\n> potential fix to below. But equally fine I think would be to push this\n> ^^ logic up into refs.c::refs_ref_iterator_begin(), which would fix both\n> at the same time.\n\nSomehow, today seems to be the day to read about \"let's push as much\ncommon things up from ref backends to the generic ref layer \" for me\n;-).\n\n\n"},{"id":"513630","messageId":"Z8j4hj/+lboANog3@nand.local","threadId":"63011","inReplyTo":"Z8cs8Y4CVpP0QHNj@nand.local","subject":"Re: The transfer.hideRefs of the upload-pack process does not work properly","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2025-03-06T01:21:10Z","receivedAt":"2025-03-06T01:21:12Z","isPatch":false,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Mar 04, 2025 at 11:40:17AM -0500, Taylor Blau wrote:\n> Ah, makes sense. Thanks, both, for looking a little deeper. I can work\n> on a fix shortly...\n\nThis took me a little longer than I had hoped, but I posted the patches\nhere:\n\n    https://lore.kernel.org/git/cover.1741223981.git.me@ttaylorr.com/\n\nThanks,\nTaylor\n"}]}