@matheusbsbs / matheus-bagliano / commits / 55d902cd61

builtin/repack.c: remove redundant pack-based bitmaps

When we write a MIDX bitmap after repacking, it is possible that the repository would be left in a state with both pack- and multi-pack reachability bitmaps. This can occur, for instance, if a pack that was kept (either by having a .keep file, or during a geometric repack in which it is not rolled up) has a bitmap file, and the repack wrote a multi-pack index and bitmap. When loading a reachability bitmap for the repository, the multi-pack one is always preferred, so the pack-based one is redundant. Let's remove it unconditionally, even if '-d' isn't passed, since there is no practical reason to keep both around. The patch below does just that. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Taylor Blau committed Oct 17, 2022 at 22:45 UTC 55d902cd61a8fd2e9b9aa4bf49b47e4dbf10ea67
2 files changed +53
builtin/repack.c
+32
@@ -661,6 +661,35 @@ static int write_midx_included_packs(struct string_list *include,
661 return finish_command(&cmd);
662 }
663
664 +static void remove_redundant_bitmaps(struct string_list *include,
665 + const char *packdir)
666 +{
667 + struct strbuf path = STRBUF_INIT;
668 + struct string_list_item *item;
669 + size_t packdir_len;
670 +
671 + strbuf_addstr(&path, packdir);
672 + strbuf_addch(&path, '/');
673 + packdir_len = path.len;
674 +
675 + /*
676 + * Remove any pack bitmaps corresponding to packs which are now
677 + * included in the MIDX.
678 + */
679 + for_each_string_list_item(item, include) {
680 + strbuf_addstr(&path, item->string);
681 + strbuf_strip_suffix(&path, ".idx");
682 + strbuf_addstr(&path, ".bitmap");
683 +
684 + if (unlink(path.buf) && errno != ENOENT)
685 + warning_errno(_("could not remove stale bitmap: %s"),
686 + path.buf);
687 +
688 + strbuf_setlen(&path, packdir_len);
689 + }
690 + strbuf_release(&path);
691 +}
692 +
693 static int write_cruft_pack(const struct pack_objects_args *args,
694 const char *pack_prefix,
695 struct string_list *names,
@@ -1059,6 +1088,9 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
1088 refs_snapshot ? get_tempfile_path(refs_snapshot) : NULL,
1089 show_progress, write_bitmaps > 0);
1090
1091 + if (!ret && write_bitmaps)
1092 + remove_redundant_bitmaps(&include, packdir);
1093 +
1094 string_list_clear(&include, 0);
1095
1096 if (ret)
t/t7700-repack.sh
+21
@@ -426,6 +426,27 @@ test_expect_success '--write-midx -b packs non-kept objects' '
426 )
427 '
428
429 +test_expect_success '--write-midx removes stale pack-based bitmaps' '
430 + rm -fr repo &&
431 + git init repo &&
432 + test_when_finished "rm -fr repo" &&
433 + (
434 + cd repo &&
435 + test_commit base &&
436 + GIT_TEST_MULTI_PACK_INDEX=0 git repack -Ab &&
437 +
438 + pack_bitmap=$(ls $objdir/pack/pack-*.bitmap) &&
439 + test_path_is_file "$pack_bitmap" &&
440 +
441 + test_commit tip &&
442 + GIT_TEST_MULTI_PACK_INDEX=0 git repack -bm &&
443 +
444 + test_path_is_file $midx &&
445 + test_path_is_file $midx-$(midx_checksum $objdir).bitmap &&
446 + test_path_is_missing $pack_bitmap
447 + )
448 +'
449 +
450 test_expect_success TTY '--quiet disables progress' '
451 test_terminal env GIT_PROGRESS_DELAY=0 \
452 git -C midx repack -ad --quiet --write-midx 2>stderr &&