diff --git a/builtin/mktree.c b/builtin/mktree.c index 4084e324768b94..45ae2af3b5f13e 100644 --- a/builtin/mktree.c +++ b/builtin/mktree.c @@ -125,7 +125,6 @@ static void mktree_line(struct repository *repo, char *buf, int nul_term_line, i oi.typep = &obj_type; if (odb_read_object_info_extended(repo->objects, &oid, &oi, OBJECT_INFO_LOOKUP_REPLACE | - OBJECT_INFO_QUICK | OBJECT_INFO_SKIP_FETCH_OBJECT) < 0) obj_type = -1; @@ -200,8 +199,11 @@ int cmd_mktree(int ac, puts(oid_to_hex(&oid)); fflush(stdout); } + for (int i = 0; i < used; i++) + free(entries[i]); used=0; /* reset tree entry buffer for re-use in batch mode */ } + free(entries); strbuf_release(&sb); return 0; diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 399acd0f225d93..751d5d34498c88 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -1786,7 +1786,7 @@ static int want_object_in_pack_mtime(const struct object_id *oid, struct multi_pack_index *m = get_multi_pack_index(files->packed); struct pack_entry e; - if (m && fill_midx_entry(m, oid, &e, NULL)) { + if (m && midx_fill_entry(m, oid, &e, NULL) == MIDX_FILL_HIT) { want = want_object_in_pack_one(e.p, oid, exclude, found_pack, found_offset, found_mtime); if (want != -1) return want; diff --git a/midx.c b/midx.c index 37f082dbdd5558..6d1c548e3dae8e 100644 --- a/midx.c +++ b/midx.c @@ -589,23 +589,23 @@ uint32_t nth_midxed_pack_int_id(struct multi_pack_index *m, uint32_t pos) (off_t)pos * MIDX_CHUNK_OFFSET_WIDTH); } -int fill_midx_entry(struct multi_pack_index *m, - const struct object_id *oid, - struct pack_entry *e, - struct packed_git **bad_pack) +enum midx_fill_result midx_fill_entry(struct multi_pack_index *m, + const struct object_id *oid, + struct pack_entry *e, + struct packed_git **bad_pack) { uint32_t pos; uint32_t pack_int_id; struct packed_git *p; if (!bsearch_midx(oid, m, &pos)) - return 0; + return MIDX_FILL_MISS; midx_for_object(&m, pos); pack_int_id = nth_midxed_pack_int_id(m, pos); if (prepare_midx_pack(m, pack_int_id)) - return 0; + return MIDX_FILL_OWNER_UNAVAILABLE; p = m->packs[pack_int_id - m->num_packs_in_base]; /* @@ -616,19 +616,19 @@ int fill_midx_entry(struct multi_pack_index *m, * loaded! */ if (!is_pack_valid(p)) - return 0; + return MIDX_FILL_OWNER_UNAVAILABLE; if (oidset_size(&p->bad_objects) && oidset_contains(&p->bad_objects, oid)) { if (bad_pack && !*bad_pack) *bad_pack = p; - return 0; + return MIDX_FILL_MISS; } e->offset = nth_midxed_offset(m, pos); e->p = p; - return 1; + return MIDX_FILL_HIT; } /* Match "foo.idx" against either "foo.pack" _or_ "foo.idx". */ @@ -1032,7 +1032,7 @@ int verify_midx_file(struct odb_source_packed *source, unsigned flags) nth_midxed_object_oid(&oid, m, pairs[i].pos); - if (!fill_midx_entry(m, &oid, &e, NULL)) { + if (midx_fill_entry(m, &oid, &e, NULL) != MIDX_FILL_HIT) { midx_report(_("failed to load pack entry for oid[%d] = %s"), pairs[i].pos, oid_to_hex(&oid)); continue; diff --git a/midx.h b/midx.h index 1f2f2d53214da5..4b768769b98a10 100644 --- a/midx.h +++ b/midx.h @@ -117,8 +117,25 @@ uint32_t nth_midxed_pack_int_id(struct multi_pack_index *m, uint32_t pos); struct object_id *nth_midxed_object_oid(struct object_id *oid, struct multi_pack_index *m, uint32_t n); -int fill_midx_entry(struct multi_pack_index *m, const struct object_id *oid, - struct pack_entry *e, struct packed_git **bad_pack); +/* + * Result of looking an object up in a multi-pack-index. MIDX_FILL_HIT means + * "e was filled in"; the two miss variants distinguish an object the midx does + * not know about (MIDX_FILL_MISS) from one it does know about but whose owning + * pack we can no longer open (MIDX_FILL_OWNER_UNAVAILABLE -- the signature of a + * concurrent repack having removed that pack). A known-bad (corrupt) object + * reports MIDX_FILL_MISS but also sets *bad_pack, if provided, to the owning + * pack so the caller can tell "corrupt" apart from "absent". + */ +enum midx_fill_result { + MIDX_FILL_MISS = 0, + MIDX_FILL_HIT, + MIDX_FILL_OWNER_UNAVAILABLE, +}; + +enum midx_fill_result midx_fill_entry(struct multi_pack_index *m, + const struct object_id *oid, + struct pack_entry *e, + struct packed_git **bad_pack); int midx_contains_pack(struct multi_pack_index *m, const char *idx_or_pack_name); int midx_layer_contains_pack(struct multi_pack_index *m, diff --git a/odb/source-packed.c b/odb/source-packed.c index 1a12a605dbc62e..90d88c0a121c5e 100644 --- a/odb/source-packed.c +++ b/odb/source-packed.c @@ -17,13 +17,18 @@ static int find_pack_entry(struct odb_source_packed *store, const struct object_id *oid, struct pack_entry *e, + enum object_info_flags flags, struct packed_git **bad_pack) { struct packfile_list_entry *l; + enum midx_fill_result midx_result = MIDX_FILL_MISS; odb_source_prepare(&store->base, 0); - if (store->midx && fill_midx_entry(store->midx, oid, e, bad_pack)) - return 1; + if (store->midx) { + midx_result = midx_fill_entry(store->midx, oid, e, bad_pack); + if (midx_result == MIDX_FILL_HIT) + return 1; + } for (l = store->packs.head; l; l = l->next) { struct packed_git *p = l->pack; @@ -35,6 +40,33 @@ static int find_pack_entry(struct odb_source_packed *store, } } + /* + * Recovery for a concurrent-repack race: a stale MIDX may still name a + * vanished owning pack even though the object survives in another pack + * the same MIDX covers. The regular fallback above skips MIDX-covered + * packs, and repreparing the on-disk pack set does not reload the + * borrowed, cached MIDX, so scan its packs directly for the survivor. + * + * Do this only on the second read, by which point repreparing packs has + * already had a chance to find an object merely relocated into a new, + * uncovered pack; only a genuine hidden duplicate reaches here. + */ + if (midx_result == MIDX_FILL_OWNER_UNAVAILABLE && + (flags & OBJECT_INFO_SECOND_READ)) { + struct multi_pack_index *m = store->midx; + uint32_t i; + + for (i = 0; i < m->num_packs + m->num_packs_in_base; i++) { + struct packed_git *p; + + if (prepare_midx_pack(m, i)) + continue; + p = nth_midxed_pack(m, i); + if (p && packfile_fill_entry(p, oid, e, bad_pack)) + return 1; + } + } + return 0; } @@ -57,7 +89,7 @@ static enum odb_read_status odb_source_packed_read_object_info(struct odb_source if (flags & OBJECT_INFO_SECOND_READ) odb_source_prepare(source, ODB_PREPARE_FLUSH_CACHES); - if (!find_pack_entry(packed, oid, &e, &bad_pack)) { + if (!find_pack_entry(packed, oid, &e, flags, &bad_pack)) { /* * The lookup may have failed because the object is known to be * corrupt in one of the packfiles. Report the object as @@ -105,7 +137,7 @@ static int odb_source_packed_read_object_stream(struct odb_read_stream **out, struct odb_source_packed *packed = odb_source_packed_downcast(source); struct pack_entry e; - if (!find_pack_entry(packed, oid, &e, NULL)) + if (!find_pack_entry(packed, oid, &e, 0, NULL)) return -1; return packfile_read_object_stream(out, oid, e.p, e.offset); @@ -611,7 +643,7 @@ static int odb_source_packed_freshen_object(struct odb_source *source, timesp = × } - if (!find_pack_entry(packed, oid, &e, NULL)) + if (!find_pack_entry(packed, oid, &e, 0, NULL)) return 0; if (e.p->is_cruft) return 0; diff --git a/replay.c b/replay.c index 463c900d6c7c56..33e21b20320e01 100644 --- a/replay.c +++ b/replay.c @@ -327,6 +327,13 @@ static struct commit *pick_regular_commit(struct repository *repo, merge_opt->ancestor = NULL; merge_opt->branch2 = NULL; + if (result->clean < 0) { + error(_("merge of %s onto %s failed"), + oid_to_hex(&pickme->object.oid), + oid_to_hex(&replayed_base->object.oid)); + return NULL; + } + if (!result->clean) return NULL; diff --git a/t/helper/test-read-midx.c b/t/helper/test-read-midx.c index 27a05da957afc2..9c5e30876102eb 100644 --- a/t/helper/test-read-midx.c +++ b/t/helper/test-read-midx.c @@ -82,7 +82,7 @@ static int read_midx_file(const char *object_dir, const char *checksum, for (i = 0; i < m->num_objects; i++) { nth_midxed_object_oid(&oid, m, i + m->num_objects_in_base); - fill_midx_entry(m, &oid, &e, NULL); + midx_fill_entry(m, &oid, &e, NULL); printf("%s %"PRIu64"\t%s\n", oid_to_hex(&oid), e.offset, e.p->pack_name); diff --git a/t/t1010-mktree.sh b/t/t1010-mktree.sh index 312fe6717a622f..cecba55d451f2d 100755 --- a/t/t1010-mktree.sh +++ b/t/t1010-mktree.sh @@ -69,4 +69,52 @@ test_expect_success 'mktree refuses to read ls-tree -r output (2)' ' test_must_fail git mktree pack-a && + echo "$b" | git pack-objects .git/objects/pack/pack >pack-b && + + # Drop the loose copies so the blobs resolve only through the + # packs the multi-pack-index names. + git prune-packed && + git multi-pack-index write && + printf "100644 blob %s\ta\n" "$a" >tree-a && + printf "100644 blob %s\tb\n" "$b" >tree-b && + + victim=".git/objects/pack/pack-$(cat pack-b)" && + mkfifo in out && + + # mktree --batch stays resident, so its pack view predates the + # repack below; feed it one tree at a time over a fifo. The + # subshell exit closes the fifos, letting mktree see EOF and quit. + (git mktree --batch out 2>err &) && + exec 9>in && + exec 8&9 && echo >&9 && read tree_a <&8 && + + # Mimic a concurrent repack: a replacement pack holds every + # object, and the pack for b loses its .idx (its .pack lingers), + # matching the order in which unlink_pack_path() removes files. + git cat-file --batch-all-objects --batch-check="%(objectname)" >oids && + git pack-objects .git/objects/pack/pack /dev/null && + rm -f "$victim.idx" && + + # Resolving b used to fail, as its QUICK lookup accepted the + # miss; without QUICK the reader repreps and finds b in the + # replacement pack. + cat tree-b >&9 && echo >&9 && read tree_b <&8 && + exec 9>&- && + + test -n "$tree_b" + ) +' + test_done diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh index 3353bc4a4dc6ed..12348b4a5f2e64 100755 --- a/t/t3650-replay-basics.sh +++ b/t/t3650-replay-basics.sh @@ -565,4 +565,38 @@ test_expect_success '--onto with --ref rejects multiple revision ranges' ' test_grep "cannot be used with multiple revision ranges" err ' +test_expect_success 'replay fails without segfault when objects are missing' ' + test_when_finished "rm -fr unreadable" && + git init unreadable && + ( + cd unreadable && + + test_write_lines l1 l2 l3 l4 l5 l6 l7 l8 >f && + git add f && + git commit -m base && + git branch base && + + test_write_lines l1 l2 l3 l4 l5 l6 l7 CHANGED >f && + git commit -am side && + git branch side && + + git switch -c onto base && + test_write_lines CHANGED l2 l3 l4 l5 l6 l7 l8 >f && + git commit -am onto && + + # The replay works while every object is readable. + git replay --onto onto base..side && + + # Removing the onto tree makes parse_tree() fail during the + # incore merge, driving clean < 0 with a NULL result tree. + onto_tree=$(git rev-parse onto^{tree}) && + obj=$(test_oid_to_path "$onto_tree") && + mv .git/objects/${obj} saved-tree && + + # Ensure replay gracefully handles the missing object + test_must_fail git replay --onto onto base..side 2>err && + test_grep -e "Could not read" -e "collecting merge info failed" err + ) +' + test_done diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh index 68143cb5b76952..2b8ff6f3ed28cd 100755 --- a/t/t5319-multi-pack-index.sh +++ b/t/t5319-multi-pack-index.sh @@ -1393,4 +1393,44 @@ test_expect_success 'pack.preferBitmapTips interprets patterns as hierarchy' ' ) ' +test_expect_success 'lookup recovers object whose midx-owning pack was removed' ' + test_when_finished "rm -fr repo" && + git init repo && + ( + cd repo && + + # "keep" ends up only in the big pack; "dup" is deliberately + # placed in two packs so the midx has to choose an owner. + test_commit keep && + echo duplicated-content >dup && + git add dup && + git commit -m dup && + dup_oid=$(git rev-parse HEAD:dup) && + + # Roll every object, including dup, into a single big pack. + git repack -adq && + + # Build a second, "moderate" pack that also contains dup, so dup + # now lives in two packs that the midx will cover. + moderate=$(echo "$dup_oid" | + git pack-objects --quiet $objdir/pack/pack) && + + # Attribute dup to the moderate pack in the midx. + git multi-pack-index write \ + --preferred-pack="pack-$moderate.idx" && + + # Simulate a concurrent "git repack" retiring the moderate pack: + # its files disappear, but the now-stale midx still names it as + # the owner of dup. A valid copy of dup survives in the big pack. + rm -f $objdir/pack/pack-$moderate.* && + + # The midx routes the lookup to the deleted pack, and the regular + # pack fallback skips midx-covered packs, so without recovery dup + # would appear missing even though it is physically present. + echo blob >expect && + git cat-file -t "$dup_oid" >actual && + test_cmp expect actual + ) +' + test_done