@@ -661,6 +661,35 @@ static int write_midx_included_packs(struct string_list *include,
return finish_command(&cmd);
}
+static void remove_redundant_bitmaps(struct string_list *include,
+ const char *packdir)
+{
+ struct strbuf path = STRBUF_INIT;
+ struct string_list_item *item;
+ size_t packdir_len;
+
+ strbuf_addstr(&path, packdir);
+ strbuf_addch(&path, '/');
+ packdir_len = path.len;
+
+ /*
+ * Remove any pack bitmaps corresponding to packs which are now
+ * included in the MIDX.
+ */
+ for_each_string_list_item(item, include) {
+ strbuf_addstr(&path, item->string);
+ strbuf_strip_suffix(&path, ".idx");
+ strbuf_addstr(&path, ".bitmap");
+
+ if (unlink(path.buf) && errno != ENOENT)
+ warning_errno(_("could not remove stale bitmap: %s"),
+ path.buf);
+
+ strbuf_setlen(&path, packdir_len);
+ }
+ strbuf_release(&path);
+}
+
static int write_cruft_pack(const struct pack_objects_args *args,
const char *pack_prefix,
struct string_list *names,
@@ -1059,6 +1088,9 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
refs_snapshot ? get_tempfile_path(refs_snapshot) : NULL,
show_progress, write_bitmaps > 0);
+ if (!ret && write_bitmaps)
+ remove_redundant_bitmaps(&include, packdir);
+
string_list_clear(&include, 0);
if (ret)
@@ -426,6 +426,27 @@ test_expect_success '--write-midx -b packs non-kept objects' '
)
'
+test_expect_success '--write-midx removes stale pack-based bitmaps' '
+ rm -fr repo &&
+ git init repo &&
+ test_when_finished "rm -fr repo" &&
+ (
+ cd repo &&
+ test_commit base &&
+ GIT_TEST_MULTI_PACK_INDEX=0 git repack -Ab &&
+
+ pack_bitmap=$(ls $objdir/pack/pack-*.bitmap) &&
+ test_path_is_file "$pack_bitmap" &&
+
+ test_commit tip &&
+ GIT_TEST_MULTI_PACK_INDEX=0 git repack -bm &&
+
+ test_path_is_file $midx &&
+ test_path_is_file $midx-$(midx_checksum $objdir).bitmap &&
+ test_path_is_missing $pack_bitmap
+ )
+'
+
test_expect_success TTY '--quiet disables progress' '
test_terminal env GIT_PROGRESS_DELAY=0 \
git -C midx repack -ad --quiet --write-midx 2>stderr &&
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> --- A small reroll to address a pair of comments from Peff. Range-diff against v1: 1: 393fd4c6db ! 1: 1e0ef7ee7f builtin/repack.c: remove redundant pack-based bitmaps @@ builtin/repack.c: static int write_midx_included_packs(struct string_list *inclu + strbuf_addstr(&path, ".bitmap"); + + if (unlink(path.buf) && errno != ENOENT) -+ die_errno(_("could not remove stale bitmap: %s"), -+ path.buf); ++ warning_errno(_("could not remove stale bitmap: %s"), ++ path.buf); + + strbuf_setlen(&path, packdir_len); + } @@ builtin/repack.c: int cmd_repack(int argc, const char **argv, const char *prefix refs_snapshot ? get_tempfile_path(refs_snapshot) : NULL, show_progress, write_bitmaps > 0); -+ if (ret) { -+ string_list_clear(&include, 0); -+ return ret; -+ } -+ -+ if (write_bitmaps) ++ if (!ret && write_bitmaps) + remove_redundant_bitmaps(&include, packdir); + string_list_clear(&include, 0); -- -- if (ret) -- return ret; - } - reprepare_packed_git(the_repository); + if (ret) ## t/t7700-repack.sh ## @@ t/t7700-repack.sh: test_expect_success '--write-midx -b packs non-kept objects' ' builtin/repack.c | 32 ++++++++++++++++++++++++++++++++ t/t7700-repack.sh | 21 +++++++++++++++++++++ 2 files changed, 53 insertions(+) -- 2.38.0.16.g393fd4c6db