Skip to content

Commit 3de89c9

Browse files
gitstergitster
committed
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>
1 parent d1a0ed1 commit 3de89c9

File tree

4 files changed

+27
-166
lines changed

4 files changed

+27
-166
lines changed

‎builtin/verify-pack.c‎

Lines changed: 25 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -1,134 +1,53 @@
11
#include "builtin.h"
22
#include "cache.h"
3-
#include "pack.h"
4-
#include "pack-revindex.h"
3+
#include "run-command.h"
54
#include "parse-options.h"
65

7-
#define MAX_CHAIN 50
8-
96
#define VERIFY_PACK_VERBOSE 01
107
#define VERIFY_PACK_STAT_ONLY 02
118

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-
769
static int verify_one_pack(const char *path, unsigned int flags)
7710
{
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;
8014
int verbose = flags & VERIFY_PACK_VERBOSE;
8115
int stat_only = flags & VERIFY_PACK_STAT_ONLY;
82-
struct packed_git *pack;
8316
int err;
8417

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";
10224

10325
/*
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".
10628
*/
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;
11535

116-
install_packed_git(pack);
36+
memset(&index_pack, 0, sizeof(index_pack));
37+
index_pack.argv = argv;
38+
index_pack.git_cmd = 1;
11739

118-
if (!stat_only)
119-
err = verify_pack(pack);
120-
else
121-
err = open_pack_index(pack);
40+
err = run_command(&index_pack);
12241

12342
if (verbose || stat_only) {
12443
if (err)
125-
printf("%s: bad\n", pack->pack_name);
44+
printf("%s: bad\n", arg.buf);
12645
else {
127-
show_pack_info(pack, flags);
12846
if (!stat_only)
129-
printf("%s: ok\n", pack->pack_name);
47+
printf("%s: ok\n", arg.buf);
13048
}
13149
}
50+
strbuf_release(&arg);
13251

13352
return err;
13453
}
@@ -159,7 +78,6 @@ int cmd_verify_pack(int argc, const char **argv, const char *prefix)
15978
for (i = 0; i < argc; i++) {
16079
if (verify_one_pack(argv[i], flags))
16180
err = 1;
162-
discard_revindex();
16381
}
16482

16583
return err;

‎cache.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -991,7 +991,6 @@ extern off_t find_pack_entry_one(const unsigned char *, struct packed_git *);
991991
extern void *unpack_entry(struct packed_git *, off_t, enum object_type *, unsigned long *);
992992
extern unsigned long unpack_object_header_buffer(const unsigned char *buf, unsigned long len, enum object_type *type, unsigned long *sizep);
993993
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 *);
995994

996995
/* Dumb servers support */
997996
extern int update_server_info(int);

‎sha1_file.c‎

Lines changed: 0 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1496,61 +1496,6 @@ static int unpack_object_header(struct packed_git *p,
14961496
return type;
14971497
}
14981498

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-
15541499
static int packed_object_info(struct packed_git *p, off_t obj_offset,
15551500
unsigned long *sizep)
15561501
{

‎t/t5302-pack-index.sh‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -226,9 +226,8 @@ test_expect_success \
226226
( while read obj
227227
do git cat-file -p $obj >/dev/null || exit 1
228228
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+
'
232231

233232
test_expect_success 'running index-pack in the object store' '
234233
rm -f .git/objects/pack/* &&

0 commit comments

Comments
 (0)