repack: silence warnings when auto-enabled bitmaps cannot be built

Depending on various config options, a full repack may not be able to
build a reachability bitmap index (e.g., if pack.packSizeLimit forces us
to write multiple packs). In these cases pack-objects may write a
warning to stderr.

Since 36eba0323d (repack: enable bitmaps by default on bare repos,
2019-03-14), we may generate these warnings even when the user did not
explicitly ask for bitmaps. This has two downsides:

- it can be confusing, if they don't know what bitmaps are

- a daemonized auto-gc will write this to its log file, and the
presence of the warning may suppress further auto-gc (until
gc.logExpiry has elapsed)

Let's have repack communicate to pack-objects that the choice to turn on
bitmaps was not made explicitly by the user, which in turn allows
pack-objects to suppress these warnings.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 267c562b1f81dad2c42e2392a76ff0a269d38f65..76ce9069467e06545652a70040b79316f0a87fb9 100644 (file)
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -96,7 +96,11 @@ static off_t reuse_packfile_offset;
 
 static int use_bitmap_index_default = 1;
 static int use_bitmap_index = -1;
-static int write_bitmap_index;
+static enum {
+       WRITE_BITMAP_FALSE = 0,
+       WRITE_BITMAP_QUIET,
+       WRITE_BITMAP_TRUE,
+} write_bitmap_index;
 static uint16_t write_bitmap_options = BITMAP_OPT_HASH_CACHE;
 
 static int exclude_promisor_objects;
@@ -892,7 +896,8 @@ static void write_pack_file(void)
                                                 nr_written, oid.hash, offset);
                        close(fd);
                        if (write_bitmap_index) {
-                               warning(_(no_split_warning));
+                               if (write_bitmap_index != WRITE_BITMAP_QUIET)
+                                       warning(_(no_split_warning));
                                write_bitmap_index = 0;
                        }
                }
@@ -1176,7 +1181,8 @@ static int add_object_entry(const struct object_id *oid, enum object_type type,
        if (!want_object_in_pack(oid, exclude, &found_pack, &found_offset)) {
                /* The pack is missing an object, so it will not have closure */
                if (write_bitmap_index) {
-                       warning(_(no_closure_warning));
+                       if (write_bitmap_index != WRITE_BITMAP_QUIET)
+                               warning(_(no_closure_warning));
                        write_bitmap_index = 0;
                }
                return 0;
@@ -3313,8 +3319,13 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
                            N_("do not hide commits by grafts"), 0),
                OPT_BOOL(0, "use-bitmap-index", &use_bitmap_index,
                         N_("use a bitmap index if available to speed up counting objects")),
-               OPT_BOOL(0, "write-bitmap-index", &write_bitmap_index,
-                        N_("write a bitmap index together with the pack index")),
+               OPT_SET_INT(0, "write-bitmap-index", &write_bitmap_index,
+                           N_("write a bitmap index together with the pack index"),
+                           WRITE_BITMAP_TRUE),
+               OPT_SET_INT_F(0, "write-bitmap-index-quiet",
+                             &write_bitmap_index,
+                             N_("write a bitmap index if possible"),
+                             WRITE_BITMAP_QUIET, PARSE_OPT_HIDDEN),
                OPT_PARSE_LIST_OBJECTS_FILTER(&filter_options),
                { OPTION_CALLBACK, 0, "missing", NULL, N_("action"),
                  N_("handling for missing objects"), PARSE_OPT_NONEG,

diff --git a/builtin/repack.c b/builtin/repack.c
index 30982ed2a2aa750b68f1613e8f14b049708b70c4..db93ca3660812e42c06d2b1dff00df1466db105f 100644 (file)
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -345,13 +345,14 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
                die(_("--keep-unreachable and -A are incompatible"));
 
        if (write_bitmaps < 0) {
-               write_bitmaps = (pack_everything & ALL_INTO_ONE) &&
-                                is_bare_repository() &&
-                                keep_pack_list.nr == 0 &&
-                                !has_pack_keep_file();
+               if (!(pack_everything & ALL_INTO_ONE) ||
+                   !is_bare_repository() ||
+                   keep_pack_list.nr != 0 ||
+                   has_pack_keep_file())
+                       write_bitmaps = 0;
        }
        if (pack_kept_objects < 0)
-               pack_kept_objects = write_bitmaps;
+               pack_kept_objects = !!write_bitmaps;
 
        if (write_bitmaps && !(pack_everything & ALL_INTO_ONE))
                die(_(incremental_bitmap_conflict_error));
@@ -375,8 +376,10 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
        argv_array_push(&cmd.args, "--indexed-objects");
        if (repository_format_partial_clone)
                argv_array_push(&cmd.args, "--exclude-promisor-objects");
-       if (write_bitmaps)
+       if (write_bitmaps > 0)
                argv_array_push(&cmd.args, "--write-bitmap-index");
+       else if (write_bitmaps < 0)
+               argv_array_push(&cmd.args, "--write-bitmap-index-quiet");
        if (use_delta_islands)
                argv_array_push(&cmd.args, "--delta-islands");
 

diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
index 8d9a358df8223a4fec6c09818aa357ef73362552..54f815b8b9e101a30312ef390effee2c65d1a635 100755 (executable)
--- a/t/t7700-repack.sh
+++ b/t/t7700-repack.sh
@@ -250,4 +250,15 @@ test_expect_success 'no bitmaps created if .keep files present' '
        test_must_be_empty actual
 '
 
+test_expect_success 'auto-bitmaps do not complain if unavailable' '
+       test_config -C bare.git pack.packSizeLimit 1M &&
+       blob=$(test-tool genrandom big $((1024*1024)) |
+              git -C bare.git hash-object -w --stdin) &&
+       git -C bare.git update-ref refs/tags/big $blob &&
+       git -C bare.git repack -ad 2>stderr &&
+       test_must_be_empty stderr &&
+       find bare.git/objects/pack -type f -name "*.bitmap" >actual &&
+       test_must_be_empty actual
+'
+
 test_done