{"thread":{"id":"44785","subject":"[PATCH v5 0/2] repack (oops)","startedAt":"2016-12-28T22:45:55Z","lastAt":"2016-12-29T02:05:15Z","messageCount":4,"participants":["David Turner","Jeff King"],"isPatch":true,"patchVersion":5,"patchTotal":2},"messages":[{"id":"308473","messageId":"1482965142-6778-1-git-send-email-dturner@twosigma.com","threadId":"44785","inReplyTo":null,"subject":"[PATCH v5 0/2] repack (oops)","fromName":"David Turner","fromEmail":"dturner@twosigma.com","sentAt":"2016-12-28T22:45:40Z","receivedAt":"2016-12-28T22:45:55Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"This version addresses Johannes Sixt's comments on v4.  Also, I\nmessed up the rebase on v4.\n\nDavid Turner (2):\n  auto gc: don't write bitmaps for incremental repacks\n  repack: die on incremental + write-bitmap-index\n\n builtin/gc.c            |  9 ++++++++-\n builtin/repack.c        |  9 +++++++++\n t/t5310-pack-bitmaps.sh |  8 +++-----\n t/t6500-gc.sh           | 25 +++++++++++++++++++++++++\n 4 files changed, 45 insertions(+), 6 deletions(-)\n\n-- \n2.8.0.rc4.22.g8ae061a\n\n"},{"id":"308474","messageId":"1482965142-6778-2-git-send-email-dturner@twosigma.com","threadId":"44785","inReplyTo":"1482965142-6778-1-git-send-email-dturner@twosigma.com","subject":"[PATCH v5 1/2] auto gc: don't write bitmaps for incremental repacks","fromName":"David Turner","fromEmail":"dturner@twosigma.com","sentAt":"2016-12-28T22:45:41Z","receivedAt":"2016-12-28T22:46:03Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"When git gc --auto does an incremental repack of loose objects, we do\nnot expect to be able to write a bitmap; it is very likely that\nobjects in the new pack will have references to objects outside of the\npack.  So we shouldn't try to write a bitmap, because doing so will\nlikely issue a warning.\n\nThis warning was making its way into gc.log.  When the gc.log was\npresent, future auto gc runs would refuse to run.\n\nPatch by Jeff King.\nBug report, test, and commit message by David Turner.\n\nSigned-off-by: David Turner <dturner@twosigma.com>\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/gc.c  |  9 ++++++++-\n t/t6500-gc.sh | 25 +++++++++++++++++++++++++\n 2 files changed, 33 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 069950d..331f219 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -191,6 +191,11 @@ static void add_repack_all_option(void)\n \t}\n }\n \n+static void add_repack_incremental_option(void)\n+{\n+       argv_array_push(&repack, \"--no-write-bitmap-index\");\n+}\n+\n static int need_to_gc(void)\n {\n \t/*\n@@ -208,7 +213,9 @@ static int need_to_gc(void)\n \t */\n \tif (too_many_packs())\n \t\tadd_repack_all_option();\n-\telse if (!too_many_loose_objects())\n+\telse if (too_many_loose_objects())\n+\t\tadd_repack_incremental_option();\n+\telse\n \t\treturn 0;\n \n \tif (run_hook_le(NULL, \"pre-auto-gc\", NULL))\ndiff --git a/t/t6500-gc.sh b/t/t6500-gc.sh\nindex 5d7d414..1762dfa 100755\n--- a/t/t6500-gc.sh\n+++ b/t/t6500-gc.sh\n@@ -43,4 +43,29 @@ test_expect_success 'gc is not aborted due to a stale symref' '\n \t)\n '\n \n+test_expect_success 'auto gc with too many loose objects does not attempt to create bitmaps' '\n+\ttest_config gc.auto 3 &&\n+\ttest_config gc.autodetach false &&\n+\ttest_config pack.writebitmaps true &&\n+\t# We need to create two object whose sha1s start with 17\n+\t# since this is what git gc counts.  As it happens, these\n+\t# two blobs will do so.\n+\ttest_commit 263 &&\n+\ttest_commit 410 &&\n+\t# Our first gc will create a pack; our second will create a second pack\n+\tgit gc --auto &&\n+\tls .git/objects/pack | sort >existing_packs &&\n+\ttest_commit 523 &&\n+\ttest_commit 790 &&\n+\n+\tgit gc --auto 2>err &&\n+\ttest_i18ngrep ! \"^warning:\" err &&\n+\tls .git/objects/pack/ | sort >post_packs &&\n+\tcomm -1 -3 existing_packs post_packs >new &&\n+\tcomm -2 -3 existing_packs post_packs >del &&\n+\ttest_line_count = 0 del && # No packs are deleted\n+\ttest_line_count = 2 new # There is one new pack and its .idx\n+'\n+\n+\n test_done\n-- \n2.8.0.rc4.22.g8ae061a\n\n"},{"id":"308475","messageId":"1482965142-6778-3-git-send-email-dturner@twosigma.com","threadId":"44785","inReplyTo":"1482965142-6778-1-git-send-email-dturner@twosigma.com","subject":"[PATCH v5 2/2] repack: die on incremental + write-bitmap-index","fromName":"David Turner","fromEmail":"dturner@twosigma.com","sentAt":"2016-12-28T22:45:42Z","receivedAt":"2016-12-28T22:46:06Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"The bitmap index only works for single packs, so requesting an\nincremental repack with bitmap indexes makes no sense.\n\nSigned-off-by: David Turner <dturner@twosigma.com>\n---\n builtin/repack.c        | 9 +++++++++\n t/t5310-pack-bitmaps.sh | 8 +++-----\n 2 files changed, 12 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/repack.c b/builtin/repack.c\nindex 80dd06b..677bc7c 100644\n--- a/builtin/repack.c\n+++ b/builtin/repack.c\n@@ -18,6 +18,12 @@ static const char *const git_repack_usage[] = {\n \tNULL\n };\n \n+static const char incremental_bitmap_conflict_error[] = N_(\n+\"Incremental repacks are incompatible with bitmap indexes.  Use\\n\"\n+\"--no-write-bitmap-index or disable the pack.writebitmaps configuration.\"\n+);\n+\n+\n static int repack_config(const char *var, const char *value, void *cb)\n {\n \tif (!strcmp(var, \"repack.usedeltabaseoffset\")) {\n@@ -206,6 +212,9 @@ int cmd_repack(int argc, const char **argv, const char *prefix)\n \tif (pack_kept_objects < 0)\n \t\tpack_kept_objects = write_bitmaps;\n \n+\tif (write_bitmaps && !(pack_everything & ALL_INTO_ONE))\n+\t\tdie(_(incremental_bitmap_conflict_error));\n+\n \tpackdir = mkpathdup(\"%s/pack\", get_object_directory());\n \tpacktmp = mkpathdup(\"%s/.tmp-%d-pack\", packdir, (int)getpid());\n \ndiff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh\nindex b4c7a6f..424bec7 100755\n--- a/t/t5310-pack-bitmaps.sh\n+++ b/t/t5310-pack-bitmaps.sh\n@@ -118,12 +118,10 @@ test_expect_success 'fetch (partial bitmap)' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success 'incremental repack cannot create bitmaps' '\n+test_expect_success 'incremental repack fails when bitmaps are requested' '\n \ttest_commit more-1 &&\n-\tfind .git/objects/pack -name \"*.bitmap\" >expect &&\n-\tgit repack -d &&\n-\tfind .git/objects/pack -name \"*.bitmap\" >actual &&\n-\ttest_cmp expect actual\n+\ttest_must_fail git repack -d 2>err &&\n+\ttest_i18ngrep \"Incremental repacks are incompatible with bitmap\" err\n '\n \n test_expect_success 'incremental repack can disable bitmaps' '\n-- \n2.8.0.rc4.22.g8ae061a\n\n"},{"id":"308479","messageId":"20161229020433.ajkhqnozxvtmwt3r@sigill.intra.peff.net","threadId":"44785","inReplyTo":"1482965142-6778-1-git-send-email-dturner@twosigma.com","subject":"Re: [PATCH v5 0/2] repack (oops)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-12-29T02:04:33Z","receivedAt":"2016-12-29T02:05:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Dec 28, 2016 at 05:45:40PM -0500, David Turner wrote:\n\n> This version addresses Johannes Sixt's comments on v4.  Also, I\n> messed up the rebase on v4.\n\nThanks. The test logic in this one looks good to me.\n\n-Peff\n"}]}