diff --git a/midx.c b/midx.c index 76c3f92cc3..05fe99f8ca 100644 --- a/midx.c +++ b/midx.c @@ -906,6 +906,48 @@ static int compare_pair_pos_vs_id(const void *_a, const void *_b) return b->pack_int_id - a->pack_int_id; } +/* + * Return whether the pack index contains an entry with both "oid" and + * "offset". A pack index may contain duplicate OIDs, so an arbitrary + * OID lookup is not enough to validate a particular offset. + * + * Do not use offset_to_pack_pos() here: it may consult an optional '.rev' + * file, which is verified separately, or build an in-memory reverse index + * that remains attached to the pack. Verify the MIDX directly against its + * source pack index instead. + */ +static int pack_index_has_oid_at_offset(struct packed_git *p, + const struct object_id *oid, + off_t offset) +{ + struct object_id candidate; + uint32_t pos, i; + + if (!bsearch_pack(oid, p, &pos)) + return 0; + + if (nth_packed_object_offset(p, pos) == offset) + return 1; + + for (i = pos; i > 0; i--) { + if (nth_packed_object_id(&candidate, p, i - 1) || + !oideq(&candidate, oid)) + break; + if (nth_packed_object_offset(p, i - 1) == offset) + return 1; + } + + for (i = pos + 1; i < p->num_objects; i++) { + if (nth_packed_object_id(&candidate, p, i) || + !oideq(&candidate, oid)) + break; + if (nth_packed_object_offset(p, i) == offset) + return 1; + } + + return 0; +} + /* * Limit calls to display_progress() for performance reasons. * The interval here was arbitrarily chosen. @@ -1015,7 +1057,6 @@ int verify_midx_file(struct odb_source_packed *source, unsigned flags) for (i = 0; i < m->num_objects + m->num_objects_in_base; i++) { struct object_id oid; struct pack_entry e; - off_t m_offset, p_offset; if (i > 0 && pairs[i-1].pack_int_id != pairs[i].pack_int_id && nth_midxed_pack(m, pairs[i-1].pack_int_id)) { @@ -1040,12 +1081,16 @@ int verify_midx_file(struct odb_source_packed *source, unsigned flags) break; } - m_offset = e.offset; - p_offset = find_pack_entry_one(&oid, e.p); - - if (m_offset != p_offset) - midx_report(_("incorrect object offset for oid[%d] = %s: %"PRIx64" != %"PRIx64), - pairs[i].pos, oid_to_hex(&oid), m_offset, p_offset); + /* + * Check that the exact offset recorded in the MIDX + * belongs to this OID. A pack index may contain + * duplicate OIDs, in which case an arbitrary OID lookup + * can return a different, equally valid copy than the + * one selected by the MIDX writer. + */ + if (!pack_index_has_oid_at_offset(e.p, &oid, e.offset)) + midx_report(_("incorrect object offset for oid[%d] = %s: %"PRIx64), + pairs[i].pos, oid_to_hex(&oid), e.offset); midx_display_sparse_progress(progress, i + 1); } diff --git a/t/helper/test-find-pack.c b/t/helper/test-find-pack.c index 28d5b1fe09..51093a7030 100644 --- a/t/helper/test-find-pack.c +++ b/t/helper/test-find-pack.c @@ -11,12 +11,15 @@ * Display the path(s), one per line, of the packfile(s) containing * the given object. * + * With '--show-offset', display the offset selected by + * find_pack_entry_one() instead of the packfile path. + * * If '--check-count ' is passed, then error out if the number of * packfiles containing the object is not . */ static const char *const find_pack_usage[] = { - "test-tool find-pack [--check-count ] ", + "test-tool find-pack [--check-count ] [--show-offset] ", NULL }; @@ -24,11 +27,13 @@ int cmd__find_pack(int argc, const char **argv) { struct object_id oid; struct packed_git *p; - int count = -1, actual_count = 0; + int count = -1, actual_count = 0, show_offset = 0; const char *prefix = setup_git_directory(the_repository); struct option options[] = { OPT_INTEGER('c', "check-count", &count, "expected number of packs"), + OPT_BOOL(0, "show-offset", &show_offset, + "show matching pack offsets"), OPT_END(), }; @@ -40,8 +45,13 @@ int cmd__find_pack(int argc, const char **argv) die("cannot parse %s as an object name", argv[0]); repo_for_each_pack(the_repository, p) { - if (find_pack_entry_one(&oid, p)) { - printf("%s\n", p->pack_name); + off_t offset = find_pack_entry_one(&oid, p); + + if (offset) { + if (show_offset) + printf("%"PRIuMAX"\n", (uintmax_t)offset); + else + printf("%s\n", p->pack_name); actual_count++; } } diff --git a/t/t5308-pack-detect-duplicates.sh b/t/t5308-pack-detect-duplicates.sh index 4ff8f5b449..493ebbc4af 100755 --- a/t/t5308-pack-detect-duplicates.sh +++ b/t/t5308-pack-detect-duplicates.sh @@ -77,6 +77,20 @@ test_expect_success 'lookup in duplicated pack' ' test_cmp expect actual ' +test_expect_success 'verify MIDX containing duplicated pack objects' ' + git multi-pack-index write && + test-tool read-midx --show-objects .git/objects >midx-objects && + midx_offset=$( + awk -v oid="$LO_SHA1" "\$1 == oid { print \$2 }"