{"thread":{"id":"37749","subject":"[PATCH] pack-objects: turn off bitmaps when we split packs","startedAt":"2014-10-17T01:11:43Z","lastAt":"2014-10-17T01:11:43Z","messageCount":1,"participants":["Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"250793","messageId":"20141017011142.GA10462@peff.net","threadId":"37749","inReplyTo":null,"subject":"[PATCH] pack-objects: turn off bitmaps when we split packs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-10-17T01:11:43Z","receivedAt":"2014-10-17T01:11:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"If a pack.packSizeLimit is set, we may split the pack data\nacross multiple packfiles. This means we cannot generate\n.bitmap files, as they require that all of the reachable\nobjects are in the same pack. We check that condition when\nwe are generating the list of objects to pack (and disable\nbitmaps if we are not packing everything), but we forgot to\nupdate it when we notice that we needed to split (which\ndoesn't happen until the actual write phase).\n\nThe resulting bitmaps are quite bogus (they mention entries\nthat do not exist in the pack!) and can cause a fetch or\npush to send insufficient objects.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nYou should generally avoid splitting packs anyway, as it introduces\nother inefficiencies, so I'm not really concerned about making bitmaps\nwork here (besides which, it would require a big change to the on-disk\nformat). The important thing here is avoiding creating the broken\nbitmaps.\n\n builtin/pack-objects.c  | 1 +\n t/t5310-pack-bitmaps.sh | 9 +++++++++\n 2 files changed, 10 insertions(+)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex d391934..a4022a7 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -811,6 +811,7 @@ static void write_pack_file(void)\n \t\t\tfixup_pack_header_footer(fd, sha1, pack_tmp_name,\n \t\t\t\t\t\t nr_written, sha1, offset);\n \t\t\tclose(fd);\n+\t\t\twrite_bitmap_index = 0;\n \t\t}\n \n \t\tif (!pack_to_stdout) {\ndiff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh\nindex 0580258..6003490 100755\n--- a/t/t5310-pack-bitmaps.sh\n+++ b/t/t5310-pack-bitmaps.sh\n@@ -170,4 +170,13 @@ test_expect_success JGIT 'jgit can read our bitmaps' '\n \t)\n '\n \n+test_expect_success 'splitting packs does not generate bogus bitmaps' '\n+\ttest-genrandom foo $((1024 * 1024)) >rand &&\n+\tgit add rand &&\n+\tgit commit -m \"commit with big file\" &&\n+\tgit -c pack.packSizeLimit=500k repack -adb &&\n+\tgit init --bare no-bitmaps.git &&\n+\tgit -C no-bitmaps.git fetch .. HEAD\n+'\n+\n test_done\n-- \n2.1.2.596.g7379948\n"}]}