repack: disable bitmaps-by-default if .keep files exist

Bitmaps aren't useful with multiple packs, and users with .keep files ended up with redundant packs when bitmaps got enabled by default in bare repos. So detect when .keep files exist and stop enabling bitmaps by default in that case. Wasteful (but otherwise harmless) race conditions with .keep files documented by Jeff King still apply and there's a chance we'd still end up with redundant data on the FS: https://public-inbox.org/git/20190623224244.GB1100@sigill.intra.peff.net/ v2: avoid subshell in test case, be multi-index aware Fixes: 36eba0323d3288a8 ("repack: enable bitmaps by default on bare repos") Signed-off-by: Eric Wong <e@80x24.org> Helped-by: Jeff King <peff@peff.net> Reported-by: Janos Farkas <chexum@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Eric Wong committed Jun 29, 2019 at 19:13 UTC 7328482253469923baddb6a3061a16fb613cd523
2 files changed +26 -2
builtin/repack.c
+16 -2
@@ -89,6 +89,17 @@ static void remove_pack_on_signal(int signo)
89 raise(signo);
90 }
91
92 +static int has_pack_keep_file(void)
93 +{
94 + struct packed_git *p;
95 +
96 + for (p = get_all_packs(the_repository); p; p = p->next) {
97 + if (p->pack_keep)
98 + return 1;
99 + }
100 + return 0;
101 +}
102 +
103 /*
104 * Adds all packs hex strings to the fname list, which do not
105 * have a corresponding .keep file. These packs are not to
@@ -343,9 +354,12 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
354 (unpack_unreachable || (pack_everything & LOOSEN_UNREACHABLE)))
355 die(_("--keep-unreachable and -A are incompatible"));
356
346 - if (write_bitmaps < 0)
357 + if (write_bitmaps < 0) {
358 write_bitmaps = (pack_everything & ALL_INTO_ONE) &&
348 - is_bare_repository();
359 + is_bare_repository() &&
360 + keep_pack_list.nr == 0 &&
361 + !has_pack_keep_file();
362 + }
363 if (pack_kept_objects < 0)
364 pack_kept_objects = write_bitmaps;
365
t/t7700-repack.sh
+10
@@ -239,4 +239,14 @@ test_expect_success 'bitmaps can be disabled on bare repos' '
239 test -z "$bitmap"
240 '
241
242 +test_expect_success 'no bitmaps created if .keep files present' '
243 + pack=$(ls bare.git/objects/pack/*.pack) &&
244 + test_path_is_file "$pack" &&
245 + keep=${pack%.pack}.keep &&
246 + >"$keep" &&
247 + git -C bare.git repack -ad &&
248 + find bare.git/objects/pack/ -type f -name "*.bitmap" >actual &&
249 + test_must_be_empty actual
250 +'
251 +
252 test_done