{"thread":{"id":"49281","subject":"[PATCH 00/11] Add 'git multi-pack-index verify' command","startedAt":"2018-09-05T14:46:35Z","lastAt":"2018-09-13T19:02:33Z","messageCount":34,"participants":["Derrick Stolee via GitGitGadget","Stefan Beller","Eric Sunshine","Derrick Stolee","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":11},"messages":[{"id":"357422","messageId":"pull.34.git.gitgitgadget@gmail.com","threadId":"49281","inReplyTo":null,"subject":"[PATCH 00/11] Add 'git multi-pack-index verify' command","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-09-05T14:46:31Z","receivedAt":"2018-09-05T14:46:35Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"The multi-pack-index file provides faster lookups in repos with many\npackfiles by duplicating the information from multiple pack-indexes into a\nsingle file. This series allows us to verify a multi-pack-index using 'git\nmulti-pack-index verify' and 'git fsck' (when core.multiPackIndex is true).\n\nThe pattern for the tests is similar to that found in t5318-commit-graph.sh.\n\nDuring testing, I found a bug in how we check for the size of off_t (we are\nnot actually checking off_t, but instead uint32_t). See \"multi-pack-index:\nfix 32-bit vs 64-bit size check\".\n\nThanks to Ævar [1], I added a commit that provides progress updates when\nchecking object offsets.\n\nBased on ds/multi-pack-index\n\n[1] \nhttps://public-inbox.org/git/20180904202729.13900-1-avarab@gmail.com/T/#u\n\nDerrick Stolee (11):\n multi-pack-index: add 'verify' verb\n multi-pack-index: verify bad header\n multi-pack-index: verify corrupt chunk lookup table\n multi-pack-index: verify packname order\n multi-pack-index: verify missing pack\n multi-pack-index: verify oid fanout order\n multi-pack-index: verify oid lookup order\n multi-pack-index: fix 32-bit vs 64-bit size check\n multi-pack-index: verify object offsets\n multi-pack-index: report progress during 'verify'\n fsck: verify multi-pack-index\n\n Documentation/git-multi-pack-index.txt | 10 ++\n builtin/fsck.c | 18 ++++\n builtin/multi-pack-index.c | 4 +-\n midx.c | 112 ++++++++++++++++----\n midx.h | 1 +\n t/t5319-multi-pack-index.sh | 136 ++++++++++++++++++++++++-\n 6 files changed, 261 insertions(+), 20 deletions(-)\n\n\nbase-commit: 6a22d521260f86dff8fe6f23ab329cebb62ba4f0\nPublished-As: https://github.com/gitgitgadget/git/releases/tags/pr-34%2Fderrickstolee%2Fmidx%2Fverify-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-34/derrickstolee/midx/verify-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/34\n-- \ngitgitgadget\n"},{"id":"357423","messageId":"8dc38afe2b4d73e940daf9a0ff7f9c0a38802d95.1536158789.git.gitgitgadget@gmail.com","threadId":"49281","inReplyTo":"pull.34.git.gitgitgadget@gmail.com","subject":"[PATCH 01/11] multi-pack-index: add 'verify' verb","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-09-05T14:46:33Z","receivedAt":"2018-09-05T14:46:37Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe multi-pack-index builtin writes multi-pack-index files, and\nuses a 'write' verb to do so. Add a 'verify' verb that checks this\nfile matches the contents of the pack-indexes it replaces.\n\nThe current implementation is a no-op, but will be extended in\nsmall increments in later commits.\n\nSigned-off-by: Derrick Stolee \n---\n Documentation/git-multi-pack-index.txt | 10 ++++++++++\n builtin/multi-pack-index.c | 4 +++-\n midx.c | 13 +++++++++++++\n midx.h | 1 +\n t/t5319-multi-pack-index.sh | 8 ++++++++\n 5 files changed, 35 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-multi-pack-index.txt b/Documentation/git-multi-pack-index.txt\nindex 1f97e79912..f7778a2c85 100644\n--- a/Documentation/git-multi-pack-index.txt\n+++ b/Documentation/git-multi-pack-index.txt\n@@ -27,6 +27,10 @@ write::\n \tWhen given as the verb, write a new MIDX file to\n \t`/packs/multi-pack-index`.\n \n+verify::\n+\tWhen given as the verb, verify the contents of the MIDX file\n+\tat `/packs/multi-pack-index`.\n+\n \n EXAMPLES\n --------\n@@ -43,6 +47,12 @@ $ git multi-pack-index write\n $ git multi-pack-index --object-dir write\n -----------------------------------------------\n \n+* Verify the MIDX file for the packfiles in the current .git folder.\n++\n+-----------------------------------------------\n+$ git multi-pack-index verify\n+-----------------------------------------------\n+\n \n SEE ALSO\n --------\ndiff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c\nindex 2633efd95d..7d567dafbc 100644\n--- a/builtin/multi-pack-index.c\n+++ b/builtin/multi-pack-index.c\n@@ -5,7 +5,7 @@\n #include \"midx.h\"\n \n static char const * const builtin_multi_pack_index_usage[] = {\n-\tN_(\"git multi-pack-index [--object-dir=] write\"),\n+\tN_(\"git multi-pack-index [--object-dir=] [write|verify]\"),\n \tNULL\n };\n \n@@ -42,6 +42,8 @@ int cmd_multi_pack_index(int argc, const char **argv,\n \n \tif (!strcmp(argv[0], \"write\"))\n \t\treturn write_midx_file(opts.object_dir);\n+\tif (!strcmp(argv[0], \"verify\"))\n+\t\treturn verify_midx_file(opts.object_dir);\n \n \tdie(_(\"unrecognized verb: %s\"), argv[0]);\n }\ndiff --git a/midx.c b/midx.c\nindex f3e8dbc108..b253bed517 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -928,3 +928,16 @@ void clear_midx_file(const char *object_dir)\n \n \tfree(midx);\n }\n+\n+int verify_midx_error;\n+\n+int verify_midx_file(const char *object_dir)\n+{\n+\tstruct multi_pack_index *m = load_multi_pack_index(object_dir, 1);\n+\tverify_midx_error = 0;\n+\n+\tif (!m)\n+\t\treturn 0;\n+\n+\treturn verify_midx_error;\n+}\ndiff --git a/midx.h b/midx.h\nindex a210f1af2a..ce80b91c68 100644\n--- a/midx.h\n+++ b/midx.h\n@@ -43,5 +43,6 @@ int prepare_multi_pack_index_one(struct repository *r, const char *object_dir, i\n \n int write_midx_file(const char *object_dir);\n void clear_midx_file(const char *object_dir);\n+int verify_midx_file(const char *object_dir);\n \n #endif\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex 6f56b38674..1c4e0e6d31 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -150,6 +150,10 @@ test_expect_success 'write midx with twelve packs' '\n \n compare_results_with_midx \"twelve packs\"\n \n+test_expect_success 'verify multi-pack-index success' '\n+\tgit multi-pack-index verify --object-dir=$objdir\n+'\n+\n test_expect_success 'repack removes multi-pack-index' '\n \ttest_path_is_file $objdir/pack/multi-pack-index &&\n \tgit repack -adf &&\n@@ -214,4 +218,8 @@ test_expect_success 'force some 64-bit offsets with pack-objects' '\n \tmidx_read_expect 1 63 5 objects64 \" large-offsets\"\n '\n \n+test_expect_success 'verify multi-pack-index with 64-bit offsets' '\n+\tgit multi-pack-index verify --object-dir=objects64\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"357424","messageId":"787e1fb61626afe79cfc846c4175d4b4d169ff42.1536158789.git.gitgitgadget@gmail.com","threadId":"49281","inReplyTo":"pull.34.git.gitgitgadget@gmail.com","subject":"[PATCH 02/11] multi-pack-index: verify bad header","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-09-05T14:46:34Z","receivedAt":"2018-09-05T14:46:38Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nWhen verifying if a multi-pack-index file is valid, we want the\ncommand to fail to signal an invalid file. Previously, we wrote\nan error to stderr and continued as if we had no multi-pack-index.\nNow, die() instead of error().\n\nAdd tests that check corrupted headers in a few ways:\n\n* Bad signature\n* Bad file version\n* Bad hash version\n* Truncated hash count\n* Extended hash count\n\nSigned-off-by: Derrick Stolee \n---\n midx.c | 18 +++++----------\n t/t5319-multi-pack-index.sh | 46 ++++++++++++++++++++++++++++++++++++-\n 2 files changed, 51 insertions(+), 13 deletions(-)\n\ndiff --git a/midx.c b/midx.c\nindex b253bed517..ec78254bb6 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -76,24 +76,18 @@ struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local\n \tm->local = local;\n \n \tm->signature = get_be32(m->data);\n-\tif (m->signature != MIDX_SIGNATURE) {\n-\t\terror(_(\"multi-pack-index signature 0x%08x does not match signature 0x%08x\"),\n+\tif (m->signature != MIDX_SIGNATURE)\n+\t\tdie(_(\"multi-pack-index signature 0x%08x does not match signature 0x%08x\"),\n \t\t m->signature, MIDX_SIGNATURE);\n-\t\tgoto cleanup_fail;\n-\t}\n \n \tm->version = m->data[MIDX_BYTE_FILE_VERSION];\n-\tif (m->version != MIDX_VERSION) {\n-\t\terror(_(\"multi-pack-index version %d not recognized\"),\n+\tif (m->version != MIDX_VERSION)\n+\t\tdie(_(\"multi-pack-index version %d not recognized\"),\n \t\t m->version);\n-\t\tgoto cleanup_fail;\n-\t}\n \n \thash_version = m->data[MIDX_BYTE_HASH_VERSION];\n-\tif (hash_version != MIDX_HASH_VERSION) {\n-\t\terror(_(\"hash version %u does not match\"), hash_version);\n-\t\tgoto cleanup_fail;\n-\t}\n+\tif (hash_version != MIDX_HASH_VERSION)\n+\t\tdie(_(\"hash version %u does not match\"), hash_version);\n \tm->hash_len = MIDX_HASH_LEN;\n \n \tm->num_chunks = m->data[MIDX_BYTE_NUM_CHUNKS];\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex 1c4e0e6d31..836d2bdb53 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -154,6 +154,51 @@ test_expect_success 'verify multi-pack-index success' '\n \tgit multi-pack-index verify --object-dir=$objdir\n '\n \n+# usage: corrupt_midx_and_verify \n+corrupt_midx_and_verify() {\n+\tPOS=$1\n+\tDATA=\"${2:-\\0}\"\n+\tOBJDIR=$3\n+\tGREPSTR=\"$4\"\n+\tFILE=$OBJDIR/pack/multi-pack-index &&\n+\tchmod a+w $FILE &&\n+\ttest_when_finished mv midx-backup $FILE &&\n+\tcp $FILE midx-backup &&\n+\tprintf \"$DATA\" | dd of=\"$FILE\" bs=1 seek=\"$POS\" conv=notrunc &&\n+\ttest_must_fail git multi-pack-index verify --object-dir=$OBJDIR 2>test_err &&\n+\tgrep -v \"^+\" test_err >err &&\n+\ttest_i18ngrep \"$GREPSTR\" err\n+}\n+\n+test_expect_success 'verify bad signature' '\n+\tcorrupt_midx_and_verify 0 \"\\00\" $objdir \\\n+\t\t\"multi-pack-index signature\"\n+'\n+\n+MIDX_BYTE_VERSION=4\n+MIDX_BYTE_OID_VERSION=5\n+MIDX_BYTE_CHUNK_COUNT=6\n+\n+test_expect_success 'verify bad version' '\n+\tcorrupt_midx_and_verify $MIDX_BYTE_VERSION \"\\00\" $objdir \\\n+\t\t\"multi-pack-index version\"\n+'\n+\n+test_expect_success 'verify bad OID version' '\n+\tcorrupt_midx_and_verify $MIDX_BYTE_OID_VERSION \"\\02\" $objdir \\\n+\t\t\"hash version\"\n+'\n+\n+test_expect_success 'verify truncated chunk count' '\n+\tcorrupt_midx_and_verify $MIDX_BYTE_CHUNK_COUNT \"\\01\" $objdir \\\n+\t\t\"missing required\"\n+'\n+\n+test_expect_success 'verify extended chunk count' '\n+\tcorrupt_midx_and_verify $MIDX_BYTE_CHUNK_COUNT \"\\07\" $objdir \\\n+\t\t\"terminating multi-pack-index chunk id appears earlier than expected\"\n+'\n+\n test_expect_success 'repack removes multi-pack-index' '\n \ttest_path_is_file $objdir/pack/multi-pack-index &&\n \tgit repack -adf &&\n@@ -191,7 +236,6 @@ test_expect_success 'multi-pack-index in an alternate' '\n \n compare_results_with_midx \"with alternate (remote midx)\"\n \n-\n # usage: corrupt_data []\n corrupt_data () {\n \tfile=$1\n-- \ngitgitgadget\n\n"},{"id":"357425","messageId":"37ee24c82bf88611808ec3bb1c36eef439c809db.1536158789.git.gitgitgadget@gmail.com","threadId":"49281","inReplyTo":"pull.34.git.gitgitgadget@gmail.com","subject":"[PATCH 04/11] multi-pack-index: verify packname order","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-09-05T14:46:37Z","receivedAt":"2018-09-05T14:46:41Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe final check we make while loading a multi-pack-index is that\nthe packfile names are in lexicographical order. Make this error\nbe a die() instead.\n\nIn order to test this condition, we need multiple packfiles.\nEarlier in t5319-multi-pack-index.sh, we tested the interaction with\n'git repack' but this limits us to one packfile in our object dir.\nMove these repack tests until after the 'verify' tests.\n\nSigned-off-by: Derrick Stolee \n---\n midx.c | 6 ++----\n t/t5319-multi-pack-index.sh | 10 ++++++++++\n 2 files changed, 12 insertions(+), 4 deletions(-)\n\ndiff --git a/midx.c b/midx.c\nindex 8b054b39ab..e655a15aed 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -157,12 +157,10 @@ struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local\n \n \t\tcur_pack_name += strlen(cur_pack_name) + 1;\n \n-\t\tif (i && strcmp(m->pack_names[i], m->pack_names[i - 1]) <= 0) {\n-\t\t\terror(_(\"multi-pack-index pack names out of order: '%s' before '%s'\"),\n+\t\tif (i && strcmp(m->pack_names[i], m->pack_names[i - 1]) <= 0)\n+\t\t\tdie(_(\"multi-pack-index pack names out of order: '%s' before '%s'\"),\n \t\t\t m->pack_names[i - 1],\n \t\t\t m->pack_names[i]);\n-\t\t\tgoto cleanup_fail;\n-\t\t}\n \t}\n \n \treturn m;\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex 5c9c499aa6..4a8f231d93 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -181,6 +181,11 @@ MIDX_BYTE_CHUNK_COUNT=6\n MIDX_HEADER_SIZE=12\n MIDX_BYTE_CHUNK_ID=$MIDX_HEADER_SIZE\n MIDX_BYTE_CHUNK_OFFSET=$(($MIDX_HEADER_SIZE + 4))\n+MIDX_NUM_CHUNKS=5\n+MIDX_CHUNK_LOOKUP_WIDTH=12\n+MIDX_OFFSET_PACKNAMES=$(($MIDX_HEADER_SIZE + \\\n+\t\t\t $MIDX_NUM_CHUNKS * $MIDX_CHUNK_LOOKUP_WIDTH))\n+MIDX_BYTE_PACKNAME_ORDER=$(($MIDX_OFFSET_PACKNAMES + 2))\n \n test_expect_success 'verify bad version' '\n \tcorrupt_midx_and_verify $MIDX_BYTE_VERSION \"\\00\" $objdir \\\n@@ -212,6 +217,11 @@ test_expect_success 'verify invalid chunk offset' '\n \t\t\"invalid chunk offset (too large)\"\n '\n \n+test_expect_success 'verify packnames out of order' '\n+\tcorrupt_midx_and_verify $MIDX_BYTE_PACKNAME_ORDER \"z\" $objdir \\\n+\t\t\"pack names out of order\"\n+'\n+\n test_expect_success 'repack removes multi-pack-index' '\n \ttest_path_is_file $objdir/pack/multi-pack-index &&\n \tgit repack -adf &&\n-- \ngitgitgadget\n\n"},{"id":"357426","messageId":"b747da415cc7eb1e47f5ac377f527957a322b034.1536158789.git.gitgitgadget@gmail.com","threadId":"49281","inReplyTo":"pull.34.git.gitgitgadget@gmail.com","subject":"[PATCH 05/11] multi-pack-index: verify missing pack","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-09-05T14:46:38Z","receivedAt":"2018-09-05T14:46:42Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nSigned-off-by: Derrick Stolee \n---\n midx.c | 16 ++++++++++++++++\n t/t5319-multi-pack-index.sh | 5 +++++\n 2 files changed, 21 insertions(+)\n\ndiff --git a/midx.c b/midx.c\nindex e655a15aed..a02b19efc1 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -926,13 +926,29 @@ void clear_midx_file(const char *object_dir)\n \n int verify_midx_error;\n \n+static void midx_report(const char *fmt, ...)\n+{\n+\tva_list ap;\n+\tverify_midx_error = 1;\n+\tva_start(ap, fmt);\n+\tvfprintf(stderr, fmt, ap);\n+\tfprintf(stderr, \"\\n\");\n+\tva_end(ap);\n+}\n+\n int verify_midx_file(const char *object_dir)\n {\n+\tuint32_t i;\n \tstruct multi_pack_index *m = load_multi_pack_index(object_dir, 1);\n \tverify_midx_error = 0;\n \n \tif (!m)\n \t\treturn 0;\n \n+\tfor (i = 0; i < m->num_packs; i++) {\n+\t\tif (prepare_midx_pack(m, i))\n+\t\t\tmidx_report(\"failed to load pack in position %d\", i);\n+\t}\n+\n \treturn verify_midx_error;\n }\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex 4a8f231d93..52b7f7905b 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -222,6 +222,11 @@ test_expect_success 'verify packnames out of order' '\n \t\t\"pack names out of order\"\n '\n \n+test_expect_success 'verify packnames out of order' '\n+\tcorrupt_midx_and_verify $MIDX_BYTE_PACKNAME_ORDER \"a\" $objdir \\\n+\t\t\"failed to load pack\"\n+'\n+\n test_expect_success 'repack removes multi-pack-index' '\n \ttest_path_is_file $objdir/pack/multi-pack-index &&\n \tgit repack -adf &&\n-- \ngitgitgadget\n\n"},{"id":"357427","messageId":"58e5c094682fb34e19860eacac788923cf3b5fdb.1536158789.git.gitgitgadget@gmail.com","threadId":"49281","inReplyTo":"pull.34.git.gitgitgadget@gmail.com","subject":"[PATCH 06/11] multi-pack-index: verify oid fanout order","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-09-05T14:46:40Z","receivedAt":"2018-09-05T14:46:43Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nSigned-off-by: Derrick Stolee \n---\n midx.c | 9 +++++++++\n t/t5319-multi-pack-index.sh | 8 ++++++++\n 2 files changed, 17 insertions(+)\n\ndiff --git a/midx.c b/midx.c\nindex a02b19efc1..dfd26b4d74 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -950,5 +950,14 @@ int verify_midx_file(const char *object_dir)\n \t\t\tmidx_report(\"failed to load pack in position %d\", i);\n \t}\n \n+\tfor (i = 0; i < 255; i++) {\n+\t\tuint32_t oid_fanout1 = ntohl(m->chunk_oid_fanout[i]);\n+\t\tuint32_t oid_fanout2 = ntohl(m->chunk_oid_fanout[i + 1]);\n+\n+\t\tif (oid_fanout1 > oid_fanout2)\n+\t\t\tmidx_report(_(\"oid fanout out of order: fanout[%d] = %\"PRIx32\" > %\"PRIx32\" = fanout[%d]\"),\n+\t\t\t\t i, oid_fanout1, oid_fanout2, i + 1);\n+\t}\n+\n \treturn verify_midx_error;\n }\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex 52b7f7905b..95ac7a6edd 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -186,6 +186,9 @@ MIDX_CHUNK_LOOKUP_WIDTH=12\n MIDX_OFFSET_PACKNAMES=$(($MIDX_HEADER_SIZE + \\\n \t\t\t $MIDX_NUM_CHUNKS * $MIDX_CHUNK_LOOKUP_WIDTH))\n MIDX_BYTE_PACKNAME_ORDER=$(($MIDX_OFFSET_PACKNAMES + 2))\n+MIDX_OFFSET_OID_FANOUT=$(($MIDX_OFFSET_PACKNAMES + 652))\n+MIDX_OID_FANOUT_WIDTH=4\n+MIDX_BYTE_OID_FANOUT_ORDER=$((MIDX_OFFSET_OID_FANOUT + 250 * $MIDX_OID_FANOUT_WIDTH + 1))\n \n test_expect_success 'verify bad version' '\n \tcorrupt_midx_and_verify $MIDX_BYTE_VERSION \"\\00\" $objdir \\\n@@ -227,6 +230,11 @@ test_expect_success 'verify packnames out of order' '\n \t\t\"failed to load pack\"\n '\n \n+test_expect_success 'verify oid fanout out of order' '\n+\tcorrupt_midx_and_verify $MIDX_BYTE_OID_FANOUT_ORDER \"\\01\" $objdir \\\n+\t\t\"oid fanout out of order\"\n+'\n+\n test_expect_success 'repack removes multi-pack-index' '\n \ttest_path_is_file $objdir/pack/multi-pack-index &&\n \tgit repack -adf &&\n-- \ngitgitgadget\n\n"},{"id":"357429","messageId":"b21772d054c1d845d5d330ca39c296d5c28b6b98.1536158789.git.gitgitgadget@gmail.com","threadId":"49281","inReplyTo":"pull.34.git.gitgitgadget@gmail.com","subject":"[PATCH 07/11] multi-pack-index: verify oid lookup order","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-09-05T14:46:41Z","receivedAt":"2018-09-05T14:46:44Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nSigned-off-by: Derrick Stolee \n---\n midx.c | 11 +++++++++++\n t/t5319-multi-pack-index.sh | 8 ++++++++\n 2 files changed, 19 insertions(+)\n\ndiff --git a/midx.c b/midx.c\nindex dfd26b4d74..06d5cfc826 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -959,5 +959,16 @@ int verify_midx_file(const char *object_dir)\n \t\t\t\t i, oid_fanout1, oid_fanout2, i + 1);\n \t}\n \n+\tfor (i = 0; i < m->num_objects - 1; i++) {\n+\t\tstruct object_id oid1, oid2;\n+\n+\t\tnth_midxed_object_oid(&oid1, m, i);\n+\t\tnth_midxed_object_oid(&oid2, m, i + 1);\n+\n+\t\tif (oidcmp(&oid1, &oid2) >= 0)\n+\t\t\tmidx_report(_(\"oid lookup out of order: oid[%d] = %s >= %s = oid[%d]\"),\n+\t\t\t\t i, oid_to_hex(&oid1), oid_to_hex(&oid2), i + 1);\n+\t}\n+\n \treturn verify_midx_error;\n }\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex 95ac7a6edd..072b1d1a74 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -175,6 +175,7 @@ test_expect_success 'verify bad signature' '\n \t\t\"multi-pack-index signature\"\n '\n \n+HASH_LEN=20\n MIDX_BYTE_VERSION=4\n MIDX_BYTE_OID_VERSION=5\n MIDX_BYTE_CHUNK_COUNT=6\n@@ -189,6 +190,8 @@ MIDX_BYTE_PACKNAME_ORDER=$(($MIDX_OFFSET_PACKNAMES + 2))\n MIDX_OFFSET_OID_FANOUT=$(($MIDX_OFFSET_PACKNAMES + 652))\n MIDX_OID_FANOUT_WIDTH=4\n MIDX_BYTE_OID_FANOUT_ORDER=$((MIDX_OFFSET_OID_FANOUT + 250 * $MIDX_OID_FANOUT_WIDTH + 1))\n+MIDX_OFFSET_OID_LOOKUP=$(($MIDX_OFFSET_OID_FANOUT + 256 * $MIDX_OID_FANOUT_WIDTH))\n+MIDX_BYTE_OID_LOOKUP=$(($MIDX_OFFSET_OID_LOOKUP + 16 * $HASH_LEN))\n \n test_expect_success 'verify bad version' '\n \tcorrupt_midx_and_verify $MIDX_BYTE_VERSION \"\\00\" $objdir \\\n@@ -235,6 +238,11 @@ test_expect_success 'verify oid fanout out of order' '\n \t\t\"oid fanout out of order\"\n '\n \n+test_expect_success 'verify oid lookup out of order' '\n+\tcorrupt_midx_and_verify $MIDX_BYTE_OID_LOOKUP \"\\00\" $objdir \\\n+\t\t\"oid lookup out of order\"\n+'\n+\n test_expect_success 'repack removes multi-pack-index' '\n \ttest_path_is_file $objdir/pack/multi-pack-index &&\n \tgit repack -adf &&\n-- \ngitgitgadget\n\n"},{"id":"357428","messageId":"b08d3f0055c7cdc0e3bcca91ed83d48ae9885c3e.1536158789.git.gitgitgadget@gmail.com","threadId":"49281","inReplyTo":"pull.34.git.gitgitgadget@gmail.com","subject":"[PATCH 08/11] multi-pack-index: fix 32-bit vs 64-bit size check","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-09-05T14:46:43Z","receivedAt":"2018-09-05T14:46:45Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nWhen loading a 64-bit offset, we intend to check that off_t can store\nthe resulting offset. However, the condition accidentally checks the\n32-bit offset to see if it is smaller than a 64-bit value. Fix it,\nand this will be covered by a test in the 'git multi-pack-index verify'\ncommand in a later commit.\n\nSigned-off-by: Derrick Stolee \n---\n midx.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/midx.c b/midx.c\nindex 06d5cfc826..80094c02a7 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -236,7 +236,7 @@ static off_t nth_midxed_offset(struct multi_pack_index *m, uint32_t pos)\n \toffset32 = get_be32(offset_data + sizeof(uint32_t));\n \n \tif (m->chunk_large_offsets && offset32 & MIDX_LARGE_OFFSET_NEEDED) {\n-\t\tif (sizeof(offset32) < sizeof(uint64_t))\n+\t\tif (sizeof(off_t) < sizeof(uint64_t))\n \t\t\tdie(_(\"multi-pack-index stores a 64-bit offset, but off_t is too small\"));\n \n \t\toffset32 ^= MIDX_LARGE_OFFSET_NEEDED;\n-- \ngitgitgadget\n\n"},{"id":"357430","messageId":"b385aa2abfc0fd6ceba670d197e1d1d702b1c387.1536158789.git.gitgitgadget@gmail.com","threadId":"49281","inReplyTo":"pull.34.git.gitgitgadget@gmail.com","subject":"[PATCH 03/11] multi-pack-index: verify corrupt chunk lookup table","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-09-05T14:46:35Z","receivedAt":"2018-09-05T14:46:45Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nSigned-off-by: Derrick Stolee \n---\n midx.c | 3 +++\n t/t5319-multi-pack-index.sh | 13 +++++++++++++\n 2 files changed, 16 insertions(+)\n\ndiff --git a/midx.c b/midx.c\nindex ec78254bb6..8b054b39ab 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -100,6 +100,9 @@ struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local\n \t\tuint64_t chunk_offset = get_be64(m->data + MIDX_HEADER_SIZE + 4 +\n \t\t\t\t\t\t MIDX_CHUNKLOOKUP_WIDTH * i);\n \n+\t\tif (chunk_offset >= m->data_len)\n+\t\t\tdie(_(\"invalid chunk offset (too large)\"));\n+\n \t\tswitch (chunk_id) {\n \t\t\tcase MIDX_CHUNKID_PACKNAMES:\n \t\t\t\tm->chunk_pack_names = m->data + chunk_offset;\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex 836d2bdb53..5c9c499aa6 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -178,6 +178,9 @@ test_expect_success 'verify bad signature' '\n MIDX_BYTE_VERSION=4\n MIDX_BYTE_OID_VERSION=5\n MIDX_BYTE_CHUNK_COUNT=6\n+MIDX_HEADER_SIZE=12\n+MIDX_BYTE_CHUNK_ID=$MIDX_HEADER_SIZE\n+MIDX_BYTE_CHUNK_OFFSET=$(($MIDX_HEADER_SIZE + 4))\n \n test_expect_success 'verify bad version' '\n \tcorrupt_midx_and_verify $MIDX_BYTE_VERSION \"\\00\" $objdir \\\n@@ -199,6 +202,16 @@ test_expect_success 'verify extended chunk count' '\n \t\t\"terminating multi-pack-index chunk id appears earlier than expected\"\n '\n \n+test_expect_success 'verify missing required chunk' '\n+\tcorrupt_midx_and_verify $MIDX_BYTE_CHUNK_ID \"\\01\" $objdir \\\n+\t\t\"missing required\"\n+'\n+\n+test_expect_success 'verify invalid chunk offset' '\n+\tcorrupt_midx_and_verify $MIDX_BYTE_CHUNK_OFFSET \"\\01\" $objdir \\\n+\t\t\"invalid chunk offset (too large)\"\n+'\n+\n test_expect_success 'repack removes multi-pack-index' '\n \ttest_path_is_file $objdir/pack/multi-pack-index &&\n \tgit repack -adf &&\n-- \ngitgitgadget\n\n"},{"id":"357432","messageId":"e1498aea45eb46feb3664413f49b70bdf048d284.1536158789.git.gitgitgadget@gmail.com","threadId":"49281","inReplyTo":"pull.34.git.gitgitgadget@gmail.com","subject":"[PATCH 09/11] multi-pack-index: verify object offsets","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-09-05T14:46:44Z","receivedAt":"2018-09-05T14:46:48Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe 'git multi-pack-index verify' command must verify the object\noffsets stored in the multi-pack-index are correct. There are two\nways the offset chunk can be incorrect: the pack-int-id and the\nobject offset.\n\nReplace the BUG() statement with a die() statement, now that we\nmay hit a bad pack-int-id during a 'verify' command on a corrupt\nmulti-pack-index, and it is covered by a test.\n\nSigned-off-by: Derrick Stolee \n---\n midx.c | 28 +++++++++++++++++++++++++++-\n t/t5319-multi-pack-index.sh | 27 +++++++++++++++++++++++++++\n 2 files changed, 54 insertions(+), 1 deletion(-)\n\ndiff --git a/midx.c b/midx.c\nindex 80094c02a7..87708dc367 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -197,7 +197,7 @@ int prepare_midx_pack(struct multi_pack_index *m, uint32_t pack_int_id)\n \tstruct strbuf pack_name = STRBUF_INIT;\n \n \tif (pack_int_id >= m->num_packs)\n-\t\tBUG(\"bad pack-int-id\");\n+\t\tdie(_(\"bad pack-int-id\"));\n \n \tif (m->packs[pack_int_id])\n \t\treturn 0;\n@@ -970,5 +970,31 @@ int verify_midx_file(const char *object_dir)\n \t\t\t\t i, oid_to_hex(&oid1), oid_to_hex(&oid2), i + 1);\n \t}\n \n+\tfor (i = 0; i < m->num_objects; i++) {\n+\t\tstruct object_id oid;\n+\t\tstruct pack_entry e;\n+\t\toff_t m_offset, p_offset;\n+\n+\t\tnth_midxed_object_oid(&oid, m, i);\n+\t\tif (!fill_midx_entry(&oid, &e, m)) {\n+\t\t\tmidx_report(_(\"failed to load pack entry for oid[%d] = %s\"),\n+\t\t\t\t i, oid_to_hex(&oid));\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tif (open_pack_index(e.p)) {\n+\t\t\tmidx_report(_(\"failed to load pack-index for packfile %s\"),\n+\t\t\t\t e.p->pack_name);\n+\t\t\tbreak;\n+\t\t}\n+\n+\t\tm_offset = e.offset;\n+\t\tp_offset = find_pack_entry_one(oid.hash, e.p);\n+\n+\t\tif (m_offset != p_offset)\n+\t\t\tmidx_report(_(\"incorrect object offset for oid[%d] = %s: %\"PRIx64\" != %\"PRIx64),\n+\t\t\t\t i, oid_to_hex(&oid), m_offset, p_offset);\n+\t}\n+\n \treturn verify_midx_error;\n }\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex 072b1d1a74..b6c34631d3 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -176,6 +176,7 @@ test_expect_success 'verify bad signature' '\n '\n \n HASH_LEN=20\n+NUM_OBJECTS=74\n MIDX_BYTE_VERSION=4\n MIDX_BYTE_OID_VERSION=5\n MIDX_BYTE_CHUNK_COUNT=6\n@@ -192,6 +193,10 @@ MIDX_OID_FANOUT_WIDTH=4\n MIDX_BYTE_OID_FANOUT_ORDER=$((MIDX_OFFSET_OID_FANOUT + 250 * $MIDX_OID_FANOUT_WIDTH + 1))\n MIDX_OFFSET_OID_LOOKUP=$(($MIDX_OFFSET_OID_FANOUT + 256 * $MIDX_OID_FANOUT_WIDTH))\n MIDX_BYTE_OID_LOOKUP=$(($MIDX_OFFSET_OID_LOOKUP + 16 * $HASH_LEN))\n+MIDX_OFFSET_OBJECT_OFFSETS=$(($MIDX_OFFSET_OID_LOOKUP + $NUM_OBJECTS * $HASH_LEN))\n+MIDX_OFFSET_WIDTH=8\n+MIDX_BYTE_PACK_INT_ID=$(($MIDX_OFFSET_OBJECT_OFFSETS + 16 * $MIDX_OFFSET_WIDTH + 2))\n+MIDX_BYTE_OFFSET=$(($MIDX_OFFSET_OBJECT_OFFSETS + 16 * $MIDX_OFFSET_WIDTH + 6))\n \n test_expect_success 'verify bad version' '\n \tcorrupt_midx_and_verify $MIDX_BYTE_VERSION \"\\00\" $objdir \\\n@@ -243,6 +248,16 @@ test_expect_success 'verify oid lookup out of order' '\n \t\t\"oid lookup out of order\"\n '\n \n+test_expect_success 'verify incorrect pack-int-id' '\n+\tcorrupt_midx_and_verify $MIDX_BYTE_PACK_INT_ID \"\\07\" $objdir \\\n+\t\t\"bad pack-int-id\"\n+'\n+\n+test_expect_success 'verify incorrect offset' '\n+\tcorrupt_midx_and_verify $MIDX_BYTE_OFFSET \"\\07\" $objdir \\\n+\t\t\"incorrect object offset\"\n+'\n+\n test_expect_success 'repack removes multi-pack-index' '\n \ttest_path_is_file $objdir/pack/multi-pack-index &&\n \tgit repack -adf &&\n@@ -310,4 +325,16 @@ test_expect_success 'verify multi-pack-index with 64-bit offsets' '\n \tgit multi-pack-index verify --object-dir=objects64\n '\n \n+NUM_OBJECTS=63\n+MIDX_OFFSET_OID_FANOUT=$((MIDX_OFFSET_PACKNAMES + 54))\n+MIDX_OFFSET_OID_LOOKUP=$((MIDX_OFFSET_OID_FANOUT + 256 * $MIDX_OID_FANOUT_WIDTH))\n+MIDX_OFFSET_OBJECT_OFFSETS=$(($MIDX_OFFSET_OID_LOOKUP + $NUM_OBJECTS * $HASH_LEN))\n+MIDX_OFFSET_LARGE_OFFSETS=$(($MIDX_OFFSET_OBJECT_OFFSETS + $NUM_OBJECTS * $MIDX_OFFSET_WIDTH))\n+MIDX_BYTE_LARGE_OFFSET=$(($MIDX_OFFSET_LARGE_OFFSETS + 3))\n+\n+test_expect_success 'verify incorrect 64-bit offset' '\n+\tcorrupt_midx_and_verify $MIDX_BYTE_LARGE_OFFSET \"\\07\" objects64 \\\n+\t\t\"incorrect object offset\"\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"357431","messageId":"acf8cfd6328deac07d1559c906505d3c0fd0b0c2.1536158789.git.gitgitgadget@gmail.com","threadId":"49281","inReplyTo":"pull.34.git.gitgitgadget@gmail.com","subject":"[PATCH 10/11] multi-pack-index: report progress during 'verify'","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-09-05T14:46:46Z","receivedAt":"2018-09-05T14:46:49Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nWhen verifying a multi-pack-index, the only action that takes\nsignificant time is checking the object offsets. For example,\nto verify a multi-pack-index containing 6.2 million objects in\nthe Linux kernel repository takes 1.3 seconds on my machine.\n99% of that time is spent looking up object offsets in each of\nthe packfiles and comparing them to the multi-pack-index offset.\n\nAdd a progress indicator for that section of the 'verify' verb.\n\nSigned-off-by: Derrick Stolee \n---\n midx.c | 6 ++++++\n 1 file changed, 6 insertions(+)\n\ndiff --git a/midx.c b/midx.c\nindex 87708dc367..489180a599 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -7,6 +7,7 @@\n #include \"object-store.h\"\n #include \"sha1-lookup.h\"\n #include \"midx.h\"\n+#include \"progress.h\"\n \n #define MIDX_SIGNATURE 0x4d494458 /* \"MIDX\" */\n #define MIDX_VERSION 1\n@@ -939,6 +940,7 @@ static void midx_report(const char *fmt, ...)\n int verify_midx_file(const char *object_dir)\n {\n \tuint32_t i;\n+\tstruct progress *progress = NULL;\n \tstruct multi_pack_index *m = load_multi_pack_index(object_dir, 1);\n \tverify_midx_error = 0;\n \n@@ -970,6 +972,7 @@ int verify_midx_file(const char *object_dir)\n \t\t\t\t i, oid_to_hex(&oid1), oid_to_hex(&oid2), i + 1);\n \t}\n \n+\tprogress = start_progress(_(\"Verifying object offsets\"), m->num_objects);\n \tfor (i = 0; i < m->num_objects; i++) {\n \t\tstruct object_id oid;\n \t\tstruct pack_entry e;\n@@ -994,7 +997,10 @@ int verify_midx_file(const char *object_dir)\n \t\tif (m_offset != p_offset)\n \t\t\tmidx_report(_(\"incorrect object offset for oid[%d] = %s: %\"PRIx64\" != %\"PRIx64),\n \t\t\t\t i, oid_to_hex(&oid), m_offset, p_offset);\n+\n+\t\tdisplay_progress(progress, i + 1);\n \t}\n+\tstop_progress(&progress);\n \n \treturn verify_midx_error;\n }\n-- \ngitgitgadget\n\n"},{"id":"357433","messageId":"09d16aff20fc16768e688c109519e4e41f9a8e36.1536158789.git.gitgitgadget@gmail.com","threadId":"49281","inReplyTo":"pull.34.git.gitgitgadget@gmail.com","subject":"[PATCH 11/11] fsck: verify multi-pack-index","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-09-05T14:46:47Z","receivedAt":"2018-09-05T14:46:51Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nWhen core.multiPackIndex is true, we may have a multi-pack-index\nin our object directory. Add calls to 'git multi-pack-index verify'\nat the end of 'git fsck' if so.\n\nSigned-off-by: Derrick Stolee \n---\n builtin/fsck.c | 18 ++++++++++++++++++\n t/t5319-multi-pack-index.sh | 13 ++++++++++++-\n 2 files changed, 30 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex 63c8578cc1..06eb421720 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -848,5 +848,23 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)\n \t\t}\n \t}\n \n+\tif (!git_config_get_bool(\"core.multipackindex\", &i) && i) {\n+\t\tstruct child_process midx_verify = CHILD_PROCESS_INIT;\n+\t\tconst char *midx_argv[] = { \"multi-pack-index\", \"verify\", NULL, NULL, NULL };\n+\n+\t\tmidx_verify.argv = midx_argv;\n+\t\tmidx_verify.git_cmd = 1;\n+\t\tif (run_command(&midx_verify))\n+\t\t\terrors_found |= ERROR_COMMIT_GRAPH;\n+\n+\t\tprepare_alt_odb(the_repository);\n+\t\tfor (alt = the_repository->objects->alt_odb_list; alt; alt = alt->next) {\n+\t\t\tmidx_argv[2] = \"--object-dir\";\n+\t\t\tmidx_argv[3] = alt->path;\n+\t\t\tif (run_command(&midx_verify))\n+\t\t\t\terrors_found |= ERROR_COMMIT_GRAPH;\n+\t\t}\n+\t}\n+\n \treturn errors_found;\n }\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex b6c34631d3..63f8718fd7 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -160,12 +160,17 @@ corrupt_midx_and_verify() {\n \tDATA=\"${2:-\\0}\"\n \tOBJDIR=$3\n \tGREPSTR=\"$4\"\n+\tCOMMAND=\"$5\"\n+\tif test -z \"$COMMAND\"\n+\tthen\n+\t\tCOMMAND=\"git multi-pack-index verify --object-dir=$OBJDIR\"\n+\tfi\n \tFILE=$OBJDIR/pack/multi-pack-index &&\n \tchmod a+w $FILE &&\n \ttest_when_finished mv midx-backup $FILE &&\n \tcp $FILE midx-backup &&\n \tprintf \"$DATA\" | dd of=\"$FILE\" bs=1 seek=\"$POS\" conv=notrunc &&\n-\ttest_must_fail git multi-pack-index verify --object-dir=$OBJDIR 2>test_err &&\n+\ttest_must_fail $COMMAND 2>test_err &&\n \tgrep -v \"^+\" test_err >err &&\n \ttest_i18ngrep \"$GREPSTR\" err\n }\n@@ -258,6 +263,12 @@ test_expect_success 'verify incorrect offset' '\n \t\t\"incorrect object offset\"\n '\n \n+test_expect_success 'git-fsck incorrect offset' '\n+\tcorrupt_midx_and_verify $MIDX_BYTE_OFFSET \"\\07\" $objdir \\\n+\t\t\"incorrect object offset\" \\\n+\t\t\"git -c core.multipackindex=true fsck\"\n+'\n+\n test_expect_success 'repack removes multi-pack-index' '\n \ttest_path_is_file $objdir/pack/multi-pack-index &&\n \tgit repack -adf &&\n-- \ngitgitgadget\n"},{"id":"357460","messageId":"CAGZ79kY7_RJfJomkpob85A4ubvXJt3BR8njkaozLgi-6ytxB9w@mail.gmail.com","threadId":"49281","inReplyTo":"37ee24c82bf88611808ec3bb1c36eef439c809db.1536158789.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 04/11] multi-pack-index: verify packname order","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-09-05T18:15:57Z","receivedAt":"2018-09-05T18:16:12Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Sep 5, 2018 at 7:46 AM Derrick Stolee via GitGitGadget\n wrote:\n>\n> From: Derrick Stolee \n>\n> The final check we make while loading a multi-pack-index is that\n> the packfile names are in lexicographical order. Make this error\n> be a die() instead.\n\nWhat is the advantage of having a die() here?\nWould the midx work under normal operation when\nnot sorted correctly?\n\nThis sounds like a hack for easy testability in this context,\nso could you clarify why this helps the regular user?\n\nThanks,\nStefan\n"},{"id":"357467","messageId":"CAPig+cR4vEcMBe4REd4Z=WMG-MR4PUnyNNBxoJacUXzB8ho6fg@mail.gmail.com","threadId":"49281","inReplyTo":"8dc38afe2b4d73e940daf9a0ff7f9c0a38802d95.1536158789.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 01/11] multi-pack-index: add 'verify' verb","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-09-05T18:59:30Z","receivedAt":"2018-09-05T18:59:44Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Sep 5, 2018 at 10:46 AM Derrick Stolee via GitGitGadget\n wrote:\n> The multi-pack-index builtin writes multi-pack-index files, and\n> uses a 'write' verb to do so. Add a 'verify' verb that checks this\n> file matches the contents of the pack-indexes it replaces.\n> [...]\n> Signed-off-by: Derrick Stolee \n> ---\n> diff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c\n> @@ -5,7 +5,7 @@\n> static char const * const builtin_multi_pack_index_usage[] = {\n> - N_(\"git multi-pack-index [--object-dir=] write\"),\n> + N_(\"git multi-pack-index [--object-dir=] [write|verify]\"),\n\nNit: The square brackets make the verb optional. You probably want\nparenthesis to indicate that one or the other is required:\n\n git multi-pack-index [--object-dir=] (write|verify)\n"},{"id":"357471","messageId":"da7eaf92-57f6-8129-149d-4260d84b4eb0@gmail.com","threadId":"49281","inReplyTo":"CAGZ79kY7_RJfJomkpob85A4ubvXJt3BR8njkaozLgi-6ytxB9w@mail.gmail.com","subject":"Re: [PATCH 04/11] multi-pack-index: verify packname order","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2018-09-05T19:11:51Z","receivedAt":"2018-09-05T19:11:58Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 9/5/2018 2:15 PM, Stefan Beller wrote:\n> On Wed, Sep 5, 2018 at 7:46 AM Derrick Stolee via GitGitGadget\n> wrote:\n>> From: Derrick Stolee \n>>\n>> The final check we make while loading a multi-pack-index is that\n>> the packfile names are in lexicographical order. Make this error\n>> be a die() instead.\n> What is the advantage of having a die() here?\n> Would the midx work under normal operation when\n> not sorted correctly?\n>\n> This sounds like a hack for easy testability in this context,\n> so could you clarify why this helps the regular user?\n\nThe multi-pack-index will not work as expected, since \nmidx_contains_pack() may provide incorrect results, and thus we will add \nthe \"missing\" packfiles to the packed_git linked list.\n\nThis _should_ be a die(), as something unexpected occurred (the file \ndoesn't match the format specification).\n\n"},{"id":"357473","messageId":"CAGZ79kZHmCyRLur=DgLx2sHhD1hCXn3pU+0CggwFQgWRzTkp5A@mail.gmail.com","threadId":"49281","inReplyTo":"da7eaf92-57f6-8129-149d-4260d84b4eb0@gmail.com","subject":"Re: [PATCH 04/11] multi-pack-index: verify packname order","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-09-05T19:14:56Z","receivedAt":"2018-09-05T19:15:10Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Sep 5, 2018 at 12:11 PM Derrick Stolee wrote:\n>\n> On 9/5/2018 2:15 PM, Stefan Beller wrote:\n> > On Wed, Sep 5, 2018 at 7:46 AM Derrick Stolee via GitGitGadget\n> > wrote:\n> >> From: Derrick Stolee \n> >>\n> >> The final check we make while loading a multi-pack-index is that\n> >> the packfile names are in lexicographical order. Make this error\n> >> be a die() instead.\n> > What is the advantage of having a die() here?\n> > Would the midx work under normal operation when\n> > not sorted correctly?\n> >\n> > This sounds like a hack for easy testability in this context,\n> > so could you clarify why this helps the regular user?\n>\n> The multi-pack-index will not work as expected, since\n> midx_contains_pack() may provide incorrect results, and thus we will add\n> the \"missing\" packfiles to the packed_git linked list.\n>\n> This _should_ be a die(), as something unexpected occurred (the file\n> doesn't match the format specification).\n\nThanks for the clarification.\n\nSo this patch actually hits 2 birds with one stone, as it fixes\na bug as well as adds the check for such corruption to the\nverify step?\n\nThanks,\nStefan\n"},{"id":"357475","messageId":"e09aa8e6-26fa-68b5-b4d1-1322b09e7974@gmail.com","threadId":"49281","inReplyTo":"CAGZ79kZHmCyRLur=DgLx2sHhD1hCXn3pU+0CggwFQgWRzTkp5A@mail.gmail.com","subject":"Re: [PATCH 04/11] multi-pack-index: verify packname order","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2018-09-05T19:28:26Z","receivedAt":"2018-09-05T19:28:31Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 9/5/2018 3:14 PM, Stefan Beller wrote:\n> On Wed, Sep 5, 2018 at 12:11 PM Derrick Stolee wrote:\n>> On 9/5/2018 2:15 PM, Stefan Beller wrote:\n>>> On Wed, Sep 5, 2018 at 7:46 AM Derrick Stolee via GitGitGadget\n>>> wrote:\n>>>> From: Derrick Stolee \n>>>>\n>>>> The final check we make while loading a multi-pack-index is that\n>>>> the packfile names are in lexicographical order. Make this error\n>>>> be a die() instead.\n>>> What is the advantage of having a die() here?\n>>> Would the midx work under normal operation when\n>>> not sorted correctly?\n>>>\n>>> This sounds like a hack for easy testability in this context,\n>>> so could you clarify why this helps the regular user?\n>> The multi-pack-index will not work as expected, since\n>> midx_contains_pack() may provide incorrect results, and thus we will add\n>> the \"missing\" packfiles to the packed_git linked list.\n>>\n>> This _should_ be a die(), as something unexpected occurred (the file\n>> doesn't match the format specification).\n> Thanks for the clarification.\n>\n> So this patch actually hits 2 birds with one stone, as it fixes\n> a bug as well as adds the check for such corruption to the\n> verify step?\nThis is a common occurrence in this series (replacing error() with \ndie()) as we are now exercising those conditions and clarifying what \nshould happen.\n"},{"id":"357476","messageId":"81279be3-6d9a-ae39-a5e9-399fd4be2a23@gmail.com","threadId":"49281","inReplyTo":"CAPig+cR4vEcMBe4REd4Z=WMG-MR4PUnyNNBxoJacUXzB8ho6fg@mail.gmail.com","subject":"Re: [PATCH 01/11] multi-pack-index: add 'verify' verb","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2018-09-05T19:37:44Z","receivedAt":"2018-09-05T19:39:22Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 9/5/2018 2:59 PM, Eric Sunshine wrote:\n> On Wed, Sep 5, 2018 at 10:46 AM Derrick Stolee via GitGitGadget\n> wrote:\n>> The multi-pack-index builtin writes multi-pack-index files, and\n>> uses a 'write' verb to do so. Add a 'verify' verb that checks this\n>> file matches the contents of the pack-indexes it replaces.\n>> [...]\n>> Signed-off-by: Derrick Stolee \n>> ---\n>> diff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c\n>> @@ -5,7 +5,7 @@\n>> static char const * const builtin_multi_pack_index_usage[] = {\n>> - N_(\"git multi-pack-index [--object-dir=] write\"),\n>> + N_(\"git multi-pack-index [--object-dir=] [write|verify]\"),\n> Nit: The square brackets make the verb optional. You probably want\n> parenthesis to indicate that one or the other is required:\n>\n> git multi-pack-index [--object-dir=] (write|verify)\nThanks for catching this mistake! This would be easy to miss and keep \naround forever.\n"},{"id":"357595","messageId":"CAPig+cSOcfQDYP+LEZxhRst__dXZUc=euoORZk0iGYrh9adm1A@mail.gmail.com","threadId":"49281","inReplyTo":"787e1fb61626afe79cfc846c4175d4b4d169ff42.1536158789.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 02/11] multi-pack-index: verify bad header","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-09-07T00:27:01Z","receivedAt":"2018-09-07T00:27:14Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Sep 5, 2018 at 10:46 AM Derrick Stolee via GitGitGadget\n wrote:\n> Add tests that check corrupted headers in a few ways:\n> [...]\n> Signed-off-by: Derrick Stolee \n> ---\n> diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\n> @@ -154,6 +154,51 @@ test_expect_success 'verify multi-pack-index success' '\n> +corrupt_midx_and_verify() {\n> + POS=$1\n> + DATA=\"${2:-\\0}\"\n> + OBJDIR=$3\n> + GREPSTR=\"$4\"\n\nIf you happen to re-roll, perhaps make these assignments part of the\n&&-chain to protect against someone coming along some day an inserting\ncode before them without realizing that the &&-chain is broken.\n\n> + FILE=$OBJDIR/pack/multi-pack-index &&\n> + chmod a+w $FILE &&\n> + test_when_finished mv midx-backup $FILE &&\n> + cp $FILE midx-backup &&\n> + printf \"$DATA\" | dd of=\"$FILE\" bs=1 seek=\"$POS\" conv=notrunc &&\n> + test_must_fail git multi-pack-index verify --object-dir=$OBJDIR 2>test_err &&\n> + grep -v \"^+\" test_err >err &&\n> + test_i18ngrep \"$GREPSTR\" err\n> +}\n"},{"id":"357596","messageId":"CAPig+cQKt4sq4bhRUoGJMV=GxHzx-AtD_5J7DNtfMngH-uJRFQ@mail.gmail.com","threadId":"49281","inReplyTo":"e1498aea45eb46feb3664413f49b70bdf048d284.1536158789.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 09/11] multi-pack-index: verify object offsets","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-09-07T00:34:02Z","receivedAt":"2018-09-07T00:34:17Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Sep 5, 2018 at 10:46 AM Derrick Stolee via GitGitGadget\n wrote:\n> Replace the BUG() statement with a die() statement, now that we\n> may hit a bad pack-int-id during a 'verify' command on a corrupt\n> multi-pack-index, and it is covered by a test.\n>\n> Signed-off-by: Derrick Stolee \n> ---\n> diff --git a/midx.c b/midx.c\n> @@ -197,7 +197,7 @@ int prepare_midx_pack(struct multi_pack_index *m, uint32_t pack_int_id)\n> if (pack_int_id >= m->num_packs)\n> - BUG(\"bad pack-int-id\");\n> + die(_(\"bad pack-int-id\"));\n\nFor someone trying to diagnose this issue, would it be helpful to know\n(that is, print out) the values of pack_int_id and num_packs?\n"},{"id":"357621","messageId":"13c86d01-7b7d-29c8-380f-379b57da9638@gmail.com","threadId":"49281","inReplyTo":"CAPig+cQKt4sq4bhRUoGJMV=GxHzx-AtD_5J7DNtfMngH-uJRFQ@mail.gmail.com","subject":"Re: [PATCH 09/11] multi-pack-index: verify object offsets","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2018-09-07T13:10:49Z","receivedAt":"2018-09-07T13:10:52Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 9/6/2018 8:34 PM, Eric Sunshine wrote:\n> On Wed, Sep 5, 2018 at 10:46 AM Derrick Stolee via GitGitGadget\n> wrote:\n>> Replace the BUG() statement with a die() statement, now that we\n>> may hit a bad pack-int-id during a 'verify' command on a corrupt\n>> multi-pack-index, and it is covered by a test.\n>>\n>> Signed-off-by: Derrick Stolee \n>> ---\n>> diff --git a/midx.c b/midx.c\n>> @@ -197,7 +197,7 @@ int prepare_midx_pack(struct multi_pack_index *m, uint32_t pack_int_id)\n>> if (pack_int_id >= m->num_packs)\n>> - BUG(\"bad pack-int-id\");\n>> + die(_(\"bad pack-int-id\"));\n> For someone trying to diagnose this issue, would it be helpful to know\n> (that is, print out) the values of pack_int_id and num_packs?\nCan do! Thanks.\n"},{"id":"358067","messageId":"pull.34.v2.git.gitgitgadget@gmail.com","threadId":"49281","inReplyTo":"pull.34.git.gitgitgadget@gmail.com","subject":"[PATCH v2 00/11] Add 'git multi-pack-index verify' command","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-09-13T18:02:12Z","receivedAt":"2018-09-13T18:02:15Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"The multi-pack-index file provides faster lookups in repos with many\npackfiles by duplicating the information from multiple pack-indexes into a\nsingle file. This series allows us to verify a multi-pack-index using 'git\nmulti-pack-index verify' and 'git fsck' (when core.multiPackIndex is true).\n\nThe pattern for the tests is similar to that found in t5318-commit-graph.sh.\n\nDuring testing, I found a bug in how we check for the size of off_t (we are\nnot actually checking off_t, but instead uint32_t). See \"multi-pack-index:\nfix 32-bit vs 64-bit size check\".\n\nThanks to Ævar [1], I added a commit that provides progress updates when\nchecking object offsets.\n\nBased on ds/multi-pack-index\n\n[1] \nhttps://public-inbox.org/git/20180904202729.13900-1-avarab@gmail.com/T/#u\n\nDerrick Stolee (11):\n multi-pack-index: add 'verify' verb\n multi-pack-index: verify bad header\n multi-pack-index: verify corrupt chunk lookup table\n multi-pack-index: verify packname order\n multi-pack-index: verify missing pack\n multi-pack-index: verify oid fanout order\n multi-pack-index: verify oid lookup order\n multi-pack-index: fix 32-bit vs 64-bit size check\n multi-pack-index: verify object offsets\n multi-pack-index: report progress during 'verify'\n fsck: verify multi-pack-index\n\n Documentation/git-multi-pack-index.txt | 10 ++\n builtin/fsck.c | 18 ++++\n builtin/multi-pack-index.c | 4 +-\n midx.c | 113 ++++++++++++++++----\n midx.h | 1 +\n t/t5319-multi-pack-index.sh | 136 ++++++++++++++++++++++++-\n 6 files changed, 262 insertions(+), 20 deletions(-)\n\n\nbase-commit: 6a22d521260f86dff8fe6f23ab329cebb62ba4f0\nPublished-As: https://github.com/gitgitgadget/git/releases/tags/pr-34%2Fderrickstolee%2Fmidx%2Fverify-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-34/derrickstolee/midx/verify-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/34\n\nRange-diff vs v1:\n\n 1: 8dc38afe2b ! 1: d8ffd84d67 multi-pack-index: add 'verify' verb\n @@ -47,7 +47,7 @@\n \n static char const * const builtin_multi_pack_index_usage[] = {\n -\tN_(\"git multi-pack-index [--object-dir=] write\"),\n -+\tN_(\"git multi-pack-index [--object-dir=] [write|verify]\"),\n ++\tN_(\"git multi-pack-index [--object-dir=] (write|verify)\"),\n \tNULL\n };\n \n 2: 787e1fb616 ! 2: 9590895830 multi-pack-index: verify bad header\n @@ -61,10 +61,10 @@\n \n +# usage: corrupt_midx_and_verify \n +corrupt_midx_and_verify() {\n -+\tPOS=$1\n -+\tDATA=\"${2:-\\0}\"\n -+\tOBJDIR=$3\n -+\tGREPSTR=\"$4\"\n ++\tPOS=$1 &&\n ++\tDATA=\"${2:-\\0}\" &&\n ++\tOBJDIR=$3 &&\n ++\tGREPSTR=\"$4\" &&\n +\tFILE=$OBJDIR/pack/multi-pack-index &&\n +\tchmod a+w $FILE &&\n +\ttest_when_finished mv midx-backup $FILE &&\n 3: b385aa2abf = 3: 2448173844 multi-pack-index: verify corrupt chunk lookup table\n 4: 37ee24c82b = 4: 947241bfdc multi-pack-index: verify packname order\n 5: b747da415c = 5: 4058867380 multi-pack-index: verify missing pack\n 6: 58e5c09468 = 6: ea1c522702 multi-pack-index: verify oid fanout order\n 7: b21772d054 = 7: 511791de91 multi-pack-index: verify oid lookup order\n 8: b08d3f0055 = 8: 210649bf83 multi-pack-index: fix 32-bit vs 64-bit size check\n 9: e1498aea45 ! 9: ef20193d59 multi-pack-index: verify object offsets\n @@ -21,7 +21,8 @@\n \n \tif (pack_int_id >= m->num_packs)\n -\t\tBUG(\"bad pack-int-id\");\n -+\t\tdie(_(\"bad pack-int-id\"));\n ++\t\tdie(_(\"bad pack-int-id: %u (%u total packs\"),\n ++\t\t pack_int_id, m->num_packs);\n \n \tif (m->packs[pack_int_id])\n \t\treturn 0;\n 10: acf8cfd632 = 10: 29ebc17161 multi-pack-index: report progress during 'verify'\n 11: 09d16aff20 ! 11: 406c88b456 fsck: verify multi-pack-index\n @@ -40,14 +40,14 @@\n --- a/t/t5319-multi-pack-index.sh\n +++ b/t/t5319-multi-pack-index.sh\n @@\n - \tDATA=\"${2:-\\0}\"\n - \tOBJDIR=$3\n - \tGREPSTR=\"$4\"\n -+\tCOMMAND=\"$5\"\n + \tDATA=\"${2:-\\0}\" &&\n + \tOBJDIR=$3 &&\n + \tGREPSTR=\"$4\" &&\n ++\tCOMMAND=\"$5\" &&\n +\tif test -z \"$COMMAND\"\n +\tthen\n +\t\tCOMMAND=\"git multi-pack-index verify --object-dir=$OBJDIR\"\n -+\tfi\n ++\tfi &&\n \tFILE=$OBJDIR/pack/multi-pack-index &&\n \tchmod a+w $FILE &&\n \ttest_when_finished mv midx-backup $FILE &&\n\n-- \ngitgitgadget\n"},{"id":"358068","messageId":"95908958302a4a27d8ba444ed7bfd867cb43cd20.1536861730.git.gitgitgadget@gmail.com","threadId":"49281","inReplyTo":"pull.34.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 02/11] multi-pack-index: verify bad header","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-09-13T18:02:15Z","receivedAt":"2018-09-13T18:02:19Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nWhen verifying if a multi-pack-index file is valid, we want the\ncommand to fail to signal an invalid file. Previously, we wrote\nan error to stderr and continued as if we had no multi-pack-index.\nNow, die() instead of error().\n\nAdd tests that check corrupted headers in a few ways:\n\n* Bad signature\n* Bad file version\n* Bad hash version\n* Truncated hash count\n* Extended hash count\n\nSigned-off-by: Derrick Stolee \n---\n midx.c | 18 +++++----------\n t/t5319-multi-pack-index.sh | 46 ++++++++++++++++++++++++++++++++++++-\n 2 files changed, 51 insertions(+), 13 deletions(-)\n\ndiff --git a/midx.c b/midx.c\nindex b253bed517..ec78254bb6 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -76,24 +76,18 @@ struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local\n \tm->local = local;\n \n \tm->signature = get_be32(m->data);\n-\tif (m->signature != MIDX_SIGNATURE) {\n-\t\terror(_(\"multi-pack-index signature 0x%08x does not match signature 0x%08x\"),\n+\tif (m->signature != MIDX_SIGNATURE)\n+\t\tdie(_(\"multi-pack-index signature 0x%08x does not match signature 0x%08x\"),\n \t\t m->signature, MIDX_SIGNATURE);\n-\t\tgoto cleanup_fail;\n-\t}\n \n \tm->version = m->data[MIDX_BYTE_FILE_VERSION];\n-\tif (m->version != MIDX_VERSION) {\n-\t\terror(_(\"multi-pack-index version %d not recognized\"),\n+\tif (m->version != MIDX_VERSION)\n+\t\tdie(_(\"multi-pack-index version %d not recognized\"),\n \t\t m->version);\n-\t\tgoto cleanup_fail;\n-\t}\n \n \thash_version = m->data[MIDX_BYTE_HASH_VERSION];\n-\tif (hash_version != MIDX_HASH_VERSION) {\n-\t\terror(_(\"hash version %u does not match\"), hash_version);\n-\t\tgoto cleanup_fail;\n-\t}\n+\tif (hash_version != MIDX_HASH_VERSION)\n+\t\tdie(_(\"hash version %u does not match\"), hash_version);\n \tm->hash_len = MIDX_HASH_LEN;\n \n \tm->num_chunks = m->data[MIDX_BYTE_NUM_CHUNKS];\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex 1c4e0e6d31..e04b5f43a2 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -154,6 +154,51 @@ test_expect_success 'verify multi-pack-index success' '\n \tgit multi-pack-index verify --object-dir=$objdir\n '\n \n+# usage: corrupt_midx_and_verify \n+corrupt_midx_and_verify() {\n+\tPOS=$1 &&\n+\tDATA=\"${2:-\\0}\" &&\n+\tOBJDIR=$3 &&\n+\tGREPSTR=\"$4\" &&\n+\tFILE=$OBJDIR/pack/multi-pack-index &&\n+\tchmod a+w $FILE &&\n+\ttest_when_finished mv midx-backup $FILE &&\n+\tcp $FILE midx-backup &&\n+\tprintf \"$DATA\" | dd of=\"$FILE\" bs=1 seek=\"$POS\" conv=notrunc &&\n+\ttest_must_fail git multi-pack-index verify --object-dir=$OBJDIR 2>test_err &&\n+\tgrep -v \"^+\" test_err >err &&\n+\ttest_i18ngrep \"$GREPSTR\" err\n+}\n+\n+test_expect_success 'verify bad signature' '\n+\tcorrupt_midx_and_verify 0 \"\\00\" $objdir \\\n+\t\t\"multi-pack-index signature\"\n+'\n+\n+MIDX_BYTE_VERSION=4\n+MIDX_BYTE_OID_VERSION=5\n+MIDX_BYTE_CHUNK_COUNT=6\n+\n+test_expect_success 'verify bad version' '\n+\tcorrupt_midx_and_verify $MIDX_BYTE_VERSION \"\\00\" $objdir \\\n+\t\t\"multi-pack-index version\"\n+'\n+\n+test_expect_success 'verify bad OID version' '\n+\tcorrupt_midx_and_verify $MIDX_BYTE_OID_VERSION \"\\02\" $objdir \\\n+\t\t\"hash version\"\n+'\n+\n+test_expect_success 'verify truncated chunk count' '\n+\tcorrupt_midx_and_verify $MIDX_BYTE_CHUNK_COUNT \"\\01\" $objdir \\\n+\t\t\"missing required\"\n+'\n+\n+test_expect_success 'verify extended chunk count' '\n+\tcorrupt_midx_and_verify $MIDX_BYTE_CHUNK_COUNT \"\\07\" $objdir \\\n+\t\t\"terminating multi-pack-index chunk id appears earlier than expected\"\n+'\n+\n test_expect_success 'repack removes multi-pack-index' '\n \ttest_path_is_file $objdir/pack/multi-pack-index &&\n \tgit repack -adf &&\n@@ -191,7 +236,6 @@ test_expect_success 'multi-pack-index in an alternate' '\n \n compare_results_with_midx \"with alternate (remote midx)\"\n \n-\n # usage: corrupt_data []\n corrupt_data () {\n \tfile=$1\n-- \ngitgitgadget\n\n"},{"id":"358069","messageId":"24481738444bf7da90dddff700dc69d653c440fb.1536861730.git.gitgitgadget@gmail.com","threadId":"49281","inReplyTo":"pull.34.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 03/11] multi-pack-index: verify corrupt chunk lookup table","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-09-13T18:02:16Z","receivedAt":"2018-09-13T18:02:20Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nSigned-off-by: Derrick Stolee \n---\n midx.c | 3 +++\n t/t5319-multi-pack-index.sh | 13 +++++++++++++\n 2 files changed, 16 insertions(+)\n\ndiff --git a/midx.c b/midx.c\nindex ec78254bb6..8b054b39ab 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -100,6 +100,9 @@ struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local\n \t\tuint64_t chunk_offset = get_be64(m->data + MIDX_HEADER_SIZE + 4 +\n \t\t\t\t\t\t MIDX_CHUNKLOOKUP_WIDTH * i);\n \n+\t\tif (chunk_offset >= m->data_len)\n+\t\t\tdie(_(\"invalid chunk offset (too large)\"));\n+\n \t\tswitch (chunk_id) {\n \t\t\tcase MIDX_CHUNKID_PACKNAMES:\n \t\t\t\tm->chunk_pack_names = m->data + chunk_offset;\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex e04b5f43a2..c54b6e7188 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -178,6 +178,9 @@ test_expect_success 'verify bad signature' '\n MIDX_BYTE_VERSION=4\n MIDX_BYTE_OID_VERSION=5\n MIDX_BYTE_CHUNK_COUNT=6\n+MIDX_HEADER_SIZE=12\n+MIDX_BYTE_CHUNK_ID=$MIDX_HEADER_SIZE\n+MIDX_BYTE_CHUNK_OFFSET=$(($MIDX_HEADER_SIZE + 4))\n \n test_expect_success 'verify bad version' '\n \tcorrupt_midx_and_verify $MIDX_BYTE_VERSION \"\\00\" $objdir \\\n@@ -199,6 +202,16 @@ test_expect_success 'verify extended chunk count' '\n \t\t\"terminating multi-pack-index chunk id appears earlier than expected\"\n '\n \n+test_expect_success 'verify missing required chunk' '\n+\tcorrupt_midx_and_verify $MIDX_BYTE_CHUNK_ID \"\\01\" $objdir \\\n+\t\t\"missing required\"\n+'\n+\n+test_expect_success 'verify invalid chunk offset' '\n+\tcorrupt_midx_and_verify $MIDX_BYTE_CHUNK_OFFSET \"\\01\" $objdir \\\n+\t\t\"invalid chunk offset (too large)\"\n+'\n+\n test_expect_success 'repack removes multi-pack-index' '\n \ttest_path_is_file $objdir/pack/multi-pack-index &&\n \tgit repack -adf &&\n-- \ngitgitgadget\n\n"},{"id":"358070","messageId":"947241bfdcc93223ea50a6721cbac2b435ca6b2e.1536861730.git.gitgitgadget@gmail.com","threadId":"49281","inReplyTo":"pull.34.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 04/11] multi-pack-index: verify packname order","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-09-13T18:02:18Z","receivedAt":"2018-09-13T18:02:21Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe final check we make while loading a multi-pack-index is that\nthe packfile names are in lexicographical order. Make this error\nbe a die() instead.\n\nIn order to test this condition, we need multiple packfiles.\nEarlier in t5319-multi-pack-index.sh, we tested the interaction with\n'git repack' but this limits us to one packfile in our object dir.\nMove these repack tests until after the 'verify' tests.\n\nSigned-off-by: Derrick Stolee \n---\n midx.c | 6 ++----\n t/t5319-multi-pack-index.sh | 10 ++++++++++\n 2 files changed, 12 insertions(+), 4 deletions(-)\n\ndiff --git a/midx.c b/midx.c\nindex 8b054b39ab..e655a15aed 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -157,12 +157,10 @@ struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local\n \n \t\tcur_pack_name += strlen(cur_pack_name) + 1;\n \n-\t\tif (i && strcmp(m->pack_names[i], m->pack_names[i - 1]) <= 0) {\n-\t\t\terror(_(\"multi-pack-index pack names out of order: '%s' before '%s'\"),\n+\t\tif (i && strcmp(m->pack_names[i], m->pack_names[i - 1]) <= 0)\n+\t\t\tdie(_(\"multi-pack-index pack names out of order: '%s' before '%s'\"),\n \t\t\t m->pack_names[i - 1],\n \t\t\t m->pack_names[i]);\n-\t\t\tgoto cleanup_fail;\n-\t\t}\n \t}\n \n \treturn m;\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex c54b6e7188..01a3cd6b00 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -181,6 +181,11 @@ MIDX_BYTE_CHUNK_COUNT=6\n MIDX_HEADER_SIZE=12\n MIDX_BYTE_CHUNK_ID=$MIDX_HEADER_SIZE\n MIDX_BYTE_CHUNK_OFFSET=$(($MIDX_HEADER_SIZE + 4))\n+MIDX_NUM_CHUNKS=5\n+MIDX_CHUNK_LOOKUP_WIDTH=12\n+MIDX_OFFSET_PACKNAMES=$(($MIDX_HEADER_SIZE + \\\n+\t\t\t $MIDX_NUM_CHUNKS * $MIDX_CHUNK_LOOKUP_WIDTH))\n+MIDX_BYTE_PACKNAME_ORDER=$(($MIDX_OFFSET_PACKNAMES + 2))\n \n test_expect_success 'verify bad version' '\n \tcorrupt_midx_and_verify $MIDX_BYTE_VERSION \"\\00\" $objdir \\\n@@ -212,6 +217,11 @@ test_expect_success 'verify invalid chunk offset' '\n \t\t\"invalid chunk offset (too large)\"\n '\n \n+test_expect_success 'verify packnames out of order' '\n+\tcorrupt_midx_and_verify $MIDX_BYTE_PACKNAME_ORDER \"z\" $objdir \\\n+\t\t\"pack names out of order\"\n+'\n+\n test_expect_success 'repack removes multi-pack-index' '\n \ttest_path_is_file $objdir/pack/multi-pack-index &&\n \tgit repack -adf &&\n-- \ngitgitgadget\n\n"},{"id":"358071","messageId":"405886738002e60b9f72d6edb10e4929c9493374.1536861730.git.gitgitgadget@gmail.com","threadId":"49281","inReplyTo":"pull.34.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 05/11] multi-pack-index: verify missing pack","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-09-13T18:02:19Z","receivedAt":"2018-09-13T18:02:23Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nSigned-off-by: Derrick Stolee \n---\n midx.c | 16 ++++++++++++++++\n t/t5319-multi-pack-index.sh | 5 +++++\n 2 files changed, 21 insertions(+)\n\ndiff --git a/midx.c b/midx.c\nindex e655a15aed..a02b19efc1 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -926,13 +926,29 @@ void clear_midx_file(const char *object_dir)\n \n int verify_midx_error;\n \n+static void midx_report(const char *fmt, ...)\n+{\n+\tva_list ap;\n+\tverify_midx_error = 1;\n+\tva_start(ap, fmt);\n+\tvfprintf(stderr, fmt, ap);\n+\tfprintf(stderr, \"\\n\");\n+\tva_end(ap);\n+}\n+\n int verify_midx_file(const char *object_dir)\n {\n+\tuint32_t i;\n \tstruct multi_pack_index *m = load_multi_pack_index(object_dir, 1);\n \tverify_midx_error = 0;\n \n \tif (!m)\n \t\treturn 0;\n \n+\tfor (i = 0; i < m->num_packs; i++) {\n+\t\tif (prepare_midx_pack(m, i))\n+\t\t\tmidx_report(\"failed to load pack in position %d\", i);\n+\t}\n+\n \treturn verify_midx_error;\n }\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex 01a3cd6b00..0a566afb05 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -222,6 +222,11 @@ test_expect_success 'verify packnames out of order' '\n \t\t\"pack names out of order\"\n '\n \n+test_expect_success 'verify packnames out of order' '\n+\tcorrupt_midx_and_verify $MIDX_BYTE_PACKNAME_ORDER \"a\" $objdir \\\n+\t\t\"failed to load pack\"\n+'\n+\n test_expect_success 'repack removes multi-pack-index' '\n \ttest_path_is_file $objdir/pack/multi-pack-index &&\n \tgit repack -adf &&\n-- \ngitgitgadget\n\n"},{"id":"358072","messageId":"ea1c522702822e65e962cf3b5178437549ee7a2d.1536861730.git.gitgitgadget@gmail.com","threadId":"49281","inReplyTo":"pull.34.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 06/11] multi-pack-index: verify oid fanout order","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-09-13T18:02:20Z","receivedAt":"2018-09-13T18:02:25Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nSigned-off-by: Derrick Stolee \n---\n midx.c | 9 +++++++++\n t/t5319-multi-pack-index.sh | 8 ++++++++\n 2 files changed, 17 insertions(+)\n\ndiff --git a/midx.c b/midx.c\nindex a02b19efc1..dfd26b4d74 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -950,5 +950,14 @@ int verify_midx_file(const char *object_dir)\n \t\t\tmidx_report(\"failed to load pack in position %d\", i);\n \t}\n \n+\tfor (i = 0; i < 255; i++) {\n+\t\tuint32_t oid_fanout1 = ntohl(m->chunk_oid_fanout[i]);\n+\t\tuint32_t oid_fanout2 = ntohl(m->chunk_oid_fanout[i + 1]);\n+\n+\t\tif (oid_fanout1 > oid_fanout2)\n+\t\t\tmidx_report(_(\"oid fanout out of order: fanout[%d] = %\"PRIx32\" > %\"PRIx32\" = fanout[%d]\"),\n+\t\t\t\t i, oid_fanout1, oid_fanout2, i + 1);\n+\t}\n+\n \treturn verify_midx_error;\n }\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex 0a566afb05..47a54e138d 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -186,6 +186,9 @@ MIDX_CHUNK_LOOKUP_WIDTH=12\n MIDX_OFFSET_PACKNAMES=$(($MIDX_HEADER_SIZE + \\\n \t\t\t $MIDX_NUM_CHUNKS * $MIDX_CHUNK_LOOKUP_WIDTH))\n MIDX_BYTE_PACKNAME_ORDER=$(($MIDX_OFFSET_PACKNAMES + 2))\n+MIDX_OFFSET_OID_FANOUT=$(($MIDX_OFFSET_PACKNAMES + 652))\n+MIDX_OID_FANOUT_WIDTH=4\n+MIDX_BYTE_OID_FANOUT_ORDER=$((MIDX_OFFSET_OID_FANOUT + 250 * $MIDX_OID_FANOUT_WIDTH + 1))\n \n test_expect_success 'verify bad version' '\n \tcorrupt_midx_and_verify $MIDX_BYTE_VERSION \"\\00\" $objdir \\\n@@ -227,6 +230,11 @@ test_expect_success 'verify packnames out of order' '\n \t\t\"failed to load pack\"\n '\n \n+test_expect_success 'verify oid fanout out of order' '\n+\tcorrupt_midx_and_verify $MIDX_BYTE_OID_FANOUT_ORDER \"\\01\" $objdir \\\n+\t\t\"oid fanout out of order\"\n+'\n+\n test_expect_success 'repack removes multi-pack-index' '\n \ttest_path_is_file $objdir/pack/multi-pack-index &&\n \tgit repack -adf &&\n-- \ngitgitgadget\n\n"},{"id":"358073","messageId":"511791de91115e0d51c1635f8ec3d337ca009971.1536861730.git.gitgitgadget@gmail.com","threadId":"49281","inReplyTo":"pull.34.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 07/11] multi-pack-index: verify oid lookup order","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-09-13T18:02:22Z","receivedAt":"2018-09-13T18:02:25Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nSigned-off-by: Derrick Stolee \n---\n midx.c | 11 +++++++++++\n t/t5319-multi-pack-index.sh | 8 ++++++++\n 2 files changed, 19 insertions(+)\n\ndiff --git a/midx.c b/midx.c\nindex dfd26b4d74..06d5cfc826 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -959,5 +959,16 @@ int verify_midx_file(const char *object_dir)\n \t\t\t\t i, oid_fanout1, oid_fanout2, i + 1);\n \t}\n \n+\tfor (i = 0; i < m->num_objects - 1; i++) {\n+\t\tstruct object_id oid1, oid2;\n+\n+\t\tnth_midxed_object_oid(&oid1, m, i);\n+\t\tnth_midxed_object_oid(&oid2, m, i + 1);\n+\n+\t\tif (oidcmp(&oid1, &oid2) >= 0)\n+\t\t\tmidx_report(_(\"oid lookup out of order: oid[%d] = %s >= %s = oid[%d]\"),\n+\t\t\t\t i, oid_to_hex(&oid1), oid_to_hex(&oid2), i + 1);\n+\t}\n+\n \treturn verify_midx_error;\n }\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex 47a54e138d..a968b9a959 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -175,6 +175,7 @@ test_expect_success 'verify bad signature' '\n \t\t\"multi-pack-index signature\"\n '\n \n+HASH_LEN=20\n MIDX_BYTE_VERSION=4\n MIDX_BYTE_OID_VERSION=5\n MIDX_BYTE_CHUNK_COUNT=6\n@@ -189,6 +190,8 @@ MIDX_BYTE_PACKNAME_ORDER=$(($MIDX_OFFSET_PACKNAMES + 2))\n MIDX_OFFSET_OID_FANOUT=$(($MIDX_OFFSET_PACKNAMES + 652))\n MIDX_OID_FANOUT_WIDTH=4\n MIDX_BYTE_OID_FANOUT_ORDER=$((MIDX_OFFSET_OID_FANOUT + 250 * $MIDX_OID_FANOUT_WIDTH + 1))\n+MIDX_OFFSET_OID_LOOKUP=$(($MIDX_OFFSET_OID_FANOUT + 256 * $MIDX_OID_FANOUT_WIDTH))\n+MIDX_BYTE_OID_LOOKUP=$(($MIDX_OFFSET_OID_LOOKUP + 16 * $HASH_LEN))\n \n test_expect_success 'verify bad version' '\n \tcorrupt_midx_and_verify $MIDX_BYTE_VERSION \"\\00\" $objdir \\\n@@ -235,6 +238,11 @@ test_expect_success 'verify oid fanout out of order' '\n \t\t\"oid fanout out of order\"\n '\n \n+test_expect_success 'verify oid lookup out of order' '\n+\tcorrupt_midx_and_verify $MIDX_BYTE_OID_LOOKUP \"\\00\" $objdir \\\n+\t\t\"oid lookup out of order\"\n+'\n+\n test_expect_success 'repack removes multi-pack-index' '\n \ttest_path_is_file $objdir/pack/multi-pack-index &&\n \tgit repack -adf &&\n-- \ngitgitgadget\n\n"},{"id":"358074","messageId":"210649bf838bf0dc338671c691cbd015bef41240.1536861730.git.gitgitgadget@gmail.com","threadId":"49281","inReplyTo":"pull.34.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 08/11] multi-pack-index: fix 32-bit vs 64-bit size check","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-09-13T18:02:23Z","receivedAt":"2018-09-13T18:02:27Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nWhen loading a 64-bit offset, we intend to check that off_t can store\nthe resulting offset. However, the condition accidentally checks the\n32-bit offset to see if it is smaller than a 64-bit value. Fix it,\nand this will be covered by a test in the 'git multi-pack-index verify'\ncommand in a later commit.\n\nSigned-off-by: Derrick Stolee \n---\n midx.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/midx.c b/midx.c\nindex 06d5cfc826..80094c02a7 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -236,7 +236,7 @@ static off_t nth_midxed_offset(struct multi_pack_index *m, uint32_t pos)\n \toffset32 = get_be32(offset_data + sizeof(uint32_t));\n \n \tif (m->chunk_large_offsets && offset32 & MIDX_LARGE_OFFSET_NEEDED) {\n-\t\tif (sizeof(offset32) < sizeof(uint64_t))\n+\t\tif (sizeof(off_t) < sizeof(uint64_t))\n \t\t\tdie(_(\"multi-pack-index stores a 64-bit offset, but off_t is too small\"));\n \n \t\toffset32 ^= MIDX_LARGE_OFFSET_NEEDED;\n-- \ngitgitgadget\n\n"},{"id":"358075","messageId":"ef20193d59bd33a4c56e8c059ea7ddefbb6a8e72.1536861730.git.gitgitgadget@gmail.com","threadId":"49281","inReplyTo":"pull.34.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 09/11] multi-pack-index: verify object offsets","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-09-13T18:02:25Z","receivedAt":"2018-09-13T18:02:28Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe 'git multi-pack-index verify' command must verify the object\noffsets stored in the multi-pack-index are correct. There are two\nways the offset chunk can be incorrect: the pack-int-id and the\nobject offset.\n\nReplace the BUG() statement with a die() statement, now that we\nmay hit a bad pack-int-id during a 'verify' command on a corrupt\nmulti-pack-index, and it is covered by a test.\n\nSigned-off-by: Derrick Stolee \n---\n midx.c | 29 ++++++++++++++++++++++++++++-\n t/t5319-multi-pack-index.sh | 27 +++++++++++++++++++++++++++\n 2 files changed, 55 insertions(+), 1 deletion(-)\n\ndiff --git a/midx.c b/midx.c\nindex 80094c02a7..47e7e6113a 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -197,7 +197,8 @@ int prepare_midx_pack(struct multi_pack_index *m, uint32_t pack_int_id)\n \tstruct strbuf pack_name = STRBUF_INIT;\n \n \tif (pack_int_id >= m->num_packs)\n-\t\tBUG(\"bad pack-int-id\");\n+\t\tdie(_(\"bad pack-int-id: %u (%u total packs\"),\n+\t\t pack_int_id, m->num_packs);\n \n \tif (m->packs[pack_int_id])\n \t\treturn 0;\n@@ -970,5 +971,31 @@ int verify_midx_file(const char *object_dir)\n \t\t\t\t i, oid_to_hex(&oid1), oid_to_hex(&oid2), i + 1);\n \t}\n \n+\tfor (i = 0; i < m->num_objects; i++) {\n+\t\tstruct object_id oid;\n+\t\tstruct pack_entry e;\n+\t\toff_t m_offset, p_offset;\n+\n+\t\tnth_midxed_object_oid(&oid, m, i);\n+\t\tif (!fill_midx_entry(&oid, &e, m)) {\n+\t\t\tmidx_report(_(\"failed to load pack entry for oid[%d] = %s\"),\n+\t\t\t\t i, oid_to_hex(&oid));\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tif (open_pack_index(e.p)) {\n+\t\t\tmidx_report(_(\"failed to load pack-index for packfile %s\"),\n+\t\t\t\t e.p->pack_name);\n+\t\t\tbreak;\n+\t\t}\n+\n+\t\tm_offset = e.offset;\n+\t\tp_offset = find_pack_entry_one(oid.hash, e.p);\n+\n+\t\tif (m_offset != p_offset)\n+\t\t\tmidx_report(_(\"incorrect object offset for oid[%d] = %s: %\"PRIx64\" != %\"PRIx64),\n+\t\t\t\t i, oid_to_hex(&oid), m_offset, p_offset);\n+\t}\n+\n \treturn verify_midx_error;\n }\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex a968b9a959..828c240389 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -176,6 +176,7 @@ test_expect_success 'verify bad signature' '\n '\n \n HASH_LEN=20\n+NUM_OBJECTS=74\n MIDX_BYTE_VERSION=4\n MIDX_BYTE_OID_VERSION=5\n MIDX_BYTE_CHUNK_COUNT=6\n@@ -192,6 +193,10 @@ MIDX_OID_FANOUT_WIDTH=4\n MIDX_BYTE_OID_FANOUT_ORDER=$((MIDX_OFFSET_OID_FANOUT + 250 * $MIDX_OID_FANOUT_WIDTH + 1))\n MIDX_OFFSET_OID_LOOKUP=$(($MIDX_OFFSET_OID_FANOUT + 256 * $MIDX_OID_FANOUT_WIDTH))\n MIDX_BYTE_OID_LOOKUP=$(($MIDX_OFFSET_OID_LOOKUP + 16 * $HASH_LEN))\n+MIDX_OFFSET_OBJECT_OFFSETS=$(($MIDX_OFFSET_OID_LOOKUP + $NUM_OBJECTS * $HASH_LEN))\n+MIDX_OFFSET_WIDTH=8\n+MIDX_BYTE_PACK_INT_ID=$(($MIDX_OFFSET_OBJECT_OFFSETS + 16 * $MIDX_OFFSET_WIDTH + 2))\n+MIDX_BYTE_OFFSET=$(($MIDX_OFFSET_OBJECT_OFFSETS + 16 * $MIDX_OFFSET_WIDTH + 6))\n \n test_expect_success 'verify bad version' '\n \tcorrupt_midx_and_verify $MIDX_BYTE_VERSION \"\\00\" $objdir \\\n@@ -243,6 +248,16 @@ test_expect_success 'verify oid lookup out of order' '\n \t\t\"oid lookup out of order\"\n '\n \n+test_expect_success 'verify incorrect pack-int-id' '\n+\tcorrupt_midx_and_verify $MIDX_BYTE_PACK_INT_ID \"\\07\" $objdir \\\n+\t\t\"bad pack-int-id\"\n+'\n+\n+test_expect_success 'verify incorrect offset' '\n+\tcorrupt_midx_and_verify $MIDX_BYTE_OFFSET \"\\07\" $objdir \\\n+\t\t\"incorrect object offset\"\n+'\n+\n test_expect_success 'repack removes multi-pack-index' '\n \ttest_path_is_file $objdir/pack/multi-pack-index &&\n \tgit repack -adf &&\n@@ -310,4 +325,16 @@ test_expect_success 'verify multi-pack-index with 64-bit offsets' '\n \tgit multi-pack-index verify --object-dir=objects64\n '\n \n+NUM_OBJECTS=63\n+MIDX_OFFSET_OID_FANOUT=$((MIDX_OFFSET_PACKNAMES + 54))\n+MIDX_OFFSET_OID_LOOKUP=$((MIDX_OFFSET_OID_FANOUT + 256 * $MIDX_OID_FANOUT_WIDTH))\n+MIDX_OFFSET_OBJECT_OFFSETS=$(($MIDX_OFFSET_OID_LOOKUP + $NUM_OBJECTS * $HASH_LEN))\n+MIDX_OFFSET_LARGE_OFFSETS=$(($MIDX_OFFSET_OBJECT_OFFSETS + $NUM_OBJECTS * $MIDX_OFFSET_WIDTH))\n+MIDX_BYTE_LARGE_OFFSET=$(($MIDX_OFFSET_LARGE_OFFSETS + 3))\n+\n+test_expect_success 'verify incorrect 64-bit offset' '\n+\tcorrupt_midx_and_verify $MIDX_BYTE_LARGE_OFFSET \"\\07\" objects64 \\\n+\t\t\"incorrect object offset\"\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"358076","messageId":"29ebc1716123a953663c43064d73b98cf2fe0cbd.1536861730.git.gitgitgadget@gmail.com","threadId":"49281","inReplyTo":"pull.34.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 10/11] multi-pack-index: report progress during 'verify'","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-09-13T18:02:26Z","receivedAt":"2018-09-13T18:02:30Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nWhen verifying a multi-pack-index, the only action that takes\nsignificant time is checking the object offsets. For example,\nto verify a multi-pack-index containing 6.2 million objects in\nthe Linux kernel repository takes 1.3 seconds on my machine.\n99% of that time is spent looking up object offsets in each of\nthe packfiles and comparing them to the multi-pack-index offset.\n\nAdd a progress indicator for that section of the 'verify' verb.\n\nSigned-off-by: Derrick Stolee \n---\n midx.c | 6 ++++++\n 1 file changed, 6 insertions(+)\n\ndiff --git a/midx.c b/midx.c\nindex 47e7e6113a..4d4c930522 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -7,6 +7,7 @@\n #include \"object-store.h\"\n #include \"sha1-lookup.h\"\n #include \"midx.h\"\n+#include \"progress.h\"\n \n #define MIDX_SIGNATURE 0x4d494458 /* \"MIDX\" */\n #define MIDX_VERSION 1\n@@ -940,6 +941,7 @@ static void midx_report(const char *fmt, ...)\n int verify_midx_file(const char *object_dir)\n {\n \tuint32_t i;\n+\tstruct progress *progress = NULL;\n \tstruct multi_pack_index *m = load_multi_pack_index(object_dir, 1);\n \tverify_midx_error = 0;\n \n@@ -971,6 +973,7 @@ int verify_midx_file(const char *object_dir)\n \t\t\t\t i, oid_to_hex(&oid1), oid_to_hex(&oid2), i + 1);\n \t}\n \n+\tprogress = start_progress(_(\"Verifying object offsets\"), m->num_objects);\n \tfor (i = 0; i < m->num_objects; i++) {\n \t\tstruct object_id oid;\n \t\tstruct pack_entry e;\n@@ -995,7 +998,10 @@ int verify_midx_file(const char *object_dir)\n \t\tif (m_offset != p_offset)\n \t\t\tmidx_report(_(\"incorrect object offset for oid[%d] = %s: %\"PRIx64\" != %\"PRIx64),\n \t\t\t\t i, oid_to_hex(&oid), m_offset, p_offset);\n+\n+\t\tdisplay_progress(progress, i + 1);\n \t}\n+\tstop_progress(&progress);\n \n \treturn verify_midx_error;\n }\n-- \ngitgitgadget\n\n"},{"id":"358077","messageId":"406c88b456c06bc42a9bb23bda48b6b3be461435.1536861730.git.gitgitgadget@gmail.com","threadId":"49281","inReplyTo":"pull.34.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 11/11] fsck: verify multi-pack-index","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-09-13T18:02:27Z","receivedAt":"2018-09-13T18:02:30Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nWhen core.multiPackIndex is true, we may have a multi-pack-index\nin our object directory. Add calls to 'git multi-pack-index verify'\nat the end of 'git fsck' if so.\n\nSigned-off-by: Derrick Stolee \n---\n builtin/fsck.c | 18 ++++++++++++++++++\n t/t5319-multi-pack-index.sh | 13 ++++++++++++-\n 2 files changed, 30 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex 63c8578cc1..06eb421720 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -848,5 +848,23 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)\n \t\t}\n \t}\n \n+\tif (!git_config_get_bool(\"core.multipackindex\", &i) && i) {\n+\t\tstruct child_process midx_verify = CHILD_PROCESS_INIT;\n+\t\tconst char *midx_argv[] = { \"multi-pack-index\", \"verify\", NULL, NULL, NULL };\n+\n+\t\tmidx_verify.argv = midx_argv;\n+\t\tmidx_verify.git_cmd = 1;\n+\t\tif (run_command(&midx_verify))\n+\t\t\terrors_found |= ERROR_COMMIT_GRAPH;\n+\n+\t\tprepare_alt_odb(the_repository);\n+\t\tfor (alt = the_repository->objects->alt_odb_list; alt; alt = alt->next) {\n+\t\t\tmidx_argv[2] = \"--object-dir\";\n+\t\t\tmidx_argv[3] = alt->path;\n+\t\t\tif (run_command(&midx_verify))\n+\t\t\t\terrors_found |= ERROR_COMMIT_GRAPH;\n+\t\t}\n+\t}\n+\n \treturn errors_found;\n }\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex 828c240389..bd8e841b81 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -160,12 +160,17 @@ corrupt_midx_and_verify() {\n \tDATA=\"${2:-\\0}\" &&\n \tOBJDIR=$3 &&\n \tGREPSTR=\"$4\" &&\n+\tCOMMAND=\"$5\" &&\n+\tif test -z \"$COMMAND\"\n+\tthen\n+\t\tCOMMAND=\"git multi-pack-index verify --object-dir=$OBJDIR\"\n+\tfi &&\n \tFILE=$OBJDIR/pack/multi-pack-index &&\n \tchmod a+w $FILE &&\n \ttest_when_finished mv midx-backup $FILE &&\n \tcp $FILE midx-backup &&\n \tprintf \"$DATA\" | dd of=\"$FILE\" bs=1 seek=\"$POS\" conv=notrunc &&\n-\ttest_must_fail git multi-pack-index verify --object-dir=$OBJDIR 2>test_err &&\n+\ttest_must_fail $COMMAND 2>test_err &&\n \tgrep -v \"^+\" test_err >err &&\n \ttest_i18ngrep \"$GREPSTR\" err\n }\n@@ -258,6 +263,12 @@ test_expect_success 'verify incorrect offset' '\n \t\t\"incorrect object offset\"\n '\n \n+test_expect_success 'git-fsck incorrect offset' '\n+\tcorrupt_midx_and_verify $MIDX_BYTE_OFFSET \"\\07\" $objdir \\\n+\t\t\"incorrect object offset\" \\\n+\t\t\"git -c core.multipackindex=true fsck\"\n+'\n+\n test_expect_success 'repack removes multi-pack-index' '\n \ttest_path_is_file $objdir/pack/multi-pack-index &&\n \tgit repack -adf &&\n-- \ngitgitgadget\n"},{"id":"358078","messageId":"d8ffd84d67471e6fbb658beedf4491e12ca50311.1536861730.git.gitgitgadget@gmail.com","threadId":"49281","inReplyTo":"pull.34.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 01/11] multi-pack-index: add 'verify' verb","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-09-13T18:02:13Z","receivedAt":"2018-09-13T18:02:38Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe multi-pack-index builtin writes multi-pack-index files, and\nuses a 'write' verb to do so. Add a 'verify' verb that checks this\nfile matches the contents of the pack-indexes it replaces.\n\nThe current implementation is a no-op, but will be extended in\nsmall increments in later commits.\n\nSigned-off-by: Derrick Stolee \n---\n Documentation/git-multi-pack-index.txt | 10 ++++++++++\n builtin/multi-pack-index.c | 4 +++-\n midx.c | 13 +++++++++++++\n midx.h | 1 +\n t/t5319-multi-pack-index.sh | 8 ++++++++\n 5 files changed, 35 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-multi-pack-index.txt b/Documentation/git-multi-pack-index.txt\nindex 1f97e79912..f7778a2c85 100644\n--- a/Documentation/git-multi-pack-index.txt\n+++ b/Documentation/git-multi-pack-index.txt\n@@ -27,6 +27,10 @@ write::\n \tWhen given as the verb, write a new MIDX file to\n \t`/packs/multi-pack-index`.\n \n+verify::\n+\tWhen given as the verb, verify the contents of the MIDX file\n+\tat `/packs/multi-pack-index`.\n+\n \n EXAMPLES\n --------\n@@ -43,6 +47,12 @@ $ git multi-pack-index write\n $ git multi-pack-index --object-dir write\n -----------------------------------------------\n \n+* Verify the MIDX file for the packfiles in the current .git folder.\n++\n+-----------------------------------------------\n+$ git multi-pack-index verify\n+-----------------------------------------------\n+\n \n SEE ALSO\n --------\ndiff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c\nindex 2633efd95d..fca70f8e4f 100644\n--- a/builtin/multi-pack-index.c\n+++ b/builtin/multi-pack-index.c\n@@ -5,7 +5,7 @@\n #include \"midx.h\"\n \n static char const * const builtin_multi_pack_index_usage[] = {\n-\tN_(\"git multi-pack-index [--object-dir=] write\"),\n+\tN_(\"git multi-pack-index [--object-dir=] (write|verify)\"),\n \tNULL\n };\n \n@@ -42,6 +42,8 @@ int cmd_multi_pack_index(int argc, const char **argv,\n \n \tif (!strcmp(argv[0], \"write\"))\n \t\treturn write_midx_file(opts.object_dir);\n+\tif (!strcmp(argv[0], \"verify\"))\n+\t\treturn verify_midx_file(opts.object_dir);\n \n \tdie(_(\"unrecognized verb: %s\"), argv[0]);\n }\ndiff --git a/midx.c b/midx.c\nindex f3e8dbc108..b253bed517 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -928,3 +928,16 @@ void clear_midx_file(const char *object_dir)\n \n \tfree(midx);\n }\n+\n+int verify_midx_error;\n+\n+int verify_midx_file(const char *object_dir)\n+{\n+\tstruct multi_pack_index *m = load_multi_pack_index(object_dir, 1);\n+\tverify_midx_error = 0;\n+\n+\tif (!m)\n+\t\treturn 0;\n+\n+\treturn verify_midx_error;\n+}\ndiff --git a/midx.h b/midx.h\nindex a210f1af2a..ce80b91c68 100644\n--- a/midx.h\n+++ b/midx.h\n@@ -43,5 +43,6 @@ int prepare_multi_pack_index_one(struct repository *r, const char *object_dir, i\n \n int write_midx_file(const char *object_dir);\n void clear_midx_file(const char *object_dir);\n+int verify_midx_file(const char *object_dir);\n \n #endif\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex 6f56b38674..1c4e0e6d31 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -150,6 +150,10 @@ test_expect_success 'write midx with twelve packs' '\n \n compare_results_with_midx \"twelve packs\"\n \n+test_expect_success 'verify multi-pack-index success' '\n+\tgit multi-pack-index verify --object-dir=$objdir\n+'\n+\n test_expect_success 'repack removes multi-pack-index' '\n \ttest_path_is_file $objdir/pack/multi-pack-index &&\n \tgit repack -adf &&\n@@ -214,4 +218,8 @@ test_expect_success 'force some 64-bit offsets with pack-objects' '\n \tmidx_read_expect 1 63 5 objects64 \" large-offsets\"\n '\n \n+test_expect_success 'verify multi-pack-index with 64-bit offsets' '\n+\tgit multi-pack-index verify --object-dir=objects64\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"358086","messageId":"87ftydkypn.fsf@evledraar.gmail.com","threadId":"49281","inReplyTo":"29ebc1716123a953663c43064d73b98cf2fe0cbd.1536861730.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 10/11] multi-pack-index: report progress during 'verify'","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-09-13T19:02:28Z","receivedAt":"2018-09-13T19:02:33Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Sep 13 2018, Derrick Stolee via GitGitGadget wrote:\n\n> +\tprogress = start_progress(_(\"Verifying object offsets\"), m->num_objects);\n\nI think in the spirit of my \"commit-graph {write,verify}: add progress\noutput\" it would be better to say:\n\n \"Verifying multi-pack-index object offsets\"\n\nI.e. both to make it clearer what's actually going on (without much\nadded verbosity), and also so that when the user turns on this feature\nit's clear what gc / fsck etc. are doing for that feature in particular,\nas with the commit-graph.\n"}]}