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>

Jeff King committed Jul 31, 2019 at 01:39 UTC 25575015cafbbb8554ff6ac82236c2a062055ae6
3 files changed +36 -11
builtin/pack-objects.c
+16 -5
@@ -96,7 +96,11 @@ static off_t reuse_packfile_offset;
96
97 static int use_bitmap_index_default = 1;
98 static int use_bitmap_index = -1;
99 -static int write_bitmap_index;
99 +static enum {
100 + WRITE_BITMAP_FALSE = 0,
101 + WRITE_BITMAP_QUIET,
102 + WRITE_BITMAP_TRUE,
103 +} write_bitmap_index;
104 static uint16_t write_bitmap_options = BITMAP_OPT_HASH_CACHE;
105
106 static int exclude_promisor_objects;
@@ -892,7 +896,8 @@ static void write_pack_file(void)
896 nr_written, oid.hash, offset);
897 close(fd);
898 if (write_bitmap_index) {
895 - warning(_(no_split_warning));
899 + if (write_bitmap_index != WRITE_BITMAP_QUIET)
900 + warning(_(no_split_warning));
901 write_bitmap_index = 0;
902 }
903 }
@@ -1176,7 +1181,8 @@ static int add_object_entry(const struct object_id *oid, enum object_type type,
1181 if (!want_object_in_pack(oid, exclude, &found_pack, &found_offset)) {
1182 /* The pack is missing an object, so it will not have closure */
1183 if (write_bitmap_index) {
1179 - warning(_(no_closure_warning));
1184 + if (write_bitmap_index != WRITE_BITMAP_QUIET)
1185 + warning(_(no_closure_warning));
1186 write_bitmap_index = 0;
1187 }
1188 return 0;
@@ -3313,8 +3319,13 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
3319 N_("do not hide commits by grafts"), 0),
3320 OPT_BOOL(0, "use-bitmap-index", &use_bitmap_index,
3321 N_("use a bitmap index if available to speed up counting objects")),
3316 - OPT_BOOL(0, "write-bitmap-index", &write_bitmap_index,
3317 - N_("write a bitmap index together with the pack index")),
3322 + OPT_SET_INT(0, "write-bitmap-index", &write_bitmap_index,
3323 + N_("write a bitmap index together with the pack index"),
3324 + WRITE_BITMAP_TRUE),
3325 + OPT_SET_INT_F(0, "write-bitmap-index-quiet",
3326 + &write_bitmap_index,
3327 + N_("write a bitmap index if possible"),
3328 + WRITE_BITMAP_QUIET, PARSE_OPT_HIDDEN),
3329 OPT_PARSE_LIST_OBJECTS_FILTER(&filter_options),
3330 { OPTION_CALLBACK, 0, "missing", NULL, N_("action"),
3331 N_("handling for missing objects"), PARSE_OPT_NONEG,
builtin/repack.c
+9 -6
@@ -345,13 +345,14 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
345 die(_("--keep-unreachable and -A are incompatible"));
346
347 if (write_bitmaps < 0) {
348 - write_bitmaps = (pack_everything & ALL_INTO_ONE) &&
349 - is_bare_repository() &&
350 - keep_pack_list.nr == 0 &&
351 - !has_pack_keep_file();
348 + if (!(pack_everything & ALL_INTO_ONE) ||
349 + !is_bare_repository() ||
350 + keep_pack_list.nr != 0 ||
351 + has_pack_keep_file())
352 + write_bitmaps = 0;
353 }
354 if (pack_kept_objects < 0)
354 - pack_kept_objects = write_bitmaps;
355 + pack_kept_objects = !!write_bitmaps;
356
357 if (write_bitmaps && !(pack_everything & ALL_INTO_ONE))
358 die(_(incremental_bitmap_conflict_error));
@@ -375,8 +376,10 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
376 argv_array_push(&cmd.args, "--indexed-objects");
377 if (repository_format_partial_clone)
378 argv_array_push(&cmd.args, "--exclude-promisor-objects");
378 - if (write_bitmaps)
379 + if (write_bitmaps > 0)
380 argv_array_push(&cmd.args, "--write-bitmap-index");
381 + else if (write_bitmaps < 0)
382 + argv_array_push(&cmd.args, "--write-bitmap-index-quiet");
383 if (use_delta_islands)
384 argv_array_push(&cmd.args, "--delta-islands");
385
t/t7700-repack.sh
+11
@@ -250,4 +250,15 @@ test_expect_success 'no bitmaps created if .keep files present' '
250 test_must_be_empty actual
251 '
252
253 +test_expect_success 'auto-bitmaps do not complain if unavailable' '
254 + test_config -C bare.git pack.packSizeLimit 1M &&
255 + blob=$(test-tool genrandom big $((1024*1024)) |
256 + git -C bare.git hash-object -w --stdin) &&
257 + git -C bare.git update-ref refs/tags/big $blob &&
258 + git -C bare.git repack -ad 2>stderr &&
259 + test_must_be_empty stderr &&
260 + find bare.git/objects/pack -type f -name "*.bitmap" >actual &&
261 + test_must_be_empty actual
262 +'
263 +
264 test_done