Re: [RFH] revision limiting sometimes ignored

On Mon, 4 Feb 2008, Junio C Hamano wrote:

I tend to agree.  In a totally connected history, the upper
bound we would need to traverse is down to the merge base of
still positive commits in the "newlist" and negative ones still
on the "list" when everybody on list becomes uninteresting.  And
if there are two unrelated histories, that traversal will need
to traverse down to respective roots.

Which sucks.

I really wonder if the right thing is not simply to admit that we consider 
the commit time meaningful (within some fudge factor!), and then do:

 - make commit warn if any parent commit date is in the future from the 
   current commit date (allow a *small* fudge factor here, say 5 minutes).

 - teach fsck to complain about parent commits being in the future from 
   their children (allow the same small fudge factor).

 - make the revision walking code realize that if times are too close to 
   each other, it should walk a bit further back...

because quite frankly, this bug only shows up when your time goes 
backwards (or stays the same, but the fudge-factor should take care of 
that too).

So the revision walking "fudge factor" could be as simple as the 
following..

		Linus

---
 revision.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/revision.c b/revision.c
index 6e85aaa..e32e1e3 100644
--- a/revision.c
+++ b/revision.c

@@ -558,6 +558,8 @@ static void cherry_pick_list(struct commit_list *list, struct rev_info *revs)
 	free_patch_ids(&ids);
 }
 
+#define FUDGE (60*60)	/* 1 hour fudge-factor */
+
 static int limit_list(struct rev_info *revs)
 {
 	struct commit_list *list = revs->commits;

@@ -579,6 +581,15 @@ static int limit_list(struct rev_info *revs)
 			return -1;
 		if (obj->flags & UNINTERESTING) {
 			mark_parents_uninteresting(commit);
+
+			/*
+			 * If we have commits on the newlist, we don't
+			 * want to do the "everybody_uninteresting()"
+			 * test until we've hit a negative commit that
+			 * is solidly in the past
+			 */
+			if (newlist && newlist->item->date < commit->date + FUDGE)
+				continue;
 			if (everybody_uninteresting(list))
 				break;
 			continue;