{"thread":{"id":"64879","subject":"[PATCH 0/3] Fix misuse of `refs_for_each_ref_in()`","startedAt":"2026-01-28T08:49:31Z","lastAt":"2026-02-26T17:39:34Z","messageCount":42,"participants":["Patrick Steinhardt","Karthik Nayak","Taylor Blau","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"534744","messageId":"20260128-b4-pks-fix-for-each-ref-in-misuse-v1-0-deccae3ea725@pks.im","threadId":"64879","inReplyTo":null,"subject":"[PATCH 0/3] Fix misuse of `refs_for_each_ref_in()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-28T08:49:19Z","receivedAt":"2026-01-28T08:49:31Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis small patch series fixes a bug I have discovered where configuring\n\"pack.preferBitmapTips\" to an exact branch will cause Git to `BUG()`.\n\nThe root cause of this bug is misuse of `refs_for_each_ref_in()`: this\nfunction accepts a prefix to yield refs for, and then strips the prefix\nfor each ref. Consequently, if passed an exact refname, then stripping\nthe prefix would make us end up with an empty refname, and that is not\nsupposed to happen.\n\nThere was one other caller that got it wrong, too, and which is also\nfixed in this patch series.\n\nThanks!\n\nPatrick\n\n---\nPatrick Steinhardt (3):\n pack-bitmap: deduplicate logic to iterate over preferred bitmap tips\n pack-bitmap: fix bug with exact ref match in \"pack.preferBitmapTips\"\n bisect: fix misuse of `refs_for_each_ref_in()`\n\n bisect.c | 8 ++++----\n builtin/pack-objects.c | 19 ++-----------------\n pack-bitmap.c | 18 +++++++++++++++++-\n pack-bitmap.h | 9 ++++++++-\n repack-midx.c | 14 +++-----------\n t/t5310-pack-bitmaps.sh | 35 +++++++++++++++++++++++++++++++++++\n t/t5319-multi-pack-index.sh | 36 ++++++++++++++++++++++++++++++++++++\n 7 files changed, 105 insertions(+), 34 deletions(-)\n\n\n---\nbase-commit: ea717645d199f6f1b66058886475db3e8c9330e9\nchange-id: 20260128-b4-pks-fix-for-each-ref-in-misuse-96ae62e313a5\n\n"},{"id":"534745","messageId":"20260128-b4-pks-fix-for-each-ref-in-misuse-v1-1-deccae3ea725@pks.im","threadId":"64879","inReplyTo":"20260128-b4-pks-fix-for-each-ref-in-misuse-v1-0-deccae3ea725@pks.im","subject":"[PATCH 1/3] pack-bitmap: deduplicate logic to iterate over preferred bitmap tips","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-28T08:49:20Z","receivedAt":"2026-01-28T08:49:33Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We have two locations that iterate over the preferred bitmap tips as\nconfigured by the user via \"pack.preferBitmapTips\". Both of these\ncallsites are subtly wrong and can lead to a `BUG()`, which we'll fix in\na subsequent commit.\n\nPrepare for this fix by unifying the two callsites into a new\n`for_each_preferred_bitmap_tip()` function.\n\nThis removes the last callsite of `bitmap_preferred_tips()` outside of\n\"pack-bitmap.c\". As such, convert the function to be local to that file\nonly.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/pack-objects.c | 19 ++-----------------\n pack-bitmap.c | 18 +++++++++++++++++-\n pack-bitmap.h | 9 ++++++++-\n repack-midx.c | 14 +++-----------\n 4 files changed, 30 insertions(+), 30 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 5846b6a293..979470e402 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -4554,22 +4554,6 @@ static int mark_bitmap_preferred_tip(const struct reference *ref, void *data UNU\n \treturn 0;\n }\n \n-static void mark_bitmap_preferred_tips(void)\n-{\n-\tstruct string_list_item *item;\n-\tconst struct string_list *preferred_tips;\n-\n-\tpreferred_tips = bitmap_preferred_tips(the_repository);\n-\tif (!preferred_tips)\n-\t\treturn;\n-\n-\tfor_each_string_list_item(item, preferred_tips) {\n-\t\trefs_for_each_ref_in(get_main_ref_store(the_repository),\n-\t\t\t\t item->string, mark_bitmap_preferred_tip,\n-\t\t\t\t NULL);\n-\t}\n-}\n-\n static inline int is_oid_uninteresting(struct repository *repo,\n \t\t\t\t struct object_id *oid)\n {\n@@ -4710,7 +4694,8 @@ static void get_object_list(struct rev_info *revs, struct strvec *argv)\n \t\tload_delta_islands(the_repository, progress);\n \n \tif (write_bitmap_index)\n-\t\tmark_bitmap_preferred_tips();\n+\t\tfor_each_preferred_bitmap_tip(the_repository, mark_bitmap_preferred_tip,\n+\t\t\t\t\t NULL);\n \n \tif (!fn_show_object)\n \t\tfn_show_object = show_object;\ndiff --git a/pack-bitmap.c b/pack-bitmap.c\nindex 972203f12b..2f5cb34009 100644\n--- a/pack-bitmap.c\n+++ b/pack-bitmap.c\n@@ -3314,7 +3314,7 @@ int bitmap_is_midx(struct bitmap_index *bitmap_git)\n \treturn !!bitmap_git->midx;\n }\n \n-const struct string_list *bitmap_preferred_tips(struct repository *r)\n+static const struct string_list *bitmap_preferred_tips(struct repository *r)\n {\n \tconst struct string_list *dest;\n \n@@ -3323,6 +3323,22 @@ const struct string_list *bitmap_preferred_tips(struct repository *r)\n \treturn NULL;\n }\n \n+void for_each_preferred_bitmap_tip(struct repository *repo,\n+\t\t\t\t each_ref_fn cb, void *cb_data)\n+{\n+\tstruct string_list_item *item;\n+\tconst struct string_list *preferred_tips;\n+\n+\tpreferred_tips = bitmap_preferred_tips(repo);\n+\tif (!preferred_tips)\n+\t\treturn;\n+\n+\tfor_each_string_list_item(item, preferred_tips) {\n+\t\trefs_for_each_ref_in(get_main_ref_store(repo),\n+\t\t\t\t item->string, cb, cb_data);\n+\t}\n+}\n+\n int bitmap_is_preferred_refname(struct repository *r, const char *refname)\n {\n \tconst struct string_list *preferred_tips = bitmap_preferred_tips(r);\ndiff --git a/pack-bitmap.h b/pack-bitmap.h\nindex 1bd7a791e2..d0611d0481 100644\n--- a/pack-bitmap.h\n+++ b/pack-bitmap.h\n@@ -5,6 +5,7 @@\n #include \"khash.h\"\n #include \"pack.h\"\n #include \"pack-objects.h\"\n+#include \"refs.h\"\n #include \"string-list.h\"\n \n struct commit;\n@@ -99,6 +100,13 @@ int for_each_bitmapped_object(struct bitmap_index *bitmap_git,\n \t\t\t show_reachable_fn show_reach,\n \t\t\t void *payload);\n \n+/*\n+ * Iterate over all references that are configured as preferred bitmap tips via\n+ * \"pack.preferBitmapTips\" and invoke the callback on each function.\n+ */\n+void for_each_preferred_bitmap_tip(struct repository *repo,\n+\t\t\t\t each_ref_fn cb, void *cb_data);\n+\n #define GIT_TEST_PACK_USE_BITMAP_BOUNDARY_TRAVERSAL \\\n \t\"GIT_TEST_PACK_USE_BITMAP_BOUNDARY_TRAVERSAL\"\n \n@@ -182,7 +190,6 @@ char *pack_bitmap_filename(struct packed_git *p);\n \n int bitmap_is_midx(struct bitmap_index *bitmap_git);\n \n-const struct string_list *bitmap_preferred_tips(struct repository *r);\n int bitmap_is_preferred_refname(struct repository *r, const char *refname);\n \n int verify_bitmap_files(struct repository *r);\ndiff --git a/repack-midx.c b/repack-midx.c\nindex 74bdfa3a6e..0682b80c42 100644\n--- a/repack-midx.c\n+++ b/repack-midx.c\n@@ -40,7 +40,6 @@ static int midx_snapshot_ref_one(const struct reference *ref, void *_data)\n void midx_snapshot_refs(struct repository *repo, struct tempfile *f)\n {\n \tstruct midx_snapshot_ref_data data;\n-\tconst struct string_list *preferred = bitmap_preferred_tips(repo);\n \n \tdata.repo = repo;\n \tdata.f = f;\n@@ -51,16 +50,9 @@ void midx_snapshot_refs(struct repository *repo, struct tempfile *f)\n \t\t die(_(\"could not open tempfile %s for writing\"),\n \t\t get_tempfile_path(f));\n \n-\tif (preferred) {\n-\t\tstruct string_list_item *item;\n-\n-\t\tdata.preferred = 1;\n-\t\tfor_each_string_list_item(item, preferred)\n-\t\t\trefs_for_each_ref_in(get_main_ref_store(repo),\n-\t\t\t\t\t item->string,\n-\t\t\t\t\t midx_snapshot_ref_one, &data);\n-\t\tdata.preferred = 0;\n-\t}\n+\tdata.preferred = 1;\n+\tfor_each_preferred_bitmap_tip(repo, midx_snapshot_ref_one, &data);\n+\tdata.preferred = 0;\n \n \trefs_for_each_ref(get_main_ref_store(repo),\n \t\t\t midx_snapshot_ref_one, &data);\n\n-- \n2.53.0.rc2.206.g60c1bca835.dirty\n\n"},{"id":"534746","messageId":"20260128-b4-pks-fix-for-each-ref-in-misuse-v1-2-deccae3ea725@pks.im","threadId":"64879","inReplyTo":"20260128-b4-pks-fix-for-each-ref-in-misuse-v1-0-deccae3ea725@pks.im","subject":"[PATCH 2/3] pack-bitmap: fix bug with exact ref match in \"pack.preferBitmapTips\"","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-28T08:49:21Z","receivedAt":"2026-01-28T08:49:35Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The \"pack.preferBitmapTips\" configuration allows the user to specify\nwhich references should be preferred when generating bitmaps. This\noption is typically expected to be set to a reference prefix, like for\nexample \"refs/heads/\".\n\nIt's not unreasonable though for a user to configure one specific\nreference as preferred. But if they do, they'll hit a `BUG()`:\n\n $ git -c pack.preferBitmapTips=refs/heads/main repack -adb\n BUG: ../refs/iterator.c:366: attempt to trim too many characters\n error: pack-objects died of signal 6\n\nThe root cause for this bug is how we enumerate these references. We\ncall `refs_for_each_ref_in()`, which will:\n\n - Yield all references that have a user-specified prefix.\n\n - Trim each of these references so that the prefix is removed.\n\nTypically, this function is called with a trailing slash, like\n\"refs/heads/\", and in that case things work alright. But if the function\nis called with the name of an existing reference then we'll try to trim\nthe full reference name, which would leave us with an empty name. And as\nthis would not really leave us with anything sensible, we call `BUG()`\ninstead of yielding this reference.\n\nOne could argue that this is a bug in `refs_for_each_ref_in()`. But the\nquestion then becomes what the correct behaviour would be:\n\n - Do we want to skip exact matches? In our case we certainly don't\n want that, as the user has asked us to generate a bitmap for it.\n\n - Do we want to yield the reference with the empty refname? That would\n lead to a somewhat weird result.\n\nNeither of these feel like viable options, so calling `BUG()` feels like\na sensible way out.\n\nThe root cause really is that we try to trim the whole refname. We can\nthus easily fix the bug itself by calling `refs_for_each_fullref_in()`\ninstead. This function behaves the same as `refs_for_each_ref_in()`,\nexcept that it doesn't strip the prefix. Consequently, it correctly\nyields also exact refnames.\n\nOne resulting weirdness is that two refs \"refs/heads/base\" and\n\"refs/heads/base-something\" would now match if the user configured\n\"refs/heads/base\" as bitmap tips. One could arguably change the\nsemantics of the configuration such that a string without a trailing\nslash needs to be an exact reference match, whereas a string with a\ntrailing slash indicates a directory hierarchy. But such a change would\npotentially cause regressions with dubious benefits, so this issue is\nignored for now.\n\nSigned-off-by: Patrick Steinhardt \n---\n pack-bitmap.c | 4 ++--\n t/t5310-pack-bitmaps.sh | 35 +++++++++++++++++++++++++++++++++++\n t/t5319-multi-pack-index.sh | 36 ++++++++++++++++++++++++++++++++++++\n 3 files changed, 73 insertions(+), 2 deletions(-)\n\ndiff --git a/pack-bitmap.c b/pack-bitmap.c\nindex 2f5cb34009..8d3b5ac037 100644\n--- a/pack-bitmap.c\n+++ b/pack-bitmap.c\n@@ -3334,8 +3334,8 @@ void for_each_preferred_bitmap_tip(struct repository *repo,\n \t\treturn;\n \n \tfor_each_string_list_item(item, preferred_tips) {\n-\t\trefs_for_each_ref_in(get_main_ref_store(repo),\n-\t\t\t\t item->string, cb, cb_data);\n+\t\trefs_for_each_fullref_in(get_main_ref_store(repo),\n+\t\t\t\t\t item->string, NULL, cb, cb_data);\n \t}\n }\n \ndiff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh\nindex 6718fb98c0..7ef91b502c 100755\n--- a/t/t5310-pack-bitmaps.sh\n+++ b/t/t5310-pack-bitmaps.sh\n@@ -466,6 +466,41 @@ test_bitmap_cases () {\n \t\t)\n \t'\n \n+\ttest_expect_success 'pack.preferBitmapTips can use direct refname' '\n+\t\tgit init repo &&\n+\t\ttest_when_finished \"rm -fr repo\" &&\n+\t\t(\n+\t\t\tcd repo &&\n+\n+\t\t\t# Create enough commits that not all will receive bitmap\n+\t\t\t# coverage even if they are all at the tip of some reference.\n+\t\t\ttest_commit_bulk --message=\"%s\" 103 &&\n+\t\t\tgit log --format=\"create refs/tags/%s %H\" HEAD >refs &&\n+\t\t\tgit update-ref --stdin commits-with-bitmap &&\n+\n+\t\t\t# Verify that we have at least one commit that did not\n+\t\t\t# receive a bitmap.\n+\t\t\tgit rev-list HEAD >commits.raw &&\n+\t\t\tsort commits &&\n+\t\t\tcomm -13 commits-with-bitmap commits >commits-wo-bitmap &&\n+\t\t\ttest_file_not_empty commits-wo-bitmap &&\n+\t\t\tcommit_id=$(head commits-wo-bitmap) &&\n+\n+\t\t\t# We now create a reference for this commit and repack\n+\t\t\t# with \"preferBitmapTips\" pointing to that exact\n+\t\t\t# reference. The expectation is that it will now be\n+\t\t\t# covered by a bitmap.\n+\t\t\tgit update-ref refs/heads/cover-me \"$commit_id\" &&\n+\t\t\tgit -c pack.preferBitmapTips=refs/heads/cover-me repack -adb &&\n+\t\t\ttest-tool bitmap list-commits >after &&\n+\t\t\ttest_grep \"$commit_id\" after\n+\t\t)\n+\t'\n+\n \ttest_expect_success 'complains about multiple pack bitmaps' '\n \t\trm -fr repo &&\n \t\tgit init repo &&\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex faae98c7e7..40d36118bd 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -1345,4 +1345,40 @@ test_expect_success 'bitmapped packs are stored via the BTMP chunk' '\n \t)\n '\n \n+test_expect_success 'pack.preferBitmapTips can use direct refname' '\n+\tgit init repo &&\n+\ttest_when_finished \"rm -fr repo\" &&\n+\t(\n+\t\tcd repo &&\n+\n+\t\t# Create enough commits that not all will receive bitmap\n+\t\t# coverage even if they are all at the tip of some reference.\n+\t\ttest_commit_bulk --message=\"%s\" 103 &&\n+\t\tgit log --format=\"create refs/tags/%s %H\" HEAD >refs &&\n+\t\tgit update-ref --stdin commits-with-bitmap &&\n+\n+\t\t# Verify that we have at least one commit that did not\n+\t\t# receive a bitmap.\n+\t\tgit rev-list HEAD >commits.raw &&\n+\t\tsort commits &&\n+\t\tcomm -13 commits-with-bitmap commits >commits-wo-bitmap &&\n+\t\ttest_file_not_empty commits-wo-bitmap &&\n+\t\tcommit_id=$(head commits-wo-bitmap) &&\n+\n+\t\t# We now create a reference for this commit and repack\n+\t\t# with \"preferBitmapTips\" pointing to that exact\n+\t\t# reference. The expectation is that it will now be\n+\t\t# covered by a bitmap.\n+\t\tgit update-ref refs/heads/cover-me \"$commit_id\" &&\n+\t\trm .git/objects/pack/multi-pack-index* &&\n+\t\tgit -c pack.preferBitmapTips=refs/heads/cover-me repack -adb --write-midx &&\n+\t\ttest-tool bitmap list-commits >after &&\n+\t\ttest_grep \"$commit_id\" after\n+\t)\n+'\n+\n test_done\n\n-- \n2.53.0.rc2.206.g60c1bca835.dirty\n\n"},{"id":"534747","messageId":"20260128-b4-pks-fix-for-each-ref-in-misuse-v1-3-deccae3ea725@pks.im","threadId":"64879","inReplyTo":"20260128-b4-pks-fix-for-each-ref-in-misuse-v1-0-deccae3ea725@pks.im","subject":"[PATCH 3/3] bisect: fix misuse of `refs_for_each_ref_in()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-28T08:49:22Z","receivedAt":"2026-01-28T08:49:37Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"All callers of `refs_for_each_ref_in()` pass in a string that is\nterminated with a trailing slash to indicate that they only want to see\nrefs in that specific ref hierarchy. This is in fact a requirement if\none wants to use this function, as the function trims the prefix from\neach yielded ref. So if there was a reference that was called\n\"refs/bisect\" as in our example, the result after trimming would be the\nempty string, and that's something we disallow.\n\nFix this by adding the trailing slash.\n\nFurthermore, taking a closer look, we strip the prefix only to re-add it\nin `mark_for_removal()`. This is somewhat roundabout, as we can instead\ncall `refs_for_each_fullref_in()` to not do any stripping at all. Do so\nto simplify the code a bit.\n\nSigned-off-by: Patrick Steinhardt \n---\n bisect.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex 326b59c0dc..4f0d1a1853 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -1180,7 +1180,7 @@ int estimate_bisect_steps(int all)\n static int mark_for_removal(const struct reference *ref, void *cb_data)\n {\n \tstruct string_list *refs = cb_data;\n-\tchar *bisect_ref = xstrfmt(\"refs/bisect%s\", ref->name);\n+\tchar *bisect_ref = xstrdup(ref->name);\n \tstring_list_append(refs, bisect_ref);\n \treturn 0;\n }\n@@ -1191,9 +1191,9 @@ int bisect_clean_state(void)\n \n \t/* There may be some refs packed during bisection */\n \tstruct string_list refs_for_removal = STRING_LIST_INIT_NODUP;\n-\trefs_for_each_ref_in(get_main_ref_store(the_repository),\n-\t\t\t \"refs/bisect\", mark_for_removal,\n-\t\t\t (void *) &refs_for_removal);\n+\trefs_for_each_fullref_in(get_main_ref_store(the_repository),\n+\t\t\t\t \"refs/bisect/\", NULL, mark_for_removal,\n+\t\t\t\t &refs_for_removal);\n \tstring_list_append(&refs_for_removal, xstrdup(\"BISECT_HEAD\"));\n \tstring_list_append(&refs_for_removal, xstrdup(\"BISECT_EXPECTED_REV\"));\n \tresult = refs_delete_refs(get_main_ref_store(the_repository),\n\n-- \n2.53.0.rc2.206.g60c1bca835.dirty\n\n"},{"id":"534752","messageId":"CAOLa=ZQ5FGoarzZFe8jg51rpL4-s9H8i-z+3XV6T0A86YevLKA@mail.gmail.com","threadId":"64879","inReplyTo":"20260128-b4-pks-fix-for-each-ref-in-misuse-v1-1-deccae3ea725@pks.im","subject":"Re: [PATCH 1/3] pack-bitmap: deduplicate logic to iterate over preferred bitmap tips","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-28T10:37:55Z","receivedAt":"2026-01-28T10:37:58Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt writes:\n\n> We have two locations that iterate over the preferred bitmap tips as\n> configured by the user via \"pack.preferBitmapTips\". Both of these\n> callsites are subtly wrong and can lead to a `BUG()`, which we'll fix in\n> a subsequent commit.\n>\n> Prepare for this fix by unifying the two callsites into a new\n> `for_each_preferred_bitmap_tip()` function.\n>\n> This removes the last callsite of `bitmap_preferred_tips()` outside of\n> \"pack-bitmap.c\". As such, convert the function to be local to that file\n> only.\n>\n> Signed-off-by: Patrick Steinhardt \n> ---\n> builtin/pack-objects.c | 19 ++-----------------\n> pack-bitmap.c | 18 +++++++++++++++++-\n> pack-bitmap.h | 9 ++++++++-\n> repack-midx.c | 14 +++-----------\n> 4 files changed, 30 insertions(+), 30 deletions(-)\n>\n> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\n> index 5846b6a293..979470e402 100644\n> --- a/builtin/pack-objects.c\n> +++ b/builtin/pack-objects.c\n> @@ -4554,22 +4554,6 @@ static int mark_bitmap_preferred_tip(const struct reference *ref, void *data UNU\n> \treturn 0;\n> }\n>\n> -static void mark_bitmap_preferred_tips(void)\n> -{\n> -\tstruct string_list_item *item;\n> -\tconst struct string_list *preferred_tips;\n> -\n> -\tpreferred_tips = bitmap_preferred_tips(the_repository);\n> -\tif (!preferred_tips)\n> -\t\treturn;\n> -\n> -\tfor_each_string_list_item(item, preferred_tips) {\n> -\t\trefs_for_each_ref_in(get_main_ref_store(the_repository),\n> -\t\t\t\t item->string, mark_bitmap_preferred_tip,\n> -\t\t\t\t NULL);\n> -\t}\n> -}\n> -\n> static inline int is_oid_uninteresting(struct repository *repo,\n> \t\t\t\t struct object_id *oid)\n> {\n> @@ -4710,7 +4694,8 @@ static void get_object_list(struct rev_info *revs, struct strvec *argv)\n> \t\tload_delta_islands(the_repository, progress);\n>\n> \tif (write_bitmap_index)\n> -\t\tmark_bitmap_preferred_tips();\n> +\t\tfor_each_preferred_bitmap_tip(the_repository, mark_bitmap_preferred_tip,\n> +\t\t\t\t\t NULL);\n>\n> \tif (!fn_show_object)\n> \t\tfn_show_object = show_object;\n> diff --git a/pack-bitmap.c b/pack-bitmap.c\n> index 972203f12b..2f5cb34009 100644\n> --- a/pack-bitmap.c\n> +++ b/pack-bitmap.c\n> @@ -3314,7 +3314,7 @@ int bitmap_is_midx(struct bitmap_index *bitmap_git)\n> \treturn !!bitmap_git->midx;\n> }\n>\n> -const struct string_list *bitmap_preferred_tips(struct repository *r)\n> +static const struct string_list *bitmap_preferred_tips(struct repository *r)\n> {\n> \tconst struct string_list *dest;\n>\n> @@ -3323,6 +3323,22 @@ const struct string_list *bitmap_preferred_tips(struct repository *r)\n> \treturn NULL;\n> }\n>\n> +void for_each_preferred_bitmap_tip(struct repository *repo,\n> +\t\t\t\t each_ref_fn cb, void *cb_data)\n> +{\n> +\tstruct string_list_item *item;\n> +\tconst struct string_list *preferred_tips;\n> +\n> +\tpreferred_tips = bitmap_preferred_tips(repo);\n> +\tif (!preferred_tips)\n> +\t\treturn;\n> +\n\nSo we move the config check here. Instead of individually checking it.\nMakes sense.\n\n> +\tfor_each_string_list_item(item, preferred_tips) {\n> +\t\trefs_for_each_ref_in(get_main_ref_store(repo),\n> +\t\t\t\t item->string, cb, cb_data);\n> +\t}\n> +}\n> +\n> int bitmap_is_preferred_refname(struct repository *r, const char *refname)\n> {\n> \tconst struct string_list *preferred_tips = bitmap_preferred_tips(r);\n> diff --git a/pack-bitmap.h b/pack-bitmap.h\n> index 1bd7a791e2..d0611d0481 100644\n> --- a/pack-bitmap.h\n> +++ b/pack-bitmap.h\n> @@ -5,6 +5,7 @@\n> #include \"khash.h\"\n> #include \"pack.h\"\n> #include \"pack-objects.h\"\n> +#include \"refs.h\"\n> #include \"string-list.h\"\n>\n> struct commit;\n> @@ -99,6 +100,13 @@ int for_each_bitmapped_object(struct bitmap_index *bitmap_git,\n> \t\t\t show_reachable_fn show_reach,\n> \t\t\t void *payload);\n>\n> +/*\n> + * Iterate over all references that are configured as preferred bitmap tips via\n> + * \"pack.preferBitmapTips\" and invoke the callback on each function.\n> + */\n> +void for_each_preferred_bitmap_tip(struct repository *repo,\n> +\t\t\t\t each_ref_fn cb, void *cb_data);\n> +\n> #define GIT_TEST_PACK_USE_BITMAP_BOUNDARY_TRAVERSAL \\\n> \t\"GIT_TEST_PACK_USE_BITMAP_BOUNDARY_TRAVERSAL\"\n>\n> @@ -182,7 +190,6 @@ char *pack_bitmap_filename(struct packed_git *p);\n>\n> int bitmap_is_midx(struct bitmap_index *bitmap_git);\n>\n> -const struct string_list *bitmap_preferred_tips(struct repository *r);\n> int bitmap_is_preferred_refname(struct repository *r, const char *refname);\n>\n> int verify_bitmap_files(struct repository *r);\n> diff --git a/repack-midx.c b/repack-midx.c\n> index 74bdfa3a6e..0682b80c42 100644\n> --- a/repack-midx.c\n> +++ b/repack-midx.c\n> @@ -40,7 +40,6 @@ static int midx_snapshot_ref_one(const struct reference *ref, void *_data)\n> void midx_snapshot_refs(struct repository *repo, struct tempfile *f)\n> {\n> \tstruct midx_snapshot_ref_data data;\n> -\tconst struct string_list *preferred = bitmap_preferred_tips(repo);\n>\n> \tdata.repo = repo;\n> \tdata.f = f;\n> @@ -51,16 +50,9 @@ void midx_snapshot_refs(struct repository *repo, struct tempfile *f)\n> \t\t die(_(\"could not open tempfile %s for writing\"),\n> \t\t get_tempfile_path(f));\n>\n> -\tif (preferred) {\n> -\t\tstruct string_list_item *item;\n> -\n> -\t\tdata.preferred = 1;\n> -\t\tfor_each_string_list_item(item, preferred)\n> -\t\t\trefs_for_each_ref_in(get_main_ref_store(repo),\n> -\t\t\t\t\t item->string,\n> -\t\t\t\t\t midx_snapshot_ref_one, &data);\n> -\t\tdata.preferred = 0;\n> -\t}\n> +\tdata.preferred = 1;\n> +\tfor_each_preferred_bitmap_tip(repo, midx_snapshot_ref_one, &data);\n> +\tdata.preferred = 0;\n>\n\nSo we no longer need to check for the config, since\n`for_each_preferred_bitmap_tip()` does that. Looks good.\n\n> \trefs_for_each_ref(get_main_ref_store(repo),\n> \t\t\t midx_snapshot_ref_one, &data);\n>\n> --\n> 2.53.0.rc2.206.g60c1bca835.dirty\n"},{"id":"534755","messageId":"CAOLa=ZQyCbVUWTOWHYK4MVV+Mcf4XMQ4rY4n-CR6a97VMCjWqg@mail.gmail.com","threadId":"64879","inReplyTo":"20260128-b4-pks-fix-for-each-ref-in-misuse-v1-2-deccae3ea725@pks.im","subject":"Re: [PATCH 2/3] pack-bitmap: fix bug with exact ref match in \"pack.preferBitmapTips\"","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-28T11:04:09Z","receivedAt":"2026-01-28T11:04:11Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt writes:\n\n> The \"pack.preferBitmapTips\" configuration allows the user to specify\n> which references should be preferred when generating bitmaps. This\n> option is typically expected to be set to a reference prefix, like for\n> example \"refs/heads/\".\n>\n> It's not unreasonable though for a user to configure one specific\n> reference as preferred. But if they do, they'll hit a `BUG()`:\n>\n> $ git -c pack.preferBitmapTips=refs/heads/main repack -adb\n> BUG: ../refs/iterator.c:366: attempt to trim too many characters\n> error: pack-objects died of signal 6\n>\n> The root cause for this bug is how we enumerate these references. We\n> call `refs_for_each_ref_in()`, which will:\n>\n> - Yield all references that have a user-specified prefix.\n>\n> - Trim each of these references so that the prefix is removed.\n>\n> Typically, this function is called with a trailing slash, like\n> \"refs/heads/\", and in that case things work alright. But if the function\n> is called with the name of an existing reference then we'll try to trim\n> the full reference name, which would leave us with an empty name. And as\n> this would not really leave us with anything sensible, we call `BUG()`\n> instead of yielding this reference.\n>\n> One could argue that this is a bug in `refs_for_each_ref_in()`. But the\n> question then becomes what the correct behaviour would be:\n>\n> - Do we want to skip exact matches? In our case we certainly don't\n> want that, as the user has asked us to generate a bitmap for it.\n>\n> - Do we want to yield the reference with the empty refname? That would\n> lead to a somewhat weird result.\n>\n> Neither of these feel like viable options, so calling `BUG()` feels like\n> a sensible way out.\n>\n> The root cause really is that we try to trim the whole refname. We can\n> thus easily fix the bug itself by calling `refs_for_each_fullref_in()`\n> instead. This function behaves the same as `refs_for_each_ref_in()`,\n> except that it doesn't strip the prefix. Consequently, it correctly\n> yields also exact refnames.\n>\n> One resulting weirdness is that two refs \"refs/heads/base\" and\n> \"refs/heads/base-something\" would now match if the user configured\n> \"refs/heads/base\" as bitmap tips. One could arguably change the\n> semantics of the configuration such that a string without a trailing\n> slash needs to be an exact reference match, whereas a string with a\n> trailing slash indicates a directory hierarchy. But such a change would\n> potentially cause regressions with dubious benefits, so this issue is\n> ignored for now.\n>\n\nWhen using `refs_for_each_ref_in()` this would yield just\n'refs/heads/base-something'. That too feels like a BUG(), so I would\nthink this is the better solution.\n\n> Signed-off-by: Patrick Steinhardt \n> ---\n> pack-bitmap.c | 4 ++--\n> t/t5310-pack-bitmaps.sh | 35 +++++++++++++++++++++++++++++++++++\n> t/t5319-multi-pack-index.sh | 36 ++++++++++++++++++++++++++++++++++++\n> 3 files changed, 73 insertions(+), 2 deletions(-)\n>\n> diff --git a/pack-bitmap.c b/pack-bitmap.c\n> index 2f5cb34009..8d3b5ac037 100644\n> --- a/pack-bitmap.c\n> +++ b/pack-bitmap.c\n> @@ -3334,8 +3334,8 @@ void for_each_preferred_bitmap_tip(struct repository *repo,\n> \t\treturn;\n>\n> \tfor_each_string_list_item(item, preferred_tips) {\n> -\t\trefs_for_each_ref_in(get_main_ref_store(repo),\n> -\t\t\t\t item->string, cb, cb_data);\n> +\t\trefs_for_each_fullref_in(get_main_ref_store(repo),\n> +\t\t\t\t\t item->string, NULL, cb, cb_data);\n> \t}\n> }\n>\n> diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh\n> index 6718fb98c0..7ef91b502c 100755\n> --- a/t/t5310-pack-bitmaps.sh\n> +++ b/t/t5310-pack-bitmaps.sh\n> @@ -466,6 +466,41 @@ test_bitmap_cases () {\n> \t\t)\n> \t'\n>\n> +\ttest_expect_success 'pack.preferBitmapTips can use direct refname' '\n> +\t\tgit init repo &&\n> +\t\ttest_when_finished \"rm -fr repo\" &&\n> +\t\t(\n> +\t\t\tcd repo &&\n> +\n> +\t\t\t# Create enough commits that not all will receive bitmap\n> +\t\t\t# coverage even if they are all at the tip of some reference.\n> +\t\t\ttest_commit_bulk --message=\"%s\" 103 &&\n> +\t\t\tgit log --format=\"create refs/tags/%s %H\" HEAD >refs &&\n> +\t\t\tgit update-ref --stdin +\n\nWe create a bunch of commits. Nit: is '--message=\"%s\"' even needed here?\nSeems to be the default behavior anyways. Since we don't provide a ref,\nit uses HEAD, so finally we'll only have one commit being referenced.\n\nBut then we also create individual tags for each of them.\n\n> +\t\t\t# Create the bitmap.\n> +\t\t\tgit repack -adb &&\n> +\t\t\ttest-tool bitmap list-commits | sort >commits-with-bitmap &&\n> +\n> +\t\t\t# Verify that we have at least one commit that did not\n> +\t\t\t# receive a bitmap.\n> +\t\t\tgit rev-list HEAD >commits.raw &&\n> +\t\t\tsort commits &&\n> +\t\t\tcomm -13 commits-with-bitmap commits >commits-wo-bitmap &&\n> +\t\t\ttest_file_not_empty commits-wo-bitmap &&\n> +\t\t\tcommit_id=$(head commits-wo-bitmap) &&\n> +\n\nAlright, so of all the commits we have, some of them won't have a bitmap\nand we pick the first one.\n\n> +\t\t\t# We now create a reference for this commit and repack\n> +\t\t\t# with \"preferBitmapTips\" pointing to that exact\n> +\t\t\t# reference. The expectation is that it will now be\n> +\t\t\t# covered by a bitmap.\n> +\t\t\tgit update-ref refs/heads/cover-me \"$commit_id\" &&\n> +\t\t\tgit -c pack.preferBitmapTips=refs/heads/cover-me repack -adb &&\n> +\t\t\ttest-tool bitmap list-commits >after &&\n> +\t\t\ttest_grep \"$commit_id\" after\n\nAlright makes sense, since we fixed the prefix issue, providing the full\nrefname appears to work now.\n\n[skip]\n\nThanks\n"},{"id":"534756","messageId":"CAOLa=ZTX1uGBgsxXFPTBujLmMgKw-X1HB1reZ2jar7yeSnzH0g@mail.gmail.com","threadId":"64879","inReplyTo":"20260128-b4-pks-fix-for-each-ref-in-misuse-v1-0-deccae3ea725@pks.im","subject":"Re: [PATCH 0/3] Fix misuse of `refs_for_each_ref_in()`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-28T11:05:53Z","receivedAt":"2026-01-28T11:05:55Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt writes:\n\n> Hi,\n>\n> this small patch series fixes a bug I have discovered where configuring\n> \"pack.preferBitmapTips\" to an exact branch will cause Git to `BUG()`.\n>\n> The root cause of this bug is misuse of `refs_for_each_ref_in()`: this\n> function accepts a prefix to yield refs for, and then strips the prefix\n> for each ref. Consequently, if passed an exact refname, then stripping\n> the prefix would make us end up with an empty refname, and that is not\n> supposed to happen.\n>\n> There was one other caller that got it wrong, too, and which is also\n> fixed in this patch series.\n>\n> Thanks!\n>\n> Patrick\n>\n\nThe patches look good, thanks for the fix!\n"},{"id":"534796","messageId":"aXrDD1H4lvBR1sF8@nand.local","threadId":"64879","inReplyTo":"20260128-b4-pks-fix-for-each-ref-in-misuse-v1-1-deccae3ea725@pks.im","subject":"Re: [PATCH 1/3] pack-bitmap: deduplicate logic to iterate over preferred bitmap tips","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-01-29T02:16:47Z","receivedAt":"2026-01-29T02:16:54Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Wed, Jan 28, 2026 at 09:49:20AM +0100, Patrick Steinhardt wrote:\n> We have two locations that iterate over the preferred bitmap tips as\n> configured by the user via \"pack.preferBitmapTips\". Both of these\n> callsites are subtly wrong and can lead to a `BUG()`, which we'll fix in\n> a subsequent commit.\n\nOK, so there is some bug here that is shared by both call-sites (one in\nthe pack-objects case for single-pack bitmaps, and another in the MIDX\ncode for multi-pack bitmaps). That bug is yet unspecified, but that\nmakes sense since the point of this patch appears to be unifying the two\nimplementations together so that both may be fixed at once.\n\nAs of yet, it's not totally clear to me what that bug is having just\nread the cover letter. I don't know how much detail it's worth getting\ninto here since you'll end up covering it in much greater detail in the\nfollowing patch, though it might be nice to include at least a taste of\nwhat's to come beyond just \"[they] are subtly wrong\".\n\n> Prepare for this fix by unifying the two callsites into a new\n> `for_each_preferred_bitmap_tip()` function.\n>\n> This removes the last callsite of `bitmap_preferred_tips()` outside of\n> \"pack-bitmap.c\". As such, convert the function to be local to that file\n> only.\n\nOK, I think hiding this implementation from outside of the compilation\nunit makes sense, however I am not sure that we should keep it as a\nseparate function.\n\n> Signed-off-by: Patrick Steinhardt \n> ---\n> builtin/pack-objects.c | 19 ++-----------------\n> pack-bitmap.c | 18 +++++++++++++++++-\n> pack-bitmap.h | 9 ++++++++-\n> repack-midx.c | 14 +++-----------\n> 4 files changed, 30 insertions(+), 30 deletions(-)\n\n> @@ -4710,7 +4694,8 @@ static void get_object_list(struct rev_info *revs, struct strvec *argv)\n> \t\tload_delta_islands(the_repository, progress);\n>\n> \tif (write_bitmap_index)\n> -\t\tmark_bitmap_preferred_tips();\n> +\t\tfor_each_preferred_bitmap_tip(the_repository, mark_bitmap_preferred_tip,\n> +\t\t\t\t\t NULL);\n\nThis one looks good to me. The function mark_bitmap_preferred_tips()\nhere is identical in its implementation to the new one introduced in\npack-bitmap.c, and the callback is reused. Good.\n\n> diff --git a/pack-bitmap.c b/pack-bitmap.c\n> index 972203f12b..2f5cb34009 100644\n> --- a/pack-bitmap.c\n> +++ b/pack-bitmap.c\n> @@ -3314,7 +3314,7 @@ int bitmap_is_midx(struct bitmap_index *bitmap_git)\n> \treturn !!bitmap_git->midx;\n> }\n>\n> -const struct string_list *bitmap_preferred_tips(struct repository *r)\n> +static const struct string_list *bitmap_preferred_tips(struct repository *r)\n> {\n> \tconst struct string_list *dest;\n>\n> @@ -3323,6 +3323,22 @@ const struct string_list *bitmap_preferred_tips(struct repository *r)\n> \treturn NULL;\n> }\n>\n> +void for_each_preferred_bitmap_tip(struct repository *repo,\n> +\t\t\t\t each_ref_fn cb, void *cb_data)\n> +{\n> +\tstruct string_list_item *item;\n> +\tconst struct string_list *preferred_tips;\n> +\n> +\tpreferred_tips = bitmap_preferred_tips(repo);\n\nOK, so this is the sole caller of bitmap_preferred_tips() you were\nreferring to earlier. That function's implementation is hidden from the\ndiff context, but it's effectively a thin wrapper around\nrepo_config_get_string_multi().\n\nI wonder if we should just inline the implementation here into its sole\ncaller. Is there a reason to keep them separate? I do not feel strongly\nhere, just thinking aloud...\n\n> +\tif (!preferred_tips)\n> +\t\treturn;\n> +\n> +\tfor_each_string_list_item(item, preferred_tips) {\n> +\t\trefs_for_each_ref_in(get_main_ref_store(repo),\n> +\t\t\t\t item->string, cb, cb_data);\n> +\t}\n\nThe rest of this function looks identical to the one from pack-objects.\n\n> +}\n> +\n> int bitmap_is_preferred_refname(struct repository *r, const char *refname)\n> {\n> \tconst struct string_list *preferred_tips = bitmap_preferred_tips(r);\n> diff --git a/pack-bitmap.h b/pack-bitmap.h\n> index 1bd7a791e2..d0611d0481 100644\n> --- a/pack-bitmap.h\n> +++ b/pack-bitmap.h\n> @@ -5,6 +5,7 @@\n> #include \"khash.h\"\n> #include \"pack.h\"\n> #include \"pack-objects.h\"\n> +#include \"refs.h\"\n\nOof. I wish that there was a way to forward-declare the each_ref_fn\ntype, but there is not AFAIK.\n\nThe rest looks good to me.\n\nThanks,\nTaylor\n"},{"id":"534797","messageId":"aXrGfGUJQ34JAmuz@nand.local","threadId":"64879","inReplyTo":"20260128-b4-pks-fix-for-each-ref-in-misuse-v1-2-deccae3ea725@pks.im","subject":"Re: [PATCH 2/3] pack-bitmap: fix bug with exact ref match in \"pack.preferBitmapTips\"","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-01-29T02:31:24Z","receivedAt":"2026-01-29T02:31:27Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Wed, Jan 28, 2026 at 09:49:21AM +0100, Patrick Steinhardt wrote:\n> The \"pack.preferBitmapTips\" configuration allows the user to specify\n> which references should be preferred when generating bitmaps. This\n> option is typically expected to be set to a reference prefix, like for\n> example \"refs/heads/\".\n>\n> It's not unreasonable though for a user to configure one specific\n> reference as preferred. But if they do, they'll hit a `BUG()`:\n>\n> $ git -c pack.preferBitmapTips=refs/heads/main repack -adb\n> BUG: ../refs/iterator.c:366: attempt to trim too many characters\n> error: pack-objects died of signal 6\n\nOops. While we should definitely not BUG() here, I am not sure I\nunderstand the desired use-case of specifying a single reference as a\nvalue for pack.preferBitmapTips.\n\nLooking at the implementation of bitmap_writer_select_commits(), we do\nnot guarantee that *any* reference specified by pack.preferBitmapTips\nwill receive a bitmap. That's because we don't necessarily enumerate the\nentire set of commits when determining which ones to bitmap.\n\nFor example, if I do something like (assuming that the bug described\nhere is fixed):\n\n $ git -c pack.preferBitmapTips=refs/heads/foo \\\n -c pack.preferBitmapTips=refs/heads/bar repack -adb\n\n, and suppose \"indexed_commits\" list has the commits pointed to by \"foo\"\nand \"bar\" next to each other. We'll look at the next batch of bitmap\ncandidates, realize that commit \"foo\" has the NEEDS_BITMAP flag set,\nmark it as chosen, and then skip ahead to the next chunk, all without\nhaving looked at \"bar\".\n\nLooking at the code, I *think* it's the case that specifying a single\npreferred bitmap tip with an exact reference name will guarantee that we\nselect it for bitmapping, but it's not the case in general.\n\n> One resulting weirdness is that two refs \"refs/heads/base\" and\n> \"refs/heads/base-something\" would now match if the user configured\n> \"refs/heads/base\" as bitmap tips. One could arguably change the\n> semantics of the configuration such that a string without a trailing\n> slash needs to be an exact reference match, whereas a string with a\n> trailing slash indicates a directory hierarchy. But such a change would\n> potentially cause regressions with dubious benefits, so this issue is\n> ignored for now.\n\n(Setting aside the for_each_ref vs. for_each_fullref issue for a\nmoment...)\n\nAm I understanding this change correctly that doing something like -c\npack.preferBitmapTips=refs/heads/foo would match both foo and foobar?\n\nIf so, I am not sure that that is a desirable interface, especially\nsince we went the opposite direction in 10e8a9352bc (refs.c: stop\nmatching non-directory prefixes in exclude patterns, 2025-03-06). Having\nthe two behave inconsistently from one another feels somewhat awkward to\nme and may lead to unexpected results.\n\nAt the very least, if we do end up going in this direction (and I am not\nnecessarily advocating that we do, since I would prefer a more\nconsistent set of behavior), we should at minimum document it in\ngit-config(1).\n\nThanks,\nTaylor\n"},{"id":"534798","messageId":"20260129081420.GA589284@coredump.intra.peff.net","threadId":"64879","inReplyTo":"20260128-b4-pks-fix-for-each-ref-in-misuse-v1-3-deccae3ea725@pks.im","subject":"Re: [PATCH 3/3] bisect: fix misuse of `refs_for_each_ref_in()`","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-01-29T08:14:20Z","receivedAt":"2026-01-29T08:14:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 28, 2026 at 09:49:22AM +0100, Patrick Steinhardt wrote:\n\n> Furthermore, taking a closer look, we strip the prefix only to re-add it\n> in `mark_for_removal()`. This is somewhat roundabout, as we can instead\n> call `refs_for_each_fullref_in()` to not do any stripping at all. Do so\n> to simplify the code a bit.\n\nYeah, I think the result is much better.\n\nWe might also want this simplification on top:\n\n-- >8 --\nSubject: [PATCH] bisect: simplify string_list memory handling\n\nWe declare the refs_for_removal string_list as NODUP, forcing us to\nmanually allocate strings we insert. And then when it comes time to\nclean up, we set strdup_strings so that string_list_clear() will free\nthem for us.\n\nThis is a confusing pattern, and can be done much more simply by just\ndeclaring the list with the DUP initializer in the first place.\n\nIt was written this way originally because one of the callsites\ngenerated the item using xstrfmt(). But that spot switched to a plain\nxstrdup() in cf01f617b9 (bisect: fix misuse of `refs_for_each_ref_in()`,\n2026-01-28). That means we can now just let the string_list code handle\nallocation itself.\n\nSigned-off-by: Jeff King \n---\nEven before cf01f617b9 we could have done:\n\n string_list_append_nodup(&refs, xstrfmt(...));\n\nto get a similar simplification, but after that commit it is even\neasier.\n\n bisect.c | 10 ++++------\n 1 file changed, 4 insertions(+), 6 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex 2cd97bc9fe..6b9e9d81c3 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -1183,8 +1183,7 @@ int estimate_bisect_steps(int all)\n static int mark_for_removal(const struct reference *ref, void *cb_data)\n {\n \tstruct string_list *refs = cb_data;\n-\tchar *bisect_ref = xstrdup(ref->name);\n-\tstring_list_append(refs, bisect_ref);\n+\tstring_list_append(refs, ref->name);\n \treturn 0;\n }\n \n@@ -1193,16 +1192,15 @@ int bisect_clean_state(void)\n \tint result = 0;\n \n \t/* There may be some refs packed during bisection */\n-\tstruct string_list refs_for_removal = STRING_LIST_INIT_NODUP;\n+\tstruct string_list refs_for_removal = STRING_LIST_INIT_DUP;\n \trefs_for_each_fullref_in(get_main_ref_store(the_repository),\n \t\t\t\t \"refs/bisect/\", NULL, mark_for_removal,\n \t\t\t\t &refs_for_removal);\n-\tstring_list_append(&refs_for_removal, xstrdup(\"BISECT_HEAD\"));\n-\tstring_list_append(&refs_for_removal, xstrdup(\"BISECT_EXPECTED_REV\"));\n+\tstring_list_append(&refs_for_removal, \"BISECT_HEAD\");\n+\tstring_list_append(&refs_for_removal, \"BISECT_EXPECTED_REV\");\n \tresult = refs_delete_refs(get_main_ref_store(the_repository),\n \t\t\t\t \"bisect: remove\", &refs_for_removal,\n \t\t\t\t REF_NO_DEREF);\n-\trefs_for_removal.strdup_strings = 1;\n \tstring_list_clear(&refs_for_removal, 0);\n \tunlink_or_warn(git_path_bisect_ancestors_ok());\n \tunlink_or_warn(git_path_bisect_log());\n-- \n2.53.0.rc2.296.g6748ed0491\n\n"},{"id":"534819","messageId":"xmqqecn8cr7e.fsf@gitster.g","threadId":"64879","inReplyTo":"aXrDD1H4lvBR1sF8@nand.local","subject":"Re: [PATCH 1/3] pack-bitmap: deduplicate logic to iterate over preferred bitmap tips","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-29T16:35:17Z","receivedAt":"2026-01-29T16:35:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau writes:\n\n>> +void for_each_preferred_bitmap_tip(struct repository *repo,\n>> +\t\t\t\t each_ref_fn cb, void *cb_data)\n>> +{\n>> +\tstruct string_list_item *item;\n>> +\tconst struct string_list *preferred_tips;\n>> +\n>> +\tpreferred_tips = bitmap_preferred_tips(repo);\n>\n> OK, so this is the sole caller of bitmap_preferred_tips() you were\n> referring to earlier. That function's implementation is hidden from the\n> diff context, but it's effectively a thin wrapper around\n> repo_config_get_string_multi().\n\nTrue. I found it easier to see from the way the patch was written\nthat this is a pure refactoring patch, though. IOW, we may want to\ndo that on top as a further rewrite, but I am not sure if it makes\nthe result easier to reason about. Such helper functions that are\nfile-scope static often help reading the logic flow of the program,\nand compilers would inline them when it is more beneficial anyway.\n\n"},{"id":"534820","messageId":"xmqq7bt0cqec.fsf@gitster.g","threadId":"64879","inReplyTo":"aXrGfGUJQ34JAmuz@nand.local","subject":"Re: [PATCH 2/3] pack-bitmap: fix bug with exact ref match in \"pack.preferBitmapTips\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-29T16:52:43Z","receivedAt":"2026-01-29T16:52:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau writes:\n\n> Looking at the implementation of bitmap_writer_select_commits(), we do\n> not guarantee that *any* reference specified by pack.preferBitmapTips\n> will receive a bitmap. That's because we don't necessarily enumerate the\n> entire set of commits when determining which ones to bitmap.\n\nHmph. Is this documented?\n\n\t... Goes and looks ...\n\nYes, it is documented. We say \"This is because ...\" but it just\nexplains it as what the chosen design of the implementation happens\nto do, without saying for what benefit the implementation was chosen,\nso it is unclear if this is designed behaviour, or more importantly,\neven if this were designed, what the rationale of choosing that\ndesign was.\n\n\"When they are so close to fall into the same chunk, there is no\npoint having bitmaps individually for them, as their bitmaps will be\nvery similar anyway, so this design saves space without sacrificing\nthe quality of the resulting set of bitmaps\" or something?\n\n\n> At the very least, if we do end up going in this direction (and I am not\n> necessarily advocating that we do, since I would prefer a more\n> consistent set of behavior), we should at minimum document it in\n> git-config(1).\n\nThe documentation says \"... reference that is a suffix of any value\nof this configuration\". Is \"refs/heads/foobar\" a \"suffix\" of\n\"refs/heads/foo\"? I actually find this phrasing fairly strange, as\nI do not think of \"refs/heads/main\" be a \"suffix\" of \"refs/heads/\".\n\n"},{"id":"534822","messageId":"xmqq343ocopx.fsf@gitster.g","threadId":"64879","inReplyTo":"20260129081420.GA589284@coredump.intra.peff.net","subject":"Re: [PATCH 3/3] bisect: fix misuse of `refs_for_each_ref_in()`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-29T17:28:58Z","receivedAt":"2026-01-29T17:29:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Wed, Jan 28, 2026 at 09:49:22AM +0100, Patrick Steinhardt wrote:\n>\n>> Furthermore, taking a closer look, we strip the prefix only to re-add it\n>> in `mark_for_removal()`. This is somewhat roundabout, as we can instead\n>> call `refs_for_each_fullref_in()` to not do any stripping at all. Do so\n>> to simplify the code a bit.\n>\n> Yeah, I think the result is much better.\n>\n> We might also want this simplification on top:\n\nVery good. Thanks, both, for improvements.\n\n> -- >8 --\n> Subject: [PATCH] bisect: simplify string_list memory handling\n>\n> We declare the refs_for_removal string_list as NODUP, forcing us to\n> manually allocate strings we insert. And then when it comes time to\n> clean up, we set strdup_strings so that string_list_clear() will free\n> them for us.\n>\n> This is a confusing pattern, and can be done much more simply by just\n> declaring the list with the DUP initializer in the first place.\n>\n> It was written this way originally because one of the callsites\n> generated the item using xstrfmt(). But that spot switched to a plain\n> xstrdup() in cf01f617b9 (bisect: fix misuse of `refs_for_each_ref_in()`,\n> 2026-01-28). That means we can now just let the string_list code handle\n> allocation itself.\n>\n> Signed-off-by: Jeff King \n> ---\n> Even before cf01f617b9 we could have done:\n>\n> string_list_append_nodup(&refs, xstrfmt(...));\n>\n> to get a similar simplification, but after that commit it is even\n> easier.\n>\n> bisect.c | 10 ++++------\n> 1 file changed, 4 insertions(+), 6 deletions(-)\n>\n> diff --git a/bisect.c b/bisect.c\n> index 2cd97bc9fe..6b9e9d81c3 100644\n> --- a/bisect.c\n> +++ b/bisect.c\n> @@ -1183,8 +1183,7 @@ int estimate_bisect_steps(int all)\n> static int mark_for_removal(const struct reference *ref, void *cb_data)\n> {\n> \tstruct string_list *refs = cb_data;\n> -\tchar *bisect_ref = xstrdup(ref->name);\n> -\tstring_list_append(refs, bisect_ref);\n> +\tstring_list_append(refs, ref->name);\n> \treturn 0;\n> }\n> \n> @@ -1193,16 +1192,15 @@ int bisect_clean_state(void)\n> \tint result = 0;\n> \n> \t/* There may be some refs packed during bisection */\n> -\tstruct string_list refs_for_removal = STRING_LIST_INIT_NODUP;\n> +\tstruct string_list refs_for_removal = STRING_LIST_INIT_DUP;\n> \trefs_for_each_fullref_in(get_main_ref_store(the_repository),\n> \t\t\t\t \"refs/bisect/\", NULL, mark_for_removal,\n> \t\t\t\t &refs_for_removal);\n> -\tstring_list_append(&refs_for_removal, xstrdup(\"BISECT_HEAD\"));\n> -\tstring_list_append(&refs_for_removal, xstrdup(\"BISECT_EXPECTED_REV\"));\n> +\tstring_list_append(&refs_for_removal, \"BISECT_HEAD\");\n> +\tstring_list_append(&refs_for_removal, \"BISECT_EXPECTED_REV\");\n> \tresult = refs_delete_refs(get_main_ref_store(the_repository),\n> \t\t\t\t \"bisect: remove\", &refs_for_removal,\n> \t\t\t\t REF_NO_DEREF);\n> -\trefs_for_removal.strdup_strings = 1;\n> \tstring_list_clear(&refs_for_removal, 0);\n> \tunlink_or_warn(git_path_bisect_ancestors_ok());\n> \tunlink_or_warn(git_path_bisect_log());\n"},{"id":"534832","messageId":"aXu2Q1TgsaUIo30+@nand.local","threadId":"64879","inReplyTo":"xmqq7bt0cqec.fsf@gitster.g","subject":"Re: [PATCH 2/3] pack-bitmap: fix bug with exact ref match in \"pack.preferBitmapTips\"","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-01-29T19:34:27Z","receivedAt":"2026-01-29T19:34:38Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Jan 29, 2026 at 08:52:43AM -0800, Junio C Hamano wrote:\n> Taylor Blau writes:\n>\n> > Looking at the implementation of bitmap_writer_select_commits(), we do\n> > not guarantee that *any* reference specified by pack.preferBitmapTips\n> > will receive a bitmap. That's because we don't necessarily enumerate the\n> > entire set of commits when determining which ones to bitmap.\n>\n> Hmph. Is this documented?\n>\n> \t... Goes and looks ...\n>\n> Yes, it is documented. We say \"This is because ...\" but it just\n> explains it as what the chosen design of the implementation happens\n> to do, without saying for what benefit the implementation was chosen,\n> so it is unclear if this is designed behaviour, or more importantly,\n> even if this were designed, what the rationale of choosing that\n> design was.\n>\n> \"When they are so close to fall into the same chunk, there is no\n> point having bitmaps individually for them, as their bitmaps will be\n> very similar anyway, so this design saves space without sacrificing\n> the quality of the resulting set of bitmaps\" or something?\n\nThe commits are generally presented in the order they are traversed\n(regardless of whether we are generating single- or multi-pack bitmaps).\nThat makes it likely that commits within the same window are likely to\ngenerate very similar bitmaps, but it is not guaranteed.\n\nWhen looking at the documentation, I ended up with the following:\n\n--- 8< ---\ndiff --git a/Documentation/config/pack.adoc b/Documentation/config/pack.adoc\nindex 75402d5579d..b65cbaaebb4 100644\n--- a/Documentation/config/pack.adoc\n+++ b/Documentation/config/pack.adoc\n@@ -168,7 +168,10 @@ pack.preferBitmapTips::\n Note that setting this configuration to `refs/foo` does not mean that\n the commits at the tips of `refs/foo/bar` and `refs/foo/baz` will\n necessarily be selected. This is because commits are selected for\n-bitmaps from within a series of windows of variable length.\n+bitmaps from within a series of windows of variable length (in order to\n+space bitmaps out throughout history), and we only select one commit per\n+window. Thus if multiple preferred commits appear in the same window,\n+only one will be selected.\n +\n If a commit at the tip of any reference which is a suffix of any value\n of this configuration is seen in a window, it is immediately given\n--- >8 ---\n\n> > At the very least, if we do end up going in this direction (and I am not\n> > necessarily advocating that we do, since I would prefer a more\n> > consistent set of behavior), we should at minimum document it in\n> > git-config(1).\n>\n> The documentation says \"... reference that is a suffix of any value\n> of this configuration\". Is \"refs/heads/foobar\" a \"suffix\" of\n> \"refs/heads/foo\"? I actually find this phrasing fairly strange, as\n> I do not think of \"refs/heads/main\" be a \"suffix\" of \"refs/heads/\".\n\nI agree, the use of \"suffix\" is confusing at best. I think if/how we\nchange this section depends on the outcome of this series, but the\noriginal intent was to say that preferring \"refs/heads/foo\" would make\nthe commits at the tips of \"refs/heads/foo/bar\" and \"refs/heads/foo/baz\"\npreferred, but not \"refs/heads/foobar\".\n\nThanks,\nTaylor\n"},{"id":"534845","messageId":"xmqq1pj89fgg.fsf@gitster.g","threadId":"64879","inReplyTo":"aXu2Q1TgsaUIo30+@nand.local","subject":"Re: [PATCH 2/3] pack-bitmap: fix bug with exact ref match in \"pack.preferBitmapTips\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-29T23:17:19Z","receivedAt":"2026-01-29T23:17:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau writes:\n\n> When looking at the documentation, I ended up with the following:\n>\n> --- 8< ---\n> diff --git a/Documentation/config/pack.adoc b/Documentation/config/pack.adoc\n> index 75402d5579d..b65cbaaebb4 100644\n> --- a/Documentation/config/pack.adoc\n> +++ b/Documentation/config/pack.adoc\n> @@ -168,7 +168,10 @@ pack.preferBitmapTips::\n> Note that setting this configuration to `refs/foo` does not mean that\n> the commits at the tips of `refs/foo/bar` and `refs/foo/baz` will\n> necessarily be selected. This is because commits are selected for\n> -bitmaps from within a series of windows of variable length.\n> +bitmaps from within a series of windows of variable length (in order to\n> +space bitmaps out throughout history), and we only select one commit per\n> +window. Thus if multiple preferred commits appear in the same window,\n> +only one will be selected.\n\nThat's certainly better.\n\n>> The documentation says \"... reference that is a suffix of any value\n>> of this configuration\". Is \"refs/heads/foobar\" a \"suffix\" of\n>> \"refs/heads/foo\"? I actually find this phrasing fairly strange, as\n>> I do not think of \"refs/heads/main\" be a \"suffix\" of \"refs/heads/\".\n>\n> I agree, the use of \"suffix\" is confusing at best. I think if/how we\n> change this section depends on the outcome of this series, but the\n> original intent was to say that preferring \"refs/heads/foo\" would make\n> the commits at the tips of \"refs/heads/foo/bar\" and \"refs/heads/foo/baz\"\n> preferred, but not \"refs/heads/foobar\".\n\nI am still not sure if naming an individual ref is an intended use,\nbut I assume that the original intent of this part of the document\nwas to specify the leading hierarchies and commits at the tip of\nrefs that appear in one of the listed hiearchies are used as\npreferred candidates to give bitmaps.\n\n pack.preferBitmapTips::\n\n\tSpecifies a ref hierarchy (e.g., \"refs/heads/\"); can be\n\tgiven multiple times to specify more than one hierarchies.\n\n\tWhen selecting which commits will receive bitmaps, prefer a\n commmit at the tip of a reference that appears in one of the\n hierarchies specified over any other commits ...\n\n\nor something?\n\n"},{"id":"534863","messageId":"aXyq2_MByjxf3TBX@pks.im","threadId":"64879","inReplyTo":"aXrDD1H4lvBR1sF8@nand.local","subject":"Re: [PATCH 1/3] pack-bitmap: deduplicate logic to iterate over preferred bitmap tips","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-30T12:58:03Z","receivedAt":"2026-01-30T12:58:10Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Jan 28, 2026 at 09:16:47PM -0500, Taylor Blau wrote:\n> On Wed, Jan 28, 2026 at 09:49:20AM +0100, Patrick Steinhardt wrote:\n> > We have two locations that iterate over the preferred bitmap tips as\n> > configured by the user via \"pack.preferBitmapTips\". Both of these\n> > callsites are subtly wrong and can lead to a `BUG()`, which we'll fix in\n> > a subsequent commit.\n> \n> OK, so there is some bug here that is shared by both call-sites (one in\n> the pack-objects case for single-pack bitmaps, and another in the MIDX\n> code for multi-pack bitmaps). That bug is yet unspecified, but that\n> makes sense since the point of this patch appears to be unifying the two\n> implementations together so that both may be fixed at once.\n> \n> As of yet, it's not totally clear to me what that bug is having just\n> read the cover letter. I don't know how much detail it's worth getting\n> into here since you'll end up covering it in much greater detail in the\n> following patch, though it might be nice to include at least a taste of\n> what's to come beyond just \"[they] are subtly wrong\".\n\nSure, can do.\n\n> > Prepare for this fix by unifying the two callsites into a new\n> > `for_each_preferred_bitmap_tip()` function.\n> >\n> > This removes the last callsite of `bitmap_preferred_tips()` outside of\n> > \"pack-bitmap.c\". As such, convert the function to be local to that file\n> > only.\n> \n> OK, I think hiding this implementation from outside of the compilation\n> unit makes sense, however I am not sure that we should keep it as a\n> separate function.\n\nI originally though the same, but there still is a second callsite of\nthis function. So...\n\n> > diff --git a/pack-bitmap.c b/pack-bitmap.c\n> > index 972203f12b..2f5cb34009 100644\n> > --- a/pack-bitmap.c\n> > +++ b/pack-bitmap.c\n> > @@ -3323,6 +3323,22 @@ const struct string_list *bitmap_preferred_tips(struct repository *r)\n> > \treturn NULL;\n> > }\n> >\n> > +void for_each_preferred_bitmap_tip(struct repository *repo,\n> > +\t\t\t\t each_ref_fn cb, void *cb_data)\n> > +{\n> > +\tstruct string_list_item *item;\n> > +\tconst struct string_list *preferred_tips;\n> > +\n> > +\tpreferred_tips = bitmap_preferred_tips(repo);\n> \n> OK, so this is the sole caller of bitmap_preferred_tips() you were\n> referring to earlier. That function's implementation is hidden from the\n> diff context, but it's effectively a thin wrapper around\n> repo_config_get_string_multi().\n> \n> I wonder if we should just inline the implementation here into its sole\n> caller. Is there a reason to keep them separate? I do not feel strongly\n> here, just thinking aloud...\n\n... I actually tried this, only to realize that the function is still\ncalled by `bitmap_is_preferred_refname()`, too.\n\n> > diff --git a/pack-bitmap.h b/pack-bitmap.h\n> > index 1bd7a791e2..d0611d0481 100644\n> > --- a/pack-bitmap.h\n> > +++ b/pack-bitmap.h\n> > @@ -5,6 +5,7 @@\n> > #include \"khash.h\"\n> > #include \"pack.h\"\n> > #include \"pack-objects.h\"\n> > +#include \"refs.h\"\n> \n> Oof. I wish that there was a way to forward-declare the each_ref_fn\n> type, but there is not AFAIK.\n\nWe could get around this by simply open-coding the function signature\nand having a forward-declaration for `struct reference`. Not sure\nthough whether that's really worth it.\n\nPatrick\n"},{"id":"534864","messageId":"aXyq4__RBadFPBoW@pks.im","threadId":"64879","inReplyTo":"xmqq343ocopx.fsf@gitster.g","subject":"Re: [PATCH 3/3] bisect: fix misuse of `refs_for_each_ref_in()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-30T12:58:11Z","receivedAt":"2026-01-30T12:58:21Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Jan 29, 2026 at 09:28:58AM -0800, Junio C Hamano wrote:\n> Jeff King writes:\n> \n> > On Wed, Jan 28, 2026 at 09:49:22AM +0100, Patrick Steinhardt wrote:\n> >\n> >> Furthermore, taking a closer look, we strip the prefix only to re-add it\n> >> in `mark_for_removal()`. This is somewhat roundabout, as we can instead\n> >> call `refs_for_each_fullref_in()` to not do any stripping at all. Do so\n> >> to simplify the code a bit.\n> >\n> > Yeah, I think the result is much better.\n> >\n> > We might also want this simplification on top:\n> \n> Very good. Thanks, both, for improvements.\n\nIndeed, I'll apply your patch on top of what I have. Thanks!\n\nPatrick\n"},{"id":"534865","messageId":"aXyq7_yo2tVv7y38@pks.im","threadId":"64879","inReplyTo":"aXrGfGUJQ34JAmuz@nand.local","subject":"Re: [PATCH 2/3] pack-bitmap: fix bug with exact ref match in \"pack.preferBitmapTips\"","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-30T12:58:23Z","receivedAt":"2026-01-30T12:58:31Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Jan 28, 2026 at 09:31:24PM -0500, Taylor Blau wrote:\n> On Wed, Jan 28, 2026 at 09:49:21AM +0100, Patrick Steinhardt wrote:\n> > The \"pack.preferBitmapTips\" configuration allows the user to specify\n> > which references should be preferred when generating bitmaps. This\n> > option is typically expected to be set to a reference prefix, like for\n> > example \"refs/heads/\".\n> >\n> > It's not unreasonable though for a user to configure one specific\n> > reference as preferred. But if they do, they'll hit a `BUG()`:\n> >\n> > $ git -c pack.preferBitmapTips=refs/heads/main repack -adb\n> > BUG: ../refs/iterator.c:366: attempt to trim too many characters\n> > error: pack-objects died of signal 6\n> \n> Oops. While we should definitely not BUG() here, I am not sure I\n> understand the desired use-case of specifying a single reference as a\n> value for pack.preferBitmapTips.\n\nThere is not really a desired use case here, I just happened to stumble\nover this bug due to playing around with the feature.\n\n> Looking at the implementation of bitmap_writer_select_commits(), we do\n> not guarantee that *any* reference specified by pack.preferBitmapTips\n> will receive a bitmap. That's because we don't necessarily enumerate the\n> entire set of commits when determining which ones to bitmap.\n\nYeah, we don't indeed.\n\n[snip]\n> > One resulting weirdness is that two refs \"refs/heads/base\" and\n> > \"refs/heads/base-something\" would now match if the user configured\n> > \"refs/heads/base\" as bitmap tips. One could arguably change the\n> > semantics of the configuration such that a string without a trailing\n> > slash needs to be an exact reference match, whereas a string with a\n> > trailing slash indicates a directory hierarchy. But such a change would\n> > potentially cause regressions with dubious benefits, so this issue is\n> > ignored for now.\n> \n> (Setting aside the for_each_ref vs. for_each_fullref issue for a\n> moment...)\n> \n> Am I understanding this change correctly that doing something like -c\n> pack.preferBitmapTips=refs/heads/foo would match both foo and foobar?\n> \n> If so, I am not sure that that is a desirable interface, especially\n> since we went the opposite direction in 10e8a9352bc (refs.c: stop\n> matching non-directory prefixes in exclude patterns, 2025-03-06). Having\n> the two behave inconsistently from one another feels somewhat awkward to\n> me and may lead to unexpected results.\n\nI don't have too much skin in the game, and as I wrote I agree that the\nthings are a bit nuanced here. I think there's two major ways to go from\nhere:\n\n - We can either fix the bug and say that we accept all references that\n start with the configured prefix. \"refs/heads/main\" _does_ start\n with the prefix \"refs/heads/main\", so it should match.\n\n - Or we can fix the bug by appending a slash to the configured prefix\n if it doesn't already have one.\n\nThe reason I picked the first option here is mostly because it allows\nfor more options rather than restricting options. The user has the\nability to both match hierarchies by appending a \"/\", and they can have\nref-prefix-matches by not doing so.\n\nI'll adapt the commit message accordingly to document my thought\nprocess, and...\n\n> At the very least, if we do end up going in this direction (and I am not\n> necessarily advocating that we do, since I would prefer a more\n> consistent set of behavior), we should at minimum document it in\n> git-config(1).\n\n... will also adapt the documentation accordingly.\n\nThat being said, I'm also open to adapt my approach here.\n\nThanks!\n\nPatrick\n"},{"id":"534868","messageId":"20260130-b4-pks-fix-for-each-ref-in-misuse-v2-0-0449b198a681@pks.im","threadId":"64879","inReplyTo":"20260128-b4-pks-fix-for-each-ref-in-misuse-v1-0-deccae3ea725@pks.im","subject":"[PATCH v2 0/4] Fix misuse of `refs_for_each_ref_in()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-30T13:27:41Z","receivedAt":"2026-01-30T13:27:59Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis small patch series fixes a bug I have discovered where configuring\n\"pack.preferBitmapTips\" to an exact branch will cause Git to `BUG()`.\n\nThe root cause of this bug is misuse of `refs_for_each_ref_in()`: this\nfunction accepts a prefix to yield refs for, and then strips the prefix\nfor each ref. Consequently, if passed an exact refname, then stripping\nthe prefix would make us end up with an empty refname, and that is not\nsupposed to happen.\n\nThere was one other caller that got it wrong, too, and which is also\nfixed in this patch series.\n\nChanges in v2:\n - Explain my thought process against why I chose to also allow exact\n ref matches in the second commit and clarify the documentation a\n bit. As said, I'm very open to changing this if my spelled-out\n thoughts are not convincing.\n - Apply Peff's patch to further simplify code.\n - Link to v1: https://lore.kernel.org/r/20260128-b4-pks-fix-for-each-ref-in-misuse-v1-0-deccae3ea725@pks.im\n\nThanks!\n\nPatrick\n\n---\nJeff King (1):\n bisect: simplify string_list memory handling\n\nPatrick Steinhardt (3):\n pack-bitmap: deduplicate logic to iterate over preferred bitmap tips\n pack-bitmap: fix bug with exact ref match in \"pack.preferBitmapTips\"\n bisect: fix misuse of `refs_for_each_ref_in()`\n\n Documentation/config/pack.adoc | 7 +++----\n bisect.c | 16 +++++++---------\n builtin/pack-objects.c | 19 ++-----------------\n pack-bitmap.c | 18 +++++++++++++++++-\n pack-bitmap.h | 9 ++++++++-\n repack-midx.c | 14 +++-----------\n t/t5310-pack-bitmaps.sh | 35 +++++++++++++++++++++++++++++++++++\n t/t5319-multi-pack-index.sh | 36 ++++++++++++++++++++++++++++++++++++\n 8 files changed, 111 insertions(+), 43 deletions(-)\n\nRange-diff versus v1:\n\n1: dcdc4a5aa2 ! 1: 2a2558bb22 pack-bitmap: deduplicate logic to iterate over preferred bitmap tips\n @@ Commit message\n \n We have two locations that iterate over the preferred bitmap tips as\n configured by the user via \"pack.preferBitmapTips\". Both of these\n - callsites are subtly wrong and can lead to a `BUG()`, which we'll fix in\n - a subsequent commit.\n + callsites are subtly wrong: when the preferred bitmap tips contain an\n + exact refname match, then we will hit a `BUG()`.\n \n - Prepare for this fix by unifying the two callsites into a new\n + Prepare for the fix by unifying the two callsites into a new\n `for_each_preferred_bitmap_tip()` function.\n \n This removes the last callsite of `bitmap_preferred_tips()` outside of\n \"pack-bitmap.c\". As such, convert the function to be local to that file\n - only.\n + only. Note that the function is still used by a second caller, so we\n + cannot just inline it.\n \n Signed-off-by: Patrick Steinhardt \n \n2: 5b14a8a680 ! 2: 5b4a33a0bd pack-bitmap: fix bug with exact ref match in \"pack.preferBitmapTips\"\n @@ Commit message\n lead to a somewhat weird result.\n \n Neither of these feel like viable options, so calling `BUG()` feels like\n - a sensible way out.\n -\n - The root cause really is that we try to trim the whole refname. We can\n - thus easily fix the bug itself by calling `refs_for_each_fullref_in()`\n - instead. This function behaves the same as `refs_for_each_ref_in()`,\n - except that it doesn't strip the prefix. Consequently, it correctly\n - yields also exact refnames.\n -\n - One resulting weirdness is that two refs \"refs/heads/base\" and\n - \"refs/heads/base-something\" would now match if the user configured\n - \"refs/heads/base\" as bitmap tips. One could arguably change the\n - semantics of the configuration such that a string without a trailing\n - slash needs to be an exact reference match, whereas a string with a\n - trailing slash indicates a directory hierarchy. But such a change would\n - potentially cause regressions with dubious benefits, so this issue is\n - ignored for now.\n + a sensible way out. The root cause ultimately is that we even try to\n + trim the whole refname in the first place. There are two possible ways\n + to fix this issue:\n +\n + - We can fix the bug by using `refs_for_each_fullref_in()` instead,\n + which does not strip the prefix at all. Consequently, we would now\n + start to accept all references that start with the configured\n + prefix, including exact matches. So if we had \"refs/heads/main\", we\n + would both match \"refs/heads/main\" and \"refs/heads/main-branch\".\n +\n + - Or we can fix the bug by appending a slash to the prefix if it\n + doesn't already have one. This would mean that we only match\n + ref hierarchies that start with this prefix.\n +\n + The first fix leaves the user with strictly _more_ configuration\n + options: they can have prefix matches by not appending a slash to the\n + configuration, and they can have ref hierarchy matches by appending one.\n +\n + Apply this fix and clarify the documentation accordingly.\n \n Signed-off-by: Patrick Steinhardt \n \n + ## Documentation/config/pack.adoc ##\n +@@ Documentation/config/pack.adoc: pack.usePathWalk::\n + \n + pack.preferBitmapTips::\n + \tWhen selecting which commits will receive bitmaps, prefer a\n +-\tcommit at the tip of any reference that is a suffix of any value\n +-\tof this configuration over any other commits in the \"selection\n +-\twindow\".\n ++\tcommmit at the tip of a reference that matches any of the\n ++\tconfigured prefixes.\n + +\n +-Note that setting this configuration to `refs/foo` does not mean that\n ++Note that setting this configuration to `refs/foo/` does not mean that\n + the commits at the tips of `refs/foo/bar` and `refs/foo/baz` will\n + necessarily be selected. This is because commits are selected for\n + bitmaps from within a series of windows of variable length.\n +\n ## pack-bitmap.c ##\n @@ pack-bitmap.c: void for_each_preferred_bitmap_tip(struct repository *repo,\n \t\treturn;\n3: 2e1bfb0894 = 3: 1579608b04 bisect: fix misuse of `refs_for_each_ref_in()`\n-: ---------- > 4: 99a80bdb37 bisect: simplify string_list memory handling\n\n---\nbase-commit: ea717645d199f6f1b66058886475db3e8c9330e9\nchange-id: 20260128-b4-pks-fix-for-each-ref-in-misuse-96ae62e313a5\n\n"},{"id":"534869","messageId":"20260130-b4-pks-fix-for-each-ref-in-misuse-v2-1-0449b198a681@pks.im","threadId":"64879","inReplyTo":"20260130-b4-pks-fix-for-each-ref-in-misuse-v2-0-0449b198a681@pks.im","subject":"[PATCH v2 1/4] pack-bitmap: deduplicate logic to iterate over preferred bitmap tips","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-30T13:27:42Z","receivedAt":"2026-01-30T13:28:06Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We have two locations that iterate over the preferred bitmap tips as\nconfigured by the user via \"pack.preferBitmapTips\". Both of these\ncallsites are subtly wrong: when the preferred bitmap tips contain an\nexact refname match, then we will hit a `BUG()`.\n\nPrepare for the fix by unifying the two callsites into a new\n`for_each_preferred_bitmap_tip()` function.\n\nThis removes the last callsite of `bitmap_preferred_tips()` outside of\n\"pack-bitmap.c\". As such, convert the function to be local to that file\nonly. Note that the function is still used by a second caller, so we\ncannot just inline it.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/pack-objects.c | 19 ++-----------------\n pack-bitmap.c | 18 +++++++++++++++++-\n pack-bitmap.h | 9 ++++++++-\n repack-midx.c | 14 +++-----------\n 4 files changed, 30 insertions(+), 30 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 5846b6a293..979470e402 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -4554,22 +4554,6 @@ static int mark_bitmap_preferred_tip(const struct reference *ref, void *data UNU\n \treturn 0;\n }\n \n-static void mark_bitmap_preferred_tips(void)\n-{\n-\tstruct string_list_item *item;\n-\tconst struct string_list *preferred_tips;\n-\n-\tpreferred_tips = bitmap_preferred_tips(the_repository);\n-\tif (!preferred_tips)\n-\t\treturn;\n-\n-\tfor_each_string_list_item(item, preferred_tips) {\n-\t\trefs_for_each_ref_in(get_main_ref_store(the_repository),\n-\t\t\t\t item->string, mark_bitmap_preferred_tip,\n-\t\t\t\t NULL);\n-\t}\n-}\n-\n static inline int is_oid_uninteresting(struct repository *repo,\n \t\t\t\t struct object_id *oid)\n {\n@@ -4710,7 +4694,8 @@ static void get_object_list(struct rev_info *revs, struct strvec *argv)\n \t\tload_delta_islands(the_repository, progress);\n \n \tif (write_bitmap_index)\n-\t\tmark_bitmap_preferred_tips();\n+\t\tfor_each_preferred_bitmap_tip(the_repository, mark_bitmap_preferred_tip,\n+\t\t\t\t\t NULL);\n \n \tif (!fn_show_object)\n \t\tfn_show_object = show_object;\ndiff --git a/pack-bitmap.c b/pack-bitmap.c\nindex 972203f12b..2f5cb34009 100644\n--- a/pack-bitmap.c\n+++ b/pack-bitmap.c\n@@ -3314,7 +3314,7 @@ int bitmap_is_midx(struct bitmap_index *bitmap_git)\n \treturn !!bitmap_git->midx;\n }\n \n-const struct string_list *bitmap_preferred_tips(struct repository *r)\n+static const struct string_list *bitmap_preferred_tips(struct repository *r)\n {\n \tconst struct string_list *dest;\n \n@@ -3323,6 +3323,22 @@ const struct string_list *bitmap_preferred_tips(struct repository *r)\n \treturn NULL;\n }\n \n+void for_each_preferred_bitmap_tip(struct repository *repo,\n+\t\t\t\t each_ref_fn cb, void *cb_data)\n+{\n+\tstruct string_list_item *item;\n+\tconst struct string_list *preferred_tips;\n+\n+\tpreferred_tips = bitmap_preferred_tips(repo);\n+\tif (!preferred_tips)\n+\t\treturn;\n+\n+\tfor_each_string_list_item(item, preferred_tips) {\n+\t\trefs_for_each_ref_in(get_main_ref_store(repo),\n+\t\t\t\t item->string, cb, cb_data);\n+\t}\n+}\n+\n int bitmap_is_preferred_refname(struct repository *r, const char *refname)\n {\n \tconst struct string_list *preferred_tips = bitmap_preferred_tips(r);\ndiff --git a/pack-bitmap.h b/pack-bitmap.h\nindex 1bd7a791e2..d0611d0481 100644\n--- a/pack-bitmap.h\n+++ b/pack-bitmap.h\n@@ -5,6 +5,7 @@\n #include \"khash.h\"\n #include \"pack.h\"\n #include \"pack-objects.h\"\n+#include \"refs.h\"\n #include \"string-list.h\"\n \n struct commit;\n@@ -99,6 +100,13 @@ int for_each_bitmapped_object(struct bitmap_index *bitmap_git,\n \t\t\t show_reachable_fn show_reach,\n \t\t\t void *payload);\n \n+/*\n+ * Iterate over all references that are configured as preferred bitmap tips via\n+ * \"pack.preferBitmapTips\" and invoke the callback on each function.\n+ */\n+void for_each_preferred_bitmap_tip(struct repository *repo,\n+\t\t\t\t each_ref_fn cb, void *cb_data);\n+\n #define GIT_TEST_PACK_USE_BITMAP_BOUNDARY_TRAVERSAL \\\n \t\"GIT_TEST_PACK_USE_BITMAP_BOUNDARY_TRAVERSAL\"\n \n@@ -182,7 +190,6 @@ char *pack_bitmap_filename(struct packed_git *p);\n \n int bitmap_is_midx(struct bitmap_index *bitmap_git);\n \n-const struct string_list *bitmap_preferred_tips(struct repository *r);\n int bitmap_is_preferred_refname(struct repository *r, const char *refname);\n \n int verify_bitmap_files(struct repository *r);\ndiff --git a/repack-midx.c b/repack-midx.c\nindex 74bdfa3a6e..0682b80c42 100644\n--- a/repack-midx.c\n+++ b/repack-midx.c\n@@ -40,7 +40,6 @@ static int midx_snapshot_ref_one(const struct reference *ref, void *_data)\n void midx_snapshot_refs(struct repository *repo, struct tempfile *f)\n {\n \tstruct midx_snapshot_ref_data data;\n-\tconst struct string_list *preferred = bitmap_preferred_tips(repo);\n \n \tdata.repo = repo;\n \tdata.f = f;\n@@ -51,16 +50,9 @@ void midx_snapshot_refs(struct repository *repo, struct tempfile *f)\n \t\t die(_(\"could not open tempfile %s for writing\"),\n \t\t get_tempfile_path(f));\n \n-\tif (preferred) {\n-\t\tstruct string_list_item *item;\n-\n-\t\tdata.preferred = 1;\n-\t\tfor_each_string_list_item(item, preferred)\n-\t\t\trefs_for_each_ref_in(get_main_ref_store(repo),\n-\t\t\t\t\t item->string,\n-\t\t\t\t\t midx_snapshot_ref_one, &data);\n-\t\tdata.preferred = 0;\n-\t}\n+\tdata.preferred = 1;\n+\tfor_each_preferred_bitmap_tip(repo, midx_snapshot_ref_one, &data);\n+\tdata.preferred = 0;\n \n \trefs_for_each_ref(get_main_ref_store(repo),\n \t\t\t midx_snapshot_ref_one, &data);\n\n-- \n2.53.0.rc2.206.g60c1bca835.dirty\n\n"},{"id":"534870","messageId":"20260130-b4-pks-fix-for-each-ref-in-misuse-v2-2-0449b198a681@pks.im","threadId":"64879","inReplyTo":"20260130-b4-pks-fix-for-each-ref-in-misuse-v2-0-0449b198a681@pks.im","subject":"[PATCH v2 2/4] pack-bitmap: fix bug with exact ref match in \"pack.preferBitmapTips\"","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-30T13:27:43Z","receivedAt":"2026-01-30T13:28:12Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The \"pack.preferBitmapTips\" configuration allows the user to specify\nwhich references should be preferred when generating bitmaps. This\noption is typically expected to be set to a reference prefix, like for\nexample \"refs/heads/\".\n\nIt's not unreasonable though for a user to configure one specific\nreference as preferred. But if they do, they'll hit a `BUG()`:\n\n $ git -c pack.preferBitmapTips=refs/heads/main repack -adb\n BUG: ../refs/iterator.c:366: attempt to trim too many characters\n error: pack-objects died of signal 6\n\nThe root cause for this bug is how we enumerate these references. We\ncall `refs_for_each_ref_in()`, which will:\n\n - Yield all references that have a user-specified prefix.\n\n - Trim each of these references so that the prefix is removed.\n\nTypically, this function is called with a trailing slash, like\n\"refs/heads/\", and in that case things work alright. But if the function\nis called with the name of an existing reference then we'll try to trim\nthe full reference name, which would leave us with an empty name. And as\nthis would not really leave us with anything sensible, we call `BUG()`\ninstead of yielding this reference.\n\nOne could argue that this is a bug in `refs_for_each_ref_in()`. But the\nquestion then becomes what the correct behaviour would be:\n\n - Do we want to skip exact matches? In our case we certainly don't\n want that, as the user has asked us to generate a bitmap for it.\n\n - Do we want to yield the reference with the empty refname? That would\n lead to a somewhat weird result.\n\nNeither of these feel like viable options, so calling `BUG()` feels like\na sensible way out. The root cause ultimately is that we even try to\ntrim the whole refname in the first place. There are two possible ways\nto fix this issue:\n\n - We can fix the bug by using `refs_for_each_fullref_in()` instead,\n which does not strip the prefix at all. Consequently, we would now\n start to accept all references that start with the configured\n prefix, including exact matches. So if we had \"refs/heads/main\", we\n would both match \"refs/heads/main\" and \"refs/heads/main-branch\".\n\n - Or we can fix the bug by appending a slash to the prefix if it\n doesn't already have one. This would mean that we only match\n ref hierarchies that start with this prefix.\n\nThe first fix leaves the user with strictly _more_ configuration\noptions: they can have prefix matches by not appending a slash to the\nconfiguration, and they can have ref hierarchy matches by appending one.\n\nApply this fix and clarify the documentation accordingly.\n\nSigned-off-by: Patrick Steinhardt \n---\n Documentation/config/pack.adoc | 7 +++----\n pack-bitmap.c | 4 ++--\n t/t5310-pack-bitmaps.sh | 35 +++++++++++++++++++++++++++++++++++\n t/t5319-multi-pack-index.sh | 36 ++++++++++++++++++++++++++++++++++++\n 4 files changed, 76 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/config/pack.adoc b/Documentation/config/pack.adoc\nindex 75402d5579..929d781552 100644\n--- a/Documentation/config/pack.adoc\n+++ b/Documentation/config/pack.adoc\n@@ -161,11 +161,10 @@ pack.usePathWalk::\n \n pack.preferBitmapTips::\n \tWhen selecting which commits will receive bitmaps, prefer a\n-\tcommit at the tip of any reference that is a suffix of any value\n-\tof this configuration over any other commits in the \"selection\n-\twindow\".\n+\tcommmit at the tip of a reference that matches any of the\n+\tconfigured prefixes.\n +\n-Note that setting this configuration to `refs/foo` does not mean that\n+Note that setting this configuration to `refs/foo/` does not mean that\n the commits at the tips of `refs/foo/bar` and `refs/foo/baz` will\n necessarily be selected. This is because commits are selected for\n bitmaps from within a series of windows of variable length.\ndiff --git a/pack-bitmap.c b/pack-bitmap.c\nindex 2f5cb34009..8d3b5ac037 100644\n--- a/pack-bitmap.c\n+++ b/pack-bitmap.c\n@@ -3334,8 +3334,8 @@ void for_each_preferred_bitmap_tip(struct repository *repo,\n \t\treturn;\n \n \tfor_each_string_list_item(item, preferred_tips) {\n-\t\trefs_for_each_ref_in(get_main_ref_store(repo),\n-\t\t\t\t item->string, cb, cb_data);\n+\t\trefs_for_each_fullref_in(get_main_ref_store(repo),\n+\t\t\t\t\t item->string, NULL, cb, cb_data);\n \t}\n }\n \ndiff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh\nindex 6718fb98c0..7ef91b502c 100755\n--- a/t/t5310-pack-bitmaps.sh\n+++ b/t/t5310-pack-bitmaps.sh\n@@ -466,6 +466,41 @@ test_bitmap_cases () {\n \t\t)\n \t'\n \n+\ttest_expect_success 'pack.preferBitmapTips can use direct refname' '\n+\t\tgit init repo &&\n+\t\ttest_when_finished \"rm -fr repo\" &&\n+\t\t(\n+\t\t\tcd repo &&\n+\n+\t\t\t# Create enough commits that not all will receive bitmap\n+\t\t\t# coverage even if they are all at the tip of some reference.\n+\t\t\ttest_commit_bulk --message=\"%s\" 103 &&\n+\t\t\tgit log --format=\"create refs/tags/%s %H\" HEAD >refs &&\n+\t\t\tgit update-ref --stdin commits-with-bitmap &&\n+\n+\t\t\t# Verify that we have at least one commit that did not\n+\t\t\t# receive a bitmap.\n+\t\t\tgit rev-list HEAD >commits.raw &&\n+\t\t\tsort commits &&\n+\t\t\tcomm -13 commits-with-bitmap commits >commits-wo-bitmap &&\n+\t\t\ttest_file_not_empty commits-wo-bitmap &&\n+\t\t\tcommit_id=$(head commits-wo-bitmap) &&\n+\n+\t\t\t# We now create a reference for this commit and repack\n+\t\t\t# with \"preferBitmapTips\" pointing to that exact\n+\t\t\t# reference. The expectation is that it will now be\n+\t\t\t# covered by a bitmap.\n+\t\t\tgit update-ref refs/heads/cover-me \"$commit_id\" &&\n+\t\t\tgit -c pack.preferBitmapTips=refs/heads/cover-me repack -adb &&\n+\t\t\ttest-tool bitmap list-commits >after &&\n+\t\t\ttest_grep \"$commit_id\" after\n+\t\t)\n+\t'\n+\n \ttest_expect_success 'complains about multiple pack bitmaps' '\n \t\trm -fr repo &&\n \t\tgit init repo &&\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex faae98c7e7..40d36118bd 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -1345,4 +1345,40 @@ test_expect_success 'bitmapped packs are stored via the BTMP chunk' '\n \t)\n '\n \n+test_expect_success 'pack.preferBitmapTips can use direct refname' '\n+\tgit init repo &&\n+\ttest_when_finished \"rm -fr repo\" &&\n+\t(\n+\t\tcd repo &&\n+\n+\t\t# Create enough commits that not all will receive bitmap\n+\t\t# coverage even if they are all at the tip of some reference.\n+\t\ttest_commit_bulk --message=\"%s\" 103 &&\n+\t\tgit log --format=\"create refs/tags/%s %H\" HEAD >refs &&\n+\t\tgit update-ref --stdin commits-with-bitmap &&\n+\n+\t\t# Verify that we have at least one commit that did not\n+\t\t# receive a bitmap.\n+\t\tgit rev-list HEAD >commits.raw &&\n+\t\tsort commits &&\n+\t\tcomm -13 commits-with-bitmap commits >commits-wo-bitmap &&\n+\t\ttest_file_not_empty commits-wo-bitmap &&\n+\t\tcommit_id=$(head commits-wo-bitmap) &&\n+\n+\t\t# We now create a reference for this commit and repack\n+\t\t# with \"preferBitmapTips\" pointing to that exact\n+\t\t# reference. The expectation is that it will now be\n+\t\t# covered by a bitmap.\n+\t\tgit update-ref refs/heads/cover-me \"$commit_id\" &&\n+\t\trm .git/objects/pack/multi-pack-index* &&\n+\t\tgit -c pack.preferBitmapTips=refs/heads/cover-me repack -adb --write-midx &&\n+\t\ttest-tool bitmap list-commits >after &&\n+\t\ttest_grep \"$commit_id\" after\n+\t)\n+'\n+\n test_done\n\n-- \n2.53.0.rc2.206.g60c1bca835.dirty\n\n"},{"id":"534871","messageId":"20260130-b4-pks-fix-for-each-ref-in-misuse-v2-3-0449b198a681@pks.im","threadId":"64879","inReplyTo":"20260130-b4-pks-fix-for-each-ref-in-misuse-v2-0-0449b198a681@pks.im","subject":"[PATCH v2 3/4] bisect: fix misuse of `refs_for_each_ref_in()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-30T13:27:44Z","receivedAt":"2026-01-30T13:28:18Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"All callers of `refs_for_each_ref_in()` pass in a string that is\nterminated with a trailing slash to indicate that they only want to see\nrefs in that specific ref hierarchy. This is in fact a requirement if\none wants to use this function, as the function trims the prefix from\neach yielded ref. So if there was a reference that was called\n\"refs/bisect\" as in our example, the result after trimming would be the\nempty string, and that's something we disallow.\n\nFix this by adding the trailing slash.\n\nFurthermore, taking a closer look, we strip the prefix only to re-add it\nin `mark_for_removal()`. This is somewhat roundabout, as we can instead\ncall `refs_for_each_fullref_in()` to not do any stripping at all. Do so\nto simplify the code a bit.\n\nSigned-off-by: Patrick Steinhardt \n---\n bisect.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex 326b59c0dc..4f0d1a1853 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -1180,7 +1180,7 @@ int estimate_bisect_steps(int all)\n static int mark_for_removal(const struct reference *ref, void *cb_data)\n {\n \tstruct string_list *refs = cb_data;\n-\tchar *bisect_ref = xstrfmt(\"refs/bisect%s\", ref->name);\n+\tchar *bisect_ref = xstrdup(ref->name);\n \tstring_list_append(refs, bisect_ref);\n \treturn 0;\n }\n@@ -1191,9 +1191,9 @@ int bisect_clean_state(void)\n \n \t/* There may be some refs packed during bisection */\n \tstruct string_list refs_for_removal = STRING_LIST_INIT_NODUP;\n-\trefs_for_each_ref_in(get_main_ref_store(the_repository),\n-\t\t\t \"refs/bisect\", mark_for_removal,\n-\t\t\t (void *) &refs_for_removal);\n+\trefs_for_each_fullref_in(get_main_ref_store(the_repository),\n+\t\t\t\t \"refs/bisect/\", NULL, mark_for_removal,\n+\t\t\t\t &refs_for_removal);\n \tstring_list_append(&refs_for_removal, xstrdup(\"BISECT_HEAD\"));\n \tstring_list_append(&refs_for_removal, xstrdup(\"BISECT_EXPECTED_REV\"));\n \tresult = refs_delete_refs(get_main_ref_store(the_repository),\n\n-- \n2.53.0.rc2.206.g60c1bca835.dirty\n\n"},{"id":"534872","messageId":"20260130-b4-pks-fix-for-each-ref-in-misuse-v2-4-0449b198a681@pks.im","threadId":"64879","inReplyTo":"20260130-b4-pks-fix-for-each-ref-in-misuse-v2-0-0449b198a681@pks.im","subject":"[PATCH v2 4/4] bisect: simplify string_list memory handling","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-30T13:27:45Z","receivedAt":"2026-01-30T13:29:56Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"From: Jeff King \n\nWe declare the refs_for_removal string_list as NODUP, forcing us to\nmanually allocate strings we insert. And then when it comes time to\nclean up, we set strdup_strings so that string_list_clear() will free\nthem for us.\n\nThis is a confusing pattern, and can be done much more simply by just\ndeclaring the list with the DUP initializer in the first place.\n\nIt was written this way originally because one of the callsites\ngenerated the item using xstrfmt(). But that spot switched to a plain\nxstrdup() in the preceding commit. That means we can now just let the\nstring_list code handle allocation itself.\n\nSigned-off-by: Jeff King \nSigned-off-by: Patrick Steinhardt \n---\n bisect.c | 10 ++++------\n 1 file changed, 4 insertions(+), 6 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex 4f0d1a1853..268f5e36f8 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -1180,8 +1180,7 @@ int estimate_bisect_steps(int all)\n static int mark_for_removal(const struct reference *ref, void *cb_data)\n {\n \tstruct string_list *refs = cb_data;\n-\tchar *bisect_ref = xstrdup(ref->name);\n-\tstring_list_append(refs, bisect_ref);\n+\tstring_list_append(refs, ref->name);\n \treturn 0;\n }\n \n@@ -1190,16 +1189,15 @@ int bisect_clean_state(void)\n \tint result = 0;\n \n \t/* There may be some refs packed during bisection */\n-\tstruct string_list refs_for_removal = STRING_LIST_INIT_NODUP;\n+\tstruct string_list refs_for_removal = STRING_LIST_INIT_DUP;\n \trefs_for_each_fullref_in(get_main_ref_store(the_repository),\n \t\t\t\t \"refs/bisect/\", NULL, mark_for_removal,\n \t\t\t\t &refs_for_removal);\n-\tstring_list_append(&refs_for_removal, xstrdup(\"BISECT_HEAD\"));\n-\tstring_list_append(&refs_for_removal, xstrdup(\"BISECT_EXPECTED_REV\"));\n+\tstring_list_append(&refs_for_removal, \"BISECT_HEAD\");\n+\tstring_list_append(&refs_for_removal, \"BISECT_EXPECTED_REV\");\n \tresult = refs_delete_refs(get_main_ref_store(the_repository),\n \t\t\t\t \"bisect: remove\", &refs_for_removal,\n \t\t\t\t REF_NO_DEREF);\n-\trefs_for_removal.strdup_strings = 1;\n \tstring_list_clear(&refs_for_removal, 0);\n \tunlink_or_warn(git_path_bisect_ancestors_ok());\n \tunlink_or_warn(git_path_bisect_log());\n\n-- \n2.53.0.rc2.206.g60c1bca835.dirty\n\n"},{"id":"534896","messageId":"xmqqqzr76nuj.fsf@gitster.g","threadId":"64879","inReplyTo":"20260130-b4-pks-fix-for-each-ref-in-misuse-v2-4-0449b198a681@pks.im","subject":"Re: [PATCH v2 4/4] bisect: simplify string_list memory handling","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-30T16:56:36Z","receivedAt":"2026-01-30T16:56:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> It was written this way originally because one of the callsites\n> generated the item using xstrfmt(). But that spot switched to a plain\n> xstrdup() in the preceding commit. That means we can now just let the\n> string_list code handle allocation itself.\n\nThanks for an extra attention to the detail of the way to refer the\nprevious change ;-).\n\nI think [2/4] is a good direction myself, but I'd prefer to hear\nTaylor's opinion as well.\n\nThanks.\n"},{"id":"534952","messageId":"aYAIVw5UMQeP3Ilr@nand.local","threadId":"64879","inReplyTo":"20260130-b4-pks-fix-for-each-ref-in-misuse-v2-2-0449b198a681@pks.im","subject":"Re: [PATCH v2 2/4] pack-bitmap: fix bug with exact ref match in \"pack.preferBitmapTips\"","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-02-02T02:13:43Z","receivedAt":"2026-02-02T02:13:45Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Fri, Jan 30, 2026 at 02:27:43PM +0100, Patrick Steinhardt wrote:\n> [...] There are two possible ways to fix this issue:\n>\n> - We can fix the bug by using `refs_for_each_fullref_in()` instead,\n> which does not strip the prefix at all. Consequently, we would now\n> start to accept all references that start with the configured\n> prefix, including exact matches. So if we had \"refs/heads/main\", we\n> would both match \"refs/heads/main\" and \"refs/heads/main-branch\".\n>\n> - Or we can fix the bug by appending a slash to the prefix if it\n> doesn't already have one. This would mean that we only match\n> ref hierarchies that start with this prefix.\n>\n> The first fix leaves the user with strictly _more_ configuration\n> options: they can have prefix matches by not appending a slash to the\n> configuration, and they can have ref hierarchy matches by appending one.\n\nI would definitely like to err on the side of more flexible\nconfiguration options, but I am still concerned that this change would\nlead to somewhat surprising behavior.\n\nA couple of thoughts:\n\n - Like I mentioned in the earlier round, 10e8a9352bc (refs.c: stop\n matching non-directory prefixes in exclude patterns, 2025-03-06)\n takes the opposite approach as what is being proposed here. I worry\n that users will find the difference in behavior between\n pack.preferBitmapTips and for-each-ref's --exclude patterns to be\n confusing.\n\n - If a user wants to list all references that start with\n \"refs/heads/ma\" in the string prefix sense (that is, matching\n \"refs/heads/ma\", \"refs/heads/main\", \"refs/heads/master\" and so on),\n then they would do\n\n $ git for-each-ref 'refs/heads/ma*'\n\n , not 'refs/heads/ma'. In fact, enumerating 'refs/heads/ma' when\n there exist references \"refs/heads/ma/foo\", \"refs/heads/ma/bar\",\n etc., for-each-ref will output those three references (but only\n \"refs/heads/ma\" itself if it exists).\n\nI suppose there is an argument to be made that we are dealing with\n\"patterns\" vs. \"prefixes\" here, but TBH I am not sure that is a\ndistinction that is well-understood by users (nor should we expect it to\nbe).\n\nThe original intent of this configuration was that \"suffix\" in this\ncontext meant directory suffix or exact match, not string suffix. The\nimplementation does not match that intent, but I think there is enough\nambiguity here that I wouldn't consider the change I'm suggesting to be\na breaking one.\n\nOverall, I think interpreting the pack.preferBitmapTips configuration as\na reference pattern gives the user both (a) more flexibility in which\nreferences to match, and (b) does so in a way that is consistent with\n10e8a9352bc and the existing behavior of for-each-ref.\n\nThanks,\nTaylor\n"},{"id":"534953","messageId":"aYAIfKW8Vd0iBun9@nand.local","threadId":"64879","inReplyTo":"xmqqqzr76nuj.fsf@gitster.g","subject":"Re: [PATCH v2 4/4] bisect: simplify string_list memory handling","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-02-02T02:14:20Z","receivedAt":"2026-02-02T02:14:22Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Fri, Jan 30, 2026 at 08:56:36AM -0800, Junio C Hamano wrote:\n> Patrick Steinhardt writes:\n>\n> > It was written this way originally because one of the callsites\n> > generated the item using xstrfmt(). But that spot switched to a plain\n> > xstrdup() in the preceding commit. That means we can now just let the\n> > string_list code handle allocation itself.\n>\n> Thanks for an extra attention to the detail of the way to refer the\n> previous change ;-).\n>\n> I think [2/4] is a good direction myself, but I'd prefer to hear\n> Taylor's opinion as well.\n\nAfter thinking it over and re-reading the second round, I am still not\nquite convinced that this is the right approach. I left some more\nthoughts on possible alternatives in my response to [2/4].\n\nThanks,\nTaylor\n"},{"id":"535312","messageId":"aYWUnM7aA5SJBdwS@pks.im","threadId":"64879","inReplyTo":"aYAIVw5UMQeP3Ilr@nand.local","subject":"Re: [PATCH v2 2/4] pack-bitmap: fix bug with exact ref match in \"pack.preferBitmapTips\"","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-02-06T07:13:32Z","receivedAt":"2026-02-06T07:13:43Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sun, Feb 01, 2026 at 09:13:43PM -0500, Taylor Blau wrote:\n> On Fri, Jan 30, 2026 at 02:27:43PM +0100, Patrick Steinhardt wrote:\n> > [...] There are two possible ways to fix this issue:\n> >\n> > - We can fix the bug by using `refs_for_each_fullref_in()` instead,\n> > which does not strip the prefix at all. Consequently, we would now\n> > start to accept all references that start with the configured\n> > prefix, including exact matches. So if we had \"refs/heads/main\", we\n> > would both match \"refs/heads/main\" and \"refs/heads/main-branch\".\n> >\n> > - Or we can fix the bug by appending a slash to the prefix if it\n> > doesn't already have one. This would mean that we only match\n> > ref hierarchies that start with this prefix.\n> >\n> > The first fix leaves the user with strictly _more_ configuration\n> > options: they can have prefix matches by not appending a slash to the\n> > configuration, and they can have ref hierarchy matches by appending one.\n> \n> I would definitely like to err on the side of more flexible\n> configuration options, but I am still concerned that this change would\n> lead to somewhat surprising behavior.\n\nI think my point is mostly that this somewhat surprising behaviour\nalready exists right now.\n\n> A couple of thoughts:\n> \n> - Like I mentioned in the earlier round, 10e8a9352bc (refs.c: stop\n> matching non-directory prefixes in exclude patterns, 2025-03-06)\n> takes the opposite approach as what is being proposed here. I worry\n> that users will find the difference in behavior between\n> pack.preferBitmapTips and for-each-ref's --exclude patterns to be\n> confusing.\n> \n> - If a user wants to list all references that start with\n> \"refs/heads/ma\" in the string prefix sense (that is, matching\n> \"refs/heads/ma\", \"refs/heads/main\", \"refs/heads/master\" and so on),\n> then they would do\n> \n> $ git for-each-ref 'refs/heads/ma*'\n> \n> , not 'refs/heads/ma'. In fact, enumerating 'refs/heads/ma' when\n> there exist references \"refs/heads/ma/foo\", \"refs/heads/ma/bar\",\n> etc., for-each-ref will output those three references (but only\n> \"refs/heads/ma\" itself if it exists).\n> \n> I suppose there is an argument to be made that we are dealing with\n> \"patterns\" vs. \"prefixes\" here, but TBH I am not sure that is a\n> distinction that is well-understood by users (nor should we expect it to\n> be).\n> \n> The original intent of this configuration was that \"suffix\" in this\n> context meant directory suffix or exact match, not string suffix. The\n> implementation does not match that intent, but I think there is enough\n> ambiguity here that I wouldn't consider the change I'm suggesting to be\n> a breaking one.\n\nI wouldn't quite frame it in the way of ambiguity, but rather in the way\nof it being very niche. I would argue that almost nobody out there will\nuse this configuration outside of hosting providers. GitHub will\nprobably use it correctly, GitLab doesn't use it at all.\n\nSo I guess it's fine overall if we introduce a breaking change here.\n\n> Overall, I think interpreting the pack.preferBitmapTips configuration as\n> a reference pattern gives the user both (a) more flexibility in which\n> references to match, and (b) does so in a way that is consistent with\n> 10e8a9352bc and the existing behavior of for-each-ref.\n\nI disagree with (a) as you can do strictly less, but being constistent\nwith (b) might be a good thing.\n\nAt the end I'm not entirely convinced by the arguments, but as I said I\ndon't have too much skin in the game, either. So let's take your\napproach.\n\nThanks!\n\nPatrick\n"},{"id":"535315","messageId":"20260206-b4-pks-fix-for-each-ref-in-misuse-v3-0-1e050c3d6a50@pks.im","threadId":"64879","inReplyTo":"20260128-b4-pks-fix-for-each-ref-in-misuse-v1-0-deccae3ea725@pks.im","subject":"[PATCH v3 0/4] Fix misuse of `refs_for_each_ref_in()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-02-06T07:49:55Z","receivedAt":"2026-02-06T07:50:06Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis small patch series fixes a bug I have discovered where configuring\n\"pack.preferBitmapTips\" to an exact branch will cause Git to `BUG()`.\n\nThe root cause of this bug is misuse of `refs_for_each_ref_in()`: this\nfunction accepts a prefix to yield refs for, and then strips the prefix\nfor each ref. Consequently, if passed an exact refname, then stripping\nthe prefix would make us end up with an empty refname, and that is not\nsupposed to happen.\n\nThere was one other caller that got it wrong, too, and which is also\nfixed in this patch series.\n\nChanges in v3:\n - Switch the approach to perform ref hierarchy matches instead, which\n is in line with the changes in 10e8a9352b (refs.c: stop matching\n non-directory prefixes in exclude patterns, 2025-03-06).\n - Link to v2: https://lore.kernel.org/r/20260130-b4-pks-fix-for-each-ref-in-misuse-v2-0-0449b198a681@pks.im\n\nChanges in v2:\n - Explain my thought process against why I chose to also allow exact\n ref matches in the second commit and clarify the documentation a\n bit. As said, I'm very open to changing this if my spelled-out\n thoughts are not convincing.\n - Apply Peff's patch to further simplify code.\n - Link to v1: https://lore.kernel.org/r/20260128-b4-pks-fix-for-each-ref-in-misuse-v1-0-deccae3ea725@pks.im\n\nThanks!\n\nPatrick\n\n---\nJeff King (1):\n bisect: simplify string_list memory handling\n\nPatrick Steinhardt (3):\n pack-bitmap: deduplicate logic to iterate over preferred bitmap tips\n pack-bitmap: fix bug with exact ref match in \"pack.preferBitmapTips\"\n bisect: fix misuse of `refs_for_each_ref_in()`\n\n Documentation/config/pack.adoc | 9 +++++----\n bisect.c | 16 +++++++---------\n builtin/pack-objects.c | 19 ++-----------------\n pack-bitmap.c | 29 +++++++++++++++++++++++++++-\n pack-bitmap.h | 9 ++++++++-\n repack-midx.c | 14 +++-----------\n t/t5310-pack-bitmaps.sh | 41 ++++++++++++++++++++++++++++++++++++++++\n t/t5319-multi-pack-index.sh | 43 ++++++++++++++++++++++++++++++++++++++++++\n 8 files changed, 137 insertions(+), 43 deletions(-)\n\nRange-diff versus v2:\n\n1: 276b593c0d = 1: 927e8bf4ae pack-bitmap: deduplicate logic to iterate over preferred bitmap tips\n2: e7a5e5e447 ! 2: 3e4ae331dd pack-bitmap: fix bug with exact ref match in \"pack.preferBitmapTips\"\n @@ Commit message\n doesn't already have one. This would mean that we only match\n ref hierarchies that start with this prefix.\n \n - The first fix leaves the user with strictly _more_ configuration\n - options: they can have prefix matches by not appending a slash to the\n - configuration, and they can have ref hierarchy matches by appending one.\n + While the first fix leaves the user with strictly _more_ configuration\n + options, we have already fixed a similar case in 10e8a9352b (refs.c:\n + stop matching non-directory prefixes in exclude patterns, 2025-03-06) by\n + using the second option. So for the sake of consistency, let's apply the\n + same fix here.\n \n - Apply this fix and clarify the documentation accordingly.\n + Clarify the documentation accordingly.\n \n Signed-off-by: Patrick Steinhardt \n \n ## Documentation/config/pack.adoc ##\n @@ Documentation/config/pack.adoc: pack.usePathWalk::\n + \tprocesses. See linkgit:git-pack-objects[1] for full details.\n \n pack.preferBitmapTips::\n ++\tSpecifies a ref hierarchy (e.g., \"refs/heads/\"); can be\n ++\tgiven multiple times to specify more than one hierarchies.\n \tWhen selecting which commits will receive bitmaps, prefer a\n -\tcommit at the tip of any reference that is a suffix of any value\n -\tof this configuration over any other commits in the \"selection\n -\twindow\".\n -+\tcommmit at the tip of a reference that matches any of the\n -+\tconfigured prefixes.\n ++\tcommmit at the tip of a reference that is contained in any of\n ++\tthe configured hierarchies.\n +\n -Note that setting this configuration to `refs/foo` does not mean that\n +Note that setting this configuration to `refs/foo/` does not mean that\n @@ Documentation/config/pack.adoc: pack.usePathWalk::\n \n ## pack-bitmap.c ##\n @@ pack-bitmap.c: void for_each_preferred_bitmap_tip(struct repository *repo,\n + {\n + \tstruct string_list_item *item;\n + \tconst struct string_list *preferred_tips;\n ++\tstruct strbuf buf = STRBUF_INIT;\n + \n + \tpreferred_tips = bitmap_preferred_tips(repo);\n + \tif (!preferred_tips)\n \t\treturn;\n \n \tfor_each_string_list_item(item, preferred_tips) {\n --\t\trefs_for_each_ref_in(get_main_ref_store(repo),\n ++\t\tconst char *pattern = item->string;\n ++\n ++\t\tif (!ends_with(pattern, \"/\")) {\n ++\t\t\tstrbuf_reset(&buf);\n ++\t\t\tstrbuf_addf(&buf, \"%s/\", pattern);\n ++\t\t\tpattern = buf.buf;\n ++\t\t}\n ++\n + \t\trefs_for_each_ref_in(get_main_ref_store(repo),\n -\t\t\t\t item->string, cb, cb_data);\n -+\t\trefs_for_each_fullref_in(get_main_ref_store(repo),\n -+\t\t\t\t\t item->string, NULL, cb, cb_data);\n ++\t\t\t\t pattern, cb, cb_data);\n \t}\n ++\n ++\tstrbuf_release(&buf);\n }\n \n + int bitmap_is_preferred_refname(struct repository *r, const char *refname)\n \n ## t/t5310-pack-bitmaps.sh ##\n @@ t/t5310-pack-bitmaps.sh: test_bitmap_cases () {\n \t\t)\n \t'\n \n -+\ttest_expect_success 'pack.preferBitmapTips can use direct refname' '\n ++\ttest_expect_success 'pack.preferBitmapTips interprets patterns as hierarchy' '\n +\t\tgit init repo &&\n +\t\ttest_when_finished \"rm -fr repo\" &&\n +\t\t(\n @@ t/t5310-pack-bitmaps.sh: test_bitmap_cases () {\n +\t\t\t# Create enough commits that not all will receive bitmap\n +\t\t\t# coverage even if they are all at the tip of some reference.\n +\t\t\ttest_commit_bulk --message=\"%s\" 103 &&\n -+\t\t\tgit log --format=\"create refs/tags/%s %H\" HEAD >refs &&\n ++\t\t\tgit log --format=\"create refs/tags/%s/tag %H\" HEAD >refs &&\n +\t\t\tgit update-ref --stdin commits-wo-bitmap &&\n +\t\t\ttest_file_not_empty commits-wo-bitmap &&\n +\t\t\tcommit_id=$(head commits-wo-bitmap) &&\n ++\t\t\tref_without_bitmap=$(git for-each-ref --points-at=\"$commit_id\" --format=\"%(refname)\") &&\n +\n -+\t\t\t# We now create a reference for this commit and repack\n -+\t\t\t# with \"preferBitmapTips\" pointing to that exact\n -+\t\t\t# reference. The expectation is that it will now be\n -+\t\t\t# covered by a bitmap.\n -+\t\t\tgit update-ref refs/heads/cover-me \"$commit_id\" &&\n -+\t\t\tgit -c pack.preferBitmapTips=refs/heads/cover-me repack -adb &&\n ++\t\t\t# When passing the full refname we do not expect a\n ++\t\t\t# bitmap to be generated, as it should be interpreted\n ++\t\t\t# as if a slash was appended to the pattern.\n ++\t\t\tgit -c pack.preferBitmapTips=\"$ref_without_bitmap\" repack -adb &&\n ++\t\t\ttest-tool bitmap list-commits >after &&\n ++\t\t\ttest_grep ! \"$commit_id\" after &&\n ++\n ++\t\t\t# But if we pass the parent directory of the ref we\n ++\t\t\t# should see a bitmap.\n ++\t\t\tref_namespace=$(dirname \"$ref_without_bitmap\") &&\n ++\t\t\tgit -c pack.preferBitmapTips=\"$ref_namespace\" repack -adb &&\n +\t\t\ttest-tool bitmap list-commits >after &&\n +\t\t\ttest_grep \"$commit_id\" after\n +\t\t)\n @@ t/t5319-multi-pack-index.sh: test_expect_success 'bitmapped packs are stored via\n \t)\n '\n \n -+test_expect_success 'pack.preferBitmapTips can use direct refname' '\n ++test_expect_success 'pack.preferBitmapTips interprets patterns as hierarchy' '\n +\tgit init repo &&\n +\ttest_when_finished \"rm -fr repo\" &&\n +\t(\n @@ t/t5319-multi-pack-index.sh: test_expect_success 'bitmapped packs are stored via\n +\t\tcomm -13 commits-with-bitmap commits >commits-wo-bitmap &&\n +\t\ttest_file_not_empty commits-wo-bitmap &&\n +\t\tcommit_id=$(head commits-wo-bitmap) &&\n ++\t\tref_without_bitmap=$(git for-each-ref --points-at=\"$commit_id\" --format=\"%(refname)\") &&\n ++\n ++\t\t# When passing the full refname we do not expect a bitmap to be\n ++\t\t# generated, as it should be interpreted as if a slash was\n ++\t\t# appended to the pattern.\n ++\t\trm .git/objects/pack/multi-pack-index* &&\n ++\t\tgit -c pack.preferBitmapTips=\"$ref_without_bitmap\" repack -adb --write-midx &&\n ++\t\ttest-tool bitmap list-commits >after &&\n ++\t\ttest_grep ! \"$commit_id\" after &&\n +\n -+\t\t# We now create a reference for this commit and repack\n -+\t\t# with \"preferBitmapTips\" pointing to that exact\n -+\t\t# reference. The expectation is that it will now be\n -+\t\t# covered by a bitmap.\n -+\t\tgit update-ref refs/heads/cover-me \"$commit_id\" &&\n ++\t\t# But if we pass the parent directory of the ref we should see\n ++\t\t# a bitmap.\n ++\t\tref_namespace=$(dirname \"$ref_without_bitmap\") &&\n +\t\trm .git/objects/pack/multi-pack-index* &&\n -+\t\tgit -c pack.preferBitmapTips=refs/heads/cover-me repack -adb --write-midx &&\n ++\t\tgit -c pack.preferBitmapTips=\"$ref_namespace\" repack -adb --write-midx &&\n +\t\ttest-tool bitmap list-commits >after &&\n +\t\ttest_grep \"$commit_id\" after\n +\t)\n3: 261523862b = 3: 8d82966f8f bisect: fix misuse of `refs_for_each_ref_in()`\n4: e9b0fd0d5f = 4: 7f68687165 bisect: simplify string_list memory handling\n\n---\nbase-commit: ea717645d199f6f1b66058886475db3e8c9330e9\nchange-id: 20260128-b4-pks-fix-for-each-ref-in-misuse-96ae62e313a5\n\n"},{"id":"535316","messageId":"20260206-b4-pks-fix-for-each-ref-in-misuse-v3-1-1e050c3d6a50@pks.im","threadId":"64879","inReplyTo":"20260206-b4-pks-fix-for-each-ref-in-misuse-v3-0-1e050c3d6a50@pks.im","subject":"[PATCH v3 1/4] pack-bitmap: deduplicate logic to iterate over preferred bitmap tips","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-02-06T07:49:56Z","receivedAt":"2026-02-06T07:50:07Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We have two locations that iterate over the preferred bitmap tips as\nconfigured by the user via \"pack.preferBitmapTips\". Both of these\ncallsites are subtly wrong: when the preferred bitmap tips contain an\nexact refname match, then we will hit a `BUG()`.\n\nPrepare for the fix by unifying the two callsites into a new\n`for_each_preferred_bitmap_tip()` function.\n\nThis removes the last callsite of `bitmap_preferred_tips()` outside of\n\"pack-bitmap.c\". As such, convert the function to be local to that file\nonly. Note that the function is still used by a second caller, so we\ncannot just inline it.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/pack-objects.c | 19 ++-----------------\n pack-bitmap.c | 18 +++++++++++++++++-\n pack-bitmap.h | 9 ++++++++-\n repack-midx.c | 14 +++-----------\n 4 files changed, 30 insertions(+), 30 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 5846b6a293..979470e402 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -4554,22 +4554,6 @@ static int mark_bitmap_preferred_tip(const struct reference *ref, void *data UNU\n \treturn 0;\n }\n \n-static void mark_bitmap_preferred_tips(void)\n-{\n-\tstruct string_list_item *item;\n-\tconst struct string_list *preferred_tips;\n-\n-\tpreferred_tips = bitmap_preferred_tips(the_repository);\n-\tif (!preferred_tips)\n-\t\treturn;\n-\n-\tfor_each_string_list_item(item, preferred_tips) {\n-\t\trefs_for_each_ref_in(get_main_ref_store(the_repository),\n-\t\t\t\t item->string, mark_bitmap_preferred_tip,\n-\t\t\t\t NULL);\n-\t}\n-}\n-\n static inline int is_oid_uninteresting(struct repository *repo,\n \t\t\t\t struct object_id *oid)\n {\n@@ -4710,7 +4694,8 @@ static void get_object_list(struct rev_info *revs, struct strvec *argv)\n \t\tload_delta_islands(the_repository, progress);\n \n \tif (write_bitmap_index)\n-\t\tmark_bitmap_preferred_tips();\n+\t\tfor_each_preferred_bitmap_tip(the_repository, mark_bitmap_preferred_tip,\n+\t\t\t\t\t NULL);\n \n \tif (!fn_show_object)\n \t\tfn_show_object = show_object;\ndiff --git a/pack-bitmap.c b/pack-bitmap.c\nindex 972203f12b..2f5cb34009 100644\n--- a/pack-bitmap.c\n+++ b/pack-bitmap.c\n@@ -3314,7 +3314,7 @@ int bitmap_is_midx(struct bitmap_index *bitmap_git)\n \treturn !!bitmap_git->midx;\n }\n \n-const struct string_list *bitmap_preferred_tips(struct repository *r)\n+static const struct string_list *bitmap_preferred_tips(struct repository *r)\n {\n \tconst struct string_list *dest;\n \n@@ -3323,6 +3323,22 @@ const struct string_list *bitmap_preferred_tips(struct repository *r)\n \treturn NULL;\n }\n \n+void for_each_preferred_bitmap_tip(struct repository *repo,\n+\t\t\t\t each_ref_fn cb, void *cb_data)\n+{\n+\tstruct string_list_item *item;\n+\tconst struct string_list *preferred_tips;\n+\n+\tpreferred_tips = bitmap_preferred_tips(repo);\n+\tif (!preferred_tips)\n+\t\treturn;\n+\n+\tfor_each_string_list_item(item, preferred_tips) {\n+\t\trefs_for_each_ref_in(get_main_ref_store(repo),\n+\t\t\t\t item->string, cb, cb_data);\n+\t}\n+}\n+\n int bitmap_is_preferred_refname(struct repository *r, const char *refname)\n {\n \tconst struct string_list *preferred_tips = bitmap_preferred_tips(r);\ndiff --git a/pack-bitmap.h b/pack-bitmap.h\nindex 1bd7a791e2..d0611d0481 100644\n--- a/pack-bitmap.h\n+++ b/pack-bitmap.h\n@@ -5,6 +5,7 @@\n #include \"khash.h\"\n #include \"pack.h\"\n #include \"pack-objects.h\"\n+#include \"refs.h\"\n #include \"string-list.h\"\n \n struct commit;\n@@ -99,6 +100,13 @@ int for_each_bitmapped_object(struct bitmap_index *bitmap_git,\n \t\t\t show_reachable_fn show_reach,\n \t\t\t void *payload);\n \n+/*\n+ * Iterate over all references that are configured as preferred bitmap tips via\n+ * \"pack.preferBitmapTips\" and invoke the callback on each function.\n+ */\n+void for_each_preferred_bitmap_tip(struct repository *repo,\n+\t\t\t\t each_ref_fn cb, void *cb_data);\n+\n #define GIT_TEST_PACK_USE_BITMAP_BOUNDARY_TRAVERSAL \\\n \t\"GIT_TEST_PACK_USE_BITMAP_BOUNDARY_TRAVERSAL\"\n \n@@ -182,7 +190,6 @@ char *pack_bitmap_filename(struct packed_git *p);\n \n int bitmap_is_midx(struct bitmap_index *bitmap_git);\n \n-const struct string_list *bitmap_preferred_tips(struct repository *r);\n int bitmap_is_preferred_refname(struct repository *r, const char *refname);\n \n int verify_bitmap_files(struct repository *r);\ndiff --git a/repack-midx.c b/repack-midx.c\nindex 74bdfa3a6e..0682b80c42 100644\n--- a/repack-midx.c\n+++ b/repack-midx.c\n@@ -40,7 +40,6 @@ static int midx_snapshot_ref_one(const struct reference *ref, void *_data)\n void midx_snapshot_refs(struct repository *repo, struct tempfile *f)\n {\n \tstruct midx_snapshot_ref_data data;\n-\tconst struct string_list *preferred = bitmap_preferred_tips(repo);\n \n \tdata.repo = repo;\n \tdata.f = f;\n@@ -51,16 +50,9 @@ void midx_snapshot_refs(struct repository *repo, struct tempfile *f)\n \t\t die(_(\"could not open tempfile %s for writing\"),\n \t\t get_tempfile_path(f));\n \n-\tif (preferred) {\n-\t\tstruct string_list_item *item;\n-\n-\t\tdata.preferred = 1;\n-\t\tfor_each_string_list_item(item, preferred)\n-\t\t\trefs_for_each_ref_in(get_main_ref_store(repo),\n-\t\t\t\t\t item->string,\n-\t\t\t\t\t midx_snapshot_ref_one, &data);\n-\t\tdata.preferred = 0;\n-\t}\n+\tdata.preferred = 1;\n+\tfor_each_preferred_bitmap_tip(repo, midx_snapshot_ref_one, &data);\n+\tdata.preferred = 0;\n \n \trefs_for_each_ref(get_main_ref_store(repo),\n \t\t\t midx_snapshot_ref_one, &data);\n\n-- \n2.53.0.239.g8d8fc8a987.dirty\n\n"},{"id":"535317","messageId":"20260206-b4-pks-fix-for-each-ref-in-misuse-v3-2-1e050c3d6a50@pks.im","threadId":"64879","inReplyTo":"20260206-b4-pks-fix-for-each-ref-in-misuse-v3-0-1e050c3d6a50@pks.im","subject":"[PATCH v3 2/4] pack-bitmap: fix bug with exact ref match in \"pack.preferBitmapTips\"","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-02-06T07:49:57Z","receivedAt":"2026-02-06T07:50:09Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The \"pack.preferBitmapTips\" configuration allows the user to specify\nwhich references should be preferred when generating bitmaps. This\noption is typically expected to be set to a reference prefix, like for\nexample \"refs/heads/\".\n\nIt's not unreasonable though for a user to configure one specific\nreference as preferred. But if they do, they'll hit a `BUG()`:\n\n $ git -c pack.preferBitmapTips=refs/heads/main repack -adb\n BUG: ../refs/iterator.c:366: attempt to trim too many characters\n error: pack-objects died of signal 6\n\nThe root cause for this bug is how we enumerate these references. We\ncall `refs_for_each_ref_in()`, which will:\n\n - Yield all references that have a user-specified prefix.\n\n - Trim each of these references so that the prefix is removed.\n\nTypically, this function is called with a trailing slash, like\n\"refs/heads/\", and in that case things work alright. But if the function\nis called with the name of an existing reference then we'll try to trim\nthe full reference name, which would leave us with an empty name. And as\nthis would not really leave us with anything sensible, we call `BUG()`\ninstead of yielding this reference.\n\nOne could argue that this is a bug in `refs_for_each_ref_in()`. But the\nquestion then becomes what the correct behaviour would be:\n\n - Do we want to skip exact matches? In our case we certainly don't\n want that, as the user has asked us to generate a bitmap for it.\n\n - Do we want to yield the reference with the empty refname? That would\n lead to a somewhat weird result.\n\nNeither of these feel like viable options, so calling `BUG()` feels like\na sensible way out. The root cause ultimately is that we even try to\ntrim the whole refname in the first place. There are two possible ways\nto fix this issue:\n\n - We can fix the bug by using `refs_for_each_fullref_in()` instead,\n which does not strip the prefix at all. Consequently, we would now\n start to accept all references that start with the configured\n prefix, including exact matches. So if we had \"refs/heads/main\", we\n would both match \"refs/heads/main\" and \"refs/heads/main-branch\".\n\n - Or we can fix the bug by appending a slash to the prefix if it\n doesn't already have one. This would mean that we only match\n ref hierarchies that start with this prefix.\n\nWhile the first fix leaves the user with strictly _more_ configuration\noptions, we have already fixed a similar case in 10e8a9352b (refs.c:\nstop matching non-directory prefixes in exclude patterns, 2025-03-06) by\nusing the second option. So for the sake of consistency, let's apply the\nsame fix here.\n\nClarify the documentation accordingly.\n\nSigned-off-by: Patrick Steinhardt \n---\n Documentation/config/pack.adoc | 9 +++++----\n pack-bitmap.c | 13 ++++++++++++-\n t/t5310-pack-bitmaps.sh | 41 ++++++++++++++++++++++++++++++++++++++++\n t/t5319-multi-pack-index.sh | 43 ++++++++++++++++++++++++++++++++++++++++++\n 4 files changed, 101 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/config/pack.adoc b/Documentation/config/pack.adoc\nindex 75402d5579..dd2de30f0c 100644\n--- a/Documentation/config/pack.adoc\n+++ b/Documentation/config/pack.adoc\n@@ -160,12 +160,13 @@ pack.usePathWalk::\n \tprocesses. See linkgit:git-pack-objects[1] for full details.\n \n pack.preferBitmapTips::\n+\tSpecifies a ref hierarchy (e.g., \"refs/heads/\"); can be\n+\tgiven multiple times to specify more than one hierarchies.\n \tWhen selecting which commits will receive bitmaps, prefer a\n-\tcommit at the tip of any reference that is a suffix of any value\n-\tof this configuration over any other commits in the \"selection\n-\twindow\".\n+\tcommmit at the tip of a reference that is contained in any of\n+\tthe configured hierarchies.\n +\n-Note that setting this configuration to `refs/foo` does not mean that\n+Note that setting this configuration to `refs/foo/` does not mean that\n the commits at the tips of `refs/foo/bar` and `refs/foo/baz` will\n necessarily be selected. This is because commits are selected for\n bitmaps from within a series of windows of variable length.\ndiff --git a/pack-bitmap.c b/pack-bitmap.c\nindex 2f5cb34009..1c93871484 100644\n--- a/pack-bitmap.c\n+++ b/pack-bitmap.c\n@@ -3328,15 +3328,26 @@ void for_each_preferred_bitmap_tip(struct repository *repo,\n {\n \tstruct string_list_item *item;\n \tconst struct string_list *preferred_tips;\n+\tstruct strbuf buf = STRBUF_INIT;\n \n \tpreferred_tips = bitmap_preferred_tips(repo);\n \tif (!preferred_tips)\n \t\treturn;\n \n \tfor_each_string_list_item(item, preferred_tips) {\n+\t\tconst char *pattern = item->string;\n+\n+\t\tif (!ends_with(pattern, \"/\")) {\n+\t\t\tstrbuf_reset(&buf);\n+\t\t\tstrbuf_addf(&buf, \"%s/\", pattern);\n+\t\t\tpattern = buf.buf;\n+\t\t}\n+\n \t\trefs_for_each_ref_in(get_main_ref_store(repo),\n-\t\t\t\t item->string, cb, cb_data);\n+\t\t\t\t pattern, cb, cb_data);\n \t}\n+\n+\tstrbuf_release(&buf);\n }\n \n int bitmap_is_preferred_refname(struct repository *r, const char *refname)\ndiff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh\nindex 6718fb98c0..310b708c5c 100755\n--- a/t/t5310-pack-bitmaps.sh\n+++ b/t/t5310-pack-bitmaps.sh\n@@ -466,6 +466,47 @@ test_bitmap_cases () {\n \t\t)\n \t'\n \n+\ttest_expect_success 'pack.preferBitmapTips interprets patterns as hierarchy' '\n+\t\tgit init repo &&\n+\t\ttest_when_finished \"rm -fr repo\" &&\n+\t\t(\n+\t\t\tcd repo &&\n+\n+\t\t\t# Create enough commits that not all will receive bitmap\n+\t\t\t# coverage even if they are all at the tip of some reference.\n+\t\t\ttest_commit_bulk --message=\"%s\" 103 &&\n+\t\t\tgit log --format=\"create refs/tags/%s/tag %H\" HEAD >refs &&\n+\t\t\tgit update-ref --stdin commits-with-bitmap &&\n+\n+\t\t\t# Verify that we have at least one commit that did not\n+\t\t\t# receive a bitmap.\n+\t\t\tgit rev-list HEAD >commits.raw &&\n+\t\t\tsort commits &&\n+\t\t\tcomm -13 commits-with-bitmap commits >commits-wo-bitmap &&\n+\t\t\ttest_file_not_empty commits-wo-bitmap &&\n+\t\t\tcommit_id=$(head commits-wo-bitmap) &&\n+\t\t\tref_without_bitmap=$(git for-each-ref --points-at=\"$commit_id\" --format=\"%(refname)\") &&\n+\n+\t\t\t# When passing the full refname we do not expect a\n+\t\t\t# bitmap to be generated, as it should be interpreted\n+\t\t\t# as if a slash was appended to the pattern.\n+\t\t\tgit -c pack.preferBitmapTips=\"$ref_without_bitmap\" repack -adb &&\n+\t\t\ttest-tool bitmap list-commits >after &&\n+\t\t\ttest_grep ! \"$commit_id\" after &&\n+\n+\t\t\t# But if we pass the parent directory of the ref we\n+\t\t\t# should see a bitmap.\n+\t\t\tref_namespace=$(dirname \"$ref_without_bitmap\") &&\n+\t\t\tgit -c pack.preferBitmapTips=\"$ref_namespace\" repack -adb &&\n+\t\t\ttest-tool bitmap list-commits >after &&\n+\t\t\ttest_grep \"$commit_id\" after\n+\t\t)\n+\t'\n+\n \ttest_expect_success 'complains about multiple pack bitmaps' '\n \t\trm -fr repo &&\n \t\tgit init repo &&\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex faae98c7e7..449353416f 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -1345,4 +1345,47 @@ test_expect_success 'bitmapped packs are stored via the BTMP chunk' '\n \t)\n '\n \n+test_expect_success 'pack.preferBitmapTips interprets patterns as hierarchy' '\n+\tgit init repo &&\n+\ttest_when_finished \"rm -fr repo\" &&\n+\t(\n+\t\tcd repo &&\n+\n+\t\t# Create enough commits that not all will receive bitmap\n+\t\t# coverage even if they are all at the tip of some reference.\n+\t\ttest_commit_bulk --message=\"%s\" 103 &&\n+\t\tgit log --format=\"create refs/tags/%s %H\" HEAD >refs &&\n+\t\tgit update-ref --stdin commits-with-bitmap &&\n+\n+\t\t# Verify that we have at least one commit that did not\n+\t\t# receive a bitmap.\n+\t\tgit rev-list HEAD >commits.raw &&\n+\t\tsort commits &&\n+\t\tcomm -13 commits-with-bitmap commits >commits-wo-bitmap &&\n+\t\ttest_file_not_empty commits-wo-bitmap &&\n+\t\tcommit_id=$(head commits-wo-bitmap) &&\n+\t\tref_without_bitmap=$(git for-each-ref --points-at=\"$commit_id\" --format=\"%(refname)\") &&\n+\n+\t\t# When passing the full refname we do not expect a bitmap to be\n+\t\t# generated, as it should be interpreted as if a slash was\n+\t\t# appended to the pattern.\n+\t\trm .git/objects/pack/multi-pack-index* &&\n+\t\tgit -c pack.preferBitmapTips=\"$ref_without_bitmap\" repack -adb --write-midx &&\n+\t\ttest-tool bitmap list-commits >after &&\n+\t\ttest_grep ! \"$commit_id\" after &&\n+\n+\t\t# But if we pass the parent directory of the ref we should see\n+\t\t# a bitmap.\n+\t\tref_namespace=$(dirname \"$ref_without_bitmap\") &&\n+\t\trm .git/objects/pack/multi-pack-index* &&\n+\t\tgit -c pack.preferBitmapTips=\"$ref_namespace\" repack -adb --write-midx &&\n+\t\ttest-tool bitmap list-commits >after &&\n+\t\ttest_grep \"$commit_id\" after\n+\t)\n+'\n+\n test_done\n\n-- \n2.53.0.239.g8d8fc8a987.dirty\n\n"},{"id":"535318","messageId":"20260206-b4-pks-fix-for-each-ref-in-misuse-v3-3-1e050c3d6a50@pks.im","threadId":"64879","inReplyTo":"20260206-b4-pks-fix-for-each-ref-in-misuse-v3-0-1e050c3d6a50@pks.im","subject":"[PATCH v3 3/4] bisect: fix misuse of `refs_for_each_ref_in()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-02-06T07:49:58Z","receivedAt":"2026-02-06T07:50:12Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"All callers of `refs_for_each_ref_in()` pass in a string that is\nterminated with a trailing slash to indicate that they only want to see\nrefs in that specific ref hierarchy. This is in fact a requirement if\none wants to use this function, as the function trims the prefix from\neach yielded ref. So if there was a reference that was called\n\"refs/bisect\" as in our example, the result after trimming would be the\nempty string, and that's something we disallow.\n\nFix this by adding the trailing slash.\n\nFurthermore, taking a closer look, we strip the prefix only to re-add it\nin `mark_for_removal()`. This is somewhat roundabout, as we can instead\ncall `refs_for_each_fullref_in()` to not do any stripping at all. Do so\nto simplify the code a bit.\n\nSigned-off-by: Patrick Steinhardt \n---\n bisect.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex 326b59c0dc..4f0d1a1853 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -1180,7 +1180,7 @@ int estimate_bisect_steps(int all)\n static int mark_for_removal(const struct reference *ref, void *cb_data)\n {\n \tstruct string_list *refs = cb_data;\n-\tchar *bisect_ref = xstrfmt(\"refs/bisect%s\", ref->name);\n+\tchar *bisect_ref = xstrdup(ref->name);\n \tstring_list_append(refs, bisect_ref);\n \treturn 0;\n }\n@@ -1191,9 +1191,9 @@ int bisect_clean_state(void)\n \n \t/* There may be some refs packed during bisection */\n \tstruct string_list refs_for_removal = STRING_LIST_INIT_NODUP;\n-\trefs_for_each_ref_in(get_main_ref_store(the_repository),\n-\t\t\t \"refs/bisect\", mark_for_removal,\n-\t\t\t (void *) &refs_for_removal);\n+\trefs_for_each_fullref_in(get_main_ref_store(the_repository),\n+\t\t\t\t \"refs/bisect/\", NULL, mark_for_removal,\n+\t\t\t\t &refs_for_removal);\n \tstring_list_append(&refs_for_removal, xstrdup(\"BISECT_HEAD\"));\n \tstring_list_append(&refs_for_removal, xstrdup(\"BISECT_EXPECTED_REV\"));\n \tresult = refs_delete_refs(get_main_ref_store(the_repository),\n\n-- \n2.53.0.239.g8d8fc8a987.dirty\n\n"},{"id":"535319","messageId":"20260206-b4-pks-fix-for-each-ref-in-misuse-v3-4-1e050c3d6a50@pks.im","threadId":"64879","inReplyTo":"20260206-b4-pks-fix-for-each-ref-in-misuse-v3-0-1e050c3d6a50@pks.im","subject":"[PATCH v3 4/4] bisect: simplify string_list memory handling","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-02-06T07:49:59Z","receivedAt":"2026-02-06T07:50:15Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"From: Jeff King \n\nWe declare the refs_for_removal string_list as NODUP, forcing us to\nmanually allocate strings we insert. And then when it comes time to\nclean up, we set strdup_strings so that string_list_clear() will free\nthem for us.\n\nThis is a confusing pattern, and can be done much more simply by just\ndeclaring the list with the DUP initializer in the first place.\n\nIt was written this way originally because one of the callsites\ngenerated the item using xstrfmt(). But that spot switched to a plain\nxstrdup() in the preceding commit. That means we can now just let the\nstring_list code handle allocation itself.\n\nSigned-off-by: Jeff King \nSigned-off-by: Patrick Steinhardt \n---\n bisect.c | 10 ++++------\n 1 file changed, 4 insertions(+), 6 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex 4f0d1a1853..268f5e36f8 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -1180,8 +1180,7 @@ int estimate_bisect_steps(int all)\n static int mark_for_removal(const struct reference *ref, void *cb_data)\n {\n \tstruct string_list *refs = cb_data;\n-\tchar *bisect_ref = xstrdup(ref->name);\n-\tstring_list_append(refs, bisect_ref);\n+\tstring_list_append(refs, ref->name);\n \treturn 0;\n }\n \n@@ -1190,16 +1189,15 @@ int bisect_clean_state(void)\n \tint result = 0;\n \n \t/* There may be some refs packed during bisection */\n-\tstruct string_list refs_for_removal = STRING_LIST_INIT_NODUP;\n+\tstruct string_list refs_for_removal = STRING_LIST_INIT_DUP;\n \trefs_for_each_fullref_in(get_main_ref_store(the_repository),\n \t\t\t\t \"refs/bisect/\", NULL, mark_for_removal,\n \t\t\t\t &refs_for_removal);\n-\tstring_list_append(&refs_for_removal, xstrdup(\"BISECT_HEAD\"));\n-\tstring_list_append(&refs_for_removal, xstrdup(\"BISECT_EXPECTED_REV\"));\n+\tstring_list_append(&refs_for_removal, \"BISECT_HEAD\");\n+\tstring_list_append(&refs_for_removal, \"BISECT_EXPECTED_REV\");\n \tresult = refs_delete_refs(get_main_ref_store(the_repository),\n \t\t\t\t \"bisect: remove\", &refs_for_removal,\n \t\t\t\t REF_NO_DEREF);\n-\trefs_for_removal.strdup_strings = 1;\n \tstring_list_clear(&refs_for_removal, 0);\n \tunlink_or_warn(git_path_bisect_ancestors_ok());\n \tunlink_or_warn(git_path_bisect_log());\n\n-- \n2.53.0.239.g8d8fc8a987.dirty\n\n"},{"id":"535370","messageId":"xmqq343dhjt3.fsf@gitster.g","threadId":"64879","inReplyTo":"20260206-b4-pks-fix-for-each-ref-in-misuse-v3-2-1e050c3d6a50@pks.im","subject":"Re: [PATCH v3 2/4] pack-bitmap: fix bug with exact ref match in \"pack.preferBitmapTips\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-06T17:18:48Z","receivedAt":"2026-02-06T17:18:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> @@ -3328,15 +3328,26 @@ void for_each_preferred_bitmap_tip(struct repository *repo,\n> {\n> \tstruct string_list_item *item;\n> \tconst struct string_list *preferred_tips;\n> +\tstruct strbuf buf = STRBUF_INIT;\n> \n> \tpreferred_tips = bitmap_preferred_tips(repo);\n> \tif (!preferred_tips)\n> \t\treturn;\n> \n> \tfor_each_string_list_item(item, preferred_tips) {\n> +\t\tconst char *pattern = item->string;\n> +\n> +\t\tif (!ends_with(pattern, \"/\")) {\n> +\t\t\tstrbuf_reset(&buf);\n> +\t\t\tstrbuf_addf(&buf, \"%s/\", pattern);\n> +\t\t\tpattern = buf.buf;\n> +\t\t}\n> +\n\nI briefly wondered if a possible alternative solution is to update\nbitmap_preferred_tips() that reads the configuration so that it\ngives the callers a string-list with \"corrected\" strings. If it can\nhave many other callers, such an approach would force everybody to\nadopt the same worldview, i.e., the configuration is meant to specify\nhierarchy prefixes and never individual refs.\n\nBut when I noticed that nobody bothers to release resources held by\npreferred_Tips, I realized that the above implementation is far\nsimpler and much better. This iterator is the only caller of the\nfile-scope static bitmap_preferred_tips() helper, and what the\nhelper returns is a borrowed piece of string_list that is owned by\nthe config subsystem, so such a change will force us to make a copy\nand then release the copy in the caller.\n\nSo, I think the above change is much better than such an alternative.\n\nThanks.\n"},{"id":"536319","messageId":"aZYGrmktI6vwp8Ow@nand.local","threadId":"64879","inReplyTo":"20260206-b4-pks-fix-for-each-ref-in-misuse-v3-0-1e050c3d6a50@pks.im","subject":"Re: [PATCH v3 0/4] Fix misuse of `refs_for_each_ref_in()`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-02-18T18:36:30Z","receivedAt":"2026-02-18T18:36:32Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Hi Patrick,\n\nOn Fri, Feb 06, 2026 at 08:49:55AM +0100, Patrick Steinhardt wrote:\n> Jeff King (1):\n> bisect: simplify string_list memory handling\n>\n> Patrick Steinhardt (3):\n> pack-bitmap: deduplicate logic to iterate over preferred bitmap tips\n> pack-bitmap: fix bug with exact ref match in \"pack.preferBitmapTips\"\n> bisect: fix misuse of `refs_for_each_ref_in()`\n\nThanks, this version looks good to me.\n\nThanks,\nTaylor\n"},{"id":"536377","messageId":"20260219-b4-pks-fix-for-each-ref-in-misuse-v4-0-57ac30172fae@pks.im","threadId":"64879","inReplyTo":"20260128-b4-pks-fix-for-each-ref-in-misuse-v1-0-deccae3ea725@pks.im","subject":"[PATCH v4 0/4] Fix misuse of `refs_for_each_ref_in()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-02-19T07:57:48Z","receivedAt":"2026-02-19T07:58:01Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis small patch series fixes a bug I have discovered where configuring\n\"pack.preferBitmapTips\" to an exact branch will cause Git to `BUG()`.\n\nThe root cause of this bug is misuse of `refs_for_each_ref_in()`: this\nfunction accepts a prefix to yield refs for, and then strips the prefix\nfor each ref. Consequently, if passed an exact refname, then stripping\nthe prefix would make us end up with an empty refname, and that is not\nsupposed to happen.\n\nThere was one other caller that got it wrong, too, and which is also\nfixed in this patch series.\n\nChanges in v4:\n - Fix a typo in the documentation.\n - Link to v3: https://lore.kernel.org/r/20260206-b4-pks-fix-for-each-ref-in-misuse-v3-0-1e050c3d6a50@pks.im\n\nChanges in v3:\n - Switch the approach to perform ref hierarchy matches instead, which\n is in line with the changes in 10e8a9352b (refs.c: stop matching\n non-directory prefixes in exclude patterns, 2025-03-06).\n - Link to v2: https://lore.kernel.org/r/20260130-b4-pks-fix-for-each-ref-in-misuse-v2-0-0449b198a681@pks.im\n\nChanges in v2:\n - Explain my thought process against why I chose to also allow exact\n ref matches in the second commit and clarify the documentation a\n bit. As said, I'm very open to changing this if my spelled-out\n thoughts are not convincing.\n - Apply Peff's patch to further simplify code.\n - Link to v1: https://lore.kernel.org/r/20260128-b4-pks-fix-for-each-ref-in-misuse-v1-0-deccae3ea725@pks.im\n\nThanks!\n\nPatrick\n\n---\nJeff King (1):\n bisect: simplify string_list memory handling\n\nPatrick Steinhardt (3):\n pack-bitmap: deduplicate logic to iterate over preferred bitmap tips\n pack-bitmap: fix bug with exact ref match in \"pack.preferBitmapTips\"\n bisect: fix misuse of `refs_for_each_ref_in()`\n\n Documentation/config/pack.adoc | 9 +++++----\n bisect.c | 16 +++++++---------\n builtin/pack-objects.c | 19 ++-----------------\n pack-bitmap.c | 29 +++++++++++++++++++++++++++-\n pack-bitmap.h | 9 ++++++++-\n repack-midx.c | 14 +++-----------\n t/t5310-pack-bitmaps.sh | 41 ++++++++++++++++++++++++++++++++++++++++\n t/t5319-multi-pack-index.sh | 43 ++++++++++++++++++++++++++++++++++++++++++\n 8 files changed, 137 insertions(+), 43 deletions(-)\n\nRange-diff versus v3:\n\n1: aa71011718 = 1: 0c2a2a122e pack-bitmap: deduplicate logic to iterate over preferred bitmap tips\n2: d15aa04be6 ! 2: a923e92ddd pack-bitmap: fix bug with exact ref match in \"pack.preferBitmapTips\"\n @@ Documentation/config/pack.adoc: pack.usePathWalk::\n -\tcommit at the tip of any reference that is a suffix of any value\n -\tof this configuration over any other commits in the \"selection\n -\twindow\".\n -+\tcommmit at the tip of a reference that is contained in any of\n ++\tcommit at the tip of a reference that is contained in any of\n +\tthe configured hierarchies.\n +\n -Note that setting this configuration to `refs/foo` does not mean that\n3: 2f1b73dd08 = 3: 7e1dbf1b4e bisect: fix misuse of `refs_for_each_ref_in()`\n4: f579e31633 = 4: 094aa2345c bisect: simplify string_list memory handling\n\n---\nbase-commit: ea717645d199f6f1b66058886475db3e8c9330e9\nchange-id: 20260128-b4-pks-fix-for-each-ref-in-misuse-96ae62e313a5\n\n"},{"id":"536378","messageId":"20260219-b4-pks-fix-for-each-ref-in-misuse-v4-1-57ac30172fae@pks.im","threadId":"64879","inReplyTo":"20260219-b4-pks-fix-for-each-ref-in-misuse-v4-0-57ac30172fae@pks.im","subject":"[PATCH v4 1/4] pack-bitmap: deduplicate logic to iterate over preferred bitmap tips","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-02-19T07:57:49Z","receivedAt":"2026-02-19T07:58:03Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We have two locations that iterate over the preferred bitmap tips as\nconfigured by the user via \"pack.preferBitmapTips\". Both of these\ncallsites are subtly wrong: when the preferred bitmap tips contain an\nexact refname match, then we will hit a `BUG()`.\n\nPrepare for the fix by unifying the two callsites into a new\n`for_each_preferred_bitmap_tip()` function.\n\nThis removes the last callsite of `bitmap_preferred_tips()` outside of\n\"pack-bitmap.c\". As such, convert the function to be local to that file\nonly. Note that the function is still used by a second caller, so we\ncannot just inline it.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/pack-objects.c | 19 ++-----------------\n pack-bitmap.c | 18 +++++++++++++++++-\n pack-bitmap.h | 9 ++++++++-\n repack-midx.c | 14 +++-----------\n 4 files changed, 30 insertions(+), 30 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 5846b6a293..979470e402 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -4554,22 +4554,6 @@ static int mark_bitmap_preferred_tip(const struct reference *ref, void *data UNU\n \treturn 0;\n }\n \n-static void mark_bitmap_preferred_tips(void)\n-{\n-\tstruct string_list_item *item;\n-\tconst struct string_list *preferred_tips;\n-\n-\tpreferred_tips = bitmap_preferred_tips(the_repository);\n-\tif (!preferred_tips)\n-\t\treturn;\n-\n-\tfor_each_string_list_item(item, preferred_tips) {\n-\t\trefs_for_each_ref_in(get_main_ref_store(the_repository),\n-\t\t\t\t item->string, mark_bitmap_preferred_tip,\n-\t\t\t\t NULL);\n-\t}\n-}\n-\n static inline int is_oid_uninteresting(struct repository *repo,\n \t\t\t\t struct object_id *oid)\n {\n@@ -4710,7 +4694,8 @@ static void get_object_list(struct rev_info *revs, struct strvec *argv)\n \t\tload_delta_islands(the_repository, progress);\n \n \tif (write_bitmap_index)\n-\t\tmark_bitmap_preferred_tips();\n+\t\tfor_each_preferred_bitmap_tip(the_repository, mark_bitmap_preferred_tip,\n+\t\t\t\t\t NULL);\n \n \tif (!fn_show_object)\n \t\tfn_show_object = show_object;\ndiff --git a/pack-bitmap.c b/pack-bitmap.c\nindex 972203f12b..2f5cb34009 100644\n--- a/pack-bitmap.c\n+++ b/pack-bitmap.c\n@@ -3314,7 +3314,7 @@ int bitmap_is_midx(struct bitmap_index *bitmap_git)\n \treturn !!bitmap_git->midx;\n }\n \n-const struct string_list *bitmap_preferred_tips(struct repository *r)\n+static const struct string_list *bitmap_preferred_tips(struct repository *r)\n {\n \tconst struct string_list *dest;\n \n@@ -3323,6 +3323,22 @@ const struct string_list *bitmap_preferred_tips(struct repository *r)\n \treturn NULL;\n }\n \n+void for_each_preferred_bitmap_tip(struct repository *repo,\n+\t\t\t\t each_ref_fn cb, void *cb_data)\n+{\n+\tstruct string_list_item *item;\n+\tconst struct string_list *preferred_tips;\n+\n+\tpreferred_tips = bitmap_preferred_tips(repo);\n+\tif (!preferred_tips)\n+\t\treturn;\n+\n+\tfor_each_string_list_item(item, preferred_tips) {\n+\t\trefs_for_each_ref_in(get_main_ref_store(repo),\n+\t\t\t\t item->string, cb, cb_data);\n+\t}\n+}\n+\n int bitmap_is_preferred_refname(struct repository *r, const char *refname)\n {\n \tconst struct string_list *preferred_tips = bitmap_preferred_tips(r);\ndiff --git a/pack-bitmap.h b/pack-bitmap.h\nindex 1bd7a791e2..d0611d0481 100644\n--- a/pack-bitmap.h\n+++ b/pack-bitmap.h\n@@ -5,6 +5,7 @@\n #include \"khash.h\"\n #include \"pack.h\"\n #include \"pack-objects.h\"\n+#include \"refs.h\"\n #include \"string-list.h\"\n \n struct commit;\n@@ -99,6 +100,13 @@ int for_each_bitmapped_object(struct bitmap_index *bitmap_git,\n \t\t\t show_reachable_fn show_reach,\n \t\t\t void *payload);\n \n+/*\n+ * Iterate over all references that are configured as preferred bitmap tips via\n+ * \"pack.preferBitmapTips\" and invoke the callback on each function.\n+ */\n+void for_each_preferred_bitmap_tip(struct repository *repo,\n+\t\t\t\t each_ref_fn cb, void *cb_data);\n+\n #define GIT_TEST_PACK_USE_BITMAP_BOUNDARY_TRAVERSAL \\\n \t\"GIT_TEST_PACK_USE_BITMAP_BOUNDARY_TRAVERSAL\"\n \n@@ -182,7 +190,6 @@ char *pack_bitmap_filename(struct packed_git *p);\n \n int bitmap_is_midx(struct bitmap_index *bitmap_git);\n \n-const struct string_list *bitmap_preferred_tips(struct repository *r);\n int bitmap_is_preferred_refname(struct repository *r, const char *refname);\n \n int verify_bitmap_files(struct repository *r);\ndiff --git a/repack-midx.c b/repack-midx.c\nindex 74bdfa3a6e..0682b80c42 100644\n--- a/repack-midx.c\n+++ b/repack-midx.c\n@@ -40,7 +40,6 @@ static int midx_snapshot_ref_one(const struct reference *ref, void *_data)\n void midx_snapshot_refs(struct repository *repo, struct tempfile *f)\n {\n \tstruct midx_snapshot_ref_data data;\n-\tconst struct string_list *preferred = bitmap_preferred_tips(repo);\n \n \tdata.repo = repo;\n \tdata.f = f;\n@@ -51,16 +50,9 @@ void midx_snapshot_refs(struct repository *repo, struct tempfile *f)\n \t\t die(_(\"could not open tempfile %s for writing\"),\n \t\t get_tempfile_path(f));\n \n-\tif (preferred) {\n-\t\tstruct string_list_item *item;\n-\n-\t\tdata.preferred = 1;\n-\t\tfor_each_string_list_item(item, preferred)\n-\t\t\trefs_for_each_ref_in(get_main_ref_store(repo),\n-\t\t\t\t\t item->string,\n-\t\t\t\t\t midx_snapshot_ref_one, &data);\n-\t\tdata.preferred = 0;\n-\t}\n+\tdata.preferred = 1;\n+\tfor_each_preferred_bitmap_tip(repo, midx_snapshot_ref_one, &data);\n+\tdata.preferred = 0;\n \n \trefs_for_each_ref(get_main_ref_store(repo),\n \t\t\t midx_snapshot_ref_one, &data);\n\n-- \n2.53.0.414.gf7e9f6c205.dirty\n\n"},{"id":"536379","messageId":"20260219-b4-pks-fix-for-each-ref-in-misuse-v4-2-57ac30172fae@pks.im","threadId":"64879","inReplyTo":"20260219-b4-pks-fix-for-each-ref-in-misuse-v4-0-57ac30172fae@pks.im","subject":"[PATCH v4 2/4] pack-bitmap: fix bug with exact ref match in \"pack.preferBitmapTips\"","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-02-19T07:57:50Z","receivedAt":"2026-02-19T08:13:17Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The \"pack.preferBitmapTips\" configuration allows the user to specify\nwhich references should be preferred when generating bitmaps. This\noption is typically expected to be set to a reference prefix, like for\nexample \"refs/heads/\".\n\nIt's not unreasonable though for a user to configure one specific\nreference as preferred. But if they do, they'll hit a `BUG()`:\n\n $ git -c pack.preferBitmapTips=refs/heads/main repack -adb\n BUG: ../refs/iterator.c:366: attempt to trim too many characters\n error: pack-objects died of signal 6\n\nThe root cause for this bug is how we enumerate these references. We\ncall `refs_for_each_ref_in()`, which will:\n\n - Yield all references that have a user-specified prefix.\n\n - Trim each of these references so that the prefix is removed.\n\nTypically, this function is called with a trailing slash, like\n\"refs/heads/\", and in that case things work alright. But if the function\nis called with the name of an existing reference then we'll try to trim\nthe full reference name, which would leave us with an empty name. And as\nthis would not really leave us with anything sensible, we call `BUG()`\ninstead of yielding this reference.\n\nOne could argue that this is a bug in `refs_for_each_ref_in()`. But the\nquestion then becomes what the correct behaviour would be:\n\n - Do we want to skip exact matches? In our case we certainly don't\n want that, as the user has asked us to generate a bitmap for it.\n\n - Do we want to yield the reference with the empty refname? That would\n lead to a somewhat weird result.\n\nNeither of these feel like viable options, so calling `BUG()` feels like\na sensible way out. The root cause ultimately is that we even try to\ntrim the whole refname in the first place. There are two possible ways\nto fix this issue:\n\n - We can fix the bug by using `refs_for_each_fullref_in()` instead,\n which does not strip the prefix at all. Consequently, we would now\n start to accept all references that start with the configured\n prefix, including exact matches. So if we had \"refs/heads/main\", we\n would both match \"refs/heads/main\" and \"refs/heads/main-branch\".\n\n - Or we can fix the bug by appending a slash to the prefix if it\n doesn't already have one. This would mean that we only match\n ref hierarchies that start with this prefix.\n\nWhile the first fix leaves the user with strictly _more_ configuration\noptions, we have already fixed a similar case in 10e8a9352b (refs.c:\nstop matching non-directory prefixes in exclude patterns, 2025-03-06) by\nusing the second option. So for the sake of consistency, let's apply the\nsame fix here.\n\nClarify the documentation accordingly.\n\nSigned-off-by: Patrick Steinhardt \n---\n Documentation/config/pack.adoc | 9 +++++----\n pack-bitmap.c | 13 ++++++++++++-\n t/t5310-pack-bitmaps.sh | 41 ++++++++++++++++++++++++++++++++++++++++\n t/t5319-multi-pack-index.sh | 43 ++++++++++++++++++++++++++++++++++++++++++\n 4 files changed, 101 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/config/pack.adoc b/Documentation/config/pack.adoc\nindex 75402d5579..fa997c8597 100644\n--- a/Documentation/config/pack.adoc\n+++ b/Documentation/config/pack.adoc\n@@ -160,12 +160,13 @@ pack.usePathWalk::\n \tprocesses. See linkgit:git-pack-objects[1] for full details.\n \n pack.preferBitmapTips::\n+\tSpecifies a ref hierarchy (e.g., \"refs/heads/\"); can be\n+\tgiven multiple times to specify more than one hierarchies.\n \tWhen selecting which commits will receive bitmaps, prefer a\n-\tcommit at the tip of any reference that is a suffix of any value\n-\tof this configuration over any other commits in the \"selection\n-\twindow\".\n+\tcommit at the tip of a reference that is contained in any of\n+\tthe configured hierarchies.\n +\n-Note that setting this configuration to `refs/foo` does not mean that\n+Note that setting this configuration to `refs/foo/` does not mean that\n the commits at the tips of `refs/foo/bar` and `refs/foo/baz` will\n necessarily be selected. This is because commits are selected for\n bitmaps from within a series of windows of variable length.\ndiff --git a/pack-bitmap.c b/pack-bitmap.c\nindex 2f5cb34009..1c93871484 100644\n--- a/pack-bitmap.c\n+++ b/pack-bitmap.c\n@@ -3328,15 +3328,26 @@ void for_each_preferred_bitmap_tip(struct repository *repo,\n {\n \tstruct string_list_item *item;\n \tconst struct string_list *preferred_tips;\n+\tstruct strbuf buf = STRBUF_INIT;\n \n \tpreferred_tips = bitmap_preferred_tips(repo);\n \tif (!preferred_tips)\n \t\treturn;\n \n \tfor_each_string_list_item(item, preferred_tips) {\n+\t\tconst char *pattern = item->string;\n+\n+\t\tif (!ends_with(pattern, \"/\")) {\n+\t\t\tstrbuf_reset(&buf);\n+\t\t\tstrbuf_addf(&buf, \"%s/\", pattern);\n+\t\t\tpattern = buf.buf;\n+\t\t}\n+\n \t\trefs_for_each_ref_in(get_main_ref_store(repo),\n-\t\t\t\t item->string, cb, cb_data);\n+\t\t\t\t pattern, cb, cb_data);\n \t}\n+\n+\tstrbuf_release(&buf);\n }\n \n int bitmap_is_preferred_refname(struct repository *r, const char *refname)\ndiff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh\nindex 6718fb98c0..310b708c5c 100755\n--- a/t/t5310-pack-bitmaps.sh\n+++ b/t/t5310-pack-bitmaps.sh\n@@ -466,6 +466,47 @@ test_bitmap_cases () {\n \t\t)\n \t'\n \n+\ttest_expect_success 'pack.preferBitmapTips interprets patterns as hierarchy' '\n+\t\tgit init repo &&\n+\t\ttest_when_finished \"rm -fr repo\" &&\n+\t\t(\n+\t\t\tcd repo &&\n+\n+\t\t\t# Create enough commits that not all will receive bitmap\n+\t\t\t# coverage even if they are all at the tip of some reference.\n+\t\t\ttest_commit_bulk --message=\"%s\" 103 &&\n+\t\t\tgit log --format=\"create refs/tags/%s/tag %H\" HEAD >refs &&\n+\t\t\tgit update-ref --stdin commits-with-bitmap &&\n+\n+\t\t\t# Verify that we have at least one commit that did not\n+\t\t\t# receive a bitmap.\n+\t\t\tgit rev-list HEAD >commits.raw &&\n+\t\t\tsort commits &&\n+\t\t\tcomm -13 commits-with-bitmap commits >commits-wo-bitmap &&\n+\t\t\ttest_file_not_empty commits-wo-bitmap &&\n+\t\t\tcommit_id=$(head commits-wo-bitmap) &&\n+\t\t\tref_without_bitmap=$(git for-each-ref --points-at=\"$commit_id\" --format=\"%(refname)\") &&\n+\n+\t\t\t# When passing the full refname we do not expect a\n+\t\t\t# bitmap to be generated, as it should be interpreted\n+\t\t\t# as if a slash was appended to the pattern.\n+\t\t\tgit -c pack.preferBitmapTips=\"$ref_without_bitmap\" repack -adb &&\n+\t\t\ttest-tool bitmap list-commits >after &&\n+\t\t\ttest_grep ! \"$commit_id\" after &&\n+\n+\t\t\t# But if we pass the parent directory of the ref we\n+\t\t\t# should see a bitmap.\n+\t\t\tref_namespace=$(dirname \"$ref_without_bitmap\") &&\n+\t\t\tgit -c pack.preferBitmapTips=\"$ref_namespace\" repack -adb &&\n+\t\t\ttest-tool bitmap list-commits >after &&\n+\t\t\ttest_grep \"$commit_id\" after\n+\t\t)\n+\t'\n+\n \ttest_expect_success 'complains about multiple pack bitmaps' '\n \t\trm -fr repo &&\n \t\tgit init repo &&\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex faae98c7e7..449353416f 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -1345,4 +1345,47 @@ test_expect_success 'bitmapped packs are stored via the BTMP chunk' '\n \t)\n '\n \n+test_expect_success 'pack.preferBitmapTips interprets patterns as hierarchy' '\n+\tgit init repo &&\n+\ttest_when_finished \"rm -fr repo\" &&\n+\t(\n+\t\tcd repo &&\n+\n+\t\t# Create enough commits that not all will receive bitmap\n+\t\t# coverage even if they are all at the tip of some reference.\n+\t\ttest_commit_bulk --message=\"%s\" 103 &&\n+\t\tgit log --format=\"create refs/tags/%s %H\" HEAD >refs &&\n+\t\tgit update-ref --stdin commits-with-bitmap &&\n+\n+\t\t# Verify that we have at least one commit that did not\n+\t\t# receive a bitmap.\n+\t\tgit rev-list HEAD >commits.raw &&\n+\t\tsort commits &&\n+\t\tcomm -13 commits-with-bitmap commits >commits-wo-bitmap &&\n+\t\ttest_file_not_empty commits-wo-bitmap &&\n+\t\tcommit_id=$(head commits-wo-bitmap) &&\n+\t\tref_without_bitmap=$(git for-each-ref --points-at=\"$commit_id\" --format=\"%(refname)\") &&\n+\n+\t\t# When passing the full refname we do not expect a bitmap to be\n+\t\t# generated, as it should be interpreted as if a slash was\n+\t\t# appended to the pattern.\n+\t\trm .git/objects/pack/multi-pack-index* &&\n+\t\tgit -c pack.preferBitmapTips=\"$ref_without_bitmap\" repack -adb --write-midx &&\n+\t\ttest-tool bitmap list-commits >after &&\n+\t\ttest_grep ! \"$commit_id\" after &&\n+\n+\t\t# But if we pass the parent directory of the ref we should see\n+\t\t# a bitmap.\n+\t\tref_namespace=$(dirname \"$ref_without_bitmap\") &&\n+\t\trm .git/objects/pack/multi-pack-index* &&\n+\t\tgit -c pack.preferBitmapTips=\"$ref_namespace\" repack -adb --write-midx &&\n+\t\ttest-tool bitmap list-commits >after &&\n+\t\ttest_grep \"$commit_id\" after\n+\t)\n+'\n+\n test_done\n\n-- \n2.53.0.414.gf7e9f6c205.dirty\n\n"},{"id":"536380","messageId":"20260219-b4-pks-fix-for-each-ref-in-misuse-v4-3-57ac30172fae@pks.im","threadId":"64879","inReplyTo":"20260219-b4-pks-fix-for-each-ref-in-misuse-v4-0-57ac30172fae@pks.im","subject":"[PATCH v4 3/4] bisect: fix misuse of `refs_for_each_ref_in()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-02-19T07:57:51Z","receivedAt":"2026-02-19T08:13:18Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"All callers of `refs_for_each_ref_in()` pass in a string that is\nterminated with a trailing slash to indicate that they only want to see\nrefs in that specific ref hierarchy. This is in fact a requirement if\none wants to use this function, as the function trims the prefix from\neach yielded ref. So if there was a reference that was called\n\"refs/bisect\" as in our example, the result after trimming would be the\nempty string, and that's something we disallow.\n\nFix this by adding the trailing slash.\n\nFurthermore, taking a closer look, we strip the prefix only to re-add it\nin `mark_for_removal()`. This is somewhat roundabout, as we can instead\ncall `refs_for_each_fullref_in()` to not do any stripping at all. Do so\nto simplify the code a bit.\n\nSigned-off-by: Patrick Steinhardt \n---\n bisect.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex 326b59c0dc..4f0d1a1853 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -1180,7 +1180,7 @@ int estimate_bisect_steps(int all)\n static int mark_for_removal(const struct reference *ref, void *cb_data)\n {\n \tstruct string_list *refs = cb_data;\n-\tchar *bisect_ref = xstrfmt(\"refs/bisect%s\", ref->name);\n+\tchar *bisect_ref = xstrdup(ref->name);\n \tstring_list_append(refs, bisect_ref);\n \treturn 0;\n }\n@@ -1191,9 +1191,9 @@ int bisect_clean_state(void)\n \n \t/* There may be some refs packed during bisection */\n \tstruct string_list refs_for_removal = STRING_LIST_INIT_NODUP;\n-\trefs_for_each_ref_in(get_main_ref_store(the_repository),\n-\t\t\t \"refs/bisect\", mark_for_removal,\n-\t\t\t (void *) &refs_for_removal);\n+\trefs_for_each_fullref_in(get_main_ref_store(the_repository),\n+\t\t\t\t \"refs/bisect/\", NULL, mark_for_removal,\n+\t\t\t\t &refs_for_removal);\n \tstring_list_append(&refs_for_removal, xstrdup(\"BISECT_HEAD\"));\n \tstring_list_append(&refs_for_removal, xstrdup(\"BISECT_EXPECTED_REV\"));\n \tresult = refs_delete_refs(get_main_ref_store(the_repository),\n\n-- \n2.53.0.414.gf7e9f6c205.dirty\n\n"},{"id":"536381","messageId":"20260219-b4-pks-fix-for-each-ref-in-misuse-v4-4-57ac30172fae@pks.im","threadId":"64879","inReplyTo":"20260219-b4-pks-fix-for-each-ref-in-misuse-v4-0-57ac30172fae@pks.im","subject":"[PATCH v4 4/4] bisect: simplify string_list memory handling","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-02-19T07:57:52Z","receivedAt":"2026-02-19T08:13:22Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"From: Jeff King \n\nWe declare the refs_for_removal string_list as NODUP, forcing us to\nmanually allocate strings we insert. And then when it comes time to\nclean up, we set strdup_strings so that string_list_clear() will free\nthem for us.\n\nThis is a confusing pattern, and can be done much more simply by just\ndeclaring the list with the DUP initializer in the first place.\n\nIt was written this way originally because one of the callsites\ngenerated the item using xstrfmt(). But that spot switched to a plain\nxstrdup() in the preceding commit. That means we can now just let the\nstring_list code handle allocation itself.\n\nSigned-off-by: Jeff King \nSigned-off-by: Patrick Steinhardt \n---\n bisect.c | 10 ++++------\n 1 file changed, 4 insertions(+), 6 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex 4f0d1a1853..268f5e36f8 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -1180,8 +1180,7 @@ int estimate_bisect_steps(int all)\n static int mark_for_removal(const struct reference *ref, void *cb_data)\n {\n \tstruct string_list *refs = cb_data;\n-\tchar *bisect_ref = xstrdup(ref->name);\n-\tstring_list_append(refs, bisect_ref);\n+\tstring_list_append(refs, ref->name);\n \treturn 0;\n }\n \n@@ -1190,16 +1189,15 @@ int bisect_clean_state(void)\n \tint result = 0;\n \n \t/* There may be some refs packed during bisection */\n-\tstruct string_list refs_for_removal = STRING_LIST_INIT_NODUP;\n+\tstruct string_list refs_for_removal = STRING_LIST_INIT_DUP;\n \trefs_for_each_fullref_in(get_main_ref_store(the_repository),\n \t\t\t\t \"refs/bisect/\", NULL, mark_for_removal,\n \t\t\t\t &refs_for_removal);\n-\tstring_list_append(&refs_for_removal, xstrdup(\"BISECT_HEAD\"));\n-\tstring_list_append(&refs_for_removal, xstrdup(\"BISECT_EXPECTED_REV\"));\n+\tstring_list_append(&refs_for_removal, \"BISECT_HEAD\");\n+\tstring_list_append(&refs_for_removal, \"BISECT_EXPECTED_REV\");\n \tresult = refs_delete_refs(get_main_ref_store(the_repository),\n \t\t\t\t \"bisect: remove\", &refs_for_removal,\n \t\t\t\t REF_NO_DEREF);\n-\trefs_for_removal.strdup_strings = 1;\n \tstring_list_clear(&refs_for_removal, 0);\n \tunlink_or_warn(git_path_bisect_ancestors_ok());\n \tunlink_or_warn(git_path_bisect_log());\n\n-- \n2.53.0.414.gf7e9f6c205.dirty\n\n"},{"id":"536412","messageId":"xmqqldgo7o72.fsf@gitster.g","threadId":"64879","inReplyTo":"aZYGrmktI6vwp8Ow@nand.local","subject":"Re: [PATCH v3 0/4] Fix misuse of `refs_for_each_ref_in()`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-19T15:22:41Z","receivedAt":"2026-02-19T15:22:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau writes:\n\n> Hi Patrick,\n>\n> On Fri, Feb 06, 2026 at 08:49:55AM +0100, Patrick Steinhardt wrote:\n>> Jeff King (1):\n>> bisect: simplify string_list memory handling\n>>\n>> Patrick Steinhardt (3):\n>> pack-bitmap: deduplicate logic to iterate over preferred bitmap tips\n>> pack-bitmap: fix bug with exact ref match in \"pack.preferBitmapTips\"\n>> bisect: fix misuse of `refs_for_each_ref_in()`\n>\n> Thanks, this version looks good to me.\n>\n> Thanks,\n> Taylor\n\nThanks, let's mark the topic for 'next' then.\n"},{"id":"537212","messageId":"xmqqwlzz2ym4.fsf@gitster.g","threadId":"64879","inReplyTo":"20260219-b4-pks-fix-for-each-ref-in-misuse-v4-0-57ac30172fae@pks.im","subject":"Re: [PATCH v4 0/4] Fix misuse of `refs_for_each_ref_in()`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-26T17:39:15Z","receivedAt":"2026-02-26T17:39:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> Changes in v4:\n> - Fix a typo in the documentation.\n> - Link to v3: https://lore.kernel.org/r/20260206-b4-pks-fix-for-each-ref-in-misuse-v3-0-1e050c3d6a50@pks.im\n>\n> Changes in v3:\n> - Switch the approach to perform ref hierarchy matches instead, which\n> is in line with the changes in 10e8a9352b (refs.c: stop matching\n> non-directory prefixes in exclude patterns, 2025-03-06).\n> - Link to v2: https://lore.kernel.org/r/20260130-b4-pks-fix-for-each-ref-in-misuse-v2-0-0449b198a681@pks.im\n\nThis round saws no activities, but I just re-read all and found them\nquite nice. Let's mark it for 'next'.\n\nThanks.\n\n"},{"id":"537213","messageId":"xmqqv7fj2yln.fsf@gitster.g","threadId":"64879","inReplyTo":"20260219-b4-pks-fix-for-each-ref-in-misuse-v4-0-57ac30172fae@pks.im","subject":"Re: [PATCH v4 0/4] Fix misuse of `refs_for_each_ref_in()`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-26T17:39:32Z","receivedAt":"2026-02-26T17:39:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> Changes in v4:\n> - Fix a typo in the documentation.\n> - Link to v3: https://lore.kernel.org/r/20260206-b4-pks-fix-for-each-ref-in-misuse-v3-0-1e050c3d6a50@pks.im\n>\n> Changes in v3:\n> - Switch the approach to perform ref hierarchy matches instead, which\n> is in line with the changes in 10e8a9352b (refs.c: stop matching\n> non-directory prefixes in exclude patterns, 2025-03-06).\n> - Link to v2: https://lore.kernel.org/r/20260130-b4-pks-fix-for-each-ref-in-misuse-v2-0-0449b198a681@pks.im\n\nThis round saw no activities, but I just re-read all four patches\nand found them quite nice. Let's mark it for 'next'.\n\nThanks.\n"}]}