pack-objects: turn off bitmaps when skipping objects

The pack bitmap format requires that we have a single bit
for each object in the pack, and that each object's bitmap
represents its complete set of reachable objects. Therefore
we have no way to represent the bitmap of an object which
references objects outside the pack.

We notice this problem while generating the bitmaps, as we
try to find the offset of a particular object and realize
that we do not have it. In this case we die, and neither the
bitmap nor the pack is generated. This is correct, but
perhaps a little unfriendly. If you have bitmaps turned on
in the config, many repacks will fail which would otherwise
succeed. E.g., incremental repacks, repacks with "-l" when
you have alternates, ".keep" files.

Instead, this patch notices early that we are omitting some
objects from the pack and turns off bitmaps (with a
warning). Note that this is not strictly correct, as it's
possible that the object being omitted is not reachable from
any other object in the pack. In practice, this is almost
never the case, and there are two advantages to doing it
this way:

1. The code is much simpler, as we do not have to cleanly
abort the bitmap-generation process midway through.

2. We do not waste time partially generating bitmaps only
to find out that some object deep in the history is not
being packed.

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 fd741970e61c674c628f2e9c8ae48e08d4996961..50d794f5d9a2c53ca596e8f04a0b61809cc91fd0 100644 (file)
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1000,6 +1000,10 @@ static void create_object_entry(const unsigned char *sha1,
        entry->no_try_delta = no_try_delta;
 }
 
+static const char no_closure_warning[] = N_(
+"disabling bitmap writing, as some objects are not being packed"
+);
+
 static int add_object_entry(const unsigned char *sha1, enum object_type type,
                            const char *name, int exclude)
 {
@@ -1010,8 +1014,14 @@ static int add_object_entry(const unsigned char *sha1, enum object_type type,
        if (have_duplicate_entry(sha1, exclude, &index_pos))
                return 0;
 
-       if (!want_object_in_pack(sha1, exclude, &found_pack, &found_offset))
+       if (!want_object_in_pack(sha1, 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));
+                       write_bitmap_index = 0;
+               }
                return 0;
+       }
 
        create_object_entry(sha1, type, pack_name_hash(name),
                            exclude, name && no_try_delta(name),

diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh
index d3a3afaba821f8b70c5f365f1046ba7a707e5085..f13525caa309e48ab32f89066a3e9661dfc25e09 100755 (executable)
--- a/t/t5310-pack-bitmaps.sh
+++ b/t/t5310-pack-bitmaps.sh
@@ -91,7 +91,10 @@ test_expect_success 'fetch (partial bitmap)' '
 
 test_expect_success 'incremental repack cannot create bitmaps' '
        test_commit more-1 &&
-       test_must_fail git repack -d
+       find .git/objects/pack -name "*.bitmap" >expect &&
+       git repack -d &&
+       find .git/objects/pack -name "*.bitmap" >actual &&
+       test_cmp expect actual
 '
 
 test_expect_success 'incremental repack can disable bitmaps' '