{"thread":{"id":"66058","subject":"[PATCH 0/5] packfile: harden handling of packs with duplicate entries","startedAt":"2026-07-24T21:05:50Z","lastAt":"2026-07-24T21:54:20Z","messageCount":7,"participants":["Taylor Blau","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"548919","messageId":"cover.1784927134.git.ttaylorr@openai.com","threadId":"66058","inReplyTo":null,"subject":"[PATCH 0/5] packfile: harden handling of packs with duplicate entries","fromName":"Taylor Blau","fromEmail":"ttaylorr@openai.com","sentAt":"2026-07-24T21:05:44Z","receivedAt":"2026-07-24T21:05:50Z","isPatch":true,"body":"Packfiles containing duplicate object entries are unusual, but Git\nalready accepts them outside of strict indexing. Both pack indexes and\nreverse indexes maintain a 1-to-1 mapping between themselves and the\nphysical layout of objects in the pack (including duplicate).\n\nWhile testing packs containing duplicate objects with MIDXs and MIDX\nbitmaps, I found various bugs which are addressed by this series. It is\norganized as follows:\n\n- The first patch establishes that reverse indexes already do the right\n  thing: they retain duplicate .idx rows. So asking cat-file to produce\n  the on-disk size of some object with '%(objectsize:disk)' produces the\n  right answer (even in cases like asking for the on-disk size of object\n  'B' in a pack layout like [A, B, A, C]).\n\n- The second fixes a bug exposed when ordinary 'REF_DELTA' lookup\n  chooses a duplicate representation of some object that creates a\n  cycle, even when a different copy of that same object could resolve\n  the delta chain without cycles.\n\n  Importantly, we only take the more expensive path after the usual\n  lookup encounters a cycle, which should hopefully be rare.\n\nThe remaining patches deal with miscellaneous MIDX and bitmap consumers\nwhich are sensitive to packs containing duplicate objects:\n\n - MIDX verification can now accept any copy of a duplicate object\n   (keyed by its OID *only*, as opposed to an (OID, offset) pair).\n\n - The 'bitmap' test helper now cleanly die()s when trying to write a\n   bitmap for packs containing duplicate objects, since the bitmap\n   writer cannot tolerate single pack bitmaps with duplicate objects.\n\n - Finally, multi-pack reuse stops treating pseudo-pack[^1] positions as\n   physical pack positions when a pack contains duplicate entries. It\n   disables optional fast paths when their mapping cannot be proven and\n   uses the existing per-object path otherwise.\n\nThis does not change index-pack's duplicate policy or make duplicate\nentries a preferred pack format. It makes existing non-strict packs\nreadable, verifiable, and safe for bitmap-assisted packing while keeping\nordinary packs on their current paths.\n\nThanks,\nTaylor\n\n[^1]: This is a good example of the types of problems this series\n  addresses. We currently assume that all objects in a MIDX's preferred\n  pack have a unique bit position, which is the same as their\n  pack-relative position. That assumption is safe as a consequence of\n  how the pseudo-pack ordering is defined, but *only* when the pack in\n  question contains no duplicate object entries.\n\nTaylor Blau (5):\n  t5308: test reverse indexes with duplicate objects\n  packfile: recover delta cycles through duplicate entries\n  midx: verify duplicate pack entries by OID and offset\n  test-tool bitmap: reject packs with duplicate objects\n  pack-bitmap: handle duplicate pack entries during MIDX reuse\n\n builtin/pack-objects.c            |  24 ++--\n midx.c                            |  59 +++++++--\n pack-bitmap.c                     |  27 +++-\n packfile.c                        | 199 ++++++++++++++++++++++++++++++\n t/helper/test-bitmap.c            |   3 +\n t/helper/test-find-pack.c         |  18 ++-\n t/t5308-pack-detect-duplicates.sh |  60 +++++++++\n t/t5309-pack-delta-cycles.sh      | 148 +++++++++++++++++++++-\n t/t5332-multi-pack-reuse.sh       |  89 +++++++++++++\n 9 files changed, 595 insertions(+), 32 deletions(-)\n\n\nbase-commit: 9a0c4701dcd5725c4184599322b52933ff5005ca\n-- \n2.55.0.383.gde07827a19\n"},{"id":"548920","messageId":"4aa08dc13f640da4036e4e50b8be7246dc9961e6.1784927134.git.ttaylorr@openai.com","threadId":"66058","inReplyTo":"cover.1784927134.git.ttaylorr@openai.com","subject":"[PATCH 1/5] t5308: test reverse indexes with duplicate objects","fromName":"Taylor Blau","fromEmail":"ttaylorr@openai.com","sentAt":"2026-07-24T21:05:50Z","receivedAt":"2026-07-24T21:05:58Z","isPatch":true,"body":"A non-strict .idx has one entry for each object in the pack, even when\nmultiple entries have the same object ID. Thus a pack ordered A, B, A,\nC has two .idx entries for A at distinct offsets. The corresponding\nper-pack reverse index must represent both entries and map them back to\nphysical pack order.\n\nExisting reverse-index tests do not cover packs with duplicate objects.\nAdd one and check that %(objectsize:disk) for B stops at the second A,\nrather than extending through it to C. Exercise both the on-disk and\nin-memory reverse-index implementations.\n\nAs part of validating Git's handling of packs containing duplicate\nobjects, cover their per-pack reverse indexes.\n\nSigned-off-by: Taylor Blau <ttaylorr@openai.com>\n---\n t/t5308-pack-detect-duplicates.sh | 39 +++++++++++++++++++++++++++++++\n 1 file changed, 39 insertions(+)\n\ndiff --git a/t/t5308-pack-detect-duplicates.sh b/t/t5308-pack-detect-duplicates.sh\nindex 0f84137867..4ff8f5b449 100755\n--- a/t/t5308-pack-detect-duplicates.sh\n+++ b/t/t5308-pack-detect-duplicates.sh\n@@ -27,6 +27,11 @@ HI_SHA1=$EMPTY_BLOB\n # duplicate runs).\n MISSING_SHA1=$(test_oid missing_oid)\n \n+# Three distinct objects for tests where physical pack order matters.\n+A=$(test_oid packlib_7_0)\n+B=$LO_SHA1\n+C=$HI_SHA1\n+\n # git will never intentionally create packfiles with\n # duplicate objects, so we have to construct them by hand.\n #\n@@ -72,6 +77,40 @@ test_expect_success 'lookup in duplicated pack' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'duplicate entries remain in pack reverse index' '\n+\tclear_packs &&\n+\t{\n+\t\tpack_header 4 &&\n+\t\tpack_obj $A &&\n+\t\tpack_obj $B &&\n+\t\tpack_obj $A &&\n+\t\tpack_obj $C\n+\t} >physical-order.pack &&\n+\tpack_trailer physical-order.pack &&\n+\n+\ttest_must_fail git index-pack --rev-index --stdin --strict \\\n+\t\t<physical-order.pack 2>err &&\n+\ttest_grep \"appears twice in the pack\" err &&\n+\n+\tgit index-pack --rev-index --stdin <physical-order.pack &&\n+\tgit show-index <\"$(ls .git/objects/pack/pack-*.idx)\" >offsets.raw &&\n+\n+\tsort -n offsets.raw | grep -A1 \"$B\" | cut -d\" \" -f1 >adjacent &&\n+\techo $(($(tail -n1 adjacent) - $(head -n1 adjacent))) >expect &&\n+\techo \"$B\" >in &&\n+\n+\tGIT_TEST_REV_INDEX_DIE_IN_MEMORY=1 \\\n+\t\tgit cat-file --batch-check=\"%(objectsize:disk)\" \\\n+\t\t<in >actual.disk &&\n+\tGIT_TEST_REV_INDEX_DIE_ON_DISK=1 \\\n+\t\tgit -c pack.readReverseIndex=false \\\n+\t\tcat-file --batch-check=\"%(objectsize:disk)\" \\\n+\t\t<in >actual.mem  &&\n+\n+\ttest_cmp expect actual.disk &&\n+\ttest_cmp expect actual.mem\n+'\n+\n test_expect_success 'index-pack can reject packs with duplicates' '\n \tclear_packs &&\n \tcreate_pack dups.pack 2 &&\n-- \n2.55.0.383.gde07827a19\n\n"},{"id":"548921","messageId":"c66a9cabba9469ab2324ccbfbcef3e789593dba3.1784927134.git.ttaylorr@openai.com","threadId":"66058","inReplyTo":"cover.1784927134.git.ttaylorr@openai.com","subject":"[PATCH 2/5] packfile: recover delta cycles through duplicate entries","fromName":"Taylor Blau","fromEmail":"ttaylorr@openai.com","sentAt":"2026-07-24T21:06:00Z","receivedAt":"2026-07-24T21:06:08Z","isPatch":true,"body":"98f8854c94 (index-pack: allow revisiting REF_DELTA chains, 2025-04-28)\nchanged t5309's recoverable-cycle case to expect index-pack to accept\nthe pack, but did not read the resulting objects. The .idx for a pack\nwith duplicate OIDs retains every physical entry, with equal OIDs in a\ncontiguous run. Ordinary REF_DELTA lookup selects one representation\nfrom such a run. If that choice closes a cycle, both type and content\nreaders follow the same physical offsets indefinitely even though\nanother representation is usable.\n\nKeep the ordinary walk, and recognize a cycle only when its existing\nstack shows an exact repeated pack offset. At that point, restart at the\nrequested object and search duplicate representations depth-first.\nTreat each OID as a node, try each entry in its .idx run, and translate\nOFS_DELTA bases back to OIDs. A bitmap marks each OID group already\nvisited. Reaching a full object records the exact offsets along the\nacyclic path so unpack_entry() can replay it; packed_to_object_type()\nneeds only the resulting type.\n\nThus, acyclic lookups continue to use the existing single-entry lookup.\nDuplicate-run scans, the visited bitmap, and OFS-to-OID translation\nremain confined to recovery after a proven cycle.\n\nExtend t5309 to read both type and content after indexing. Cover a\nroot-level full duplicate, a mixed REF/OFS cycle, and a tail into a\nthree-object cycle which must backtrack before taking an alternate\nREF_DELTA/OFS_DELTA path to a full base. Keep the fixtures\nhash-independent so the same cases run under SHA-1 and SHA-256. This\nadds the reader validation missing from the earlier acceptance test.\n\nSigned-off-by: Taylor Blau <ttaylorr@openai.com>\n---\n packfile.c                   | 199 +++++++++++++++++++++++++++++++++++\n t/t5309-pack-delta-cycles.sh | 148 ++++++++++++++++++++++++--\n 2 files changed, 341 insertions(+), 6 deletions(-)\n\ndiff --git a/packfile.c b/packfile.c\nindex 0eee45055f..6049d1fd8c 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -10,6 +10,7 @@\n #include \"dir.h\"\n #include \"packfile.h\"\n #include \"delta.h\"\n+#include \"ewah/ewok.h\"\n #include \"hash-lookup.h\"\n #include \"commit.h\"\n #include \"object.h\"\n@@ -1077,6 +1078,160 @@ static int get_delta_base_oid(struct packed_git *p,\n \t\treturn -1;\n }\n \n+/*\n+ * Search duplicate representations for a chain ending in a full object.\n+ * Representations of the same OID are interchangeable as delta bases.\n+ *\n+ * Each path entry is the search frame for one OID. It walks that OID's\n+ * contiguous .idx entries and retains the chosen entry's .pack offset\n+ * for replay.\n+ */\n+struct delta_path_entry {\n+\toff_t selected_offset; /* zero until a candidate is selected */\n+\tuint32_t next; /* index position of the next candidate */\n+\tuint32_t remaining; /* total number of candidates remaining */\n+};\n+\n+struct delta_path {\n+\tstruct delta_path_entry *entries;\n+\tsize_t nr, alloc;\n+};\n+\n+static int push_oid_group(struct packed_git *p,\n+\t\t\t  const struct object_id *oid,\n+\t\t\t  struct bitmap *visited, struct delta_path *path)\n+{\n+\tstruct object_id candidate;\n+\tstruct delta_path_entry *entry;\n+\tuint32_t first_index_pos, group_index_pos, index_pos;\n+\n+\t/*\n+\t * Determine the range of index positions referring to duplicate\n+\t * copies of the given object.\n+\t *\n+\t * Any position within that range is OK, since we will determine\n+\t * the exact range below.\n+\t */\n+\tif (!bsearch_pack(oid, p, &group_index_pos))\n+\t\treturn 0;\n+\tif (bitmap_get(visited, group_index_pos))\n+\t\treturn 0;\n+\n+\tfirst_index_pos = group_index_pos;\n+\twhile (first_index_pos > 0) {\n+\t\tif (nth_packed_object_id(&candidate, p, first_index_pos - 1) < 0)\n+\t\t\treturn -1;\n+\t\tif (!oideq(&candidate, oid))\n+\t\t\tbreak;\n+\t\tfirst_index_pos--;\n+\t}\n+\n+\tfor (index_pos = first_index_pos; index_pos < p->num_objects; index_pos++) {\n+\t\tif (nth_packed_object_id(&candidate, p, index_pos) < 0)\n+\t\t\treturn -1;\n+\t\tif (!oideq(&candidate, oid))\n+\t\t\tbreak;\n+\t}\n+\n+\tbitmap_set(visited, group_index_pos);\n+\n+\tALLOC_GROW(path->entries, path->nr + 1, path->alloc);\n+\tentry = &path->entries[path->nr++];\n+\tentry->selected_offset = 0;\n+\tentry->next = first_index_pos;\n+\tentry->remaining = index_pos - first_index_pos;\n+\n+\treturn 1;\n+}\n+\n+static enum object_type find_delta_path(struct packed_git *p,\n+\t\t\t\t\tstruct pack_window **w_curs,\n+\t\t\t\t\toff_t offset,\n+\t\t\t\t\tstruct delta_path *path)\n+{\n+\tstruct object_id oid;\n+\tuint32_t pack_pos;\n+\tstruct bitmap *visited;\n+\tenum object_type result = OBJ_BAD;\n+\n+\tif (offset_to_pack_pos(p, offset, &pack_pos) < 0)\n+\t\treturn OBJ_BAD;\n+\tif (nth_packed_object_id(&oid, p, pack_pos_to_index(p, pack_pos)) < 0)\n+\t\treturn OBJ_BAD;\n+\n+\tvisited = bitmap_new();\n+\tif (push_oid_group(p, &oid, visited, path) != 1)\n+\t\tgoto done;\n+\n+\t/*\n+\t * Search depth-first for a chain ending in a full object. Each frame\n+\t * tries every representation of one OID; a delta pushes its base OID,\n+\t * while exhausting a frame backtracks to its parent.\n+\t */\n+\twhile (path->nr) {\n+\t\tstruct delta_path_entry *entry = &path->entries[path->nr - 1];\n+\t\tenum object_type candidate_type;\n+\t\toff_t curpos;\n+\t\tsize_t size;\n+\n+\t\t/*\n+\t\t * This OID has no path to a full object. Let its parent try\n+\t\t * another representation; exhausting the root fails the search.\n+\t\t */\n+\t\tif (!entry->remaining) {\n+\t\t\tpath->nr--;\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tentry->selected_offset =\n+\t\t\tnth_packed_object_offset(p, entry->next++);\n+\t\tentry->remaining--;\n+\t\tcurpos = entry->selected_offset;\n+\t\tcandidate_type = unpack_object_header(p, w_curs, &curpos, &size);\n+\n+\t\t/*\n+\t\t * A full object terminates the chain, and its type is\n+\t\t * inherited by every delta above it. A delta continues\n+\t\t * at its base; any other type rejects only this\n+\t\t * representation.\n+\t\t */\n+\t\tswitch (candidate_type) {\n+\t\tcase OBJ_COMMIT:\n+\t\tcase OBJ_TREE:\n+\t\tcase OBJ_BLOB:\n+\t\tcase OBJ_TAG:\n+\t\t\tresult = candidate_type;\n+\t\t\tgoto done;\n+\t\tcase OBJ_OFS_DELTA:\n+\t\tcase OBJ_REF_DELTA:\n+\t\t\tbreak;\n+\t\tdefault:\n+\t\t\t/*\n+\t\t\t * A bad or unknown type rejects only this copy;\n+\t\t\t * another representation of the same OID may\n+\t\t\t * still work.\n+\t\t\t */\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\t/*\n+\t\t * Descend to this delta's base. A malformed reference\n+\t\t * or a missing or already-visited base rejects this\n+\t\t * copy. A newly pushed base is examined next; an index\n+\t\t * error aborts the search.\n+\t\t */\n+\t\tif (get_delta_base_oid(p, w_curs, curpos, &oid, candidate_type,\n+\t\t\t\t       entry->selected_offset))\n+\t\t\tcontinue;\n+\t\tif (push_oid_group(p, &oid, visited, path) < 0)\n+\t\t\tgoto done;\n+\t}\n+\n+done:\n+\tbitmap_free(visited);\n+\treturn result;\n+}\n+\n static int retry_bad_packed_offset(struct repository *r,\n \t\t\t\t   struct packed_git *p,\n \t\t\t\t   off_t obj_offset)\n@@ -1105,11 +1260,27 @@ static enum object_type packed_to_object_type(struct repository *r,\n {\n \toff_t small_poi_stack[POI_STACK_PREALLOC];\n \toff_t *poi_stack = small_poi_stack;\n+\toff_t root_offset = obj_offset;\n \tint poi_stack_nr = 0, poi_stack_alloc = POI_STACK_PREALLOC;\n \n \twhile (type == OBJ_OFS_DELTA || type == OBJ_REF_DELTA) {\n \t\toff_t base_offset;\n \t\tsize_t size;\n+\n+\t\tif (poi_stack_nr > 0 && poi_stack_nr % 2 == 0 &&\n+\t\t    obj_offset == poi_stack[poi_stack_nr / 2]) {\n+\t\t\tstruct delta_path path = { 0 };\n+\t\t\t/*\n+\t\t\t * Normal lookup returned to the same pack\n+\t\t\t * entry. Restart from the requested object\n+\t\t\t * using alternate representations.\n+\t\t\t */\n+\t\t\ttype = find_delta_path(p, w_curs, root_offset, &path);\n+\t\t\tfree(path.entries);\n+\t\t\tif (type == OBJ_BAD)\n+\t\t\t\tgoto unwind;\n+\t\t\tbreak;\n+\t\t}\n \t\t/* Push the object we're going to leave behind */\n \t\tif (poi_stack_nr >= poi_stack_alloc && poi_stack == small_poi_stack) {\n \t\t\tpoi_stack_alloc = alloc_nr(poi_stack_nr);\n@@ -1525,9 +1696,11 @@ void *unpack_entry(struct repository *r, struct packed_git *p, off_t obj_offset,\n {\n \tstruct pack_window *w_curs = NULL;\n \toff_t curpos = obj_offset;\n+\toff_t root_offset = obj_offset;\n \tvoid *data = NULL;\n \tsize_t size;\n \tenum object_type type;\n+\tstruct delta_path path = { 0 };\n \tstruct unpack_entry_stack_ent small_delta_stack[UNPACK_ENTRY_STACK_PREALLOC];\n \tstruct unpack_entry_stack_ent *delta_stack = small_delta_stack;\n \tint delta_stack_nr = 0, delta_stack_alloc = UNPACK_ENTRY_STACK_PREALLOC;\n@@ -1553,6 +1726,22 @@ void *unpack_entry(struct repository *r, struct packed_git *p, off_t obj_offset,\n \t\t\tbreak;\n \t\t}\n \n+\t\tif (!path.nr &&\n+\t\t    delta_stack_nr > 0 && delta_stack_nr % 2 == 0 &&\n+\t\t    obj_offset == delta_stack[delta_stack_nr / 2].obj_offset) {\n+\t\t\t/*\n+\t\t\t * Normal lookup returned to the same pack\n+\t\t\t * entry. Find an acyclic path if one exists,\n+\t\t\t * discard this walk, and replay that path.\n+\t\t\t */\n+\t\t\tif (find_delta_path(p, &w_curs, root_offset,\n+\t\t\t\t\t    &path) == OBJ_BAD)\n+\t\t\t\tbreak;\n+\t\t\tdelta_stack_nr = 0;\n+\t\t\tcurpos = obj_offset = path.entries[0].selected_offset;\n+\t\t\tcontinue;\n+\t\t}\n+\n \t\tif (do_check_packed_object_crc && p->index_version > 1) {\n \t\t\tuint32_t pack_pos, index_pos;\n \t\t\toff_t len;\n@@ -1591,6 +1780,15 @@ void *unpack_entry(struct repository *r, struct packed_git *p, off_t obj_offset,\n \t\t\tbreak;\n \t\t}\n \n+\t\t/* Use the base chosen by recovery, not the one from normal lookup. */\n+\t\tif (path.nr) {\n+\t\t\tsize_t path_pos = (size_t)delta_stack_nr + 1;\n+\n+\t\t\tif (path_pos >= path.nr)\n+\t\t\t\tBUG(\"alternate delta path ends in a delta\");\n+\t\t\tbase_offset = path.entries[path_pos].selected_offset;\n+\t\t}\n+\n \t\t/* push object, proceed to base */\n \t\tif (delta_stack_nr >= delta_stack_alloc\n \t\t    && delta_stack == small_delta_stack) {\n@@ -1731,6 +1929,7 @@ void *unpack_entry(struct repository *r, struct packed_git *p, off_t obj_offset,\n \n out:\n \tunuse_pack(&w_curs);\n+\tfree(path.entries);\n \n \tif (delta_stack != small_delta_stack)\n \t\tfree(delta_stack);\ndiff --git a/t/t5309-pack-delta-cycles.sh b/t/t5309-pack-delta-cycles.sh\nindex 6b03675d91..f613950e38 100755\n--- a/t/t5309-pack-delta-cycles.sh\n+++ b/t/t5309-pack-delta-cycles.sh\n@@ -9,6 +9,83 @@ test_description='test index-pack handling of delta cycles in packfiles'\n A=$(test_oid packlib_7_0)\n B=$(test_oid packlib_7_76)\n \n+# Copy the entries from a complete pack without its header or trailer.\n+pack_entries () {\n+\tentry_size=$(wc -c <\"$1\") &&\n+\tdd if=\"$1\" bs=1 skip=12 \\\n+\t\tcount=$((entry_size - 12 - $(test_oid rawsz))) 2>/dev/null\n+}\n+\n+# B as an OFS_DELTA against A at the given one-byte distance.\n+pack_obj_b_ofs_a () {\n+\tpack_obj \"$B\" \"$A\" >b-ref.tmp &&\n+\tprintf \"\\145\" &&\n+\tprintf \"\\\\$(printf \"%03o\" \"$1\")\" &&\n+\tdd if=b-ref.tmp bs=1 skip=$((1 + $(test_oid rawsz))) 2>/dev/null\n+}\n+\n+# Return the base of the first one-byte-header REF_DELTA for the given OID.\n+first_ref_base () {\n+\tidx=$(echo .git/objects/pack/*.idx) &&\n+\toffset=$(git show-index <\"$idx\" |\n+\t\tawk -v oid=\"$1\" '$2 == oid { print $1; exit }') &&\n+\tdd if=\"${idx%.idx}.pack\" bs=1 skip=$((offset + 1)) \\\n+\t\tcount=$(test_oid rawsz) 2>/dev/null |\n+\ttest-tool hexdump |\n+\ttr -d \" \\n\"\n+}\n+\n+# The order of equal-OID entries in the .idx is unspecified. Retain a pack\n+# which selects $1 as a delta against $2. Unless $3 is \"-\", also require the\n+# first copy searched during recovery to be a REF_DELTA against $3.\n+install_cycle () {\n+\tcycle_oid=$1 &&\n+\tcycle_base=$2 &&\n+\tfirst_base=$3 &&\n+\tshift 3 &&\n+\tfor pack\n+\tdo\n+\t\tclear_packs &&\n+\t\tgit index-pack --fix-thin --stdin <\"$pack\" &&\n+\t\tselected_base=$(echo \"$cycle_oid\" |\n+\t\t\tgit cat-file --batch-check=\"%(deltabase)\") ||\n+\t\treturn 1\n+\t\tif test \"$selected_base\" = \"$cycle_base\" &&\n+\t\t   { test \"$first_base\" = \"-\" ||\n+\t\t     test \"$(first_ref_base \"$cycle_oid\")\" = \"$first_base\"; }\n+\t\tthen\n+\t\t\treturn 0\n+\t\tfi\n+\tdone\n+\treturn 1\n+}\n+\n+make_cycle_pack () {\n+\tcycle_pack=$1 &&\n+\tshift &&\n+\ttest-tool -C alt-source pack-deltas --num-objects=6 >refs.tmp <<-EOF &&\n+\tREF_DELTA $T $X\n+\tREF_DELTA $X $1\n+\tREF_DELTA $Y $Z\n+\tREF_DELTA $Z $X\n+\tREF_DELTA $X $2\n+\tREF_DELTA $X $3\n+\tEOF\n+\t{\n+\t\tpack_header 8 &&\n+\t\tpack_entries refs.tmp &&\n+\t\tcat a-full &&\n+\t\tpack_obj_b_ofs_a \"$a_full_size\"\n+\t} >\"$cycle_pack\" &&\n+\tpack_trailer \"$cycle_pack\"\n+}\n+\n+check_blob () {\n+\ttest \"$(git cat-file -t \"$1\")\" = blob &&\n+\tgit cat-file blob \"$1\" >actual &&\n+\ttest_cmp_bin \"$2\" actual\n+}\n+\n # double-check our hand-constucted packs\n test_expect_success 'index-pack works with a single delta (A->B)' '\n \tclear_packs &&\n@@ -67,18 +144,77 @@ test_expect_success 'failover to an object in another pack' '\n '\n \n test_expect_success 'failover to a duplicate object in the same pack' '\n-\tclear_packs &&\n+\t{\n+\t\tpack_header 3 &&\n+\t\tpack_obj $A &&\n+\t\tpack_obj $B $A &&\n+\t\tpack_obj $A $B\n+\t} >recoverable-1.pack &&\n+\tpack_trailer recoverable-1.pack &&\n \t{\n \t\tpack_header 3 &&\n \t\tpack_obj $A $B &&\n \t\tpack_obj $B $A &&\n \t\tpack_obj $A\n-\t} >recoverable.pack &&\n-\tpack_trailer recoverable.pack &&\n+\t} >recoverable-2.pack &&\n+\tpack_trailer recoverable-2.pack &&\n \n-\t# This cycle does not fail since the existence of a full copy\n-\t# of A in the pack allows us to resolve the cycle.\n-\tgit index-pack --fix-thin --stdin <recoverable.pack\n+\t# The selected copy of A is part of the cycle, but the full copy\n+\t# lets both type and content lookups resolve it.\n+\tinstall_cycle \"$A\" \"$B\" - recoverable-1.pack recoverable-2.pack &&\n+\tprintf \"\\7\\0\" >expect &&\n+\tcheck_blob \"$A\" expect\n+'\n+\n+test_expect_success 'failover from a mixed REF/OFS cycle' '\n+\tpack_obj \"$A\" \"$B\" >a-ref &&\n+\tpack_obj \"$B\" >b-full &&\n+\ta_ref_size=$(wc -c <a-ref) &&\n+\tb_full_size=$(wc -c <b-full) &&\n+\n+\t{\n+\t\tpack_header 3 &&\n+\t\tcat a-ref &&\n+\t\tcat b-full &&\n+\t\tpack_obj_b_ofs_a \"$((a_ref_size + b_full_size))\"\n+\t} >mixed-1.pack &&\n+\tpack_trailer mixed-1.pack &&\n+\t{\n+\t\tpack_header 3 &&\n+\t\tcat a-ref &&\n+\t\tpack_obj_b_ofs_a \"$a_ref_size\" &&\n+\t\tcat b-full\n+\t} >mixed-2.pack &&\n+\tpack_trailer mixed-2.pack &&\n+\n+\t# The REF_DELTA for A selects the OFS_DELTA copy of B; the\n+\t# full B is its escape.\n+\tinstall_cycle \"$B\" \"$A\" - mixed-1.pack mixed-2.pack &&\n+\tprintf \"\\7\\0\" >expect &&\n+\tcheck_blob \"$A\" expect\n+'\n+\n+test_expect_success 'failover after a tail into a three-object delta cycle' '\n+\tgit init alt-source &&\n+\tprintf \"\\7\\76\" |\n+\t\tgit -C alt-source hash-object -w --stdin >/dev/null &&\n+\tX=$(printf x | git -C alt-source hash-object -w --stdin) &&\n+\tY=$(printf y | git -C alt-source hash-object -w --stdin) &&\n+\tZ=$(printf z | git -C alt-source hash-object -w --stdin) &&\n+\tprintf \"tail T\\n\" >tail &&\n+\tT=$(git -C alt-source hash-object -w --stdin <tail) &&\n+\n+\tpack_obj \"$A\" >a-full &&\n+\ta_full_size=$(wc -c <a-full) &&\n+\tmake_cycle_pack alternate-1.pack \"$B\" \"$Y\" \"$Y\" &&\n+\tmake_cycle_pack alternate-2.pack \"$Y\" \"$B\" \"$Y\" &&\n+\tmake_cycle_pack alternate-3.pack \"$Y\" \"$Y\" \"$B\" &&\n+\n+\t# Lookup of T follows T->X->Y->Z->X. Recovery must exhaust that\n+\t# branch, then use X->B->A, whose final edge is an OFS_DELTA.\n+\tinstall_cycle \"$X\" \"$Y\" \"$Y\" \\\n+\t\talternate-1.pack alternate-2.pack alternate-3.pack &&\n+\tcheck_blob \"$T\" tail\n '\n \n test_expect_success 'index-pack works with thin pack A->B->C with B on disk' '\n-- \n2.55.0.383.gde07827a19\n\n"},{"id":"548922","messageId":"5c9dc6880fff33cd6061663cfd170c5daf871dfc.1784927134.git.ttaylorr@openai.com","threadId":"66058","inReplyTo":"cover.1784927134.git.ttaylorr@openai.com","subject":"[PATCH 3/5] midx: verify duplicate pack entries by OID and offset","fromName":"Taylor Blau","fromEmail":"ttaylorr@openai.com","sentAt":"2026-07-24T21:06:09Z","receivedAt":"2026-07-24T21:06:22Z","isPatch":true,"body":"A MIDX retains one entry per OID, while a non-strict pack index can\ncontain the same OID at several offsets. verify_midx_file() compares the\nrecorded offset with find_pack_entry_one(), which may return a different\nmember of that duplicate run and falsely report a valid MIDX as corrupt.\n\nCheck instead that the exact OID/offset pair exists anywhere in the\ncontiguous duplicate run in the source pack. That is the relevant\ninvariant: same-pack duplicates have no canonical representation, and\nevery matching physical copy is valid, including those recorded by\nexisting MIDXs.\n\nThis matches reader behavior. The OOFF chunk records the selected\n(pack, offset), and midx_to_pack_pos() reconstructs its RIDX position\nfrom that pair. If midx_pair_to_pack_pos() is given an unselected\nduplicate offset, the lookup misses and its sole caller falls back from\npartial reuse to normal packing. Readers therefore remain consistent\nwith whichever representation the MIDX records.\n\nWrite a MIDX over the existing duplicate-pack fixture, assert that its\nselected offset differs from find_pack_entry_one(), and verify it. Then\nrun fsck as an application-level check.\n\nSigned-off-by: Taylor Blau <ttaylorr@openai.com>\n---\n midx.c                            | 59 +++++++++++++++++++++++++++----\n t/helper/test-find-pack.c         | 18 +++++++---\n t/t5308-pack-detect-duplicates.sh | 14 ++++++++\n 3 files changed, 80 insertions(+), 11 deletions(-)\n\ndiff --git a/midx.c b/midx.c\nindex 76c3f92cc3..05fe99f8ca 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -906,6 +906,48 @@ static int compare_pair_pos_vs_id(const void *_a, const void *_b)\n \treturn b->pack_int_id - a->pack_int_id;\n }\n \n+/*\n+ * Return whether the pack index contains an entry with both \"oid\" and\n+ * \"offset\". A pack index may contain duplicate OIDs, so an arbitrary\n+ * OID lookup is not enough to validate a particular offset.\n+ *\n+ * Do not use offset_to_pack_pos() here: it may consult an optional '.rev'\n+ * file, which is verified separately, or build an in-memory reverse index\n+ * that remains attached to the pack. Verify the MIDX directly against its\n+ * source pack index instead.\n+ */\n+static int pack_index_has_oid_at_offset(struct packed_git *p,\n+\t\t\t\t\tconst struct object_id *oid,\n+\t\t\t\t\toff_t offset)\n+{\n+\tstruct object_id candidate;\n+\tuint32_t pos, i;\n+\n+\tif (!bsearch_pack(oid, p, &pos))\n+\t\treturn 0;\n+\n+\tif (nth_packed_object_offset(p, pos) == offset)\n+\t\treturn 1;\n+\n+\tfor (i = pos; i > 0; i--) {\n+\t\tif (nth_packed_object_id(&candidate, p, i - 1) ||\n+\t\t    !oideq(&candidate, oid))\n+\t\t\tbreak;\n+\t\tif (nth_packed_object_offset(p, i - 1) == offset)\n+\t\t\treturn 1;\n+\t}\n+\n+\tfor (i = pos + 1; i < p->num_objects; i++) {\n+\t\tif (nth_packed_object_id(&candidate, p, i) ||\n+\t\t    !oideq(&candidate, oid))\n+\t\t\tbreak;\n+\t\tif (nth_packed_object_offset(p, i) == offset)\n+\t\t\treturn 1;\n+\t}\n+\n+\treturn 0;\n+}\n+\n /*\n  * Limit calls to display_progress() for performance reasons.\n  * The interval here was arbitrarily chosen.\n@@ -1015,7 +1057,6 @@ int verify_midx_file(struct odb_source_packed *source, unsigned flags)\n \tfor (i = 0; i < m->num_objects + m->num_objects_in_base; i++) {\n \t\tstruct object_id oid;\n \t\tstruct pack_entry e;\n-\t\toff_t m_offset, p_offset;\n \n \t\tif (i > 0 && pairs[i-1].pack_int_id != pairs[i].pack_int_id &&\n \t\t    nth_midxed_pack(m, pairs[i-1].pack_int_id)) {\n@@ -1040,12 +1081,16 @@ int verify_midx_file(struct odb_source_packed *source, unsigned flags)\n \t\t\tbreak;\n \t\t}\n \n-\t\tm_offset = e.offset;\n-\t\tp_offset = find_pack_entry_one(&oid, e.p);\n-\n-\t\tif (m_offset != p_offset)\n-\t\t\tmidx_report(_(\"incorrect object offset for oid[%d] = %s: %\"PRIx64\" != %\"PRIx64),\n-\t\t\t\t    pairs[i].pos, oid_to_hex(&oid), m_offset, p_offset);\n+\t\t/*\n+\t\t * Check that the exact offset recorded in the MIDX\n+\t\t * belongs to this OID. A pack index may contain\n+\t\t * duplicate OIDs, in which case an arbitrary OID lookup\n+\t\t * can return a different, equally valid copy than the\n+\t\t * one selected by the MIDX writer.\n+\t\t */\n+\t\tif (!pack_index_has_oid_at_offset(e.p, &oid, e.offset))\n+\t\t\tmidx_report(_(\"incorrect object offset for oid[%d] = %s: %\"PRIx64),\n+\t\t\t\t    pairs[i].pos, oid_to_hex(&oid), e.offset);\n \n \t\tmidx_display_sparse_progress(progress, i + 1);\n \t}\ndiff --git a/t/helper/test-find-pack.c b/t/helper/test-find-pack.c\nindex 28d5b1fe09..51093a7030 100644\n--- a/t/helper/test-find-pack.c\n+++ b/t/helper/test-find-pack.c\n@@ -11,12 +11,15 @@\n  * Display the path(s), one per line, of the packfile(s) containing\n  * the given object.\n  *\n+ * With '--show-offset', display the offset selected by\n+ * find_pack_entry_one() instead of the packfile path.\n+ *\n  * If '--check-count <n>' is passed, then error out if the number of\n  * packfiles containing the object is not <n>.\n  */\n \n static const char *const find_pack_usage[] = {\n-\t\"test-tool find-pack [--check-count <n>] <object>\",\n+\t\"test-tool find-pack [--check-count <n>] [--show-offset] <object>\",\n \tNULL\n };\n \n@@ -24,11 +27,13 @@ int cmd__find_pack(int argc, const char **argv)\n {\n \tstruct object_id oid;\n \tstruct packed_git *p;\n-\tint count = -1, actual_count = 0;\n+\tint count = -1, actual_count = 0, show_offset = 0;\n \tconst char *prefix = setup_git_directory(the_repository);\n \n \tstruct option options[] = {\n \t\tOPT_INTEGER('c', \"check-count\", &count, \"expected number of packs\"),\n+\t\tOPT_BOOL(0, \"show-offset\", &show_offset,\n+\t\t\t \"show matching pack offsets\"),\n \t\tOPT_END(),\n \t};\n \n@@ -40,8 +45,13 @@ int cmd__find_pack(int argc, const char **argv)\n \t\tdie(\"cannot parse %s as an object name\", argv[0]);\n \n \trepo_for_each_pack(the_repository, p) {\n-\t\tif (find_pack_entry_one(&oid, p)) {\n-\t\t\tprintf(\"%s\\n\", p->pack_name);\n+\t\toff_t offset = find_pack_entry_one(&oid, p);\n+\n+\t\tif (offset) {\n+\t\t\tif (show_offset)\n+\t\t\t\tprintf(\"%\"PRIuMAX\"\\n\", (uintmax_t)offset);\n+\t\t\telse\n+\t\t\t\tprintf(\"%s\\n\", p->pack_name);\n \t\t\tactual_count++;\n \t\t}\n \t}\ndiff --git a/t/t5308-pack-detect-duplicates.sh b/t/t5308-pack-detect-duplicates.sh\nindex 4ff8f5b449..493ebbc4af 100755\n--- a/t/t5308-pack-detect-duplicates.sh\n+++ b/t/t5308-pack-detect-duplicates.sh\n@@ -77,6 +77,20 @@ test_expect_success 'lookup in duplicated pack' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'verify MIDX containing duplicated pack objects' '\n+\tgit multi-pack-index write &&\n+\ttest-tool read-midx --show-objects .git/objects >midx-objects &&\n+\tmidx_offset=$(\n+\t\tawk -v oid=\"$LO_SHA1\" \"\\$1 == oid { print \\$2 }\" <midx-objects\n+\t) &&\n+\tlookup_offset=$(\n+\t\ttest-tool find-pack --check-count=1 --show-offset \"$LO_SHA1\"\n+\t) &&\n+\ttest \"$midx_offset\" -ne \"$lookup_offset\" &&\n+\tgit multi-pack-index verify &&\n+\tgit fsck --full\n+'\n+\n test_expect_success 'duplicate entries remain in pack reverse index' '\n \tclear_packs &&\n \t{\n-- \n2.55.0.383.gde07827a19\n\n"},{"id":"548923","messageId":"355a9f849c1a98f47dad3edfc8251e28ee179272.1784927134.git.ttaylorr@openai.com","threadId":"66058","inReplyTo":"cover.1784927134.git.ttaylorr@openai.com","subject":"[PATCH 4/5] test-tool bitmap: reject packs with duplicate objects","fromName":"Taylor Blau","fromEmail":"ttaylorr@openai.com","sentAt":"2026-07-24T21:06:20Z","receivedAt":"2026-07-24T21:06:26Z","isPatch":true,"body":"The bitmap writer builds its object-to-position map in packing_data,\nwhose hash table permits one entry per OID. The bitmap test helper\naccepts an arbitrary existing pack, so a pack with duplicate entries\ncalls packlist_alloc() twice for the same OID and trips its internal\nBUG().\n\nDetect duplicate OIDs while ingesting the pack and die before calling\npacklist_alloc(). This preserves the internal uniqueness check for\nother callers while making unsupported helper input fail gracefully.\n\nReuse t5308's existing duplicate pack to exercise the fatal\ndiagnostic.\n\nSigned-off-by: Taylor Blau <ttaylorr@openai.com>\n---\n t/helper/test-bitmap.c            | 3 +++\n t/t5308-pack-detect-duplicates.sh | 7 +++++++\n 2 files changed, 10 insertions(+)\n\ndiff --git a/t/helper/test-bitmap.c b/t/helper/test-bitmap.c\nindex 8547ef67e2..6f851e0421 100644\n--- a/t/helper/test-bitmap.c\n+++ b/t/helper/test-bitmap.c\n@@ -50,6 +50,9 @@ static int add_packed_object(const struct object_id *oid,\n \n \toi.typep = &type;\n \n+\tif (packlist_find(packed, oid))\n+\t\tdie(\"pack contains duplicate object %s\", oid_to_hex(oid));\n+\n \tentry = packlist_alloc(packed, oid);\n \tentry->idx.offset = nth_packed_object_offset(pack, pos);\n \tif (packed_object_info(NULL, pack, entry->idx.offset, &oi) < 0)\ndiff --git a/t/t5308-pack-detect-duplicates.sh b/t/t5308-pack-detect-duplicates.sh\nindex 493ebbc4af..c6273a1aeb 100755\n--- a/t/t5308-pack-detect-duplicates.sh\n+++ b/t/t5308-pack-detect-duplicates.sh\n@@ -59,6 +59,13 @@ test_expect_success 'index-pack will allow duplicate objects by default' '\n \tgit index-pack --stdin <dups.pack\n '\n \n+test_expect_success 'bitmap writer rejects duplicate objects' '\n+\tpack=$(ls .git/objects/pack/pack-*.pack) &&\n+\ttest_must_fail test-tool bitmap write \"$(basename \"$pack\")\" \\\n+\t\t</dev/null 2>err &&\n+\ttest_grep \"fatal: pack contains duplicate object\" err\n+'\n+\n test_expect_success 'create batch-check test vectors' '\n \tcat >input <<-EOF &&\n \t$LO_SHA1\n-- \n2.55.0.383.gde07827a19\n\n"},{"id":"548924","messageId":"de07827a19dcab2dc3447017718f45cc51541b92.1784927134.git.ttaylorr@openai.com","threadId":"66058","inReplyTo":"cover.1784927134.git.ttaylorr@openai.com","subject":"[PATCH 5/5] pack-bitmap: handle duplicate pack entries during MIDX reuse","fromName":"Taylor Blau","fromEmail":"ttaylorr@openai.com","sentAt":"2026-07-24T21:06:24Z","receivedAt":"2026-07-24T21:06:33Z","isPatch":true,"body":"A MIDX pseudo-pack assigns one bit position to each selected OID. Its\npreferred-pack fast paths handle duplicate OIDs across packs in the\nMIDX, where the MIDX chooses one copy, but assume that each individual\npack contains one entry per OID.\n\nConsider a preferred pack ordered [A, B, A, C]. The MIDX retains one\ncopy of A, leaving three pseudo-pack positions for four physical\nentries. This creates two problems:\n\n - In MIDX-backed single-pack reuse, bitmap_nr came from\n   pack->num_objects and therefore counted both copies of A. Read the\n   selected count from the root MIDX layer BTMP chunk instead. If that\n   layer has no BTMP chunk, skip pack reuse and let the ordinary packing\n   path handle the bitmap-selected objects. MIDXs without BTMP lose\n   preferred-pack reuse until rewritten, rather than requiring a scan of\n   the entire pseudo-pack to retain an optional optimization.\n\n - Correcting the range length still does not align the two orderings.\n   Once one copy of A is omitted, MIDX position N need not name physical\n   pack entry N.\n\nThat mismatch matters during both selection and output. Whole-word\nselection bypasses the per-object check that the exact physical base of\na delta is present. Verbatim reuse copies the first N pack entries for\nthe first N bits. The per-object writer likewise used each bit as a\nphysical pack position.\n\nUse the direct mapping only when the range begins at zero and its\nselected count equals the physical entry count. Since selected entries\nremain in pack-offset order, equal counts mean that none was omitted and\nthe two positions coincide.\n\nFor every other MIDX range, let whole-word selection fall through to\nthe existing per-bit path, which resolves the selected MIDX offset to a\nphysical pack position before checking the delta base. Disable verbatim\nprefix reuse, and perform the same translation in the per-object writer.\n\nLeave classic single-pack bitmap handling unchanged. The production\nwriter creates those bitmaps only for the pack it just wrote, which has\none entry per OID. The test helper can target an existing pack, but\nrejects duplicate OIDs.\n\nCover the A, B, A, C case in a MIDX-backed preferred pack. Check reuse\nwith BTMP, then hide BTMP and check that the optional reuse path is\nskipped. Also make B a delta against the omitted copy of A and require\nnormal packing to handle it.\n\nSigned-off-by: Taylor Blau <ttaylorr@openai.com>\n---\n builtin/pack-objects.c      | 24 +++++-----\n pack-bitmap.c               | 27 ++++++++++-\n t/t5332-multi-pack-reuse.sh | 89 +++++++++++++++++++++++++++++++++++++\n 3 files changed, 125 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 3673b14b89..c333922961 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -1182,12 +1182,12 @@ static size_t write_reused_pack_verbatim(struct bitmapped_pack *reuse_packfile,\n \tsize_t pos = 0;\n \tsize_t end;\n \n-\tif (reuse_packfile->bitmap_pos) {\n+\tif (reuse_packfile->bitmap_pos ||\n+\t    reuse_packfile->bitmap_nr != reuse_packfile->p->num_objects) {\n \t\t/*\n-\t\t * We can't reuse whole chunks verbatim out of\n-\t\t * non-preferred packs since we can't guarantee that\n-\t\t * all duplicate objects were resolved in favor of\n-\t\t * that pack.\n+\t\t * We can't reuse whole chunks verbatim from non-preferred\n+\t\t * packs or packs with entries missing from the bitmap because\n+\t\t * bitmap and pack positions may differ.\n \t\t *\n \t\t * Even if we have a whole eword_t worth of bits that\n \t\t * could be reused, there may be objects between the\n@@ -1196,7 +1196,7 @@ static size_t write_reused_pack_verbatim(struct bitmapped_pack *reuse_packfile,\n \t\t * pack, causing us to send duplicate or unwanted\n \t\t * objects.\n \t\t *\n-\t\t * Handle non-preferred packs from within\n+\t\t * Handle these packs from within\n \t\t * write_reused_pack(), which inspects and reuses\n \t\t * individual bits.\n \t\t */\n@@ -1263,20 +1263,18 @@ static void write_reused_pack(struct bitmapped_pack *reuse_packfile,\n \t\t\tif (pos + offset >= reuse_packfile->bitmap_pos + reuse_packfile->bitmap_nr)\n \t\t\t\tgoto done;\n \n-\t\t\tif (reuse_packfile->bitmap_pos) {\n+\t\t\tif (reuse_packfile->bitmap_pos ||\n+\t\t\t    reuse_packfile->bitmap_nr != reuse_packfile->p->num_objects) {\n \t\t\t\t/*\n-\t\t\t\t * When doing multi-pack reuse on a\n-\t\t\t\t * non-preferred pack, translate bit positions\n-\t\t\t\t * from the MIDX pseudo-pack order back to their\n-\t\t\t\t * pack-relative positions before attempting\n-\t\t\t\t * reuse.\n+\t\t\t\t * Translate MIDX bitmap positions which do not\n+\t\t\t\t * correspond directly to physical pack positions.\n \t\t\t\t */\n \t\t\t\tstruct multi_pack_index *m = reuse_packfile->from_midx;\n \t\t\t\tuint32_t midx_pos;\n \t\t\t\toff_t pack_ofs;\n \n \t\t\t\tif (!m)\n-\t\t\t\t\tBUG(\"non-zero bitmap position without MIDX\");\n+\t\t\t\t\tBUG(\"cannot translate bitmap position without MIDX\");\n \n \t\t\t\tmidx_pos = pack_pos_to_midx(m, pos + offset);\n \t\t\t\tpack_ofs = nth_midxed_offset(m, midx_pos);\ndiff --git a/pack-bitmap.c b/pack-bitmap.c\nindex d8dc4ae8d1..8141290436 100644\n--- a/pack-bitmap.c\n+++ b/pack-bitmap.c\n@@ -2383,7 +2383,8 @@ static void reuse_partial_packfile_from_bitmap_1(struct bitmap_index *bitmap_git\n \tstruct pack_window *w_curs = NULL;\n \tsize_t pos = pack->bitmap_pos / BITS_IN_EWORD;\n \n-\tif (!pack->bitmap_pos) {\n+\tif (!pack->bitmap_pos &&\n+\t    pack->bitmap_nr == pack->p->num_objects) {\n \t\t/*\n \t\t * If we're processing the first (in the case of a MIDX, the\n \t\t * preferred pack) or the only (in the case of single-pack\n@@ -2399,6 +2400,9 @@ static void reuse_partial_packfile_from_bitmap_1(struct bitmap_index *bitmap_git\n \t\t *   all ties are broken in favor of that pack (i.e. the one\n \t\t *   we're currently processing). So any duplicate bases will be\n \t\t *   resolved in favor of the pack we're processing.\n+\t\t *\n+\t\t * The range must also contain every physical pack entry so that\n+\t\t * bitmap and pack positions correspond.\n \t\t */\n \t\twhile (pos < result->word_alloc &&\n \t\t       pos < pack->bitmap_nr / BITS_IN_EWORD &&\n@@ -2527,14 +2531,26 @@ void reuse_partial_packfile_from_bitmap(struct bitmap_index *bitmap_git,\n \t} else {\n \t\tstruct packed_git *pack;\n \t\tuint32_t pack_int_id;\n+\t\tuint32_t bitmap_nr;\n \n \t\tif (bitmap_is_midx(bitmap_git)) {\n+\t\t\tstruct bitmapped_pack bitmapped_pack;\n \t\t\tstruct multi_pack_index *m = bitmap_git->midx;\n \t\t\tuint32_t preferred_pack_pos;\n \n \t\t\twhile (m->base_midx)\n \t\t\t\tm = m->base_midx;\n \n+\t\t\tif (!m->chunk_bitmapped_packs) {\n+\t\t\t\t/*\n+\t\t\t\t * Without BTMP, determining the preferred\n+\t\t\t\t * pack's range requires scanning the pseudo-pack.\n+\t\t\t\t * Skip reuse and leave the bitmap result for\n+\t\t\t\t * normal packing.\n+\t\t\t\t */\n+\t\t\t\treturn;\n+\t\t\t}\n+\n \t\t\tif (midx_preferred_pack(m, &preferred_pack_pos) < 0) {\n \t\t\t\twarning(_(\"unable to compute preferred pack, disabling pack-reuse\"));\n \t\t\t\treturn;\n@@ -2542,6 +2558,12 @@ void reuse_partial_packfile_from_bitmap(struct bitmap_index *bitmap_git,\n \n \t\t\tpack = nth_midxed_pack(m, preferred_pack_pos);\n \t\t\tpack_int_id = preferred_pack_pos;\n+\n+\t\t\tif (nth_bitmapped_pack(m, &bitmapped_pack,\n+\t\t\t\t\t       pack_int_id) < 0)\n+\t\t\t\treturn;\n+\t\t\tpack = bitmapped_pack.p;\n+\t\t\tbitmap_nr = bitmapped_pack.bitmap_nr;\n \t\t} else {\n \t\t\tpack = bitmap_git->pack;\n \t\t\t/*\n@@ -2554,13 +2576,14 @@ void reuse_partial_packfile_from_bitmap(struct bitmap_index *bitmap_git,\n \t\t\t * that we do not expect to read this field.\n \t\t\t */\n \t\t\tpack_int_id = -1;\n+\t\t\tbitmap_nr = pack->num_objects;\n \t\t}\n \n \t\tif (is_pack_valid(pack)) {\n \t\t\tALLOC_GROW(packs, packs_nr + 1, packs_alloc);\n \t\t\tpacks[packs_nr].p = pack;\n \t\t\tpacks[packs_nr].pack_int_id = pack_int_id;\n-\t\t\tpacks[packs_nr].bitmap_nr = pack->num_objects;\n+\t\t\tpacks[packs_nr].bitmap_nr = bitmap_nr;\n \t\t\tpacks[packs_nr].bitmap_pos = 0;\n \t\t\tpacks[packs_nr].from_midx = bitmap_git->midx;\n \t\t\tpacks_nr++;\ndiff --git a/t/t5332-multi-pack-reuse.sh b/t/t5332-multi-pack-reuse.sh\nindex 881ce668e1..126ab9df99 100755\n--- a/t/t5332-multi-pack-reuse.sh\n+++ b/t/t5332-multi-pack-reuse.sh\n@@ -4,6 +4,7 @@ test_description='pack-objects multi-pack reuse'\n \n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-bitmap.sh\n+. \"$TEST_DIRECTORY\"/lib-pack.sh\n \n GIT_TEST_MULTI_PACK_INDEX=0\n GIT_TEST_MULTI_PACK_INDEX_WRITE_INCREMENTAL=0\n@@ -32,6 +33,14 @@ pack_position () {\n \tgrep \"$1\" objects | cut -d\" \" -f1\n }\n \n+# B as an OFS_DELTA against A at the given one-byte distance.\n+pack_obj_b_ofs_a () {\n+\tpack_obj \"$B\" \"$A\" >b-ref.tmp &&\n+\tprintf \"\\145\" &&\n+\tprintf \"\\\\$(printf \"%03o\" \"$1\")\" &&\n+\tdd if=b-ref.tmp bs=1 skip=$((1 + $(test_oid rawsz))) 2>/dev/null\n+}\n+\n # test_pack_objects_reused_all <pack-reused> <packs-reused>\n test_pack_objects_reused_all () {\n \t: >trace2.txt &&\n@@ -288,4 +297,84 @@ test_expect_success 'duplicate objects with verbatim reuse' '\n \t)\n '\n \n+test_expect_success 'reuse with intra-pack duplicate objects' '\n+\tgit init intra-pack-duplicate-objects &&\n+\t(\n+\t\tcd intra-pack-duplicate-objects &&\n+\n+\t\t# Make enough objects to exercise whole-word reuse.\n+\t\ttest_commit_bulk 20 &&\n+\t\ttest_commit --printf A a \"\\7\\0\" &&\n+\t\ttest_commit --printf B b \"\\7\\76\" &&\n+\n+\t\tobjects_nr=$(git rev-list --count --objects --all) &&\n+\t\tgit rev-list --objects --all |\n+\t\tcut -d\" \" -f1 >objects &&\n+\t\tA=$(test_oid packlib_7_0) &&\n+\t\tB=$(test_oid packlib_7_76) &&\n+\t\tgrep -v -e \"^$A$\" -e \"^$B$\" objects >rest &&\n+\t\tpack_obj \"$A\" >a-full &&\n+\t\tpack_obj \"$B\" >b-full &&\n+\t\twhile read oid\n+\t\tdo\n+\t\t\tpack_obj \"$oid\" || exit 1\n+\t\tdone <rest >rest.entries &&\n+\t\t{\n+\t\t\t# Arrange the pack as A, B, A, C..., so that physical\n+\t\t\t# positions diverge from MIDX pseudo-pack order.\n+\t\t\tpack_header $((objects_nr + 1)) &&\n+\t\t\tcat a-full b-full a-full rest.entries\n+\t\t} >duplicate.pack &&\n+\t\tpack_trailer duplicate.pack &&\n+\t\tclear_packs &&\n+\t\tgit index-pack --stdin <duplicate.pack &&\n+\n+\t\tgit multi-pack-index write --bitmap &&\n+\t\tgit config pack.allowPackReuse single &&\n+\t\ttest_pack_objects_reused_all \"$objects_nr\" 1 &&\n+\t\trm -f got.idx &&\n+\t\ttest_env GIT_TEST_MIDX_READ_BTMP=false \\\n+\t\t\ttest_pack_objects_reused_all 0 0\n+\t)\n+'\n+\n+test_expect_success 'omit delta whose duplicate base is not selected' '\n+\t(\n+\t\tcd intra-pack-duplicate-objects &&\n+\n+\t\tA=$(test_oid packlib_7_0) &&\n+\t\tB=$(test_oid packlib_7_76) &&\n+\t\tobjects_nr=$(wc -l <objects) &&\n+\n+\t\ta_size=$(wc -c <a-full) &&\n+\t\ttest \"$((2 * a_size))\" -lt 128 &&\n+\n+\t\t# The .idx order of duplicate OIDs is unspecified. Try a delta\n+\t\t# against each copy and keep the pack whose base was omitted.\n+\t\tfor distance in \"$((2 * a_size))\" \"$a_size\"\n+\t\tdo\n+\t\t\tbase_offset=$((12 + 2 * a_size - distance)) &&\n+\t\t\tpack_obj_b_ofs_a \"$distance\" >b-delta &&\n+\t\t\t{\n+\t\t\t\tpack_header \"$((objects_nr + 1))\" &&\n+\t\t\t\tcat a-full a-full b-delta rest.entries\n+\t\t\t} >candidate.pack &&\n+\t\t\tpack_trailer candidate.pack &&\n+\t\t\tclear_packs &&\n+\t\t\tgit index-pack --stdin <candidate.pack &&\n+\t\t\tgit multi-pack-index write --bitmap || return 1\n+\n+\t\t\tselected_offset=$(\n+\t\t\t\ttest-tool read-midx --show-objects \"$objdir\" |\n+\t\t\t\tawk -v oid=\"$A\" \"\\$1 == oid { print \\$2 }\"\n+\t\t\t) || return 1\n+\t\t\ttest -n \"$selected_offset\" || return 1\n+\t\t\ttest \"$selected_offset\" = \"$base_offset\" || break\n+\t\tdone &&\n+\t\ttest \"$selected_offset\" != \"$base_offset\" &&\n+\n+\t\ttest_pack_objects_reused_all \"$((objects_nr - 1))\" 1\n+\t)\n+'\n+\n test_done\n-- \n2.55.0.383.gde07827a19\n"},{"id":"548931","messageId":"xmqqecgs3vg6.fsf@gitster.g","threadId":"66058","inReplyTo":"cover.1784927134.git.ttaylorr@openai.com","subject":"Re: [PATCH 0/5] packfile: harden handling of packs with duplicate entries","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-24T21:54:17Z","receivedAt":"2026-07-24T21:54:20Z","isPatch":true,"body":"Taylor Blau <ttaylorr@openai.com> writes:\n\n> Packfiles containing duplicate object entries are unusual, but Git\n> already accepts them outside of strict indexing.\n\nThat is looser than what we intended.  \"unusual\" -> \"invalid\".  It\nis just like Git does not immediately complain until \"git fsck\",\n\"git repack\", or otherwise \"git\" tries to use the object if you\ncorrupted loose object files under .git/objects/??/ directories.\n\nDetecting such a problematic pack early before the problem spreads\nis a very welcome change.\n"}]}