{"thread":{"id":"20462","subject":"[PATCH 1/3] t5510: harden the way verify-pack is used","startedAt":"2009-08-08T03:36:32Z","lastAt":"2009-08-08T04:11:05Z","messageCount":4,"participants":["Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"119948","messageId":"1249702594-7815-1-git-send-email-gitster@pobox.com","threadId":"20462","inReplyTo":null,"subject":"[PATCH 1/3] t5510: harden the way verify-pack is used","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-08T03:36:32Z","receivedAt":"2009-08-08T03:36:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The test ignored the exit status from verify pack command, and also relied\non not seeing any delta chain statistics.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n t/t5510-fetch.sh |   11 ++++++++---\n 1 files changed, 8 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex bee3424..d13c806 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -9,6 +9,11 @@ test_description='Per branch config variables affects \"git fetch\".\n \n D=`pwd`\n \n+test_bundle_object_count () {\n+\tgit verify-pack -v \"$1\" >verify.out &&\n+\ttest \"$2\" = $(grep '^[0-9a-f]\\{40\\} ' verify.out | wc -l)\n+}\n+\n test_expect_success setup '\n \techo >file original &&\n \tgit add file &&\n@@ -146,6 +151,7 @@ test_expect_success 'unbundle 1' '\n \ttest_must_fail git fetch \"$D/bundle1\" master:master\n '\n \n+\n test_expect_success 'bundle 1 has only 3 files ' '\n \tcd \"$D\" &&\n \t(\n@@ -156,8 +162,7 @@ test_expect_success 'bundle 1 has only 3 files ' '\n \t\tcat\n \t) <bundle1 >bundle.pack &&\n \tgit index-pack bundle.pack &&\n-\tverify=$(git verify-pack -v bundle.pack) &&\n-\ttest 4 = $(echo \"$verify\" | wc -l)\n+\ttest_bundle_object_count bundle.pack 3\n '\n \n test_expect_success 'unbundle 2' '\n@@ -180,7 +185,7 @@ test_expect_success 'bundle does not prerequisite objects' '\n \t\tcat\n \t) <bundle3 >bundle.pack &&\n \tgit index-pack bundle.pack &&\n-\ttest 4 = $(git verify-pack -v bundle.pack | wc -l)\n+\ttest_bundle_object_count bundle.pack 3\n '\n \n test_expect_success 'bundle should be able to create a full history' '\n-- \n1.6.4.151.g786db\n"},{"id":"119949","messageId":"1249702594-7815-2-git-send-email-gitster@pobox.com","threadId":"20462","inReplyTo":"1249702594-7815-1-git-send-email-gitster@pobox.com","subject":"[PATCH 2/3] verify-pack -v: do not report \"chain length 0\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-08T03:36:33Z","receivedAt":"2009-08-08T03:36:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When making a histogram of delta chain length in the pack, the program\ncollects number of objects whose delta depth exceeds the MAX_CHAIN limit\nin histogram[0], and showed it as the number of items that exceeds the\nlimit correctly.  HOWEVER, it also showed the same number labeled as\n\"chain length = 0\".\n\nIn fact, we are not showing the number of objects whose chain length is\nzero, i.e. the base objects.  Correct this.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin-verify-pack.c |   27 +++++++++++++++++++--------\n 1 files changed, 19 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin-verify-pack.c b/builtin-verify-pack.c\nindex ebd6dff..b5bd28e 100644\n--- a/builtin-verify-pack.c\n+++ b/builtin-verify-pack.c\n@@ -8,10 +8,13 @@\n \n static void show_pack_info(struct packed_git *p)\n {\n-\tuint32_t nr_objects, i, chain_histogram[MAX_CHAIN+1];\n+\tuint32_t nr_objects, i;\n+\tint cnt;\n+\tunsigned long chain_histogram[MAX_CHAIN+1], baseobjects;\n \n \tnr_objects = p->num_objects;\n \tmemset(chain_histogram, 0, sizeof(chain_histogram));\n+\tbaseobjects = 0;\n \n \tfor (i = 0; i < nr_objects; i++) {\n \t\tconst unsigned char *sha1;\n@@ -30,9 +33,11 @@ static void show_pack_info(struct packed_git *p)\n \t\t\t\t\t\t &delta_chain_length,\n \t\t\t\t\t\t base_sha1);\n \t\tprintf(\"%s \", sha1_to_hex(sha1));\n-\t\tif (!delta_chain_length)\n+\t\tif (!delta_chain_length) {\n \t\t\tprintf(\"%-6s %lu %lu %\"PRIuMAX\"\\n\",\n \t\t\t       type, size, store_size, (uintmax_t)offset);\n+\t\t\tbaseobjects++;\n+\t\t}\n \t\telse {\n \t\t\tprintf(\"%-6s %lu %lu %\"PRIuMAX\" %u %s\\n\",\n \t\t\t       type, size, store_size, (uintmax_t)offset,\n@@ -44,15 +49,21 @@ static void show_pack_info(struct packed_git *p)\n \t\t}\n \t}\n \n-\tfor (i = 0; i <= MAX_CHAIN; i++) {\n-\t\tif (!chain_histogram[i])\n+\tif (baseobjects)\n+\t\tprintf(\"non delta: %lu object%s\\n\",\n+\t\t       baseobjects, baseobjects > 1 ? \"s\" : \"\");\n+\n+\tfor (cnt = 1; cnt <= MAX_CHAIN; cnt++) {\n+\t\tif (!chain_histogram[cnt])\n \t\t\tcontinue;\n-\t\tprintf(\"chain length = %\"PRIu32\": %\"PRIu32\" object%s\\n\", i,\n-\t\t       chain_histogram[i], chain_histogram[i] > 1 ? \"s\" : \"\");\n+\t\tprintf(\"chain length = %d: %lu object%s\\n\", cnt,\n+\t\t       chain_histogram[cnt],\n+\t\t       chain_histogram[cnt] > 1 ? \"s\" : \"\");\n \t}\n \tif (chain_histogram[0])\n-\t\tprintf(\"chain length > %d: %\"PRIu32\" object%s\\n\", MAX_CHAIN,\n-\t\t       chain_histogram[0], chain_histogram[0] > 1 ? \"s\" : \"\");\n+\t\tprintf(\"chain length > %d: %lu object%s\\n\", MAX_CHAIN,\n+\t\t       chain_histogram[0],\n+\t\t       chain_histogram[0] > 1 ? \"s\" : \"\");\n }\n \n static int verify_one_pack(const char *path, int verbose)\n-- \n1.6.4.151.g786db\n"},{"id":"119950","messageId":"1249702594-7815-3-git-send-email-gitster@pobox.com","threadId":"20462","inReplyTo":"1249702594-7815-2-git-send-email-gitster@pobox.com","subject":"[PATCH 3/3] verify-pack --stat-only: show histogram without verifying","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-08T03:36:34Z","receivedAt":"2009-08-08T03:36:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When this option is given, the command does not verify the pack contents,\nbut shows the delta chain histogram.  If used with --verbose, the usual\nlist of objects is also shown.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/git-verify-pack.txt |    8 +++++-\n builtin-verify-pack.c             |   49 +++++++++++++++++++++++++------------\n 2 files changed, 40 insertions(+), 17 deletions(-)\n\ndiff --git a/Documentation/git-verify-pack.txt b/Documentation/git-verify-pack.txt\nindex d791a80..97f7f91 100644\n--- a/Documentation/git-verify-pack.txt\n+++ b/Documentation/git-verify-pack.txt\n@@ -25,7 +25,13 @@ OPTIONS\n -v::\n --verbose::\n \tAfter verifying the pack, show list of objects contained\n-\tin the pack.\n+\tin the pack and a histogram of delta chain length.\n+\n+-s::\n+--stat-only::\n+\tDo not verify the pack contents; only show the histogram of delta\n+\tchain length.  With `--verbose`, list of objects is also shown.\n+\n \\--::\n \tDo not interpret any more arguments as options.\n \ndiff --git a/builtin-verify-pack.c b/builtin-verify-pack.c\nindex b5bd28e..b6079ae 100644\n--- a/builtin-verify-pack.c\n+++ b/builtin-verify-pack.c\n@@ -6,10 +6,14 @@\n \n #define MAX_CHAIN 50\n \n-static void show_pack_info(struct packed_git *p)\n+#define VERIFY_PACK_VERBOSE 01\n+#define VERIFY_PACK_STAT_ONLY 02\n+\n+static void show_pack_info(struct packed_git *p, unsigned int flags)\n {\n \tuint32_t nr_objects, i;\n \tint cnt;\n+\tint stat_only = flags & VERIFY_PACK_STAT_ONLY;\n \tunsigned long chain_histogram[MAX_CHAIN+1], baseobjects;\n \n \tnr_objects = p->num_objects;\n@@ -32,16 +36,19 @@ static void show_pack_info(struct packed_git *p)\n \t\ttype = packed_object_info_detail(p, offset, &size, &store_size,\n \t\t\t\t\t\t &delta_chain_length,\n \t\t\t\t\t\t base_sha1);\n-\t\tprintf(\"%s \", sha1_to_hex(sha1));\n+\t\tif (!stat_only)\n+\t\t\tprintf(\"%s \", sha1_to_hex(sha1));\n \t\tif (!delta_chain_length) {\n-\t\t\tprintf(\"%-6s %lu %lu %\"PRIuMAX\"\\n\",\n-\t\t\t       type, size, store_size, (uintmax_t)offset);\n+\t\t\tif (!stat_only)\n+\t\t\t\tprintf(\"%-6s %lu %lu %\"PRIuMAX\"\\n\",\n+\t\t\t\t       type, size, store_size, (uintmax_t)offset);\n \t\t\tbaseobjects++;\n \t\t}\n \t\telse {\n-\t\t\tprintf(\"%-6s %lu %lu %\"PRIuMAX\" %u %s\\n\",\n-\t\t\t       type, size, store_size, (uintmax_t)offset,\n-\t\t\t       delta_chain_length, sha1_to_hex(base_sha1));\n+\t\t\tif (!stat_only)\n+\t\t\t\tprintf(\"%-6s %lu %lu %\"PRIuMAX\" %u %s\\n\",\n+\t\t\t\t       type, size, store_size, (uintmax_t)offset,\n+\t\t\t\t       delta_chain_length, sha1_to_hex(base_sha1));\n \t\t\tif (delta_chain_length <= MAX_CHAIN)\n \t\t\t\tchain_histogram[delta_chain_length]++;\n \t\t\telse\n@@ -66,10 +73,12 @@ static void show_pack_info(struct packed_git *p)\n \t\t       chain_histogram[0] > 1 ? \"s\" : \"\");\n }\n \n-static int verify_one_pack(const char *path, int verbose)\n+static int verify_one_pack(const char *path, unsigned int flags)\n {\n \tchar arg[PATH_MAX];\n \tint len;\n+\tint verbose = flags & VERIFY_PACK_VERBOSE;\n+\tint stat_only = flags & VERIFY_PACK_STAT_ONLY;\n \tstruct packed_git *pack;\n \tint err;\n \n@@ -105,14 +114,19 @@ static int verify_one_pack(const char *path, int verbose)\n \t\treturn error(\"packfile %s not found.\", arg);\n \n \tinstall_packed_git(pack);\n-\terr = verify_pack(pack);\n \n-\tif (verbose) {\n+\tif (!stat_only)\n+\t\terr = verify_pack(pack);\n+\telse\n+\t\terr = open_pack_index(pack);\n+\n+\tif (verbose || stat_only) {\n \t\tif (err)\n \t\t\tprintf(\"%s: bad\\n\", pack->pack_name);\n \t\telse {\n-\t\t\tshow_pack_info(pack);\n-\t\t\tprintf(\"%s: ok\\n\", pack->pack_name);\n+\t\t\tshow_pack_info(pack, flags);\n+\t\t\tif (!stat_only)\n+\t\t\t\tprintf(\"%s: ok\\n\", pack->pack_name);\n \t\t}\n \t}\n \n@@ -120,17 +134,20 @@ static int verify_one_pack(const char *path, int verbose)\n }\n \n static const char * const verify_pack_usage[] = {\n-\t\"git verify-pack [-v|--verbose] <pack>...\",\n+\t\"git verify-pack [-v|--verbose] [-s|--stat-only] <pack>...\",\n \tNULL\n };\n \n int cmd_verify_pack(int argc, const char **argv, const char *prefix)\n {\n \tint err = 0;\n-\tint verbose = 0;\n+\tunsigned int flags = 0;\n \tint i;\n \tconst struct option verify_pack_options[] = {\n-\t\tOPT__VERBOSE(&verbose),\n+\t\tOPT_BIT('v', \"verbose\", &flags, \"verbose\",\n+\t\t\tVERIFY_PACK_VERBOSE),\n+\t\tOPT_BIT('s', \"stat-only\", &flags, \"show statistics only\",\n+\t\t\tVERIFY_PACK_STAT_ONLY),\n \t\tOPT_END()\n \t};\n \n@@ -140,7 +157,7 @@ int cmd_verify_pack(int argc, const char **argv, const char *prefix)\n \tif (argc < 1)\n \t\tusage_with_options(verify_pack_usage, verify_pack_options);\n \tfor (i = 0; i < argc; i++) {\n-\t\tif (verify_one_pack(argv[i], verbose))\n+\t\tif (verify_one_pack(argv[i], flags))\n \t\t\terr = 1;\n \t\tdiscard_revindex();\n \t}\n-- \n1.6.4.151.g786db\n"},{"id":"119951","messageId":"20090808041105.GA20339@coredump.intra.peff.net","threadId":"20462","inReplyTo":"1249702594-7815-2-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 2/3] verify-pack -v: do not report \"chain length 0\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-08T04:11:05Z","receivedAt":"2009-08-08T04:11:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Aug 07, 2009 at 08:36:33PM -0700, Junio C Hamano wrote:\n\n> +\tif (baseobjects)\n> +\t\tprintf(\"non delta: %lu object%s\\n\",\n> +\t\t       baseobjects, baseobjects > 1 ? \"s\" : \"\");\n\nNit: this should be \"non-delta\" or \"no delta\".\n\n-Peff\n"}]}