{"thread":{"id":"30751","subject":"[PATCH 1/3] revision: \"simplify\" options imply topo-order sort","startedAt":"2012-06-08T22:53:26Z","lastAt":"2012-06-08T22:53:29Z","messageCount":4,"participants":["Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"193196","messageId":"1339196009-14555-1-git-send-email-gitster@pobox.com","threadId":"30751","inReplyTo":null,"subject":"[PATCH 0/3] rev-list --simplify-{merges,by-decoration} --first-parent","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-08T22:53:26Z","receivedAt":"2012-06-08T22:53:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The deeper history simplification using merge information (not to be\nconfused with \"merge simplification\") was not prepared to be used\nwith the --first-parent option, but it was possible to trigger this\ncombination from gitk.\n\nThe fix is fairly straight-forward. When showing only first-parent\nchain of history, the later parents won't matter in the result, so\nwe can just rewrite the parents and make the history a single strand\nof pearls.\n\nShowing first-parent-only history and then attempting to simplify\nmerges does not make much sense, but simplify-by-decoration may be a\nvalid thing to ask.\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: cull side parents before running simplify-merges\n\n revision.c | 30 +++++++++++++++++++++---------\n 1 file changed, 21 insertions(+), 9 deletions(-)\n\n-- \n1.7.11.rc2.29.g85b30f3\n"},{"id":"193195","messageId":"1339196009-14555-2-git-send-email-gitster@pobox.com","threadId":"30751","inReplyTo":"1339196009-14555-1-git-send-email-gitster@pobox.com","subject":"[PATCH 1/3] revision: \"simplify\" options imply topo-order sort","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-08T22:53:27Z","receivedAt":"2012-06-08T22:53:27Z","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\nSigned-off-by: Junio C Hamano \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.rc2.29.g85b30f3\n"},{"id":"193198","messageId":"1339196009-14555-3-git-send-email-gitster@pobox.com","threadId":"30751","inReplyTo":"1339196009-14555-1-git-send-email-gitster@pobox.com","subject":"[PATCH 2/3] revision: note the lack of free() in simplify_merges()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-08T22:53:28Z","receivedAt":"2012-06-08T22:53:28Z","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 \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.rc2.29.g85b30f3\n"},{"id":"193197","messageId":"1339196009-14555-4-git-send-email-gitster@pobox.com","threadId":"30751","inReplyTo":"1339196009-14555-1-git-send-email-gitster@pobox.com","subject":"[PATCH 3/3] revision: cull side parents before running simplify-merges","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-08T22:53:29Z","receivedAt":"2012-06-08T22:53:29Z","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\nRemove these side parents from parents list before starting to\nfurther simplifying the result.\n\nSigned-off-by: Junio C Hamano \n---\n revision.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/revision.c b/revision.c\nindex 814b96f..d93eb7c 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2031,6 +2031,9 @@ static void simplify_merges(struct rev_info *revs)\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\tif (revs->first_parent_only &&\n+\t\t commit->parents && commit->parents->next)\n+\t\t\tcommit->parents->next = NULL;\n \t\tcommit_list_insert(commit, &yet_to_do);\n \t}\n \twhile (yet_to_do) {\n-- \n1.7.11.rc2.29.g85b30f3\n"}]}