Re: [PATCH 2/3] repack: silence warnings when auto-enabled bitmaps cannot be built
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 31, 2019, 20:26 UTC
- Message-ID
- <xmqqftmmosrf.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <20190731053927.GB16941@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 13 quoted lines
> @@ -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),The receiving end of this communication is pretty easy to follow. I'd have named an option to trigger "if possible" behaviour after that "if possible" phrase and not "quiet", but this is entirely internal that it does not matter.
Show 17 quoted lines
> diff --git a/builtin/repack.c b/builtin/repack.c
> index 30982ed2a2..db93ca3660 100644
> --- 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;This side of communication is a bit harder to follow, but not impossible ;-) We leave it "negative" to signal "the user did not specify, but we enabled it by default" here.
> } > if (pack_kept_objects < 0) > - pack_kept_objects = write_bitmaps; > + pack_kept_objects = !!write_bitmaps;
And non-zero write_bitmaps replaces "true" in the older world.
> if (write_bitmaps && !(pack_everything & ALL_INTO_ONE))
So this does not have to change.
Show 10 quoted lines
> 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");
And "enabled by user" and "enabled by default" are mapped to the two options.
All makes sense.
Thanks.