From a52f00711321f1c60f9e8ee4434cb5ddc43f6b64 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 8 Jun 2012 14:47:08 -0700 Subject: [PATCH 1/3] revision: "simplify" options imply topo-order sort The code internally runs sort_in_topo_order() already; it is more clear to spell it out in the option parsing phase, instead of adding a special case in simplify_merges() function. --- revision.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/revision.c b/revision.c index 935e7a7ba4..00aaefe885 100644 --- a/revision.c +++ b/revision.c @@ -1358,11 +1358,13 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg revs->topo_order = 1; } else if (!strcmp(arg, "--simplify-merges")) { revs->simplify_merges = 1; + revs->topo_order = 1; revs->rewrite_parents = 1; revs->simplify_history = 0; revs->limited = 1; } else if (!strcmp(arg, "--simplify-by-decoration")) { revs->simplify_merges = 1; + revs->topo_order = 1; revs->rewrite_parents = 1; revs->simplify_history = 0; revs->simplify_by_decoration = 1; @@ -2016,8 +2018,6 @@ static void simplify_merges(struct rev_info *revs) struct commit_list *list; struct commit_list *yet_to_do, **tail; - if (!revs->topo_order) - sort_in_topological_order(&revs->commits, revs->lifo); if (!revs->prune) return; From ab9d75a8d7e48e03ab0be9fc5e38902f1c173b87 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 8 Jun 2012 14:50:22 -0700 Subject: [PATCH 2/3] revision: note the lack of free() in simplify_merges() Among the three similar-looking loops that walk singly linked commit_list, the first one is only peeking and the same list is later used for real work. Leave a comment not to mistakenly free its elements there. Signed-off-by: Junio C Hamano --- revision.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/revision.c b/revision.c index 00aaefe885..814b96ff53 100644 --- a/revision.c +++ b/revision.c @@ -2015,23 +2015,31 @@ static struct commit_list **simplify_one(struct rev_info *revs, struct commit *c static void simplify_merges(struct rev_info *revs) { - struct commit_list *list; + struct commit_list *list, *next; struct commit_list *yet_to_do, **tail; + struct commit *commit; if (!revs->prune) return; /* feed the list reversed */ yet_to_do = NULL; - for (list = revs->commits; list; list = list->next) - commit_list_insert(list->item, &yet_to_do); + for (list = revs->commits; list; list = next) { + commit = list->item; + next = list->next; + /* + * Do not free(list) here yet; the original list + * is used later in this function. + */ + commit_list_insert(commit, &yet_to_do); + } while (yet_to_do) { list = yet_to_do; yet_to_do = NULL; tail = &yet_to_do; while (list) { - struct commit *commit = list->item; - struct commit_list *next = list->next; + commit = list->item; + next = list->next; free(list); list = next; tail = simplify_one(revs, commit, tail); @@ -2043,9 +2051,10 @@ static void simplify_merges(struct rev_info *revs) revs->commits = NULL; tail = &revs->commits; while (list) { - struct commit *commit = list->item; - struct commit_list *next = list->next; struct merge_simplify_state *st; + + commit = list->item; + next = list->next; free(list); list = next; st = locate_simplify_state(revs, commit); From 6e513ba3a624a57fcf9e0f316ba54b3e0c951286 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 8 Jun 2012 14:56:03 -0700 Subject: [PATCH 3/3] revision: ignore side parents while running simplify-merges The simplify_merges() function needs to look at all history chain to find the closest ancestor that is relevant after the simplification, but after --first-parent traversal, side parents haven't been marked for relevance (they are irrelevant by definition due to the nature of first-parent-only traversal) nor culled from the parents list of resulting commits. We cannot simply remove these side parents from the parents list, as the output phase still wants to see the parents. Instead, teach simplify_one() and its callees to ignore the later parents. Signed-off-by: Junio C Hamano --- revision.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/revision.c b/revision.c index 814b96ff53..d1a4ef5da0 100644 --- a/revision.c +++ b/revision.c @@ -1949,8 +1949,9 @@ static struct commit_list **simplify_one(struct rev_info *revs, struct commit *c } /* - * Do we know what commit all of our parents should be rewritten to? - * Otherwise we are not ready to rewrite this one yet. + * Do we know what commit all of our parents that matter + * should be rewritten to? Otherwise we are not ready to + * rewrite this one yet. */ for (cnt = 0, p = commit->parents; p; p = p->next) { pst = locate_simplify_state(revs, p->item); @@ -1958,6 +1959,8 @@ static struct commit_list **simplify_one(struct rev_info *revs, struct commit *c tail = &commit_list_insert(p->item, tail)->next; cnt++; } + if (revs->first_parent_only) + break; } if (cnt) { tail = &commit_list_insert(commit, tail)->next; @@ -1970,8 +1973,13 @@ static struct commit_list **simplify_one(struct rev_info *revs, struct commit *c for (p = commit->parents; p; p = p->next) { pst = locate_simplify_state(revs, p->item); p->item = pst->simplified; + if (revs->first_parent_only) + break; } - cnt = remove_duplicate_parents(commit); + if (!revs->first_parent_only) + cnt = remove_duplicate_parents(commit); + else + cnt = 1; /* * It is possible that we are a merge and one side branch