Re: [RFH] revision limiting sometimes ignored

[Junio C Hamano <gitster@pobox.com>]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> 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).

Hmmm.  In other words, you are punished for trying to build on
top of somebody else who screwed up.  That sucks.

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

And this is even worse.

But I think these are sane thing to do regardless.  To prevent
problematic commits from contaminating other people, we could
add default post-receive hook and perhaps a new pre-merge hook
to inspect the newly obtained history to warn and reject a merge
(including fast-forward) if it contains commits far into the
future.  We would also need a mechanism to force a merge with
such a clock-skewed history if we did so.

>  - 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).

> @@ -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;

We picked up commit which we know is the youngest in the "list".
Because we push into newlist as we traverse, newlist is sorted
by date, and the element pointed by it is the youngest in there.
That is something we have processed earlier, so it ought to be
younger than this commit.  Otherwise we have found a problematic
clock skew.  Ok, I think it should work.

The clock skew t6009 artificially introduces needs to be
shortened for this, though.