@matheusbsbs / matheus-bagliano / commits / 5d3cd09a80

midx: reject empty `--preferred-pack`'s

The soon-to-be-implemented multi-pack bitmap treats object in the first bit position specially by assuming that all objects in the pack it was selected from are also represented from that pack in the MIDX. In other words, the pack from which the first object was selected must also have all of its other objects selected from that same pack in the MIDX in case of any duplicates. But this assumption relies on the fact that there is at least one object in that pack to begin with; otherwise the object in the first bit position isn't from a preferred pack, in which case we can no longer assume that all objects in that pack were also selected from the same pack. Guard this assumption by checking the number of objects in the given preferred pack, and failing if the given pack is empty. To make sure we can safely perform this check, open any packs which are contained in an existing MIDX via prepare_midx_pack(). The same is done for new packs via the add_pack_to_midx() callback, but packs picked up from a previous MIDX will not yet have these opened. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Taylor Blau committed Aug 31, 2021 at 16:52 UTC 5d3cd09a80819009636ae85c5171d9a4057eb372
3 files changed +49 -3
Documentation/git-multi-pack-index.txt
+3 -3
@@ -37,9 +37,9 @@ write::
37 --
38 --preferred-pack=<pack>::
39 Optionally specify the tie-breaking pack used when
40 - multiple packs contain the same object. If not given,
41 - ties are broken in favor of the pack with the lowest
42 - mtime.
40 + multiple packs contain the same object. `<pack>` must
41 + contain at least one object. If not given, ties are
42 + broken in favor of the pack with the lowest mtime.
43 --
44
45 verify::
midx.c
+29
@@ -934,6 +934,25 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *
934 ctx.info[ctx.nr].pack_name = xstrdup(ctx.m->pack_names[i]);
935 ctx.info[ctx.nr].p = NULL;
936 ctx.info[ctx.nr].expired = 0;
937 +
938 + if (flags & MIDX_WRITE_REV_INDEX) {
939 + /*
940 + * If generating a reverse index, need to have
941 + * packed_git's loaded to compare their
942 + * mtimes and object count.
943 + */
944 + if (prepare_midx_pack(the_repository, ctx.m, i)) {
945 + error(_("could not load pack"));
946 + result = 1;
947 + goto cleanup;
948 + }
949 +
950 + if (open_pack_index(ctx.m->packs[i]))
951 + die(_("could not open index for %s"),
952 + ctx.m->packs[i]->pack_name);
953 + ctx.info[ctx.nr].p = ctx.m->packs[i];
954 + }
955 +
956 ctx.nr++;
957 }
958 }
@@ -961,6 +980,16 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *
980 }
981 }
982
983 + if (ctx.preferred_pack_idx > -1) {
984 + struct packed_git *preferred = ctx.info[ctx.preferred_pack_idx].p;
985 + if (!preferred->num_objects) {
986 + error(_("cannot select preferred pack %s with no objects"),
987 + preferred->pack_name);
988 + result = 1;
989 + goto cleanup;
990 + }
991 + }
992 +
993 ctx.entries = get_sorted_entries(ctx.m, ctx.info, ctx.nr, &ctx.entries_nr,
994 ctx.preferred_pack_idx);
995
t/t5319-multi-pack-index.sh
+17
@@ -305,6 +305,23 @@ test_expect_success 'midx picks objects from preferred pack' '
305 )
306 '
307
308 +test_expect_success 'preferred packs must be non-empty' '
309 + test_when_finished rm -rf preferred.git &&
310 + git init preferred.git &&
311 + (
312 + cd preferred.git &&
313 +
314 + test_commit base &&
315 + git repack -ad &&
316 +
317 + empty="$(git pack-objects $objdir/pack/pack </dev/null)" &&
318 +
319 + test_must_fail git multi-pack-index write \
320 + --preferred-pack=pack-$empty.pack 2>err &&
321 + grep "with no objects" err
322 + )
323 +'
324 +
325 test_expect_success 'verify multi-pack-index success' '
326 git multi-pack-index verify --object-dir=$objdir
327 '