{"thread":{"id":"28864","subject":"[PATCH v2 0/4] fsck improvements","startedAt":"2011-11-07T02:59:22Z","lastAt":"2011-11-07T04:33:03Z","messageCount":6,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":4},"messages":[{"id":"178990","messageId":"1320634766-24511-1-git-send-email-pclouds@gmail.com","threadId":"28864","inReplyTo":null,"subject":"[PATCH v2 0/4] fsck improvements","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-11-07T02:59:22Z","receivedAt":"2011-11-07T02:59:22Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"2/4 is updated to check as many objects as possible.\n4/4 has new doc update and explains why --verbose turn progress off\n\nNguyễn Thái Ngọc Duy (4):\n  fsck: return error code when verify_pack() goes wrong\n  verify_packfile(): check as many object as possible in a pack\n  fsck: avoid reading every object twice\n  fsck: print progress\n\n Documentation/git-fsck.txt |   11 ++++++-\n builtin/fsck.c             |   78 +++++++++++++++++++++++++++++++++++---------\n pack-check.c               |   28 +++++++++++-----\n pack.h                     |    6 +++-\n 4 files changed, 96 insertions(+), 27 deletions(-)\n\n-- \n1.7.4.74.g639db\n"},{"id":"178991","messageId":"1320634766-24511-2-git-send-email-pclouds@gmail.com","threadId":"28864","inReplyTo":"1320634766-24511-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2 1/4] fsck: return error code when verify_pack() goes wrong","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-11-07T02:59:23Z","receivedAt":"2011-11-07T02:59:23Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n builtin/fsck.c |    4 +++-\n 1 files changed, 3 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex df1a88b..4ead98d 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -29,6 +29,7 @@ static int write_lost_and_found;\n static int verbose;\n #define ERROR_OBJECT 01\n #define ERROR_REACHABLE 02\n+#define ERROR_PACK 04\n \n #ifdef NO_D_INO_IN_DIRENT\n #define SORT_DIRENT 0\n@@ -626,7 +627,8 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)\n \t\tprepare_packed_git();\n \t\tfor (p = packed_git; p; p = p->next)\n \t\t\t/* verify gives error messages itself */\n-\t\t\tverify_pack(p);\n+\t\t\tif (verify_pack(p))\n+\t\t\t\terrors_found |= ERROR_PACK;\n \n \t\tfor (p = packed_git; p; p = p->next) {\n \t\t\tuint32_t j, num;\n-- \n1.7.4.74.g639db\n"},{"id":"178992","messageId":"1320634766-24511-3-git-send-email-pclouds@gmail.com","threadId":"28864","inReplyTo":"1320634766-24511-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2 2/4] verify_packfile(): check as many object as possible in a pack","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-11-07T02:59:24Z","receivedAt":"2011-11-07T02:59:24Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"verify_packfile() checks for whole pack integerity first, then each\nobject individually. Once we get past whole pack check, we can\nidentify all objects in the pack. If there's an error with one object,\nwe should continue to check the next objects to salvage as many\nobjects as possible instead of stopping the process.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n pack-check.c |    9 ++-------\n 1 files changed, 2 insertions(+), 7 deletions(-)\n\ndiff --git a/pack-check.c b/pack-check.c\nindex 0c19b6e..7ac9b3a 100644\n--- a/pack-check.c\n+++ b/pack-check.c\n@@ -113,18 +113,13 @@ static int verify_packfile(struct packed_git *p,\n \t\t\t\t\t    p->pack_name, (uintmax_t)offset);\n \t\t}\n \t\tdata = unpack_entry(p, entries[i].offset, &type, &size);\n-\t\tif (!data) {\n+\t\tif (!data)\n \t\t\terr = error(\"cannot unpack %s from %s at offset %\"PRIuMAX\"\",\n \t\t\t\t    sha1_to_hex(entries[i].sha1), p->pack_name,\n \t\t\t\t    (uintmax_t)entries[i].offset);\n-\t\t\tbreak;\n-\t\t}\n-\t\tif (check_sha1_signature(entries[i].sha1, data, size, typename(type))) {\n+\t\telse if (check_sha1_signature(entries[i].sha1, data, size, typename(type)))\n \t\t\terr = error(\"packed %s from %s is corrupt\",\n \t\t\t\t    sha1_to_hex(entries[i].sha1), p->pack_name);\n-\t\t\tfree(data);\n-\t\t\tbreak;\n-\t\t}\n \t\tfree(data);\n \t}\n \tfree(entries);\n-- \n1.7.4.74.g639db\n"},{"id":"178993","messageId":"1320634766-24511-4-git-send-email-pclouds@gmail.com","threadId":"28864","inReplyTo":"1320634766-24511-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2 3/4] fsck: avoid reading every object twice","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-11-07T02:59:25Z","receivedAt":"2011-11-07T02:59:25Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"During verify_pack() all objects are read for SHA-1 check. Then\nfsck_sha1() is called on every object, which read the object again\n(fsck_sha1 -> parse_object -> read_sha1_file).\n\nAvoid reading an object twice, do fsck_sha1 while we have an object\nuncompressed data in verify_pack.\n\nOn git.git, with this patch I got:\n\n$ /usr/bin/time ./git fsck >/dev/null\n98.97user 0.90system 1:40.01elapsed 99%CPU (0avgtext+0avgdata 616624maxresident)k\n0inputs+0outputs (0major+194186minor)pagefaults 0swaps\n\nWithout it:\n\n$ /usr/bin/time ./git fsck >/dev/null\n231.23user 2.35system 3:53.82elapsed 99%CPU (0avgtext+0avgdata 636688maxresident)k\n0inputs+0outputs (0major+461629minor)pagefaults 0swaps\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n builtin/fsck.c |   42 +++++++++++++++++++++++++-----------------\n pack-check.c   |   13 ++++++++++---\n pack.h         |    5 ++++-\n 3 files changed, 39 insertions(+), 21 deletions(-)\n\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex 4ead98d..0603f64 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -282,14 +282,8 @@ static void check_connectivity(void)\n \t}\n }\n \n-static int fsck_sha1(const unsigned char *sha1)\n+static int fsck_obj(struct object *obj)\n {\n-\tstruct object *obj = parse_object(sha1);\n-\tif (!obj) {\n-\t\terrors_found |= ERROR_OBJECT;\n-\t\treturn error(\"%s: object corrupt or missing\",\n-\t\t\t     sha1_to_hex(sha1));\n-\t}\n \tif (obj->flags & SEEN)\n \t\treturn 0;\n \tobj->flags |= SEEN;\n@@ -332,6 +326,29 @@ static int fsck_sha1(const unsigned char *sha1)\n \treturn 0;\n }\n \n+static int fsck_sha1(const unsigned char *sha1)\n+{\n+\tstruct object *obj = parse_object(sha1);\n+\tif (!obj) {\n+\t\terrors_found |= ERROR_OBJECT;\n+\t\treturn error(\"%s: object corrupt or missing\",\n+\t\t\t     sha1_to_hex(sha1));\n+\t}\n+\treturn fsck_obj(obj);\n+}\n+\n+static int fsck_obj_buffer(const unsigned char *sha1, enum object_type type,\n+\t\t\t   unsigned long size, void *buffer, int *eaten)\n+{\n+\tstruct object *obj;\n+\tobj = parse_object_buffer(sha1, type, size, buffer, eaten);\n+\tif (!obj) {\n+\t\terrors_found |= ERROR_OBJECT;\n+\t\treturn error(\"%s: object corrupt or missing\", sha1_to_hex(sha1));\n+\t}\n+\treturn fsck_obj(obj);\n+}\n+\n /*\n  * This is the sorting chunk size: make it reasonably\n  * big so that we can sort well..\n@@ -627,17 +644,8 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)\n \t\tprepare_packed_git();\n \t\tfor (p = packed_git; p; p = p->next)\n \t\t\t/* verify gives error messages itself */\n-\t\t\tif (verify_pack(p))\n+\t\t\tif (verify_pack(p, fsck_obj_buffer))\n \t\t\t\terrors_found |= ERROR_PACK;\n-\n-\t\tfor (p = packed_git; p; p = p->next) {\n-\t\t\tuint32_t j, num;\n-\t\t\tif (open_pack_index(p))\n-\t\t\t\tcontinue;\n-\t\t\tnum = p->num_objects;\n-\t\t\tfor (j = 0; j < num; j++)\n-\t\t\t\tfsck_sha1(nth_packed_object_sha1(p, j));\n-\t\t}\n \t}\n \n \theads = 0;\ndiff --git a/pack-check.c b/pack-check.c\nindex 7ac9b3a..3b48b86 100644\n--- a/pack-check.c\n+++ b/pack-check.c\n@@ -42,7 +42,8 @@ int check_pack_crc(struct packed_git *p, struct pack_window **w_curs,\n }\n \n static int verify_packfile(struct packed_git *p,\n-\t\tstruct pack_window **w_curs)\n+\t\t\t   struct pack_window **w_curs,\n+\t\t\t   verify_fn fn)\n {\n \toff_t index_size = p->index_size;\n \tconst unsigned char *index_base = p->index_data;\n@@ -120,6 +121,12 @@ static int verify_packfile(struct packed_git *p,\n \t\telse if (check_sha1_signature(entries[i].sha1, data, size, typename(type)))\n \t\t\terr = error(\"packed %s from %s is corrupt\",\n \t\t\t\t    sha1_to_hex(entries[i].sha1), p->pack_name);\n+\t\telse if (fn) {\n+\t\t\tint eaten = 0;\n+\t\t\tfn(entries[i].sha1, type, size, data, &eaten);\n+\t\t\tif (eaten)\n+\t\t\t\tdata = NULL;\n+\t\t}\n \t\tfree(data);\n \t}\n \tfree(entries);\n@@ -150,7 +157,7 @@ int verify_pack_index(struct packed_git *p)\n \treturn err;\n }\n \n-int verify_pack(struct packed_git *p)\n+int verify_pack(struct packed_git *p, verify_fn fn)\n {\n \tint err = 0;\n \tstruct pack_window *w_curs = NULL;\n@@ -159,7 +166,7 @@ int verify_pack(struct packed_git *p)\n \tif (!p->index_data)\n \t\treturn -1;\n \n-\terr |= verify_packfile(p, &w_curs);\n+\terr |= verify_packfile(p, &w_curs, fn);\n \tunuse_pack(&w_curs);\n \n \treturn err;\ndiff --git a/pack.h b/pack.h\nindex 722a54e..70f3c29 100644\n--- a/pack.h\n+++ b/pack.h\n@@ -70,10 +70,13 @@ struct pack_idx_entry {\n \toff_t offset;\n };\n \n+\n+typedef int (*verify_fn)(const unsigned char*, enum object_type, unsigned long, void*, int*);\n+\n extern const char *write_idx_file(const char *index_name, struct pack_idx_entry **objects, int nr_objects, const struct pack_idx_option *, unsigned char *sha1);\n extern int check_pack_crc(struct packed_git *p, struct pack_window **w_curs, off_t offset, off_t len, unsigned int nr);\n extern int verify_pack_index(struct packed_git *);\n-extern int verify_pack(struct packed_git *);\n+extern int verify_pack(struct packed_git *, verify_fn fn);\n extern void fixup_pack_header_footer(int, unsigned char *, const char *, uint32_t, unsigned char *, off_t);\n extern char *index_pack_lockfile(int fd);\n extern int encode_in_pack_object_header(enum object_type, uintmax_t, unsigned char *);\n-- \n1.7.4.74.g639db\n"},{"id":"178994","messageId":"1320634766-24511-5-git-send-email-pclouds@gmail.com","threadId":"28864","inReplyTo":"1320634766-24511-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2 4/4] fsck: print progress","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-11-07T02:59:26Z","receivedAt":"2011-11-07T02:59:26Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"fsck is usually a long process and it would be nice if it prints\nprogress from time to time.\n\nProgress meter is not printed when --verbose is given because\n--verbose prints a lot, there's no need for \"alive\" indicator.\nProgress meter may provide \"% complete\" information but it would\nbe lost anyway in the flood of text.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Documentation/git-fsck.txt |   11 ++++++++++-\n builtin/fsck.c             |   40 ++++++++++++++++++++++++++++++++++++++--\n pack-check.c               |   14 +++++++++++---\n pack.h                     |    3 ++-\n 4 files changed, 61 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-fsck.txt b/Documentation/git-fsck.txt\nindex a2a508d..0a17b42 100644\n--- a/Documentation/git-fsck.txt\n+++ b/Documentation/git-fsck.txt\n@@ -10,7 +10,8 @@ SYNOPSIS\n --------\n [verse]\n 'git fsck' [--tags] [--root] [--unreachable] [--cache] [--no-reflogs]\n-\t [--[no-]full] [--strict] [--verbose] [--lost-found] [<object>*]\n+\t [--[no-]full] [--strict] [--verbose] [--lost-found]\n+\t [--[no-]progress] [<object>*]\n \n DESCRIPTION\n -----------\n@@ -72,6 +73,14 @@ index file, all SHA1 references in .git/refs/*, and all reflogs (unless\n \ta blob, the contents are written into the file, rather than\n \tits object name.\n \n+--progress::\n+--no-progress::\n+\tProgress status is reported on the standard error stream by\n+\tdefault when it is attached to a terminal, unless\n+\t--no-progress or --verbose is specified. --progress forces\n+\tprogress status even if the standard error stream is not\n+\tdirected to a terminal.\n+\n It tests SHA1 and general object sanity, and it does full tracking of\n the resulting reachability and everything else. It prints out any\n corruption it finds (missing or bad objects), and if you use the\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex 0603f64..c4b1ca6 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -11,6 +11,7 @@\n #include \"fsck.h\"\n #include \"parse-options.h\"\n #include \"dir.h\"\n+#include \"progress.h\"\n \n #define REACHABLE 0x0001\n #define SEEN      0x0002\n@@ -27,6 +28,7 @@ static const char *head_points_at;\n static int errors_found;\n static int write_lost_and_found;\n static int verbose;\n+static int show_progress = -1;\n #define ERROR_OBJECT 01\n #define ERROR_REACHABLE 02\n #define ERROR_PACK 04\n@@ -138,7 +140,11 @@ static int traverse_one_object(struct object *obj)\n \n static int traverse_reachable(void)\n {\n+\tstruct progress *progress = NULL;\n+\tunsigned int nr = 0;\n \tint result = 0;\n+\tif (show_progress)\n+\t\tprogress = start_progress_delay(\"Checking connectivity\", 0, 0, 2);\n \twhile (pending.nr) {\n \t\tstruct object_array_entry *entry;\n \t\tstruct object *obj;\n@@ -146,7 +152,9 @@ static int traverse_reachable(void)\n \t\tentry = pending.objects + --pending.nr;\n \t\tobj = entry->item;\n \t\tresult |= traverse_one_object(obj);\n+\t\tdisplay_progress(progress, ++nr);\n \t}\n+\tstop_progress(&progress);\n \treturn !!result;\n }\n \n@@ -530,15 +538,20 @@ static void get_default_heads(void)\n static void fsck_object_dir(const char *path)\n {\n \tint i;\n+\tstruct progress *progress = NULL;\n \n \tif (verbose)\n \t\tfprintf(stderr, \"Checking object directory\\n\");\n \n+\tif (show_progress)\n+\t\tprogress = start_progress(\"Checking object directories\", 256);\n \tfor (i = 0; i < 256; i++) {\n \t\tstatic char dir[4096];\n \t\tsprintf(dir, \"%s/%02x\", path, i);\n \t\tfsck_dir(i, dir);\n+\t\tdisplay_progress(progress, i+1);\n \t}\n+\tstop_progress(&progress);\n \tfsck_sha1_list();\n }\n \n@@ -609,6 +622,7 @@ static struct option fsck_opts[] = {\n \tOPT_BOOLEAN(0, \"strict\", &check_strict, \"enable more strict checking\"),\n \tOPT_BOOLEAN(0, \"lost-found\", &write_lost_and_found,\n \t\t\t\t\"write dangling objects in .git/lost-found\"),\n+\tOPT_BOOL   (0, \"progress\", &show_progress, \"show progress\"),\n \tOPT_END(),\n };\n \n@@ -621,6 +635,12 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)\n \tread_replace_refs = 0;\n \n \targc = parse_options(argc, argv, prefix, fsck_opts, fsck_usage, 0);\n+\n+\tif (show_progress == -1)\n+\t\tshow_progress = isatty(2);\n+\tif (verbose)\n+\t\tshow_progress = 0;\n+\n \tif (write_lost_and_found) {\n \t\tcheck_full = 1;\n \t\tinclude_reflogs = 0;\n@@ -640,12 +660,28 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)\n \n \tif (check_full) {\n \t\tstruct packed_git *p;\n+\t\tuint32_t total = 0, count = 0;\n+\t\tstruct progress *progress = NULL;\n \n \t\tprepare_packed_git();\n-\t\tfor (p = packed_git; p; p = p->next)\n+\n+\t\tif (show_progress) {\n+\t\t\tfor (p = packed_git; p; p = p->next) {\n+\t\t\t\tif (open_pack_index(p))\n+\t\t\t\t\tcontinue;\n+\t\t\t\ttotal += p->num_objects;\n+\t\t\t}\n+\n+\t\t\tprogress = start_progress(\"Checking objects\", total);\n+\t\t}\n+\t\tfor (p = packed_git; p; p = p->next) {\n \t\t\t/* verify gives error messages itself */\n-\t\t\tif (verify_pack(p, fsck_obj_buffer))\n+\t\t\tif (verify_pack(p, fsck_obj_buffer,\n+\t\t\t\t\tprogress, count))\n \t\t\t\terrors_found |= ERROR_PACK;\n+\t\t\tcount += p->num_objects;\n+\t\t}\n+\t\tstop_progress(&progress);\n \t}\n \n \theads = 0;\ndiff --git a/pack-check.c b/pack-check.c\nindex 3b48b86..63a595c 100644\n--- a/pack-check.c\n+++ b/pack-check.c\n@@ -1,6 +1,7 @@\n #include \"cache.h\"\n #include \"pack.h\"\n #include \"pack-revindex.h\"\n+#include \"progress.h\"\n \n struct idx_entry {\n \toff_t                offset;\n@@ -43,7 +44,9 @@ int check_pack_crc(struct packed_git *p, struct pack_window **w_curs,\n \n static int verify_packfile(struct packed_git *p,\n \t\t\t   struct pack_window **w_curs,\n-\t\t\t   verify_fn fn)\n+\t\t\t   verify_fn fn,\n+\t\t\t   struct progress *progress, uint32_t base_count)\n+\n {\n \toff_t index_size = p->index_size;\n \tconst unsigned char *index_base = p->index_data;\n@@ -127,8 +130,12 @@ static int verify_packfile(struct packed_git *p,\n \t\t\tif (eaten)\n \t\t\t\tdata = NULL;\n \t\t}\n+\t\tif (((base_count + i) & 1023) == 0)\n+\t\t\tdisplay_progress(progress, base_count + i);\n \t\tfree(data);\n+\n \t}\n+\tdisplay_progress(progress, base_count + i);\n \tfree(entries);\n \n \treturn err;\n@@ -157,7 +164,8 @@ int verify_pack_index(struct packed_git *p)\n \treturn err;\n }\n \n-int verify_pack(struct packed_git *p, verify_fn fn)\n+int verify_pack(struct packed_git *p, verify_fn fn,\n+\t\tstruct progress *progress, uint32_t base_count)\n {\n \tint err = 0;\n \tstruct pack_window *w_curs = NULL;\n@@ -166,7 +174,7 @@ int verify_pack(struct packed_git *p, verify_fn fn)\n \tif (!p->index_data)\n \t\treturn -1;\n \n-\terr |= verify_packfile(p, &w_curs, fn);\n+\terr |= verify_packfile(p, &w_curs, fn, progress, base_count);\n \tunuse_pack(&w_curs);\n \n \treturn err;\ndiff --git a/pack.h b/pack.h\nindex 70f3c29..324a1d7 100644\n--- a/pack.h\n+++ b/pack.h\n@@ -71,12 +71,13 @@ struct pack_idx_entry {\n };\n \n \n+struct progress;\n typedef int (*verify_fn)(const unsigned char*, enum object_type, unsigned long, void*, int*);\n \n extern const char *write_idx_file(const char *index_name, struct pack_idx_entry **objects, int nr_objects, const struct pack_idx_option *, unsigned char *sha1);\n extern int check_pack_crc(struct packed_git *p, struct pack_window **w_curs, off_t offset, off_t len, unsigned int nr);\n extern int verify_pack_index(struct packed_git *);\n-extern int verify_pack(struct packed_git *, verify_fn fn);\n+extern int verify_pack(struct packed_git *, verify_fn fn, struct progress *, uint32_t);\n extern void fixup_pack_header_footer(int, unsigned char *, const char *, uint32_t, unsigned char *, off_t);\n extern char *index_pack_lockfile(int fd);\n extern int encode_in_pack_object_header(enum object_type, uintmax_t, unsigned char *);\n-- \n1.7.4.74.g639db\n"},{"id":"178996","messageId":"7vsjm04kn4.fsf@alter.siamese.dyndns.org","threadId":"28864","inReplyTo":"1320634766-24511-5-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v2 4/4] fsck: print progress","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-11-07T04:33:03Z","receivedAt":"2011-11-07T04:33:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks. Will queue.\n"}]}