{"thread":{"id":"30876","subject":"[PATCH v2 0/3] \"log --first-parent --simplify-merges/by-decoration\"","startedAt":"2012-06-22T22:27:37Z","lastAt":"2012-06-22T22:27:40Z","messageCount":4,"participants":["Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":3},"messages":[{"id":"194110","messageId":"1340404061-11619-1-git-send-email-gitster@pobox.com","threadId":"30876","inReplyTo":null,"subject":"[PATCH v2 0/3] \"log --first-parent --simplify-merges/by-decoration\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-22T22:27:37Z","receivedAt":"2012-06-22T22:27:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"It is unclear what it means to \"simplify-merges\" while traversing\nonly the \"first-parent\" ancestry chain, but the combination of the\noptions makes the simplification logic to use in-core commit objects\nthat haven't been examined for relevance, either producing incorrect\nresult or taking too long to produce any output.\n\nTeach the simplification logic to ignore commits that the\nfirst-parent traversal logic ignored when both are in effect to work\naround the issue.\n\nJunio C Hamano (3):\n  revision: \"simplify\" options imply topo-order sort\n  revision: note the lack of free() in simplify_merges()\n  revision: ignore side parents while running simplify-merges\n\n revision.c | 41 +++++++++++++++++++++++++++++------------\n 1 file changed, 29 insertions(+), 12 deletions(-)\n\n-- \n1.7.11.1.29.gf71be5c\n"},{"id":"194111","messageId":"1340404061-11619-2-git-send-email-gitster@pobox.com","threadId":"30876","inReplyTo":"1340404061-11619-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 1/3] revision: \"simplify\" options imply topo-order sort","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-22T22:27:38Z","receivedAt":"2012-06-22T22:27:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The code internally runs sort_in_topo_order() already; it is more clear\nto spell it out in the option parsing phase, instead of adding a special\ncase in simplify_merges() function.\n---\n revision.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 935e7a7..00aaefe 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1358,11 +1358,13 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->topo_order = 1;\n \t} else if (!strcmp(arg, \"--simplify-merges\")) {\n \t\trevs->simplify_merges = 1;\n+\t\trevs->topo_order = 1;\n \t\trevs->rewrite_parents = 1;\n \t\trevs->simplify_history = 0;\n \t\trevs->limited = 1;\n \t} else if (!strcmp(arg, \"--simplify-by-decoration\")) {\n \t\trevs->simplify_merges = 1;\n+\t\trevs->topo_order = 1;\n \t\trevs->rewrite_parents = 1;\n \t\trevs->simplify_history = 0;\n \t\trevs->simplify_by_decoration = 1;\n@@ -2016,8 +2018,6 @@ static void simplify_merges(struct rev_info *revs)\n \tstruct commit_list *list;\n \tstruct commit_list *yet_to_do, **tail;\n \n-\tif (!revs->topo_order)\n-\t\tsort_in_topological_order(&revs->commits, revs->lifo);\n \tif (!revs->prune)\n \t\treturn;\n \n-- \n1.7.11.1.29.gf71be5c\n"},{"id":"194112","messageId":"1340404061-11619-3-git-send-email-gitster@pobox.com","threadId":"30876","inReplyTo":"1340404061-11619-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 2/3] revision: note the lack of free() in simplify_merges()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-22T22:27:39Z","receivedAt":"2012-06-22T22:27:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Among the three similar-looking loops that walk singly linked\ncommit_list, the first one is only peeking and the same list is\nlater used for real work.  Leave a comment not to mistakenly\nfree its elements there.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n revision.c | 23 ++++++++++++++++-------\n 1 file changed, 16 insertions(+), 7 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 00aaefe..814b96f 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2015,23 +2015,31 @@ static struct commit_list **simplify_one(struct rev_info *revs, struct commit *c\n \n static void simplify_merges(struct rev_info *revs)\n {\n-\tstruct commit_list *list;\n+\tstruct commit_list *list, *next;\n \tstruct commit_list *yet_to_do, **tail;\n+\tstruct commit *commit;\n \n \tif (!revs->prune)\n \t\treturn;\n \n \t/* feed the list reversed */\n \tyet_to_do = NULL;\n-\tfor (list = revs->commits; list; list = list->next)\n-\t\tcommit_list_insert(list->item, &yet_to_do);\n+\tfor (list = revs->commits; list; list = next) {\n+\t\tcommit = list->item;\n+\t\tnext = list->next;\n+\t\t/*\n+\t\t * Do not free(list) here yet; the original list\n+\t\t * is used later in this function.\n+\t\t */\n+\t\tcommit_list_insert(commit, &yet_to_do);\n+\t}\n \twhile (yet_to_do) {\n \t\tlist = yet_to_do;\n \t\tyet_to_do = NULL;\n \t\ttail = &yet_to_do;\n \t\twhile (list) {\n-\t\t\tstruct commit *commit = list->item;\n-\t\t\tstruct commit_list *next = list->next;\n+\t\t\tcommit = list->item;\n+\t\t\tnext = list->next;\n \t\t\tfree(list);\n \t\t\tlist = next;\n \t\t\ttail = simplify_one(revs, commit, tail);\n@@ -2043,9 +2051,10 @@ static void simplify_merges(struct rev_info *revs)\n \trevs->commits = NULL;\n \ttail = &revs->commits;\n \twhile (list) {\n-\t\tstruct commit *commit = list->item;\n-\t\tstruct commit_list *next = list->next;\n \t\tstruct merge_simplify_state *st;\n+\n+\t\tcommit = list->item;\n+\t\tnext = list->next;\n \t\tfree(list);\n \t\tlist = next;\n \t\tst = locate_simplify_state(revs, commit);\n-- \n1.7.11.1.29.gf71be5c\n"},{"id":"194113","messageId":"1340404061-11619-4-git-send-email-gitster@pobox.com","threadId":"30876","inReplyTo":"1340404061-11619-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 3/3] revision: ignore side parents while running simplify-merges","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-22T22:27:40Z","receivedAt":"2012-06-22T22:27:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The simplify_merges() function needs to look at all history chain to\nfind the closest ancestor that is relevant after the simplification,\nbut after --first-parent traversal, side parents haven't been marked\nfor relevance (they are irrelevant by definition due to the nature\nof first-parent-only traversal) nor culled from the parents list of\nresulting commits.\n\nWe cannot simply remove these side parents from the parents list, as\nthe output phase still wants to see the parents.  Instead, teach\nsimplify_one() and its callees to ignore the later parents.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n revision.c | 14 +++++++++++---\n 1 file changed, 11 insertions(+), 3 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 814b96f..d1a4ef5 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1949,8 +1949,9 @@ static struct commit_list **simplify_one(struct rev_info *revs, struct commit *c\n \t}\n \n \t/*\n-\t * Do we know what commit all of our parents should be rewritten to?\n-\t * Otherwise we are not ready to rewrite this one yet.\n+\t * Do we know what commit all of our parents that matter\n+\t * should be rewritten to?  Otherwise we are not ready to\n+\t * rewrite this one yet.\n \t */\n \tfor (cnt = 0, p = commit->parents; p; p = p->next) {\n \t\tpst = locate_simplify_state(revs, p->item);\n@@ -1958,6 +1959,8 @@ static struct commit_list **simplify_one(struct rev_info *revs, struct commit *c\n \t\t\ttail = &commit_list_insert(p->item, tail)->next;\n \t\t\tcnt++;\n \t\t}\n+\t\tif (revs->first_parent_only)\n+\t\t\tbreak;\n \t}\n \tif (cnt) {\n \t\ttail = &commit_list_insert(commit, tail)->next;\n@@ -1970,8 +1973,13 @@ static struct commit_list **simplify_one(struct rev_info *revs, struct commit *c\n \tfor (p = commit->parents; p; p = p->next) {\n \t\tpst = locate_simplify_state(revs, p->item);\n \t\tp->item = pst->simplified;\n+\t\tif (revs->first_parent_only)\n+\t\t\tbreak;\n \t}\n-\tcnt = remove_duplicate_parents(commit);\n+\tif (!revs->first_parent_only)\n+\t\tcnt = remove_duplicate_parents(commit);\n+\telse\n+\t\tcnt = 1;\n \n \t/*\n \t * It is possible that we are a merge and one side branch\n-- \n1.7.11.1.29.gf71be5c\n"}]}