Re: [RFH] revision limiting sometimes ignored
- From
Linus Torvalds <torvalds@linux-foundation.org>
- Date
- Feb 5, 2008, 21:23 UTC
- Message-ID
- <alpine.LFD.1.00.0802051300050.3110@woody.linux-foundation.org>
- In-Reply-To
- <7vir13g9hx.fsf@gitster.siamese.dyndns.org>
On Mon, 4 Feb 2008, Junio C Hamano wrote:
Show 9 quoted lines
> > 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;