* [PATCH 1/3] t5510: harden the way verify-pack is used @ 2009-08-08 3:36 Junio C Hamano 2009-08-08 3:36 ` [PATCH 2/3] verify-pack -v: do not report "chain length 0" Junio C Hamano 0 siblings, 1 reply; 4+ messages in thread From: Junio C Hamano @ 2009-08-08 3:36 UTC (permalink / raw) To: git The test ignored the exit status from verify pack command, and also relied on not seeing any delta chain statistics. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- t/t5510-fetch.sh | 11 ++++++++--- 1 files changed, 8 insertions(+), 3 deletions(-) diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index bee3424..d13c806 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -9,6 +9,11 @@ test_description='Per branch config variables affects "git fetch". D=`pwd` +test_bundle_object_count () { + git verify-pack -v "$1" >verify.out && + test "$2" = $(grep '^[0-9a-f]\{40\} ' verify.out | wc -l) +} + test_expect_success setup ' echo >file original && git add file && @@ -146,6 +151,7 @@ test_expect_success 'unbundle 1' ' test_must_fail git fetch "$D/bundle1" master:master ' + test_expect_success 'bundle 1 has only 3 files ' ' cd "$D" && ( @@ -156,8 +162,7 @@ test_expect_success 'bundle 1 has only 3 files ' ' cat ) <bundle1 >bundle.pack && git index-pack bundle.pack && - verify=$(git verify-pack -v bundle.pack) && - test 4 = $(echo "$verify" | wc -l) + test_bundle_object_count bundle.pack 3 ' test_expect_success 'unbundle 2' ' @@ -180,7 +185,7 @@ test_expect_success 'bundle does not prerequisite objects' ' cat ) <bundle3 >bundle.pack && git index-pack bundle.pack && - test 4 = $(git verify-pack -v bundle.pack | wc -l) + test_bundle_object_count bundle.pack 3 ' test_expect_success 'bundle should be able to create a full history' ' -- 1.6.4.151.g786db ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/3] verify-pack -v: do not report "chain length 0" 2009-08-08 3:36 [PATCH 1/3] t5510: harden the way verify-pack is used Junio C Hamano @ 2009-08-08 3:36 ` Junio C Hamano 2009-08-08 3:36 ` [PATCH 3/3] verify-pack --stat-only: show histogram without verifying Junio C Hamano 2009-08-08 4:11 ` [PATCH 2/3] verify-pack -v: do not report "chain length 0" Jeff King 0 siblings, 2 replies; 4+ messages in thread From: Junio C Hamano @ 2009-08-08 3:36 UTC (permalink / raw) To: git When making a histogram of delta chain length in the pack, the program collects number of objects whose delta depth exceeds the MAX_CHAIN limit in histogram[0], and showed it as the number of items that exceeds the limit correctly. HOWEVER, it also showed the same number labeled as "chain length = 0". In fact, we are not showing the number of objects whose chain length is zero, i.e. the base objects. Correct this. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- builtin-verify-pack.c | 27 +++++++++++++++++++-------- 1 files changed, 19 insertions(+), 8 deletions(-) diff --git a/builtin-verify-pack.c b/builtin-verify-pack.c index ebd6dff..b5bd28e 100644 --- a/builtin-verify-pack.c +++ b/builtin-verify-pack.c @@ -8,10 +8,13 @@ static void show_pack_info(struct packed_git *p) { - uint32_t nr_objects, i, chain_histogram[MAX_CHAIN+1]; + uint32_t nr_objects, i; + int cnt; + unsigned long chain_histogram[MAX_CHAIN+1], baseobjects; nr_objects = p->num_objects; memset(chain_histogram, 0, sizeof(chain_histogram)); + baseobjects = 0; for (i = 0; i < nr_objects; i++) { const unsigned char *sha1; @@ -30,9 +33,11 @@ static void show_pack_info(struct packed_git *p) &delta_chain_length, base_sha1); printf("%s ", sha1_to_hex(sha1)); - if (!delta_chain_length) + if (!delta_chain_length) { printf("%-6s %lu %lu %"PRIuMAX"\n", type, size, store_size, (uintmax_t)offset); + baseobjects++; + } else { printf("%-6s %lu %lu %"PRIuMAX" %u %s\n", type, size, store_size, (uintmax_t)offset, @@ -44,15 +49,21 @@ static void show_pack_info(struct packed_git *p) } } - for (i = 0; i <= MAX_CHAIN; i++) { - if (!chain_histogram[i]) + if (baseobjects) + printf("non delta: %lu object%s\n", + baseobjects, baseobjects > 1 ? "s" : ""); + + for (cnt = 1; cnt <= MAX_CHAIN; cnt++) { + if (!chain_histogram[cnt]) continue; - printf("chain length = %"PRIu32": %"PRIu32" object%s\n", i, - chain_histogram[i], chain_histogram[i] > 1 ? "s" : ""); + printf("chain length = %d: %lu object%s\n", cnt, + chain_histogram[cnt], + chain_histogram[cnt] > 1 ? "s" : ""); } if (chain_histogram[0]) - printf("chain length > %d: %"PRIu32" object%s\n", MAX_CHAIN, - chain_histogram[0], chain_histogram[0] > 1 ? "s" : ""); + printf("chain length > %d: %lu object%s\n", MAX_CHAIN, + chain_histogram[0], + chain_histogram[0] > 1 ? "s" : ""); } static int verify_one_pack(const char *path, int verbose) -- 1.6.4.151.g786db ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 3/3] verify-pack --stat-only: show histogram without verifying 2009-08-08 3:36 ` [PATCH 2/3] verify-pack -v: do not report "chain length 0" Junio C Hamano @ 2009-08-08 3:36 ` Junio C Hamano 2009-08-08 4:11 ` [PATCH 2/3] verify-pack -v: do not report "chain length 0" Jeff King 1 sibling, 0 replies; 4+ messages in thread From: Junio C Hamano @ 2009-08-08 3:36 UTC (permalink / raw) To: git When this option is given, the command does not verify the pack contents, but shows the delta chain histogram. If used with --verbose, the usual list of objects is also shown. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- Documentation/git-verify-pack.txt | 8 +++++- builtin-verify-pack.c | 49 +++++++++++++++++++++++++------------ 2 files changed, 40 insertions(+), 17 deletions(-) diff --git a/Documentation/git-verify-pack.txt b/Documentation/git-verify-pack.txt index d791a80..97f7f91 100644 --- a/Documentation/git-verify-pack.txt +++ b/Documentation/git-verify-pack.txt @@ -25,7 +25,13 @@ OPTIONS -v:: --verbose:: After verifying the pack, show list of objects contained - in the pack. + in the pack and a histogram of delta chain length. + +-s:: +--stat-only:: + Do not verify the pack contents; only show the histogram of delta + chain length. With `--verbose`, list of objects is also shown. + \--:: Do not interpret any more arguments as options. diff --git a/builtin-verify-pack.c b/builtin-verify-pack.c index b5bd28e..b6079ae 100644 --- a/builtin-verify-pack.c +++ b/builtin-verify-pack.c @@ -6,10 +6,14 @@ #define MAX_CHAIN 50 -static void show_pack_info(struct packed_git *p) +#define VERIFY_PACK_VERBOSE 01 +#define VERIFY_PACK_STAT_ONLY 02 + +static void show_pack_info(struct packed_git *p, unsigned int flags) { uint32_t nr_objects, i; int cnt; + int stat_only = flags & VERIFY_PACK_STAT_ONLY; unsigned long chain_histogram[MAX_CHAIN+1], baseobjects; nr_objects = p->num_objects; @@ -32,16 +36,19 @@ static void show_pack_info(struct packed_git *p) type = packed_object_info_detail(p, offset, &size, &store_size, &delta_chain_length, base_sha1); - printf("%s ", sha1_to_hex(sha1)); + if (!stat_only) + printf("%s ", sha1_to_hex(sha1)); if (!delta_chain_length) { - printf("%-6s %lu %lu %"PRIuMAX"\n", - type, size, store_size, (uintmax_t)offset); + if (!stat_only) + printf("%-6s %lu %lu %"PRIuMAX"\n", + type, size, store_size, (uintmax_t)offset); baseobjects++; } else { - printf("%-6s %lu %lu %"PRIuMAX" %u %s\n", - type, size, store_size, (uintmax_t)offset, - delta_chain_length, sha1_to_hex(base_sha1)); + if (!stat_only) + printf("%-6s %lu %lu %"PRIuMAX" %u %s\n", + type, size, store_size, (uintmax_t)offset, + delta_chain_length, sha1_to_hex(base_sha1)); if (delta_chain_length <= MAX_CHAIN) chain_histogram[delta_chain_length]++; else @@ -66,10 +73,12 @@ static void show_pack_info(struct packed_git *p) chain_histogram[0] > 1 ? "s" : ""); } -static int verify_one_pack(const char *path, int verbose) +static int verify_one_pack(const char *path, unsigned int flags) { char arg[PATH_MAX]; int len; + int verbose = flags & VERIFY_PACK_VERBOSE; + int stat_only = flags & VERIFY_PACK_STAT_ONLY; struct packed_git *pack; int err; @@ -105,14 +114,19 @@ static int verify_one_pack(const char *path, int verbose) return error("packfile %s not found.", arg); install_packed_git(pack); - err = verify_pack(pack); - if (verbose) { + if (!stat_only) + err = verify_pack(pack); + else + err = open_pack_index(pack); + + if (verbose || stat_only) { if (err) printf("%s: bad\n", pack->pack_name); else { - show_pack_info(pack); - printf("%s: ok\n", pack->pack_name); + show_pack_info(pack, flags); + if (!stat_only) + printf("%s: ok\n", pack->pack_name); } } @@ -120,17 +134,20 @@ static int verify_one_pack(const char *path, int verbose) } static const char * const verify_pack_usage[] = { - "git verify-pack [-v|--verbose] <pack>...", + "git verify-pack [-v|--verbose] [-s|--stat-only] <pack>...", NULL }; int cmd_verify_pack(int argc, const char **argv, const char *prefix) { int err = 0; - int verbose = 0; + unsigned int flags = 0; int i; const struct option verify_pack_options[] = { - OPT__VERBOSE(&verbose), + OPT_BIT('v', "verbose", &flags, "verbose", + VERIFY_PACK_VERBOSE), + OPT_BIT('s', "stat-only", &flags, "show statistics only", + VERIFY_PACK_STAT_ONLY), OPT_END() }; @@ -140,7 +157,7 @@ int cmd_verify_pack(int argc, const char **argv, const char *prefix) if (argc < 1) usage_with_options(verify_pack_usage, verify_pack_options); for (i = 0; i < argc; i++) { - if (verify_one_pack(argv[i], verbose)) + if (verify_one_pack(argv[i], flags)) err = 1; discard_revindex(); } -- 1.6.4.151.g786db ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 2/3] verify-pack -v: do not report "chain length 0" 2009-08-08 3:36 ` [PATCH 2/3] verify-pack -v: do not report "chain length 0" Junio C Hamano 2009-08-08 3:36 ` [PATCH 3/3] verify-pack --stat-only: show histogram without verifying Junio C Hamano @ 2009-08-08 4:11 ` Jeff King 1 sibling, 0 replies; 4+ messages in thread From: Jeff King @ 2009-08-08 4:11 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On Fri, Aug 07, 2009 at 08:36:33PM -0700, Junio C Hamano wrote: > + if (baseobjects) > + printf("non delta: %lu object%s\n", > + baseobjects, baseobjects > 1 ? "s" : ""); Nit: this should be "non-delta" or "no delta". -Peff ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-08-08 4:11 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-08-08 3:36 [PATCH 1/3] t5510: harden the way verify-pack is used Junio C Hamano 2009-08-08 3:36 ` [PATCH 2/3] verify-pack -v: do not report "chain length 0" Junio C Hamano 2009-08-08 3:36 ` [PATCH 3/3] verify-pack --stat-only: show histogram without verifying Junio C Hamano 2009-08-08 4:11 ` [PATCH 2/3] verify-pack -v: do not report "chain length 0" Jeff King
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).