threads / discuss / 1891

Make time-based commit filtering work with topological ordering

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

## tl;dr

4 messages between Sep 20, 2005 and Sep 21, 2005.

replies: 3people: 2as markdown or json

Linus Torvalds· Sep 20, 2005, 21:00 UTC · lore

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")) {

Junio C Hamano· Sep 20, 2005, 22:28 UTC · re: Linus Torvalds · lore

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

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

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

Linus Torvalds· Sep 20, 2005, 22:50 UTC · re: Junio C Hamano · lore

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

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

Linus Torvalds· Sep 21, 2005, 00:55 UTC · re: Linus Torvalds · lore

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

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")) {