Make time-based commit filtering work with topological ordering

Subsystems: the rest

4 messages, 2 authors, 2016-06-15 · open the first message on its own page

Make time-based commit filtering work with topological ordering

From: Linus Torvalds <torvalds@osdl.org>
Date: 2016-06-15 22:42:07

The trick is to consider the time-based filtering a limiter, the same way 
we do for release ranges.

That means that the time-based filtering runs _before_ the topological 
sorting, which makes it meaningful again. It also simplifies the code 
logic.

This makes "gitk" useful with time ranges.

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
---

diff --git a/rev-list.c b/rev-list.c
--- a/rev-list.c
+++ b/rev-list.c

@@ -82,12 +82,6 @@ static int filter_commit(struct commit *
 		return STOP;
 	if (commit->object.flags & (UNINTERESTING|SHOWN))
 		return CONTINUE;
-	if (min_age != -1 && (commit->date > min_age))
-		return CONTINUE;
-	if (max_age != -1 && (commit->date < max_age)) {
-		stop_traversal=1;
-		return merge_order?CONTINUE:STOP;
-	}
 	if (max_count != -1 && !max_count--)
 		return STOP;
 	if (no_merges && (commit->parents && commit->parents->next))

@@ -374,6 +368,8 @@ static struct commit_list *limit_list(st
 		struct commit *commit = pop_most_recent_commit(&list, SEEN);
 		struct object *obj = &commit->object;
 
+		if (max_age != -1 && (commit->date < max_age))
+			obj->flags |= UNINTERESTING;
 		if (unpacked && has_sha1_pack(obj->sha1))
 			obj->flags |= UNINTERESTING;
 		if (obj->flags & UNINTERESTING) {

@@ -382,6 +378,8 @@ static struct commit_list *limit_list(st
 				break;
 			continue;
 		}
+		if (min_age != -1 && (commit->date > min_age))
+			continue;
 		p = &commit_list_insert(commit, p)->next;
 	}
 	if (tree_objects)

@@ -494,10 +492,12 @@ int main(int argc, char **argv)
 		}
 		if (!strncmp(arg, "--max-age=", 10)) {
 			max_age = atoi(arg + 10);
+			limited = 1;
 			continue;
 		}
 		if (!strncmp(arg, "--min-age=", 10)) {
 			min_age = atoi(arg + 10);
+			limited = 1;
 			continue;
 		}
 		if (!strcmp(arg, "--header")) {

Re: Make time-based commit filtering work with topological ordering

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:42:07

This seems to break t6001 merge-order with max-age test.

* FAIL 23: --max-age=c3, --merge-order

Re: Make time-based commit filtering work with topological ordering

From: Linus Torvalds <torvalds@osdl.org>
Date: 2016-06-15 22:42:07


On Tue, 20 Sep 2005, Junio C Hamano wrote:

This seems to break t6001 merge-order with max-age test.

Yes. 

Merge-order with ages doesn't make sense. I'd seriously suggest removing
--merge-order entirely, but in the meantime I think you should remove 
anything that tests dates and merge-order.

Merge-order doesn't use the normal limit rules, but does its own home-brew
limiting.  It could do the date-limiting on its own, if it wanted to, but 
I doubt it would make sense.

Try the example I had without my patches:

        [torvalds@g5 git]$ git-rev-list $(git-rev-parse --since="8 days ago" HEAD) | wc
             80      80    3280
        [torvalds@g5 git]$ git-rev-list --topo-order $(git-rev-parse --since="8 days ago" HEAD) | wc
             10      10     410

and I bet the same thing was true for --merge-order. And that's just not 
_sensible_. It doesn't make sense that a time limiter would depend on the 
order, but the way it used to be done it always did so for very 
fundamental reasons.

So the old code was broken. Any test for it was broken too.

		Linus

Re: Make time-based commit filtering work with topological ordering

From: Linus Torvalds <torvalds@osdl.org>
Date: 2016-06-15 22:42:07


On Tue, 20 Sep 2005, Linus Torvalds wrote:

Merge-order doesn't use the normal limit rules, but does its own home-brew
limiting.  It could do the date-limiting on its own, if it wanted to, but 
I doubt it would make sense.

Actually, looking closely, merge-order actually had it's own hack in the 
old filter_commit() code to make it work sanely. 

This is an updated patch that allows merge-order to continue to work. 

		Linus

---
Subject: Make time-based commit filtering work with topological ordering

The trick is to consider the time-based filtering a limiter, the same way
we do for release ranges.

That means that the time-based filtering runs _before_ the topological
sorting, which makes it meaningful again. It also simplifies the code
logic.

This makes "gitk" useful with time ranges.

[ Second version: --merge-order now unaffected by the re-org ]

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
---

diff --git a/rev-list.c b/rev-list.c
--- a/rev-list.c
+++ b/rev-list.c

@@ -86,7 +86,7 @@ static int filter_commit(struct commit *
 		return CONTINUE;
 	if (max_age != -1 && (commit->date < max_age)) {
 		stop_traversal=1;
-		return merge_order?CONTINUE:STOP;
+		return CONTINUE;
 	}
 	if (max_count != -1 && !max_count--)
 		return STOP;

@@ -374,6 +374,8 @@ static struct commit_list *limit_list(st
 		struct commit *commit = pop_most_recent_commit(&list, SEEN);
 		struct object *obj = &commit->object;
 
+		if (max_age != -1 && (commit->date < max_age))
+			obj->flags |= UNINTERESTING;
 		if (unpacked && has_sha1_pack(obj->sha1))
 			obj->flags |= UNINTERESTING;
 		if (obj->flags & UNINTERESTING) {

@@ -382,6 +384,8 @@ static struct commit_list *limit_list(st
 				break;
 			continue;
 		}
+		if (min_age != -1 && (commit->date > min_age))
+			continue;
 		p = &commit_list_insert(commit, p)->next;
 	}
 	if (tree_objects)

@@ -494,10 +498,12 @@ int main(int argc, char **argv)
 		}
 		if (!strncmp(arg, "--max-age=", 10)) {
 			max_age = atoi(arg + 10);
+			limited = 1;
 			continue;
 		}
 		if (!strncmp(arg, "--min-age=", 10)) {
 			min_age = atoi(arg + 10);
+			limited = 1;
 			continue;
 		}
 		if (!strcmp(arg, "--header")) {

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help