verify-pack: use index-pack --verify

This finally gets rid of the inefficient verify-pack implementation that walks objects in the packfile in their object name order and replaces it with a call to index-pack --verify. As a side effect, it also removes packed_object_info_detail() API which is rather expensive. As this changes the way errors are reported (verify-pack used to rely on the usual runtime error detection routine unpack_entry() to diagnose the CRC errors in an entry in the *.idx file; index-pack --verify checks the whole *.idx file in one go), update a test that expected the string "CRC" to appear in the error message. Signed-off-by: Junio C Hamano <gitster@pobox.com>

Junio C Hamano committed Jun 3, 2011 at 15:32 UTC 3de89c9d4216d0fdc11bd1141c419ac4d0d35fed
4 files changed +27 -166
builtin/verify-pack.c
+25 -107
@@ -1,134 +1,53 @@
1 #include "builtin.h"
2 #include "cache.h"
3 -#include "pack.h"
4 -#include "pack-revindex.h"
3 +#include "run-command.h"
4 #include "parse-options.h"
5
7 -#define MAX_CHAIN 50
8 -
6 #define VERIFY_PACK_VERBOSE 01
7 #define VERIFY_PACK_STAT_ONLY 02
8
12 -static void show_pack_info(struct packed_git *p, unsigned int flags)
13 -{
14 - uint32_t nr_objects, i;
15 - int cnt;
16 - int stat_only = flags & VERIFY_PACK_STAT_ONLY;
17 - unsigned long chain_histogram[MAX_CHAIN+1], baseobjects;
18 -
19 - nr_objects = p->num_objects;
20 - memset(chain_histogram, 0, sizeof(chain_histogram));
21 - baseobjects = 0;
22 -
23 - for (i = 0; i < nr_objects; i++) {
24 - const unsigned char *sha1;
25 - unsigned char base_sha1[20];
26 - const char *type;
27 - unsigned long size;
28 - unsigned long store_size;
29 - off_t offset;
30 - unsigned int delta_chain_length;
31 -
32 - sha1 = nth_packed_object_sha1(p, i);
33 - if (!sha1)
34 - die("internal error pack-check nth-packed-object");
35 - offset = nth_packed_object_offset(p, i);
36 - type = packed_object_info_detail(p, offset, &size, &store_size,
37 - &delta_chain_length,
38 - base_sha1);
39 - if (!stat_only)
40 - printf("%s ", sha1_to_hex(sha1));
41 - if (!delta_chain_length) {
42 - if (!stat_only)
43 - printf("%-6s %lu %lu %"PRIuMAX"\n",
44 - type, size, store_size, (uintmax_t)offset);
45 - baseobjects++;
46 - }
47 - else {
48 - if (!stat_only)
49 - printf("%-6s %lu %lu %"PRIuMAX" %u %s\n",
50 - type, size, store_size, (uintmax_t)offset,
51 - delta_chain_length, sha1_to_hex(base_sha1));
52 - if (delta_chain_length <= MAX_CHAIN)
53 - chain_histogram[delta_chain_length]++;
54 - else
55 - chain_histogram[0]++;
56 - }
57 - }
58 -
59 - if (baseobjects)
60 - printf("non delta: %lu object%s\n",
61 - baseobjects, baseobjects > 1 ? "s" : "");
62 -
63 - for (cnt = 1; cnt <= MAX_CHAIN; cnt++) {
64 - if (!chain_histogram[cnt])
65 - continue;
66 - printf("chain length = %d: %lu object%s\n", cnt,
67 - chain_histogram[cnt],
68 - chain_histogram[cnt] > 1 ? "s" : "");
69 - }
70 - if (chain_histogram[0])
71 - printf("chain length > %d: %lu object%s\n", MAX_CHAIN,
72 - chain_histogram[0],
73 - chain_histogram[0] > 1 ? "s" : "");
74 -}
75 -
9 static int verify_one_pack(const char *path, unsigned int flags)
10 {
78 - char arg[PATH_MAX];
79 - int len;
11 + struct child_process index_pack;
12 + const char *argv[] = {"index-pack", NULL, NULL, NULL };
13 + struct strbuf arg = STRBUF_INIT;
14 int verbose = flags & VERIFY_PACK_VERBOSE;
15 int stat_only = flags & VERIFY_PACK_STAT_ONLY;
82 - struct packed_git *pack;
16 int err;
17
85 - len = strlcpy(arg, path, PATH_MAX);
86 - if (len >= PATH_MAX)
87 - return error("name too long: %s", path);
88 -
89 - /*
90 - * In addition to "foo.idx" we accept "foo.pack" and "foo";
91 - * normalize these forms to "foo.idx" for add_packed_git().
92 - */
93 - if (has_extension(arg, ".pack")) {
94 - strcpy(arg + len - 5, ".idx");
95 - len--;
96 - } else if (!has_extension(arg, ".idx")) {
97 - if (len + 4 >= PATH_MAX)
98 - return error("name too long: %s.idx", arg);
99 - strcpy(arg + len, ".idx");
100 - len += 4;
101 - }
18 + if (stat_only)
19 + argv[1] = "--verify-stat-only";
20 + else if (verbose)
21 + argv[1] = "--verify-stat";
22 + else
23 + argv[1] = "--verify";
24
25 /*
104 - * add_packed_git() uses our buffer (containing "foo.idx") to
105 - * build the pack filename ("foo.pack"). Make sure it fits.
26 + * In addition to "foo.pack" we accept "foo.idx" and "foo";
27 + * normalize these forms to "foo.pack" for "index-pack --verify".
28 */
107 - if (len + 1 >= PATH_MAX) {
108 - arg[len - 4] = '\0';
109 - return error("name too long: %s.pack", arg);
110 - }
111 -
112 - pack = add_packed_git(arg, len, 1);
113 - if (!pack)
114 - return error("packfile %s not found.", arg);
29 + strbuf_addstr(&arg, path);
30 + if (has_extension(arg.buf, ".idx"))
31 + strbuf_splice(&arg, arg.len - 3, 3, "pack", 4);
32 + else if (!has_extension(arg.buf, ".pack"))
33 + strbuf_add(&arg, ".pack", 5);
34 + argv[2] = arg.buf;
35
116 - install_packed_git(pack);
36 + memset(&index_pack, 0, sizeof(index_pack));
37 + index_pack.argv = argv;
38 + index_pack.git_cmd = 1;
39
118 - if (!stat_only)
119 - err = verify_pack(pack);
120 - else
121 - err = open_pack_index(pack);
40 + err = run_command(&index_pack);
41
42 if (verbose || stat_only) {
43 if (err)
125 - printf("%s: bad\n", pack->pack_name);
44 + printf("%s: bad\n", arg.buf);
45 else {
127 - show_pack_info(pack, flags);
46 if (!stat_only)
129 - printf("%s: ok\n", pack->pack_name);
47 + printf("%s: ok\n", arg.buf);
48 }
49 }
50 + strbuf_release(&arg);
51
52 return err;
53 }
@@ -159,7 +78,6 @@ int cmd_verify_pack(int argc, const char **argv, const char *prefix)
78 for (i = 0; i < argc; i++) {
79 if (verify_one_pack(argv[i], flags))
80 err = 1;
162 - discard_revindex();
81 }
82
83 return err;
cache.h
-1
@@ -991,7 +991,6 @@ extern off_t find_pack_entry_one(const unsigned char *, struct packed_git *);
991 extern void *unpack_entry(struct packed_git *, off_t, enum object_type *, unsigned long *);
992 extern unsigned long unpack_object_header_buffer(const unsigned char *buf, unsigned long len, enum object_type *type, unsigned long *sizep);
993 extern unsigned long get_size_from_delta(struct packed_git *, struct pack_window **, off_t);
994 -extern const char *packed_object_info_detail(struct packed_git *, off_t, unsigned long *, unsigned long *, unsigned int *, unsigned char *);
994
995 /* Dumb servers support */
996 extern int update_server_info(int);
sha1_file.c
-55
@@ -1496,61 +1496,6 @@ static int unpack_object_header(struct packed_git *p,
1496 return type;
1497 }
1498
1499 -const char *packed_object_info_detail(struct packed_git *p,
1500 - off_t obj_offset,
1501 - unsigned long *size,
1502 - unsigned long *store_size,
1503 - unsigned int *delta_chain_length,
1504 - unsigned char *base_sha1)
1505 -{
1506 - struct pack_window *w_curs = NULL;
1507 - off_t curpos;
1508 - unsigned long dummy;
1509 - unsigned char *next_sha1;
1510 - enum object_type type;
1511 - struct revindex_entry *revidx;
1512 -
1513 - *delta_chain_length = 0;
1514 - curpos = obj_offset;
1515 - type = unpack_object_header(p, &w_curs, &curpos, size);
1516 -
1517 - revidx = find_pack_revindex(p, obj_offset);
1518 - *store_size = revidx[1].offset - obj_offset;
1519 -
1520 - for (;;) {
1521 - switch (type) {
1522 - default:
1523 - die("pack %s contains unknown object type %d",
1524 - p->pack_name, type);
1525 - case OBJ_COMMIT:
1526 - case OBJ_TREE:
1527 - case OBJ_BLOB:
1528 - case OBJ_TAG:
1529 - unuse_pack(&w_curs);
1530 - return typename(type);
1531 - case OBJ_OFS_DELTA:
1532 - obj_offset = get_delta_base(p, &w_curs, &curpos, type, obj_offset);
1533 - if (!obj_offset)
1534 - die("pack %s contains bad delta base reference of type %s",
1535 - p->pack_name, typename(type));
1536 - if (*delta_chain_length == 0) {
1537 - revidx = find_pack_revindex(p, obj_offset);
1538 - hashcpy(base_sha1, nth_packed_object_sha1(p, revidx->nr));
1539 - }
1540 - break;
1541 - case OBJ_REF_DELTA:
1542 - next_sha1 = use_pack(p, &w_curs, curpos, NULL);
1543 - if (*delta_chain_length == 0)
1544 - hashcpy(base_sha1, next_sha1);
1545 - obj_offset = find_pack_entry_one(next_sha1, p);
1546 - break;
1547 - }
1548 - (*delta_chain_length)++;
1549 - curpos = obj_offset;
1550 - type = unpack_object_header(p, &w_curs, &curpos, &dummy);
1551 - }
1552 -}
1553 -
1499 static int packed_object_info(struct packed_git *p, off_t obj_offset,
1500 unsigned long *sizep)
1501 {
t/t5302-pack-index.sh
+2 -3
@@ -226,9 +226,8 @@ test_expect_success \
226 ( while read obj
227 do git cat-file -p $obj >/dev/null || exit 1
228 done <obj-list ) &&
229 - err=$(test_must_fail git verify-pack \
230 - ".git/objects/pack/pack-${pack1}.pack" 2>&1) &&
231 - echo "$err" | grep "CRC mismatch"'
229 + test_must_fail git verify-pack ".git/objects/pack/pack-${pack1}.pack"
230 +'
231
232 test_expect_success 'running index-pack in the object store' '
233 rm -f .git/objects/pack/* &&