{"thread":{"id":"10640","subject":"[PATCH 1/5] pretty describe: add name info to struct commit","startedAt":"2007-11-04T11:48:22Z","lastAt":"2007-11-04T15:06:58Z","messageCount":4,"participants":["René Scharfe","Alex Riesen","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"58251","messageId":"472DB186.9030909@lsrfire.ath.cx","threadId":"10640","inReplyTo":null,"subject":"[PATCH 1/5] pretty describe: add name info to struct commit","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2007-11-04T11:48:22Z","receivedAt":"2007-11-04T11:48:22Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"This patch prepares the lib'ification of git-describe.  As a library\nfunction, it can't use commit marks or the util field, as they might\nalready be used by the caller.\n\nConvert all usage of commit marks and the util field to use the three\nnew members of struct commit (name, name_flags, name_prio) instead.\nA new function, clear_commit_name_flags(), should be used to clear\nthem after the work is done.\n\nadd_to_known_names() is merged into get_name(); struct commit_name is\nmerged into struct possible_tag; loading the name is done once before\ndescribe() is called instead of doing it inside, but protected by a\nstatic variable.\n\nThe name unconditionally has the prefix \"refs/\" cut from it, but not\nmore, so a prefix of \"tags/\" remains for names found under\n\"refs/tags/\".  This is dealt with when displaying.  Together with the\nchange to pass a minimum name \"priority\" via the cb_data pointer,\nthis cleans get_name() from any use of global variables, making it\nready to serve as a library function (or rather its wrapper, the new\nfunction load_commit_names()).\n\nSigned-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>\n---\n builtin-describe.c |  103 ++++++++++++++++++++++------------------------------\n commit.c           |   21 +++++++++++\n commit.h           |    4 ++\n 3 files changed, 69 insertions(+), 59 deletions(-)\n\ndiff --git a/builtin-describe.c b/builtin-describe.c\nindex 6eeb9b5..82c25c3 100644\n--- a/builtin-describe.c\n+++ b/builtin-describe.c\n@@ -20,42 +20,21 @@ static int tags;\t/* But allow any tags if --tags is specified */\n static int abbrev = DEFAULT_ABBREV;\n static int max_candidates = 10;\n \n-struct commit_name {\n-\tint prio; /* annotated tag = 2, tag = 1, head = 0 */\n-\tchar path[FLEX_ARRAY]; /* more */\n-};\n static const char *prio_names[] = {\n \t\"head\", \"lightweight\", \"annotated\",\n };\n \n-static void add_to_known_names(const char *path,\n-\t\t\t       struct commit *commit,\n-\t\t\t       int prio)\n-{\n-\tstruct commit_name *e = commit->util;\n-\tif (!e || e->prio < prio) {\n-\t\tsize_t len = strlen(path)+1;\n-\t\tfree(e);\n-\t\te = xmalloc(sizeof(struct commit_name) + len);\n-\t\te->prio = prio;\n-\t\tmemcpy(e->path, path, len);\n-\t\tcommit->util = e;\n-\t}\n-}\n-\n static int get_name(const char *path, const unsigned char *sha1, int flag, void *cb_data)\n {\n \tstruct commit *commit = lookup_commit_reference_gently(sha1, 1);\n \tstruct object *object;\n+\tint min_prio = *((int *)cb_data);\n \tint prio;\n \n \tif (!commit)\n \t\treturn 0;\n \tobject = parse_object(sha1);\n-\t/* If --all, then any refs are used.\n-\t * If --tags, then any tags are used.\n-\t * Otherwise only annotated tags are used.\n-\t */\n+\n \tif (!prefixcmp(path, \"refs/tags/\")) {\n \t\tif (object->type == OBJ_TAG)\n \t\t\tprio = 2;\n@@ -64,19 +43,28 @@ static int get_name(const char *path, const unsigned char *sha1, int flag, void\n \t}\n \telse\n \t\tprio = 0;\n+\tif (prio < min_prio)\n+\t\treturn 0;\n \n-\tif (!all) {\n-\t\tif (!prio)\n-\t\t\treturn 0;\n-\t\tif (!tags && prio < 2)\n+\tif (commit->name) {\n+\t\tif (commit->name_prio >= prio)\n \t\t\treturn 0;\n+\t\tfree(commit->name);\n \t}\n-\tadd_to_known_names(all ? path + 5 : path + 10, commit, prio);\n+\tcommit->name = xstrdup(path + 5);\n+\tcommit->name_prio = prio;\n+\n \treturn 0;\n }\n \n+void load_commit_names(int min_prio)\n+{\n+\tfor_each_ref(get_name, &min_prio);\n+}\n+\n struct possible_tag {\n-\tstruct commit_name *name;\n+\tchar *name;\n+\tint prio; /* annotated tag = 2, tag = 1, head = 0 */\n \tint depth;\n \tint found_order;\n \tunsigned flag_within;\n@@ -86,8 +74,8 @@ static int compare_pt(const void *a_, const void *b_)\n {\n \tstruct possible_tag *a = (struct possible_tag *)a_;\n \tstruct possible_tag *b = (struct possible_tag *)b_;\n-\tif (a->name->prio != b->name->prio)\n-\t\treturn b->name->prio - a->name->prio;\n+\tif (a->prio != b->prio)\n+\t\treturn b->prio - a->prio;\n \tif (a->depth != b->depth)\n \t\treturn a->depth - b->depth;\n \tif (a->found_order != b->found_order)\n@@ -104,11 +92,11 @@ static unsigned long finish_depth_computation(\n \t\tstruct commit *c = pop_commit(list);\n \t\tstruct commit_list *parents = c->parents;\n \t\tseen_commits++;\n-\t\tif (c->object.flags & best->flag_within) {\n+\t\tif (c->name_flags & best->flag_within) {\n \t\t\tstruct commit_list *a = *list;\n \t\t\twhile (a) {\n \t\t\t\tstruct commit *i = a->item;\n-\t\t\t\tif (!(i->object.flags & best->flag_within))\n+\t\t\t\tif (!(i->name_flags & best->flag_within))\n \t\t\t\t\tbreak;\n \t\t\t\ta = a->next;\n \t\t\t}\n@@ -119,9 +107,9 @@ static unsigned long finish_depth_computation(\n \t\twhile (parents) {\n \t\t\tstruct commit *p = parents->item;\n \t\t\tparse_commit(p);\n-\t\t\tif (!(p->object.flags & SEEN))\n+\t\t\tif (!(p->name_flags & SEEN))\n \t\t\t\tinsert_by_date(p, list);\n-\t\t\tp->object.flags |= c->object.flags;\n+\t\t\tp->name_flags |= c->name_flags;\n \t\t\tparents = parents->next;\n \t\t}\n \t}\n@@ -133,8 +121,6 @@ static void describe(const char *arg, int last_one)\n \tunsigned char sha1[20];\n \tstruct commit *cmit, *gave_up_on = NULL;\n \tstruct commit_list *list;\n-\tstatic int initialized = 0;\n-\tstruct commit_name *n;\n \tstruct possible_tag all_matches[MAX_TAGS];\n \tunsigned int match_cnt = 0, annotated_cnt = 0, cur_match;\n \tunsigned long seen_commits = 0;\n@@ -145,14 +131,8 @@ static void describe(const char *arg, int last_one)\n \tif (!cmit)\n \t\tdie(\"%s is not a valid '%s' object\", arg, commit_type);\n \n-\tif (!initialized) {\n-\t\tinitialized = 1;\n-\t\tfor_each_ref(get_name, NULL);\n-\t}\n-\n-\tn = cmit->util;\n-\tif (n) {\n-\t\tprintf(\"%s\\n\", n->path);\n+\tif (cmit->name) {\n+\t\tprintf(\"%s\\n\", all ? cmit->name : cmit->name + 5);\n \t\treturn;\n \t}\n \n@@ -160,22 +140,22 @@ static void describe(const char *arg, int last_one)\n \t\tfprintf(stderr, \"searching to describe %s\\n\", arg);\n \n \tlist = NULL;\n-\tcmit->object.flags = SEEN;\n+\tcmit->name_flags = SEEN;\n \tcommit_list_insert(cmit, &list);\n \twhile (list) {\n \t\tstruct commit *c = pop_commit(&list);\n \t\tstruct commit_list *parents = c->parents;\n \t\tseen_commits++;\n-\t\tn = c->util;\n-\t\tif (n) {\n+\t\tif (c->name) {\n \t\t\tif (match_cnt < max_candidates) {\n \t\t\t\tstruct possible_tag *t = &all_matches[match_cnt++];\n-\t\t\t\tt->name = n;\n+\t\t\t\tt->name = all ? c->name : c->name + 5;\n+\t\t\t\tt->prio = c->name_prio;\n \t\t\t\tt->depth = seen_commits - 1;\n \t\t\t\tt->flag_within = 1u << match_cnt;\n \t\t\t\tt->found_order = match_cnt;\n-\t\t\t\tc->object.flags |= t->flag_within;\n-\t\t\t\tif (n->prio == 2)\n+\t\t\t\tc->name_flags |= t->flag_within;\n+\t\t\t\tif (c->name_prio == 2)\n \t\t\t\t\tannotated_cnt++;\n \t\t\t}\n \t\t\telse {\n@@ -185,7 +165,7 @@ static void describe(const char *arg, int last_one)\n \t\t}\n \t\tfor (cur_match = 0; cur_match < match_cnt; cur_match++) {\n \t\t\tstruct possible_tag *t = &all_matches[cur_match];\n-\t\t\tif (!(c->object.flags & t->flag_within))\n+\t\t\tif (!(c->name_flags & t->flag_within))\n \t\t\t\tt->depth++;\n \t\t}\n \t\tif (annotated_cnt && !list) {\n@@ -197,9 +177,9 @@ static void describe(const char *arg, int last_one)\n \t\twhile (parents) {\n \t\t\tstruct commit *p = parents->item;\n \t\t\tparse_commit(p);\n-\t\t\tif (!(p->object.flags & SEEN))\n+\t\t\tif (!(p->name_flags & SEEN))\n \t\t\t\tinsert_by_date(p, &list);\n-\t\t\tp->object.flags |= c->object.flags;\n+\t\t\tp->name_flags |= c->name_flags;\n \t\t\tparents = parents->next;\n \t\t}\n \t}\n@@ -220,8 +200,7 @@ static void describe(const char *arg, int last_one)\n \t\tfor (cur_match = 0; cur_match < match_cnt; cur_match++) {\n \t\t\tstruct possible_tag *t = &all_matches[cur_match];\n \t\t\tfprintf(stderr, \" %-11s %8d %s\\n\",\n-\t\t\t\tprio_names[t->name->prio],\n-\t\t\t\tt->depth, t->name->path);\n+\t\t\t\tprio_names[t->prio], t->depth, t->name);\n \t\t}\n \t\tfprintf(stderr, \"traversed %lu commits\\n\", seen_commits);\n \t\tif (gave_up_on) {\n@@ -233,14 +212,14 @@ static void describe(const char *arg, int last_one)\n \t\t}\n \t}\n \tif (abbrev == 0)\n-\t\tprintf(\"%s\\n\", all_matches[0].name->path );\n+\t\tprintf(\"%s\\n\", all_matches[0].name);\n \telse\n-\t\tprintf(\"%s-%d-g%s\\n\", all_matches[0].name->path,\n+\t\tprintf(\"%s-%d-g%s\\n\", all_matches[0].name,\n \t\t       all_matches[0].depth,\n \t\t       find_unique_abbrev(cmit->object.sha1, abbrev));\n \n \tif (!last_one)\n-\t\tclear_commit_marks(cmit, -1);\n+\t\tclear_commit_name_flags(cmit);\n }\n \n int cmd_describe(int argc, const char **argv, const char *prefix)\n@@ -275,6 +254,12 @@ int cmd_describe(int argc, const char **argv, const char *prefix)\n \t\treturn cmd_name_rev(3 + argc, args, prefix);\n \t}\n \n+\t/* If --all, then any refs are used.\n+\t * If --tags, then any tags are used.\n+\t * Otherwise only annotated tags are used.\n+\t */\n+\tload_commit_names(all ? 0 : (tags ? 1 : 2));\n+\n \tif (argc == 0) {\n \t\tdescribe(\"HEAD\", 1);\n \t} else {\ndiff --git a/commit.c b/commit.c\nindex ac24266..24b7268 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -460,6 +460,27 @@ void clear_commit_marks(struct commit *commit, unsigned int mark)\n \t}\n }\n \n+void clear_commit_name_flags(struct commit *commit)\n+{\n+\twhile (commit) {\n+\t\tstruct commit_list *parents;\n+\n+\t\tif (!commit->name_flags)\n+\t\t\treturn;\n+\n+\t\tcommit->name_flags = 0;\n+\n+\t\tparents = commit->parents;\n+\t\tif (!parents)\n+\t\t\treturn;\n+\n+\t\twhile ((parents = parents->next))\n+\t\t\tclear_commit_name_flags(parents->item);\n+\n+\t\tcommit = commit->parents->item;\n+\t}\n+}\n+\n /*\n  * Generic support for pretty-printing the header\n  */\ndiff --git a/commit.h b/commit.h\nindex b661503..80e94b9 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -18,6 +18,9 @@ struct commit {\n \tstruct commit_list *parents;\n \tstruct tree *tree;\n \tchar *buffer;\n+\tchar *name;\n+\tunsigned int name_flags;\n+\tchar name_prio;\n };\n \n extern int save_commit_buffer;\n@@ -78,6 +81,7 @@ struct commit *pop_most_recent_commit(struct commit_list **list,\n struct commit *pop_commit(struct commit_list **stack);\n \n void clear_commit_marks(struct commit *commit, unsigned int mark);\n+void clear_commit_name_flags(struct commit *commit);\n \n /*\n  * Performs an in-place topological sort of list supplied.\n-- \n1.5.3.5.529.ge3d6d\n"},{"id":"58269","messageId":"20071104140700.GB3078@steel.home","threadId":"10640","inReplyTo":"472DB186.9030909@lsrfire.ath.cx","subject":"Re: [PATCH 1/5] pretty describe: add name info to struct commit","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-11-04T14:07:00Z","receivedAt":"2007-11-04T14:07:00Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"René Scharfe, Sun, Nov 04, 2007 12:48:22 +0100:\n> diff --git a/commit.h b/commit.h\n> index b661503..80e94b9 100644\n> --- a/commit.h\n> +++ b/commit.h\n> @@ -18,6 +18,9 @@ struct commit {\n>  \tstruct commit_list *parents;\n>  \tstruct tree *tree;\n>  \tchar *buffer;\n> +\tchar *name;\n> +\tunsigned int name_flags;\n> +\tchar name_prio;\n>  };\n\nIt increases size of struct commit by ~12 bytes (assuming 4byte\nallignment), and this is a popular structure. Besides, the three new\nfields used by only git-describe, which nobody has in their top-ten\nused commands (see \"best git practices\" thread). If the fields are so\nbadly needed (and the information can't (really?) be stored somewhere\nelse), maybe they could be at least compressed when not used:\n\n\tstruct commit_name_info {\n\t\tunsigned int name_flags;\n\t\tchar name_prio;\n\t\tchar name[FLEX_ARRAY];\n\t};\n\n\tstruct commit {\n\t\tstruct commit_list *parents;\n\t\tstruct tree *tree;\n\t\tchar *buffer;\n\t\tstruct commit_name_info *name_info;\n\t};\n\nBTW, do we still have some bits left in struct object->flags?\nTo, for example, switch between normal and expanded structure of\nsecond-level object (i.e. commit and commit-with-names).\n"},{"id":"58273","messageId":"Pine.LNX.4.64.0711041420430.4362@racer.site","threadId":"10640","inReplyTo":"20071104140700.GB3078@steel.home","subject":"Re: [PATCH 1/5] pretty describe: add name info to struct commit","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-04T14:22:28Z","receivedAt":"2007-11-04T14:22:28Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 4 Nov 2007, Alex Riesen wrote:\n\n> Ren? Scharfe, Sun, Nov 04, 2007 12:48:22 +0100:\n> > diff --git a/commit.h b/commit.h\n> > index b661503..80e94b9 100644\n> > --- a/commit.h\n> > +++ b/commit.h\n> > @@ -18,6 +18,9 @@ struct commit {\n> >  \tstruct commit_list *parents;\n> >  \tstruct tree *tree;\n> >  \tchar *buffer;\n> > +\tchar *name;\n> > +\tunsigned int name_flags;\n> > +\tchar name_prio;\n> >  };\n> \n> It increases size of struct commit by ~12 bytes (assuming 4byte\n> allignment), and this is a popular structure.\n\nI was just about to say something similar.\n\nBut there is a wonderful solution: Have a look at decorate.[ch].  This is \nthe structure you should use IMHO.\n\nCiao,\nDscho\n"},{"id":"58279","messageId":"472DE012.7040001@lsrfire.ath.cx","threadId":"10640","inReplyTo":"20071104140700.GB3078@steel.home","subject":"Re: [PATCH 1/5] pretty describe: add name info to struct commit","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2007-11-04T15:06:58Z","receivedAt":"2007-11-04T15:06:58Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Alex Riesen schrieb:\n> René Scharfe, Sun, Nov 04, 2007 12:48:22 +0100:\n>> diff --git a/commit.h b/commit.h\n>> index b661503..80e94b9 100644\n>> --- a/commit.h\n>> +++ b/commit.h\n>> @@ -18,6 +18,9 @@ struct commit {\n>>  \tstruct commit_list *parents;\n>>  \tstruct tree *tree;\n>>  \tchar *buffer;\n>> +\tchar *name;\n>> +\tunsigned int name_flags;\n>> +\tchar name_prio;\n>>  };\n> \n> It increases size of struct commit by ~12 bytes (assuming 4byte\n> allignment), and this is a popular structure. Besides, the three new\n> fields used by only git-describe, which nobody has in their top-ten\n> used commands (see \"best git practices\" thread).\n\nTrue.  When I was looking for a place for the name info I was a bit\nworried about this increase, but dismissed it after looking at the\nkernel repository: there are ca. 140000 commits, which means my patch\nincreased memory usage by 2MB for commands that operate on all commits\nat the same time.  I haven't taken any measurements to back up this\nestimate, though..\n\nI had looked briefly at the decorate stuff that Dscho mentioned in\nanother reply, but I can't remember why I didn't use it.  Guess I wasn't\nmotivated enough by those 2MB. ;-)  I'll take another look.\n\nThanks,\nRené\n"}]}