On Fri, Dec 23, 2016 at 02:43:35PM -0500, David Turner wrote:
> The bitmap index only works for single packs, so requesting an
> incremental repack with bitmap indexes makes no sense.
OK. I doubt this will come up much after the git-gc change from the
first patch. And it should already be printing a warning in this case
anyway[1]. But I do not mind turning the warning into a hard failure; it
may make things more obvious for the user. The only weird thing is that
the bitmap option may come from the config, so this effectively means
you can never run "git repack -d" in a repo with bitmap config. I'm not
sure if anybody cares or not, with the new git-gc patch.
[1] Technically "repack -d" could actually create bitmaps (with no
warning) in a repo that has no existing packs. But that's probably
an uninteresting corner case (the user should say "-ad" there, and
the "-a" ends up being a noop).
> @@ -206,6 +212,9 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
> if (pack_kept_objects < 0)
> pack_kept_objects = write_bitmaps;
>
> + if (write_bitmaps && !(pack_everything & ALL_INTO_ONE))
> + die(incremental_bitmap_conflict_error);
I double-checked that ALL_INTO_ONE covers both -A and -a, which I think
should be sufficient here.
> -test_expect_success 'incremental repack cannot create bitmaps' '
> +test_expect_success 'incremental repack fails when bitmaps are requested' '
> test_commit more-1 &&
> find .git/objects/pack -name "*.bitmap" >expect &&
> - git repack -d &&
> + test_must_fail git repack -d 2>err &&
> + test_i18ngrep "Incremental repacks are incompatible with bitmap" err &&
> find .git/objects/pack -name "*.bitmap" >actual &&
> test_cmp expect actual
> '
The final `find` is probably overkill now after we know the command
failed and reported the error. But it does not hurt to be thorough. :)
-Peff