{"thread":{"id":"32887","subject":"[PATCH v3 1/4] git-count-objects.txt: describe each line in -v output","startedAt":"2013-02-12T09:27:53Z","lastAt":"2013-02-15T23:20:51Z","messageCount":17,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano","Duy Nguyen"],"isPatch":true,"patchVersion":3,"patchTotal":4},"messages":[{"id":"209342","messageId":"1360661277-17273-1-git-send-email-pclouds@gmail.com","threadId":"32887","inReplyTo":null,"subject":"[PATCH v3 0/4] count-objects improvements","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-02-12T09:27:53Z","receivedAt":"2013-02-12T09:27:53Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Compared to v2 [1], this version\n - fixes sparse warning\n - restructures 2/3 (now 3/4) to make it easier to read\n - report \"path too long\" instead of \"garbage found\" in\n   .git/path/too/long/pack-xxx.pack case\n - changes output prefix \"error:\" to \"warning:\"\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/215378/focus=215744\n\nNguyễn Thái Ngọc Duy (4):\n  git-count-objects.txt: describe each line in -v output\n  sha1_file: reorder code in prepare_packed_git_one()\n  count-objects: report garbage files in pack directory too\n  count-objects: report how much disk space taken by garbage files\n\n Documentation/git-count-objects.txt |  22 ++++++--\n builtin/count-objects.c             |  43 +++++++++++---\n cache.h                             |   3 +\n sha1_file.c                         | 108 +++++++++++++++++++++++++++++++-----\n 4 files changed, 148 insertions(+), 28 deletions(-)\n\n-- \n1.8.1.2.536.gf441e6d\n"},{"id":"209338","messageId":"1360661277-17273-2-git-send-email-pclouds@gmail.com","threadId":"32887","inReplyTo":"1360661277-17273-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v3 1/4] git-count-objects.txt: describe each line in -v output","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-02-12T09:27:54Z","receivedAt":"2013-02-12T09:27:54Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The current description requires a bit of guessing (what clause\ncorresponds to what printed line?) and lacks information, such as\nthe unit of size and size-pack.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Documentation/git-count-objects.txt | 20 +++++++++++++++-----\n 1 file changed, 15 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-count-objects.txt b/Documentation/git-count-objects.txt\nindex 23c80ce..e816823 100644\n--- a/Documentation/git-count-objects.txt\n+++ b/Documentation/git-count-objects.txt\n@@ -20,11 +20,21 @@ OPTIONS\n -------\n -v::\n --verbose::\n-\tIn addition to the number of loose objects and disk\n-\tspace consumed, it reports the number of in-pack\n-\tobjects, number of packs, disk space consumed by those packs,\n-\tand number of objects that can be removed by running\n-\t`git prune-packed`.\n+\tReport in more detail:\n++\n+count: the number of loose objects\n++\n+size: disk space consumed by loose objects, in KiB\n++\n+in-pack: the number of in-pack objects\n++\n+size-pack: disk space consumed by the packs, in KiB\n++\n+prune-packable: the number of loose objects that are also present in\n+the packs. These objects could be pruned using `git prune-packed`.\n++\n+garbage: the number of files in loose object database that are not\n+valid loose objects\n \n GIT\n ---\n-- \n1.8.1.2.536.gf441e6d\n"},{"id":"209339","messageId":"1360661277-17273-3-git-send-email-pclouds@gmail.com","threadId":"32887","inReplyTo":"1360661277-17273-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v3 2/4] sha1_file: reorder code in prepare_packed_git_one()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-02-12T09:27:55Z","receivedAt":"2013-02-12T09:27:55Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The current loop does\n\n\twhile (...) {\n\t\tif (!not .idx file)\n\t\t\tcontinue;\n\t\tprocess .idx file;\n\t}\n\nand is reordered to\n\n\twhile (...) {\n\t\tif (!.idx file) {\n\t\t\tprocess .idx file;\n\t\t}\n\t}\n\nThis makes it easier to add new extension file processing.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n sha1_file.c | 33 +++++++++++++++++----------------\n 1 file changed, 17 insertions(+), 16 deletions(-)\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 40b2329..8d7da1d 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -1024,27 +1024,28 @@ static void prepare_packed_git_one(char *objdir, int local)\n \t\tint namelen = strlen(de->d_name);\n \t\tstruct packed_git *p;\n \n-\t\tif (!has_extension(de->d_name, \".idx\"))\n-\t\t\tcontinue;\n-\n \t\tif (len + namelen + 1 > sizeof(path))\n \t\t\tcontinue;\n \n-\t\t/* Don't reopen a pack we already have. */\n \t\tstrcpy(path + len, de->d_name);\n-\t\tfor (p = packed_git; p; p = p->next) {\n-\t\t\tif (!memcmp(path, p->pack_name, len + namelen - 4))\n-\t\t\t\tbreak;\n+\n+\t\tif (has_extension(de->d_name, \".idx\")) {\n+\t\t\t/* Don't reopen a pack we already have. */\n+\t\t\tfor (p = packed_git; p; p = p->next) {\n+\t\t\t\tif (!memcmp(path, p->pack_name, len + namelen - 4))\n+\t\t\t\t\tbreak;\n+\t\t\t}\n+\t\t\tif (p)\n+\t\t\t\tcontinue;\n+\t\t\t/*\n+\t\t\t * See if it really is a valid .idx file with\n+\t\t\t * corresponding .pack file that we can map.\n+\t\t\t */\n+\t\t\tp = add_packed_git(path, len + namelen, local);\n+\t\t\tif (!p)\n+\t\t\t\tcontinue;\n+\t\t\tinstall_packed_git(p);\n \t\t}\n-\t\tif (p)\n-\t\t\tcontinue;\n-\t\t/* See if it really is a valid .idx file with corresponding\n-\t\t * .pack file that we can map.\n-\t\t */\n-\t\tp = add_packed_git(path, len + namelen, local);\n-\t\tif (!p)\n-\t\t\tcontinue;\n-\t\tinstall_packed_git(p);\n \t}\n \tclosedir(dir);\n }\n-- \n1.8.1.2.536.gf441e6d\n"},{"id":"209340","messageId":"1360661277-17273-4-git-send-email-pclouds@gmail.com","threadId":"32887","inReplyTo":"1360661277-17273-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v3 3/4] count-objects: report garbage files in pack directory too","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-02-12T09:27:56Z","receivedAt":"2013-02-12T09:27:56Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"prepare_packed_git_one() is modified to allow count-objects to hook a\nreport function to so we don't need to duplicate the pack searching\nlogic in count-objects.c. When report_pack_garbage is NULL, the\noverhead is insignificant.\n\nThe garbage is reported with warning() instead of error() in packed\ngarbage case because it's not an error to have garbage. Loose garbage\nis still reported as errors and will be converted to warnings later.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Documentation/git-count-objects.txt |  4 +-\n builtin/count-objects.c             | 22 +++++++++-\n cache.h                             |  3 ++\n sha1_file.c                         | 83 ++++++++++++++++++++++++++++++++++++-\n 4 files changed, 107 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-count-objects.txt b/Documentation/git-count-objects.txt\nindex e816823..1611d7c 100644\n--- a/Documentation/git-count-objects.txt\n+++ b/Documentation/git-count-objects.txt\n@@ -33,8 +33,8 @@ size-pack: disk space consumed by the packs, in KiB\n prune-packable: the number of loose objects that are also present in\n the packs. These objects could be pruned using `git prune-packed`.\n +\n-garbage: the number of files in loose object database that are not\n-valid loose objects\n+garbage: the number of files in object database that are not valid\n+loose objects nor valid packs\n \n GIT\n ---\ndiff --git a/builtin/count-objects.c b/builtin/count-objects.c\nindex 9afaa88..639c9a5 100644\n--- a/builtin/count-objects.c\n+++ b/builtin/count-objects.c\n@@ -9,6 +9,24 @@\n #include \"builtin.h\"\n #include \"parse-options.h\"\n \n+static unsigned long garbage;\n+\n+static void real_report_garbage(const char *desc,\n+\t\t\t\tconst char *path, int len,\n+\t\t\t\tconst char *name)\n+{\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tif (len && name)\n+\t\tstrbuf_addf(&sb, \"%.*s/%s\", len, path, name);\n+\telse if (!len && name)\n+\t\tstrbuf_addf(&sb, \"%s%s\", path, name);\n+\telse\n+\t\tstrbuf_addf(&sb, \"%s\", path);\n+\twarning(\"%s: %s\", desc, sb.buf);\n+\tgarbage++;\n+\tstrbuf_release(&sb);\n+}\n+\n static void count_objects(DIR *d, char *path, int len, int verbose,\n \t\t\t  unsigned long *loose,\n \t\t\t  off_t *loose_size,\n@@ -76,7 +94,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)\n \tconst char *objdir = get_object_directory();\n \tint len = strlen(objdir);\n \tchar *path = xmalloc(len + 50);\n-\tunsigned long loose = 0, packed = 0, packed_loose = 0, garbage = 0;\n+\tunsigned long loose = 0, packed = 0, packed_loose = 0;\n \toff_t loose_size = 0;\n \tstruct option opts[] = {\n \t\tOPT__VERBOSE(&verbose, N_(\"be verbose\")),\n@@ -87,6 +105,8 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)\n \t/* we do not take arguments other than flags for now */\n \tif (argc)\n \t\tusage_with_options(count_objects_usage, opts);\n+\tif (verbose)\n+\t\treport_garbage = real_report_garbage;\n \tmemcpy(path, objdir, len);\n \tif (len && objdir[len-1] != '/')\n \t\tpath[len++] = '/';\ndiff --git a/cache.h b/cache.h\nindex 7339f21..e486499 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1051,6 +1051,9 @@ extern const char *parse_feature_value(const char *feature_list, const char *fea\n \n extern struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_path);\n \n+/* A hook for count-objects to report invalid files in pack directory */\n+extern void (*report_garbage)(const char *desc, const char *path, int len, const char *name);\n+\n extern void prepare_packed_git(void);\n extern void reprepare_packed_git(void);\n extern void install_packed_git(struct packed_git *pack);\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 8d7da1d..290e348 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -21,6 +21,7 @@\n #include \"sha1-lookup.h\"\n #include \"bulk-checkin.h\"\n #include \"streaming.h\"\n+#include \"dir.h\"\n \n #ifndef O_NOATIME\n #if defined(__linux__) && (defined(__i386__) || defined(__PPC__))\n@@ -1000,6 +1001,52 @@ void install_packed_git(struct packed_git *pack)\n \tpacked_git = pack;\n }\n \n+void (*report_garbage)(const char *desc, const char *path,\n+\t\t       int len, const char *name);\n+\n+static void report_pack_garbage(struct string_list *list)\n+{\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tstruct packed_git *p;\n+\tint i;\n+\n+\tif (!report_garbage)\n+\t\treturn;\n+\n+\tsort_string_list(list);\n+\n+\tfor (p = packed_git; p; p = p->next) {\n+\t\tstruct string_list_item *item;\n+\t\tif (!p->pack_local)\n+\t\t\tcontinue;\n+\t\tstrbuf_reset(&sb);\n+\t\tstrbuf_add(&sb, p->pack_name,\n+\t\t\t   strlen(p->pack_name) - 5); /* \".pack\" */\n+\t\titem = string_list_lookup(list, sb.buf);\n+\t\tif (!item)\n+\t\t\tcontinue;\n+\t\t/*\n+\t\t * string_list_lookup does not guarantee to return the\n+\t\t * first matched string if it's duplicated.\n+\t\t */\n+\t\twhile (item - list->items &&\n+\t\t       !strcmp(item[-1].string, item->string))\n+\t\t\titem--;\n+\t\twhile (item - list->items < list->nr &&\n+\t\t       !strcmp(item->string, sb.buf)) {\n+\t\t\titem->util = NULL; /* non-garbage mark */\n+\t\t\titem++;\n+\t\t}\n+\t}\n+\tfor (i = 0; i < list->nr; i++) {\n+\t\tstruct string_list_item *item = list->items + i;\n+\t\tif (!item->util)\n+\t\t\tcontinue;\n+\t\treport_garbage(\"garbage found\", item->string, 0, item->util);\n+\t}\n+\tstrbuf_release(&sb);\n+}\n+\n static void prepare_packed_git_one(char *objdir, int local)\n {\n \t/* Ensure that this buffer is large enough so that we can\n@@ -1009,6 +1056,7 @@ static void prepare_packed_git_one(char *objdir, int local)\n \tint len;\n \tDIR *dir;\n \tstruct dirent *de;\n+\tstruct string_list garbage = STRING_LIST_INIT_DUP;\n \n \tsprintf(path, \"%s/pack\", objdir);\n \tlen = strlen(path);\n@@ -1024,7 +1072,14 @@ static void prepare_packed_git_one(char *objdir, int local)\n \t\tint namelen = strlen(de->d_name);\n \t\tstruct packed_git *p;\n \n-\t\tif (len + namelen + 1 > sizeof(path))\n+\t\tif (len + namelen + 1 > sizeof(path)) {\n+\t\t\tif (report_garbage)\n+\t\t\t\treport_garbage(\"path too long\",\n+\t\t\t\t\t       path, len - 1, de->d_name);\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tif (is_dot_or_dotdot(de->d_name))\n \t\t\tcontinue;\n \n \t\tstrcpy(path + len, de->d_name);\n@@ -1045,9 +1100,33 @@ static void prepare_packed_git_one(char *objdir, int local)\n \t\t\tif (!p)\n \t\t\t\tcontinue;\n \t\t\tinstall_packed_git(p);\n-\t\t}\n+\t\t} else if (!report_garbage) {\n+\t\t\t/*\n+\t\t\t * the rest of this if-chain requires\n+\t\t\t * report_garbage != NULL. Stop the chain if\n+\t\t\t * report_garbage is NULL.\n+\t\t\t */\n+\t\t\t;\n+\t\t} else if (has_extension(de->d_name, \".pack\")) {\n+\t\t\tstruct string_list_item *item;\n+\t\t\tint n = strlen(path) - 5;\n+\t\t\titem = string_list_append_nodup(&garbage,\n+\t\t\t\t\t\t\t xstrndup(path, n));\n+\t\t\titem->util = \".pack\";\n+\t\t\tcontinue;\n+\t\t} else if (has_extension(de->d_name, \".idx\")) {\n+\t\t\tstruct string_list_item *item;\n+\t\t\tint n = strlen(path) - 4;\n+\t\t\titem = string_list_append_nodup(&garbage,\n+\t\t\t\t\t\t\txstrndup(path, n));\n+\t\t\titem->util = \".idx\";\n+\t\t\tcontinue;\n+\t\t} else\n+\t\t\treport_garbage(\"garbage found\", path, 0, NULL);\n \t}\n \tclosedir(dir);\n+\treport_pack_garbage(&garbage);\n+\tstring_list_clear(&garbage, 0);\n }\n \n static int sort_pack(const void *a_, const void *b_)\n-- \n1.8.1.2.536.gf441e6d\n"},{"id":"209341","messageId":"1360661277-17273-5-git-send-email-pclouds@gmail.com","threadId":"32887","inReplyTo":"1360661277-17273-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v3 4/4] count-objects: report how much disk space taken by garbage files","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-02-12T09:27:57Z","receivedAt":"2013-02-12T09:27:57Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Also issue warnings on loose garbages instead of errors as a result of\nusing report_garbage() function in count_objects()\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Documentation/git-count-objects.txt |  2 ++\n builtin/count-objects.c             | 21 +++++++++++++--------\n 2 files changed, 15 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/git-count-objects.txt b/Documentation/git-count-objects.txt\nindex 1611d7c..da6e72e 100644\n--- a/Documentation/git-count-objects.txt\n+++ b/Documentation/git-count-objects.txt\n@@ -35,6 +35,8 @@ the packs. These objects could be pruned using `git prune-packed`.\n +\n garbage: the number of files in object database that are not valid\n loose objects nor valid packs\n++\n+size-garbage: disk space consumed by garbage files, in KiB\n \n GIT\n ---\ndiff --git a/builtin/count-objects.c b/builtin/count-objects.c\nindex 639c9a5..75feee5 100644\n--- a/builtin/count-objects.c\n+++ b/builtin/count-objects.c\n@@ -10,12 +10,15 @@\n #include \"parse-options.h\"\n \n static unsigned long garbage;\n+static off_t size_garbage;\n \n static void real_report_garbage(const char *desc,\n \t\t\t\tconst char *path, int len,\n \t\t\t\tconst char *name)\n {\n \tstruct strbuf sb = STRBUF_INIT;\n+\tstruct stat st;\n+\n \tif (len && name)\n \t\tstrbuf_addf(&sb, \"%.*s/%s\", len, path, name);\n \telse if (!len && name)\n@@ -23,6 +26,10 @@ static void real_report_garbage(const char *desc,\n \telse\n \t\tstrbuf_addf(&sb, \"%s\", path);\n \twarning(\"%s: %s\", desc, sb.buf);\n+\n+\tif (!stat(sb.buf, &st))\n+\t\tsize_garbage += st.st_size;\n+\n \tgarbage++;\n \tstrbuf_release(&sb);\n }\n@@ -30,8 +37,7 @@ static void real_report_garbage(const char *desc,\n static void count_objects(DIR *d, char *path, int len, int verbose,\n \t\t\t  unsigned long *loose,\n \t\t\t  off_t *loose_size,\n-\t\t\t  unsigned long *packed_loose,\n-\t\t\t  unsigned long *garbage)\n+\t\t\t  unsigned long *packed_loose)\n {\n \tstruct dirent *ent;\n \twhile ((ent = readdir(d)) != NULL) {\n@@ -63,11 +69,9 @@ static void count_objects(DIR *d, char *path, int len, int verbose,\n \t\t\t\t(*loose_size) += xsize_t(on_disk_bytes(st));\n \t\t}\n \t\tif (bad) {\n-\t\t\tif (verbose) {\n-\t\t\t\terror(\"garbage found: %.*s/%s\",\n-\t\t\t\t      len + 2, path, ent->d_name);\n-\t\t\t\t(*garbage)++;\n-\t\t\t}\n+\t\t\tif (verbose)\n+\t\t\t\treport_garbage(\"garbage found\",\n+\t\t\t\t\t       path, len + 2, ent->d_name);\n \t\t\tcontinue;\n \t\t}\n \t\t(*loose)++;\n@@ -117,7 +121,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)\n \t\tif (!d)\n \t\t\tcontinue;\n \t\tcount_objects(d, path, len, verbose,\n-\t\t\t      &loose, &loose_size, &packed_loose, &garbage);\n+\t\t\t      &loose, &loose_size, &packed_loose);\n \t\tclosedir(d);\n \t}\n \tif (verbose) {\n@@ -142,6 +146,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)\n \t\tprintf(\"size-pack: %lu\\n\", (unsigned long) (size_pack / 1024));\n \t\tprintf(\"prune-packable: %lu\\n\", packed_loose);\n \t\tprintf(\"garbage: %lu\\n\", garbage);\n+\t\tprintf(\"size-garbage: %lu\\n\", (unsigned long) (size_garbage / 1024));\n \t}\n \telse\n \t\tprintf(\"%lu objects, %lu kilobytes\\n\",\n-- \n1.8.1.2.536.gf441e6d\n"},{"id":"209377","messageId":"7va9r9igze.fsf@alter.siamese.dyndns.org","threadId":"32887","inReplyTo":"1360661277-17273-4-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v3 3/4] count-objects: report garbage files in pack directory too","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-12T17:23:17Z","receivedAt":"2013-02-12T17:23:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:\n\n> +/* A hook for count-objects to report invalid files in pack directory */\n> +extern void (*report_garbage)(const char *desc, const char *path, int len, const char *name);\n\nWe may want to document the strange way the last three parameters\nare used somewhere.  e.g.\n\n\tshows \"path\" (if \"name\" is NULL), or prepends \"path\" in\n\tfront of name (otherwise); only for the latter, \"path\" can\n\tbe a string that is not NUL-terminated but its length\n\tspecified with \"len\" and in that case a slash is inserted\n\tbetween the path and the \"name\".\n\nWhen described clearly, it sounds somewhat ugly and incoherent API,\neven though it covers the immediate need X-<.\n\n> +\tsort_string_list(list);\n> +\n> +\tfor (p = packed_git; p; p = p->next) {\n> +\t\tstruct string_list_item *item;\n> +\t\tif (!p->pack_local)\n> +\t\t\tcontinue;\n> +\t\tstrbuf_reset(&sb);\n> +\t\tstrbuf_add(&sb, p->pack_name,\n> +\t\t\t   strlen(p->pack_name) - 5); /* \".pack\" */\n> +\t\titem = string_list_lookup(list, sb.buf);\n> +\t\tif (!item)\n> +\t\t\tcontinue;\n> +\t\t/*\n> +\t\t * string_list_lookup does not guarantee to return the\n> +\t\t * first matched string if it's duplicated.\n> +\t\t */\n\nDo we need to even allow duplication?  Why does prepare_packed_git_one()\nbelow have to split the pathname into the base and extension in the first\nplace?\n\nIf you collect all pathnames that end with known extensions (\".idx\",\n\".pack\" and \".keep\") in the sorted \"garbage\" list as separate entries in\nprepare_packed_git_one(), report_pack_garbage() can do the right thing\nwithout trusting or iterating over packed_git list at all, I think.  As\nthe entries are sorted, .pack, .idx and all other valid .ext are grouped\ntogether for the same basename.  If you see a group that have both .pack\nand .idx, the group is good.  Otherwise, everybody in the group is bad\n(e.g. a lonely .pack file without .idx is an unusable garbage).\n\nHow about doing it something along this line, perhaps?\n\n\tint i;\n\tint beginning_of_this_name = -1;\n        int seen_bits = 0; /* 01 for .idx, 02 for .pack */\n\tfor (i = 0; i < list->nr; i++) {\n        \tif (beginning_of_this_name < 0)\n                \tbeginning_of_this_name = i;\n\t\telse if (list->items[i] and list->items[beginning_of_this_name]\n                \t share the same basename)\n\t\t\t; /* keep scanning */\n\t\telse {\n                \t/* one name ended at (i-1) */\n\t\t\tif (seen_bits == 3)\n\t\t\t\t; /* both .idx and .pack exist; good */\n\t\t\telse\n\t\t\t        report_garbage_for_one_name(list, beginning_of_this_name, i,\n                                \t\tseen_bits);\n\t\t\tseen_bits = 0;\n                        beginning_of_this_name = i;\n\t\t}\n                if (list->items[i] is \".idx\")\n\t\t\tseen_bits |= 1;\n                if (list->items[i] is \".pack\")\n\t\t\tseen_bits |= 2;\n\n\t}\n\tif (0 <= beginning_of_this_name && seen_bits != 3)\n\t        report_garbages_for_one_name(list, beginning_of_this_name, list->nr, seen_bits);\n\nwith a helper function report_garbage_for_one_name() that would look like this:\n\n\treport_garbage_for_one_name(...) {\n                int j;\n                const char *msg;\n                switch (seen_bits) {\n                case 0: msg = \"no corresponding .idx nor .pack\"; break;\n                case 1: msg = \"no corresponding .pack\"; break;\n                case 2: msg = \"no corresponding .idx; break;\n                }\n                for (j = beginning_of_this_name; j < i; j++)\n                        report_garbage(msg, list->items[j]);\n\t}\n\nFor the above to work, prepare_packed_git_one() needs to retain only the\npaths with known extensions in garbage list. \"pack-deadbeef.unk\" can and\nshould be reported as a garbage immediately when it is seen without being\nplaced in the list.\n\n> @@ -1045,9 +1100,33 @@ static void prepare_packed_git_one(char *objdir, int local)\n>  \t\t\tif (!p)\n>  \t\t\t\tcontinue;\n>  \t\t\tinstall_packed_git(p);\n> -\t\t}\n> +\t\t} else if (!report_garbage) {\n> +\t\t\t/*\n> +\t\t\t * the rest of this if-chain requires\n> +\t\t\t * report_garbage != NULL. Stop the chain if\n> +\t\t\t * report_garbage is NULL.\n> +\t\t\t */\n> +\t\t\t;\n> +\t\t} else if (has_extension(de->d_name, \".pack\")) {\n> +\t\t\tstruct string_list_item *item;\n> +\t\t\tint n = strlen(path) - 5;\n> +\t\t\titem = string_list_append_nodup(&garbage,\n> +\t\t\t\t\t\t\t xstrndup(path, n));\n> +\t\t\titem->util = \".pack\";\n> +\t\t\tcontinue;\n> +\t\t} else if (has_extension(de->d_name, \".idx\")) {\n> +\t\t\tstruct string_list_item *item;\n> +\t\t\tint n = strlen(path) - 4;\n> +\t\t\titem = string_list_append_nodup(&garbage,\n> +\t\t\t\t\t\t\txstrndup(path, n));\n> +\t\t\titem->util = \".idx\";\n> +\t\t\tcontinue;\n> +\t\t} else\n> +\t\t\treport_garbage(\"garbage found\", path, 0, NULL);\n\nHmm, where is a \".keep\" file handled in this flow?\n\nThe structure of the if/else cascade is much nicer than the earlier\niterations, but wouldn't it be even more clear to do this?\n\n\tif (is .idx file) {\n\t\t... do that .idx thing ...\n\t}\n\n\tif (!report_garbage)\n        \tcontinue; /* it does not matter what the file is */\n\n\tif (is .pack) {\n\t\t... remember that we saw this .pack ...\n\t} else if (is .idx) {\n\t\t... remember that we saw this .idx ...\n\t} else if (is .keep) {\n\t\t... remember that we saw this .keep ...\n        } else {\n        \t... all else --- report as garbage immediately ...\n\t}\n"},{"id":"209452","messageId":"1360746799-3668-1-git-send-email-pclouds@gmail.com","threadId":"32887","inReplyTo":"7va9r9igze.fsf@alter.siamese.dyndns.org","subject":"[PATCH v4 0/4] count-objects improvements","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-02-13T09:13:15Z","receivedAt":"2013-02-13T09:13:15Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Feb 13, 2013 at 12:23 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:\n>\n>> +/* A hook for count-objects to report invalid files in pack directory */\n>> +extern void (*report_garbage)(const char *desc, const char *path, int len, const char *name);\n>\n> We may want to document the strange way the last three parameters\n> are used somewhere.  e.g.\n>\n>         shows \"path\" (if \"name\" is NULL), or prepends \"path\" in\n>         front of name (otherwise); only for the latter, \"path\" can\n>         be a string that is not NUL-terminated but its length\n>         specified with \"len\" and in that case a slash is inserted\n>         between the path and the \"name\".\n>\n> When described clearly, it sounds somewhat ugly and incoherent API,\n> even though it covers the immediate need X-<.\n\nOne of the reasons why I did not export it explicitly. Changed it to\n\nvoid (*report_garbage)(const char *desc, const char *path);\n\nand pushed the ugly part back to callers.\n\n> How about doing it something along this line, perhaps?\n>\n>         int i;\n>         int beginning_of_this_name = -1;\n>         int seen_bits = 0; /* 01 for .idx, 02 for .pack */\n>         for (i = 0; i < list->nr; i++) {\n>                 if (beginning_of_this_name < 0)\n>                         beginning_of_this_name = i;\n>                 else if (list->items[i] and list->items[beginning_of_this_name]\n>                          share the same basename)\n>                         ; /* keep scanning */\n>                 else {\n>                         /* one name ended at (i-1) */\n>                         if (seen_bits == 3)\n>                                 ; /* both .idx and .pack exist; good */\n>                         else\n>                                 report_garbage_for_one_name(list, beginning_of_this_name, i,\n>                                                 seen_bits);\n>                         seen_bits = 0;\n>                         beginning_of_this_name = i;\n>                 }\n>                 if (list->items[i] is \".idx\")\n>                         seen_bits |= 1;\n>                 if (list->items[i] is \".pack\")\n>                         seen_bits |= 2;\n>\n>         }\n>         if (0 <= beginning_of_this_name && seen_bits != 3)\n>                 report_garbages_for_one_name(list, beginning_of_this_name, list->nr, seen_bits);\n>\n> with a helper function report_garbage_for_one_name() that would look like this:\n>\n>         report_garbage_for_one_name(...) {\n>                 int j;\n>                 const char *msg;\n>                 switch (seen_bits) {\n>                 case 0: msg = \"no corresponding .idx nor .pack\"; break;\n>                 case 1: msg = \"no corresponding .pack\"; break;\n>                 case 2: msg = \"no corresponding .idx; break;\n>                 }\n>                 for (j = beginning_of_this_name; j < i; j++)\n>                         report_garbage(msg, list->items[j]);\n>         }\n>\n> For the above to work, prepare_packed_git_one() needs to retain only the\n> paths with known extensions in garbage list. \"pack-deadbeef.unk\" can and\n> should be reported as a garbage immediately when it is seen without being\n> placed in the list.\n\nYup. Looks good.\n\n>> +             } else if (has_extension(de->d_name, \".idx\")) {\n>> +                     struct string_list_item *item;\n>> +                     int n = strlen(path) - 4;\n>> +                     item = string_list_append_nodup(&garbage,\n>> +                                                     xstrndup(path, n));\n>> +                     item->util = \".idx\";\n>> +                     continue;\n>> +             } else\n>> +                     report_garbage(\"garbage found\", path, 0, NULL);\n>\n> Hmm, where is a \".keep\" file handled in this flow?\n\nApparently I smoked/drank while coding or something. .idx is supposed\nto be .keep. This calls for a test to guard my code (part of this v4).\n\n> The structure of the if/else cascade is much nicer than the earlier\n> iterations, but wouldn't it be even more clear to do this?\n>\n>         if (is .idx file) {\n>                 ... do that .idx thing ...\n>         }\n>\n>         if (!report_garbage)\n>                 continue; /* it does not matter what the file is */\n>\n>         if (is .pack) {\n>                 ... remember that we saw this .pack ...\n>         } else if (is .idx) {\n>                 ... remember that we saw this .idx ...\n>         } else if (is .keep) {\n>                 ... remember that we saw this .keep ...\n>         } else {\n>                 ... all else --- report as garbage immediately ...\n>         }\n\nDone. 2/4 is updated to make sure the \"if (is .idx file)\" block does\nnot shortcut the loop with \"continue;\" so that we always get .idx\nfile in the end of the loop.\n\nNguyễn Thái Ngọc Duy (4):\n  git-count-objects.txt: describe each line in -v output\n  sha1_file: reorder code in prepare_packed_git_one()\n  count-objects: report garbage files in pack directory too\n  count-objects: report how much disk space taken by garbage files\n\n Documentation/git-count-objects.txt |  22 ++++++--\n builtin/count-objects.c             |  30 ++++++++---\n cache.h                             |   3 ++\n sha1_file.c                         | 101 +++++++++++++++++++++++++++++++-----\n t/t5304-prune.sh                    |  26 ++++++++++\n 5 files changed, 156 insertions(+), 26 deletions(-)\n\n-- \n1.8.1.2.536.gf441e6d\n"},{"id":"209453","messageId":"1360746799-3668-2-git-send-email-pclouds@gmail.com","threadId":"32887","inReplyTo":"1360746799-3668-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v4 1/4] git-count-objects.txt: describe each line in -v output","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-02-13T09:13:16Z","receivedAt":"2013-02-13T09:13:16Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The current description requires a bit of guessing (what clause\ncorresponds to what printed line?) and lacks information, such as\nthe unit of size and size-pack.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Documentation/git-count-objects.txt | 20 +++++++++++++++-----\n 1 file changed, 15 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-count-objects.txt b/Documentation/git-count-objects.txt\nindex 23c80ce..e816823 100644\n--- a/Documentation/git-count-objects.txt\n+++ b/Documentation/git-count-objects.txt\n@@ -20,11 +20,21 @@ OPTIONS\n -------\n -v::\n --verbose::\n-\tIn addition to the number of loose objects and disk\n-\tspace consumed, it reports the number of in-pack\n-\tobjects, number of packs, disk space consumed by those packs,\n-\tand number of objects that can be removed by running\n-\t`git prune-packed`.\n+\tReport in more detail:\n++\n+count: the number of loose objects\n++\n+size: disk space consumed by loose objects, in KiB\n++\n+in-pack: the number of in-pack objects\n++\n+size-pack: disk space consumed by the packs, in KiB\n++\n+prune-packable: the number of loose objects that are also present in\n+the packs. These objects could be pruned using `git prune-packed`.\n++\n+garbage: the number of files in loose object database that are not\n+valid loose objects\n \n GIT\n ---\n-- \n1.8.1.2.536.gf441e6d\n"},{"id":"209454","messageId":"1360746799-3668-3-git-send-email-pclouds@gmail.com","threadId":"32887","inReplyTo":"1360746799-3668-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v4 2/4] sha1_file: reorder code in prepare_packed_git_one()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-02-13T09:13:17Z","receivedAt":"2013-02-13T09:13:17Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The current loop does\n\n\twhile (...) {\n\t\tif (!not .idx file)\n\t\t\tcontinue;\n\t\tprocess .idx file;\n\t}\n\nand is reordered to\n\n\twhile (...) {\n\t\tif (!.idx file) {\n\t\t\tprocess .idx file;\n\t\t}\n\t}\n\nThis makes it easier to add new extension file processing.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n sha1_file.c | 30 ++++++++++++++----------------\n 1 file changed, 14 insertions(+), 16 deletions(-)\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 40b2329..239bee7 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -1024,27 +1024,25 @@ static void prepare_packed_git_one(char *objdir, int local)\n \t\tint namelen = strlen(de->d_name);\n \t\tstruct packed_git *p;\n \n-\t\tif (!has_extension(de->d_name, \".idx\"))\n-\t\t\tcontinue;\n-\n \t\tif (len + namelen + 1 > sizeof(path))\n \t\t\tcontinue;\n \n-\t\t/* Don't reopen a pack we already have. */\n \t\tstrcpy(path + len, de->d_name);\n-\t\tfor (p = packed_git; p; p = p->next) {\n-\t\t\tif (!memcmp(path, p->pack_name, len + namelen - 4))\n-\t\t\t\tbreak;\n+\n+\t\tif (has_extension(de->d_name, \".idx\")) {\n+\t\t\t/* Don't reopen a pack we already have. */\n+\t\t\tfor (p = packed_git; p; p = p->next) {\n+\t\t\t\tif (!memcmp(path, p->pack_name, len + namelen - 4))\n+\t\t\t\t\tbreak;\n+\t\t\t}\n+\t\t\tif (p == NULL &&\n+\t\t\t    /*\n+\t\t\t     * See if it really is a valid .idx file with\n+\t\t\t     * corresponding .pack file that we can map.\n+\t\t\t     */\n+\t\t\t    (p = add_packed_git(path, len + namelen, local)) != NULL)\n+\t\t\t\tinstall_packed_git(p);\n \t\t}\n-\t\tif (p)\n-\t\t\tcontinue;\n-\t\t/* See if it really is a valid .idx file with corresponding\n-\t\t * .pack file that we can map.\n-\t\t */\n-\t\tp = add_packed_git(path, len + namelen, local);\n-\t\tif (!p)\n-\t\t\tcontinue;\n-\t\tinstall_packed_git(p);\n \t}\n \tclosedir(dir);\n }\n-- \n1.8.1.2.536.gf441e6d\n"},{"id":"209455","messageId":"1360746799-3668-4-git-send-email-pclouds@gmail.com","threadId":"32887","inReplyTo":"1360746799-3668-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v4 3/4] count-objects: report garbage files in pack directory too","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-02-13T09:13:18Z","receivedAt":"2013-02-13T09:13:18Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"prepare_packed_git_one() is modified to allow count-objects to hook a\nreport function to so we don't need to duplicate the pack searching\nlogic in count-objects.c. When report_pack_garbage is NULL, the\noverhead is insignificant.\n\nThe garbage is reported with warning() instead of error() in packed\ngarbage case because it's not an error to have garbage. Loose garbage\nis still reported as errors and will be converted to warnings later.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Documentation/git-count-objects.txt |  4 +-\n builtin/count-objects.c             | 12 +++++-\n cache.h                             |  3 ++\n sha1_file.c                         | 77 ++++++++++++++++++++++++++++++++++++-\n t/t5304-prune.sh                    | 26 +++++++++++++\n 5 files changed, 118 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-count-objects.txt b/Documentation/git-count-objects.txt\nindex e816823..1611d7c 100644\n--- a/Documentation/git-count-objects.txt\n+++ b/Documentation/git-count-objects.txt\n@@ -33,8 +33,8 @@ size-pack: disk space consumed by the packs, in KiB\n prune-packable: the number of loose objects that are also present in\n the packs. These objects could be pruned using `git prune-packed`.\n +\n-garbage: the number of files in loose object database that are not\n-valid loose objects\n+garbage: the number of files in object database that are not valid\n+loose objects nor valid packs\n \n GIT\n ---\ndiff --git a/builtin/count-objects.c b/builtin/count-objects.c\nindex 9afaa88..1706c8b 100644\n--- a/builtin/count-objects.c\n+++ b/builtin/count-objects.c\n@@ -9,6 +9,14 @@\n #include \"builtin.h\"\n #include \"parse-options.h\"\n \n+static unsigned long garbage;\n+\n+static void real_report_garbage(const char *desc, const char *path)\n+{\n+\twarning(\"%s: %s\", desc, path);\n+\tgarbage++;\n+}\n+\n static void count_objects(DIR *d, char *path, int len, int verbose,\n \t\t\t  unsigned long *loose,\n \t\t\t  off_t *loose_size,\n@@ -76,7 +84,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)\n \tconst char *objdir = get_object_directory();\n \tint len = strlen(objdir);\n \tchar *path = xmalloc(len + 50);\n-\tunsigned long loose = 0, packed = 0, packed_loose = 0, garbage = 0;\n+\tunsigned long loose = 0, packed = 0, packed_loose = 0;\n \toff_t loose_size = 0;\n \tstruct option opts[] = {\n \t\tOPT__VERBOSE(&verbose, N_(\"be verbose\")),\n@@ -87,6 +95,8 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)\n \t/* we do not take arguments other than flags for now */\n \tif (argc)\n \t\tusage_with_options(count_objects_usage, opts);\n+\tif (verbose)\n+\t\treport_garbage = real_report_garbage;\n \tmemcpy(path, objdir, len);\n \tif (len && objdir[len-1] != '/')\n \t\tpath[len++] = '/';\ndiff --git a/cache.h b/cache.h\nindex 7339f21..73de68c 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1051,6 +1051,9 @@ extern const char *parse_feature_value(const char *feature_list, const char *fea\n \n extern struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_path);\n \n+/* A hook for count-objects to report invalid files in pack directory */\n+extern void (*report_garbage)(const char *desc, const char *path);\n+\n extern void prepare_packed_git(void);\n extern void reprepare_packed_git(void);\n extern void install_packed_git(struct packed_git *pack);\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 239bee7..5bedf78 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -21,6 +21,7 @@\n #include \"sha1-lookup.h\"\n #include \"bulk-checkin.h\"\n #include \"streaming.h\"\n+#include \"dir.h\"\n \n #ifndef O_NOATIME\n #if defined(__linux__) && (defined(__i386__) || defined(__PPC__))\n@@ -1000,6 +1001,57 @@ void install_packed_git(struct packed_git *pack)\n \tpacked_git = pack;\n }\n \n+void (*report_garbage)(const char *desc, const char *path);\n+\n+static void report_helper(const struct string_list *list,\n+\t\t\t  int seen_bits, int first, int last)\n+{\n+\tconst char *msg;\n+\tswitch (seen_bits) {\n+\tcase 0: msg = \"no corresponding .idx nor .pack\"; break;\n+\tcase 1: msg = \"no corresponding .idx\"; break;\n+\tcase 2: msg = \"no corresponding .pack\"; break;\n+\tdefault:\n+\t\treturn;\n+\t}\n+\tfor (; first <= last; first++)\n+\t\treport_garbage(msg, list->items[first].string);\n+}\n+\n+static void report_pack_garbage(struct string_list *list)\n+{\n+\tint i, baselen = -1, first = 0, seen_bits = 0;\n+\n+\tif (!report_garbage)\n+\t\treturn;\n+\n+\tsort_string_list(list);\n+\n+\tfor (i = 0; i < list->nr; i++) {\n+\t\tconst char *path = list->items[i].string;\n+\t\tif (baselen != -1 &&\n+\t\t    strncmp(path, list->items[first].string, baselen)) {\n+\t\t\treport_helper(list, seen_bits, first, i - 1);\n+\t\t\tbaselen = -1;\n+\t\t\tseen_bits = 0;\n+\t\t}\n+\t\tif (baselen == -1) {\n+\t\t\tconst char *dot = strrchr(path, '.');\n+\t\t\tif (!dot) {\n+\t\t\t\treport_garbage(\"garbage found\", path);\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t\tbaselen = dot - path + 1;\n+\t\t\tfirst = i;\n+\t\t}\n+\t\tif (!strcmp(path + baselen, \"pack\"))\n+\t\t\tseen_bits |= 1;\n+\t\telse if (!strcmp(path + baselen, \"idx\"))\n+\t\t\tseen_bits |= 2;\n+\t}\n+\treport_helper(list, seen_bits, first, list->nr - 1);\n+}\n+\n static void prepare_packed_git_one(char *objdir, int local)\n {\n \t/* Ensure that this buffer is large enough so that we can\n@@ -1009,6 +1061,7 @@ static void prepare_packed_git_one(char *objdir, int local)\n \tint len;\n \tDIR *dir;\n \tstruct dirent *de;\n+\tstruct string_list garbage = STRING_LIST_INIT_DUP;\n \n \tsprintf(path, \"%s/pack\", objdir);\n \tlen = strlen(path);\n@@ -1024,7 +1077,17 @@ static void prepare_packed_git_one(char *objdir, int local)\n \t\tint namelen = strlen(de->d_name);\n \t\tstruct packed_git *p;\n \n-\t\tif (len + namelen + 1 > sizeof(path))\n+\t\tif (len + namelen + 1 > sizeof(path)) {\n+\t\t\tif (report_garbage) {\n+\t\t\t\tstruct strbuf sb = STRBUF_INIT;\n+\t\t\t\tstrbuf_addf(&sb, \"%.*s/%s\", len - 1, path, de->d_name);\n+\t\t\t\treport_garbage(\"path too long\", sb.buf);\n+\t\t\t\tstrbuf_release(&sb);\n+\t\t\t}\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tif (is_dot_or_dotdot(de->d_name))\n \t\t\tcontinue;\n \n \t\tstrcpy(path + len, de->d_name);\n@@ -1043,8 +1106,20 @@ static void prepare_packed_git_one(char *objdir, int local)\n \t\t\t    (p = add_packed_git(path, len + namelen, local)) != NULL)\n \t\t\t\tinstall_packed_git(p);\n \t\t}\n+\n+\t\tif (!report_garbage)\n+\t\t\tcontinue;\n+\n+\t\tif (has_extension(de->d_name, \".idx\") ||\n+\t\t    has_extension(de->d_name, \".pack\") ||\n+\t\t    has_extension(de->d_name, \".keep\"))\n+\t\t\tstring_list_append(&garbage, path);\n+\t\telse\n+\t\t\treport_garbage(\"garbage found\", path);\n \t}\n \tclosedir(dir);\n+\treport_pack_garbage(&garbage);\n+\tstring_list_clear(&garbage, 0);\n }\n \n static int sort_pack(const void *a_, const void *b_)\ndiff --git a/t/t5304-prune.sh b/t/t5304-prune.sh\nindex d645328..e4bb3a1 100755\n--- a/t/t5304-prune.sh\n+++ b/t/t5304-prune.sh\n@@ -195,4 +195,30 @@ test_expect_success 'gc: prune old objects after local clone' '\n \t)\n '\n \n+test_expect_success 'garbage report in count-objects -v' '\n+\t: >.git/objects/pack/foo &&\n+\t: >.git/objects/pack/foo.bar &&\n+\t: >.git/objects/pack/foo.keep &&\n+\t: >.git/objects/pack/foo.pack &&\n+\t: >.git/objects/pack/fake.bar &&\n+\t: >.git/objects/pack/fake.keep &&\n+\t: >.git/objects/pack/fake.pack &&\n+\t: >.git/objects/pack/fake.idx &&\n+\t: >.git/objects/pack/fake2.keep &&\n+\t: >.git/objects/pack/fake3.idx &&\n+\tgit count-objects -v 2>stderr &&\n+\tgrep \"index file .git/objects/pack/fake.idx is too small\" stderr &&\n+\tgrep \"^warning:\" stderr | sort >actual &&\n+\tcat >expected <<\\EOF &&\n+warning: garbage found: .git/objects/pack/fake.bar\n+warning: garbage found: .git/objects/pack/foo\n+warning: garbage found: .git/objects/pack/foo.bar\n+warning: no corresponding .idx nor .pack: .git/objects/pack/fake2.keep\n+warning: no corresponding .idx: .git/objects/pack/foo.keep\n+warning: no corresponding .idx: .git/objects/pack/foo.pack\n+warning: no corresponding .pack: .git/objects/pack/fake3.idx\n+EOF\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n1.8.1.2.536.gf441e6d\n"},{"id":"209456","messageId":"1360746799-3668-5-git-send-email-pclouds@gmail.com","threadId":"32887","inReplyTo":"1360746799-3668-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v4 4/4] count-objects: report how much disk space taken by garbage files","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-02-13T09:13:19Z","receivedAt":"2013-02-13T09:13:19Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Also issue warnings on loose garbages instead of errors as a result of\nusing report_garbage() function in count_objects()\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Documentation/git-count-objects.txt |  2 ++\n builtin/count-objects.c             | 18 ++++++++++++------\n 2 files changed, 14 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-count-objects.txt b/Documentation/git-count-objects.txt\nindex 1611d7c..da6e72e 100644\n--- a/Documentation/git-count-objects.txt\n+++ b/Documentation/git-count-objects.txt\n@@ -35,6 +35,8 @@ the packs. These objects could be pruned using `git prune-packed`.\n +\n garbage: the number of files in object database that are not valid\n loose objects nor valid packs\n++\n+size-garbage: disk space consumed by garbage files, in KiB\n \n GIT\n ---\ndiff --git a/builtin/count-objects.c b/builtin/count-objects.c\nindex 1706c8b..3a01a8d 100644\n--- a/builtin/count-objects.c\n+++ b/builtin/count-objects.c\n@@ -10,9 +10,13 @@\n #include \"parse-options.h\"\n \n static unsigned long garbage;\n+static off_t size_garbage;\n \n static void real_report_garbage(const char *desc, const char *path)\n {\n+\tstruct stat st;\n+\tif (!stat(path, &st))\n+\t\tsize_garbage += st.st_size;\n \twarning(\"%s: %s\", desc, path);\n \tgarbage++;\n }\n@@ -20,8 +24,7 @@ static void real_report_garbage(const char *desc, const char *path)\n static void count_objects(DIR *d, char *path, int len, int verbose,\n \t\t\t  unsigned long *loose,\n \t\t\t  off_t *loose_size,\n-\t\t\t  unsigned long *packed_loose,\n-\t\t\t  unsigned long *garbage)\n+\t\t\t  unsigned long *packed_loose)\n {\n \tstruct dirent *ent;\n \twhile ((ent = readdir(d)) != NULL) {\n@@ -54,9 +57,11 @@ static void count_objects(DIR *d, char *path, int len, int verbose,\n \t\t}\n \t\tif (bad) {\n \t\t\tif (verbose) {\n-\t\t\t\terror(\"garbage found: %.*s/%s\",\n-\t\t\t\t      len + 2, path, ent->d_name);\n-\t\t\t\t(*garbage)++;\n+\t\t\t\tstruct strbuf sb = STRBUF_INIT;\n+\t\t\t\tstrbuf_addf(&sb, \"%.*s/%s\",\n+\t\t\t\t\t    len + 2, path, ent->d_name);\n+\t\t\t\treport_garbage(\"garbage found\", sb.buf);\n+\t\t\t\tstrbuf_release(&sb);\n \t\t\t}\n \t\t\tcontinue;\n \t\t}\n@@ -107,7 +112,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)\n \t\tif (!d)\n \t\t\tcontinue;\n \t\tcount_objects(d, path, len, verbose,\n-\t\t\t      &loose, &loose_size, &packed_loose, &garbage);\n+\t\t\t      &loose, &loose_size, &packed_loose);\n \t\tclosedir(d);\n \t}\n \tif (verbose) {\n@@ -132,6 +137,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)\n \t\tprintf(\"size-pack: %lu\\n\", (unsigned long) (size_pack / 1024));\n \t\tprintf(\"prune-packable: %lu\\n\", packed_loose);\n \t\tprintf(\"garbage: %lu\\n\", garbage);\n+\t\tprintf(\"size-garbage: %lu\\n\", (unsigned long) (size_garbage / 1024));\n \t}\n \telse\n \t\tprintf(\"%lu objects, %lu kilobytes\\n\",\n-- \n1.8.1.2.536.gf441e6d\n"},{"id":"209474","messageId":"7vehgkb43v.fsf@alter.siamese.dyndns.org","threadId":"32887","inReplyTo":"1360746799-3668-4-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v4 3/4] count-objects: report garbage files in pack directory too","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-13T15:55:32Z","receivedAt":"2013-02-13T15:55:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> prepare_packed_git_one() is modified to allow count-objects to hook a\n> report function to so we don't need to duplicate the pack searching\n> logic in count-objects.c. When report_pack_garbage is NULL, the\n> overhead is insignificant.\n>\n> The garbage is reported with warning() instead of error() in packed\n> garbage case because it's not an error to have garbage. Loose garbage\n> is still reported as errors and will be converted to warnings later.\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n> ---\n\nThanks.\n\nTests look good and the series is getting much closer.\n\n> diff --git a/sha1_file.c b/sha1_file.c\n> index 239bee7..5bedf78 100644\n> --- a/sha1_file.c\n> +++ b/sha1_file.c\n> @@ -21,6 +21,7 @@\n>  #include \"sha1-lookup.h\"\n>  #include \"bulk-checkin.h\"\n>  #include \"streaming.h\"\n> +#include \"dir.h\"\n>  \n>  #ifndef O_NOATIME\n>  #if defined(__linux__) && (defined(__i386__) || defined(__PPC__))\n> @@ -1000,6 +1001,57 @@ void install_packed_git(struct packed_git *pack)\n>  \tpacked_git = pack;\n>  }\n>  \n> +void (*report_garbage)(const char *desc, const char *path);\n> +\n> +static void report_helper(const struct string_list *list,\n> +\t\t\t  int seen_bits, int first, int last)\n> +{\n> +\tconst char *msg;\n> +\tswitch (seen_bits) {\n> +\tcase 0: msg = \"no corresponding .idx nor .pack\"; break;\n> +\tcase 1: msg = \"no corresponding .idx\"; break;\n> +\tcase 2: msg = \"no corresponding .pack\"; break;\n\nThat's dense.\n\n> +\tdefault:\n> +\t\treturn;\n> +\t}\n> +\tfor (; first <= last; first++)\n\nThis looks odd.  If you use the usual last+1 convention between the\ncaller and callee, you do not have to do this, or call this function\nwith \"i - 1\" and \"list->nr -1\" as the last parameter.\n\n> +static void report_pack_garbage(struct string_list *list)\n> +{\n> +\tint i, baselen = -1, first = 0, seen_bits = 0;\n> +\n> +\tif (!report_garbage)\n> +\t\treturn;\n> +\n> +\tsort_string_list(list);\n> +\n> +\tfor (i = 0; i < list->nr; i++) {\n> +\t\tconst char *path = list->items[i].string;\n> +\t\tif (baselen != -1 &&\n> +\t\t    strncmp(path, list->items[first].string, baselen)) {\n> +\t\t\treport_helper(list, seen_bits, first, i - 1);\n> +\t\t\tbaselen = -1;\n> +\t\t\tseen_bits = 0;\n> +\t\t}\n> +\t\tif (baselen == -1) {\n> +\t\t\tconst char *dot = strrchr(path, '.');\n> +\t\t\tif (!dot) {\n> +\t\t\t\treport_garbage(\"garbage found\", path);\n> +\t\t\t\tcontinue;\n> +\t\t\t}\n> +\t\t\tbaselen = dot - path + 1;\n> +\t\t\tfirst = i;\n> +\t\t}\n> +\t\tif (!strcmp(path + baselen, \"pack\"))\n> +\t\t\tseen_bits |= 1;\n> +\t\telse if (!strcmp(path + baselen, \"idx\"))\n> +\t\t\tseen_bits |= 2;\n> +\t}\n> +\treport_helper(list, seen_bits, first, list->nr - 1);\n> +}\n\n> @@ -1009,6 +1061,7 @@ static void prepare_packed_git_one(char *objdir, int local)\n>  \tint len;\n>  \tDIR *dir;\n>  \tstruct dirent *de;\n> +\tstruct string_list garbage = STRING_LIST_INIT_DUP;\n>  \n>  \tsprintf(path, \"%s/pack\", objdir);\n>  \tlen = strlen(path);\n> ...\n> @@ -1043,8 +1106,20 @@ static void prepare_packed_git_one(char *objdir, int local)\n>  \t\t\t    (p = add_packed_git(path, len + namelen, local)) != NULL)\n>  \t\t\t\tinstall_packed_git(p);\n>  \t\t}\n> +\n> +\t\tif (!report_garbage)\n> +\t\t\tcontinue;\n> +\n> +\t\tif (has_extension(de->d_name, \".idx\") ||\n> +\t\t    has_extension(de->d_name, \".pack\") ||\n> +\t\t    has_extension(de->d_name, \".keep\"))\n> +\t\t\tstring_list_append(&garbage, path);\n\nIt might be OK to put .pack and .keep in the same \"if (A || B)\" as\nit may happen to be that they do not need any special treatment\nright now, but I do not think this is a good idea in general.\n\nYou would want to do things differently for \".idx\", e.g.\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 5bedf78..450521f 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -1076,6 +1076,7 @@ static void prepare_packed_git_one(char *objdir, int local)\n \twhile ((de = readdir(dir)) != NULL) {\n \t\tint namelen = strlen(de->d_name);\n \t\tstruct packed_git *p;\n+\t\tint is_a_bad_idx = 0;\n \n \t\tif (len + namelen + 1 > sizeof(path)) {\n \t\t\tif (report_garbage) {\n@@ -1105,12 +1106,14 @@ static void prepare_packed_git_one(char *objdir, int local)\n \t\t\t     */\n \t\t\t    (p = add_packed_git(path, len + namelen, local)) != NULL)\n \t\t\t\tinstall_packed_git(p);\n+\t\t\telse\n+\t\t\t\tis_a_bad_idx = 1;\n \t\t}\n \n \t\tif (!report_garbage)\n \t\t\tcontinue;\n \n-\t\tif (has_extension(de->d_name, \".idx\") ||\n+\t\tif ((has_extension(de->d_name, \".idx\") && !is_a_bad_idx) ||\n \t\t    has_extension(de->d_name, \".pack\") ||\n \t\t    has_extension(de->d_name, \".keep\"))\n \t\t\tstring_list_append(&garbage, path);\n\n\nso that you can say something about .pack/.keep files that do not\nhave a working .idx file.  In the above example, the only special\nthing you would do for .idx is just to check if it is a bad one, but\nin later patches you may have to do different things in the body\n(i.e. something else in addition to string_list_append(&garbage))\nnot just in the condition.  Collapsing these into a condition to a\nsingle \"if (A||B||C)\" may be suffering from a lack of foresight.\n\n> diff --git a/t/t5304-prune.sh b/t/t5304-prune.sh\n> index d645328..e4bb3a1 100755\n> --- a/t/t5304-prune.sh\n> +++ b/t/t5304-prune.sh\n> @@ -195,4 +195,30 @@ test_expect_success 'gc: prune old objects after local clone' '\n>  \t)\n>  '\n>  \n> +test_expect_success 'garbage report in count-objects -v' '\n> +\t: >.git/objects/pack/foo &&\n> +\t: >.git/objects/pack/foo.bar &&\n> +\t: >.git/objects/pack/foo.keep &&\n> +\t: >.git/objects/pack/foo.pack &&\n> +\t: >.git/objects/pack/fake.bar &&\n> +\t: >.git/objects/pack/fake.keep &&\n> +\t: >.git/objects/pack/fake.pack &&\n> +\t: >.git/objects/pack/fake.idx &&\n> +\t: >.git/objects/pack/fake2.keep &&\n> +\t: >.git/objects/pack/fake3.idx &&\n> +\tgit count-objects -v 2>stderr &&\n> +\tgrep \"index file .git/objects/pack/fake.idx is too small\" stderr &&\n\nThe above suggested change will make a difference to\nfake.{pack,keep} because of this breakage, I think.\n\n> +\tgrep \"^warning:\" stderr | sort >actual &&\n> +\tcat >expected <<\\EOF &&\n> +warning: garbage found: .git/objects/pack/fake.bar\n> +warning: garbage found: .git/objects/pack/foo\n> +warning: garbage found: .git/objects/pack/foo.bar\n> +warning: no corresponding .idx nor .pack: .git/objects/pack/fake2.keep\n> +warning: no corresponding .idx: .git/objects/pack/foo.keep\n> +warning: no corresponding .idx: .git/objects/pack/foo.pack\n> +warning: no corresponding .pack: .git/objects/pack/fake3.idx\n> +EOF\n> +\ttest_cmp expected actual\n> +'\n> +\n>  test_done\n"},{"id":"209481","messageId":"7vpq049msu.fsf@alter.siamese.dyndns.org","threadId":"32887","inReplyTo":"7vehgkb43v.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v4 3/4] count-objects: report garbage files in pack directory too","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-13T16:54:41Z","receivedAt":"2013-02-13T16:54:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n>> +\t\tif (has_extension(de->d_name, \".idx\") ||\n>> +\t\t    has_extension(de->d_name, \".pack\") ||\n>> +\t\t    has_extension(de->d_name, \".keep\"))\n>> +\t\t\tstring_list_append(&garbage, path);\n>\n> It might be OK to put .pack and .keep in the same \"if (A || B)\" as\n> it may happen to be that they do not need any special treatment\n> right now, but I do not think this is a good idea in general.\n\nActually I take this part back.  I can see that the condition part\ngrow over time but I do not think the body should.  That is the\nwhole point of collecting paths that cannot be judged as garbage by\nthemselves into a list; we shouldn't be doing anything else by\ndefinition in the body.\n\nEverything else I said in the review still stands, though..\n\nThanks.\n"},{"id":"209510","messageId":"CACsJy8DxRuML1+gSCZaGC1hA4MU9sWnoGJGZMbeUVdaipOH7vA@mail.gmail.com","threadId":"32887","inReplyTo":"7vehgkb43v.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v4 3/4] count-objects: report garbage files in pack directory too","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-02-14T09:24:11Z","receivedAt":"2013-02-14T09:24:11Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Feb 13, 2013 at 10:55 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> +     default:\n>> +             return;\n>> +     }\n>> +     for (; first <= last; first++)\n>\n> This looks odd.  If you use the usual last+1 convention between the\n> caller and callee, you do not have to do this, or call this function\n> with \"i - 1\" and \"list->nr -1\" as the last parameter.\n\nI know. I just don't know how to name the variable to say \"the element\nafter the last one\".\n-- \nDuy\n"},{"id":"209531","messageId":"7v8v6q3k32.fsf@alter.siamese.dyndns.org","threadId":"32887","inReplyTo":"CACsJy8DxRuML1+gSCZaGC1hA4MU9sWnoGJGZMbeUVdaipOH7vA@mail.gmail.com","subject":"Re: [PATCH v4 3/4] count-objects: report garbage files in pack directory too","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-14T17:02:09Z","receivedAt":"2013-02-14T17:02:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen <pclouds@gmail.com> writes:\n\n> On Wed, Feb 13, 2013 at 10:55 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>>> +     default:\n>>> +             return;\n>>> +     }\n>>> +     for (; first <= last; first++)\n>>\n>> This looks odd.  If you use the usual last+1 convention between the\n>> caller and callee, you do not have to do this, or call this function\n>> with \"i - 1\" and \"list->nr -1\" as the last parameter.\n>\n> I know. I just don't know how to name the variable to say \"the element\n> after the last one\".\n\nIn case it was unclear, by the \"usual last+1 convention\", I meant\nthat it is perfectly normal to write\n\n\tfor (i = first; i < last; i++)\n\tfor (i = begin; i < end; i++)\n\nin C.  Saying these as\n\n\tfor (i = first; i < beyond_last; i++)\n\tfor (i = begin; i < beyond_end; i++)\n\nlook non-C.\n"},{"id":"209551","messageId":"1360930030-21211-1-git-send-email-pclouds@gmail.com","threadId":"32887","inReplyTo":"7vehgkb43v.fsf@alter.siamese.dyndns.org","subject":"[PATCH v4+ 3/4] count-objects: report garbage files in pack directory too","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-02-15T12:07:10Z","receivedAt":"2013-02-15T12:07:10Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"prepare_packed_git_one() is modified to allow count-objects to hook a\nreport function to so we don't need to duplicate the pack searching\nlogic in count-objects.c. When report_pack_garbage is NULL, the\noverhead is insignificant.\n\nThe garbage is reported with warning() instead of error() in packed\ngarbage case because it's not an error to have garbage. Loose garbage\nis still reported as errors and will be converted to warnings later.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Documentation/git-count-objects.txt |  4 +-\n builtin/count-objects.c             | 12 +++++-\n cache.h                             |  3 ++\n sha1_file.c                         | 83 ++++++++++++++++++++++++++++++++++++-\n t/t5304-prune.sh                    | 26 ++++++++++++\n 5 files changed, 124 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-count-objects.txt b/Documentation/git-count-objects.txt\nindex e816823..1611d7c 100644\n--- a/Documentation/git-count-objects.txt\n+++ b/Documentation/git-count-objects.txt\n@@ -33,8 +33,8 @@ size-pack: disk space consumed by the packs, in KiB\n prune-packable: the number of loose objects that are also present in\n the packs. These objects could be pruned using `git prune-packed`.\n +\n-garbage: the number of files in loose object database that are not\n-valid loose objects\n+garbage: the number of files in object database that are not valid\n+loose objects nor valid packs\n \n GIT\n ---\ndiff --git a/builtin/count-objects.c b/builtin/count-objects.c\nindex 9afaa88..1706c8b 100644\n--- a/builtin/count-objects.c\n+++ b/builtin/count-objects.c\n@@ -9,6 +9,14 @@\n #include \"builtin.h\"\n #include \"parse-options.h\"\n \n+static unsigned long garbage;\n+\n+static void real_report_garbage(const char *desc, const char *path)\n+{\n+\twarning(\"%s: %s\", desc, path);\n+\tgarbage++;\n+}\n+\n static void count_objects(DIR *d, char *path, int len, int verbose,\n \t\t\t  unsigned long *loose,\n \t\t\t  off_t *loose_size,\n@@ -76,7 +84,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)\n \tconst char *objdir = get_object_directory();\n \tint len = strlen(objdir);\n \tchar *path = xmalloc(len + 50);\n-\tunsigned long loose = 0, packed = 0, packed_loose = 0, garbage = 0;\n+\tunsigned long loose = 0, packed = 0, packed_loose = 0;\n \toff_t loose_size = 0;\n \tstruct option opts[] = {\n \t\tOPT__VERBOSE(&verbose, N_(\"be verbose\")),\n@@ -87,6 +95,8 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)\n \t/* we do not take arguments other than flags for now */\n \tif (argc)\n \t\tusage_with_options(count_objects_usage, opts);\n+\tif (verbose)\n+\t\treport_garbage = real_report_garbage;\n \tmemcpy(path, objdir, len);\n \tif (len && objdir[len-1] != '/')\n \t\tpath[len++] = '/';\ndiff --git a/cache.h b/cache.h\nindex 7339f21..73de68c 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1051,6 +1051,9 @@ extern const char *parse_feature_value(const char *feature_list, const char *fea\n \n extern struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_path);\n \n+/* A hook for count-objects to report invalid files in pack directory */\n+extern void (*report_garbage)(const char *desc, const char *path);\n+\n extern void prepare_packed_git(void);\n extern void reprepare_packed_git(void);\n extern void install_packed_git(struct packed_git *pack);\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 239bee7..16967d3 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -21,6 +21,7 @@\n #include \"sha1-lookup.h\"\n #include \"bulk-checkin.h\"\n #include \"streaming.h\"\n+#include \"dir.h\"\n \n #ifndef O_NOATIME\n #if defined(__linux__) && (defined(__i386__) || defined(__PPC__))\n@@ -1000,6 +1001,63 @@ void install_packed_git(struct packed_git *pack)\n \tpacked_git = pack;\n }\n \n+void (*report_garbage)(const char *desc, const char *path);\n+\n+static void report_helper(const struct string_list *list,\n+\t\t\t  int seen_bits, int first, int last)\n+{\n+\tconst char *msg;\n+\tswitch (seen_bits) {\n+\tcase 0:\n+\t\tmsg = \"no corresponding .idx nor .pack\";\n+\t\tbreak;\n+\tcase 1:\n+\t\tmsg = \"no corresponding .idx\";\n+\t\tbreak;\n+\tcase 2:\n+\t\tmsg = \"no corresponding .pack\";\n+\t\tbreak;\n+\tdefault:\n+\t\treturn;\n+\t}\n+\tfor (; first < last; first++)\n+\t\treport_garbage(msg, list->items[first].string);\n+}\n+\n+static void report_pack_garbage(struct string_list *list)\n+{\n+\tint i, baselen = -1, first = 0, seen_bits = 0;\n+\n+\tif (!report_garbage)\n+\t\treturn;\n+\n+\tsort_string_list(list);\n+\n+\tfor (i = 0; i < list->nr; i++) {\n+\t\tconst char *path = list->items[i].string;\n+\t\tif (baselen != -1 &&\n+\t\t    strncmp(path, list->items[first].string, baselen)) {\n+\t\t\treport_helper(list, seen_bits, first, i);\n+\t\t\tbaselen = -1;\n+\t\t\tseen_bits = 0;\n+\t\t}\n+\t\tif (baselen == -1) {\n+\t\t\tconst char *dot = strrchr(path, '.');\n+\t\t\tif (!dot) {\n+\t\t\t\treport_garbage(\"garbage found\", path);\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t\tbaselen = dot - path + 1;\n+\t\t\tfirst = i;\n+\t\t}\n+\t\tif (!strcmp(path + baselen, \"pack\"))\n+\t\t\tseen_bits |= 1;\n+\t\telse if (!strcmp(path + baselen, \"idx\"))\n+\t\t\tseen_bits |= 2;\n+\t}\n+\treport_helper(list, seen_bits, first, list->nr);\n+}\n+\n static void prepare_packed_git_one(char *objdir, int local)\n {\n \t/* Ensure that this buffer is large enough so that we can\n@@ -1009,6 +1067,7 @@ static void prepare_packed_git_one(char *objdir, int local)\n \tint len;\n \tDIR *dir;\n \tstruct dirent *de;\n+\tstruct string_list garbage = STRING_LIST_INIT_DUP;\n \n \tsprintf(path, \"%s/pack\", objdir);\n \tlen = strlen(path);\n@@ -1024,7 +1083,17 @@ static void prepare_packed_git_one(char *objdir, int local)\n \t\tint namelen = strlen(de->d_name);\n \t\tstruct packed_git *p;\n \n-\t\tif (len + namelen + 1 > sizeof(path))\n+\t\tif (len + namelen + 1 > sizeof(path)) {\n+\t\t\tif (report_garbage) {\n+\t\t\t\tstruct strbuf sb = STRBUF_INIT;\n+\t\t\t\tstrbuf_addf(&sb, \"%.*s/%s\", len - 1, path, de->d_name);\n+\t\t\t\treport_garbage(\"path too long\", sb.buf);\n+\t\t\t\tstrbuf_release(&sb);\n+\t\t\t}\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tif (is_dot_or_dotdot(de->d_name))\n \t\t\tcontinue;\n \n \t\tstrcpy(path + len, de->d_name);\n@@ -1043,8 +1112,20 @@ static void prepare_packed_git_one(char *objdir, int local)\n \t\t\t    (p = add_packed_git(path, len + namelen, local)) != NULL)\n \t\t\t\tinstall_packed_git(p);\n \t\t}\n+\n+\t\tif (!report_garbage)\n+\t\t\tcontinue;\n+\n+\t\tif (has_extension(de->d_name, \".idx\") ||\n+\t\t    has_extension(de->d_name, \".pack\") ||\n+\t\t    has_extension(de->d_name, \".keep\"))\n+\t\t\tstring_list_append(&garbage, path);\n+\t\telse\n+\t\t\treport_garbage(\"garbage found\", path);\n \t}\n \tclosedir(dir);\n+\treport_pack_garbage(&garbage);\n+\tstring_list_clear(&garbage, 0);\n }\n \n static int sort_pack(const void *a_, const void *b_)\ndiff --git a/t/t5304-prune.sh b/t/t5304-prune.sh\nindex d645328..e4bb3a1 100755\n--- a/t/t5304-prune.sh\n+++ b/t/t5304-prune.sh\n@@ -195,4 +195,30 @@ test_expect_success 'gc: prune old objects after local clone' '\n \t)\n '\n \n+test_expect_success 'garbage report in count-objects -v' '\n+\t: >.git/objects/pack/foo &&\n+\t: >.git/objects/pack/foo.bar &&\n+\t: >.git/objects/pack/foo.keep &&\n+\t: >.git/objects/pack/foo.pack &&\n+\t: >.git/objects/pack/fake.bar &&\n+\t: >.git/objects/pack/fake.keep &&\n+\t: >.git/objects/pack/fake.pack &&\n+\t: >.git/objects/pack/fake.idx &&\n+\t: >.git/objects/pack/fake2.keep &&\n+\t: >.git/objects/pack/fake3.idx &&\n+\tgit count-objects -v 2>stderr &&\n+\tgrep \"index file .git/objects/pack/fake.idx is too small\" stderr &&\n+\tgrep \"^warning:\" stderr | sort >actual &&\n+\tcat >expected <<\\EOF &&\n+warning: garbage found: .git/objects/pack/fake.bar\n+warning: garbage found: .git/objects/pack/foo\n+warning: garbage found: .git/objects/pack/foo.bar\n+warning: no corresponding .idx nor .pack: .git/objects/pack/fake2.keep\n+warning: no corresponding .idx: .git/objects/pack/foo.keep\n+warning: no corresponding .idx: .git/objects/pack/foo.pack\n+warning: no corresponding .pack: .git/objects/pack/fake3.idx\n+EOF\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n1.8.1.2.536.gf441e6d\n"},{"id":"209570","messageId":"7vliapf9kc.fsf@alter.siamese.dyndns.org","threadId":"32887","inReplyTo":"1360930030-21211-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v4+ 3/4] count-objects: report garbage files in pack directory too","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-15T23:20:51Z","receivedAt":"2013-02-15T23:20:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> prepare_packed_git_one() is modified to allow count-objects to hook a\n> report function to so we don't need to duplicate the pack searching\n> logic in count-objects.c. When report_pack_garbage is NULL, the\n> overhead is insignificant.\n>\n> The garbage is reported with warning() instead of error() in packed\n> garbage case because it's not an error to have garbage. Loose garbage\n> is still reported as errors and will be converted to warnings later.\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n> ---\n\nWill replace the one from the other day and advance the topic to 'next'.\n\nThanks.\n"}]}