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>

diff --git a/builtin/repack.c b/builtin/repack.c
index caca11392713eb92816d1e503f286bfe0d9be78a..73250b243157d75530067cdd9238b90adb3004a6 100644 (file)
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -89,6 +89,17 @@ static void remove_pack_on_signal(int signo)
        raise(signo);
 }
 
+static int has_pack_keep_file(void)
+{
+       struct packed_git *p;
+
+       for (p = get_all_packs(the_repository); p; p = p->next) {
+               if (p->pack_keep)
+                       return 1;
+       }
+       return 0;
+}
+
 /*
  * Adds all packs hex strings to the fname list, which do not
  * 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)
            (unpack_unreachable || (pack_everything & LOOSEN_UNREACHABLE)))
                die(_("--keep-unreachable and -A are incompatible"));
 
-       if (write_bitmaps < 0)
+       if (write_bitmaps < 0) {
                write_bitmaps = (pack_everything & ALL_INTO_ONE) &&
-                                is_bare_repository();
+                                is_bare_repository() &&
+                                keep_pack_list.nr == 0 &&
+                                !has_pack_keep_file();
+       }
        if (pack_kept_objects < 0)
                pack_kept_objects = write_bitmaps;
 

diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
index 86d05160a3589cefd7ad21dbf64c655658f26342..0e9af832c9790f3e329fc59d61b8bc177c151b48 100755 (executable)
--- a/t/t7700-repack.sh
+++ b/t/t7700-repack.sh
@@ -239,4 +239,14 @@ test_expect_success 'bitmaps can be disabled on bare repos' '
        test -z "$bitmap"
 '
 
+test_expect_success 'no bitmaps created if .keep files present' '
+       pack=$(ls bare.git/objects/pack/*.pack) &&
+       test_path_is_file "$pack" &&
+       keep=${pack%.pack}.keep &&
+       >"$keep" &&
+       git -C bare.git repack -ad &&
+       find bare.git/objects/pack/ -type f -name "*.bitmap" >actual &&
+       test_must_be_empty actual
+'
+
 test_done