Re: git describe --contains doesn't work properly for a commit

From: Jeff King <peff@peff.net>
Date: Mar 5, 2015, 05:12 UTC
Message-ID: <20150305051211.GA3344@peff.net>
In-Reply-To: <xmqq7fuw8pgq.fsf@gitster.dls.corp.google.com>

On Wed, Mar 04, 2015 at 12:41:57PM -0800, Junio C Hamano wrote:

> > Calculating them is simple. Caching and storage is the bigger question.
> 
> Yes, also having to handle the ones whose generation numbers haven't
> been computed yet adds to the complexity.

I'm not sure it's that bad. If you cache generation numbers for all known commits when you repack, then worst case you have to traverse all commits not in the pack.

> This one, and $gmane/264101, are a few instances of this known issue
> raised here recently.

If $gmane/264101 is caused by clock skew, I'd find that disturbing. Those algorithms are supposed to be "correct, but slower" in the face of skew, not ever incorrect.

Show 8 quoted lines

> I have been wondering if we can do something
> along the following (these are not alternatives) as a cheaper
> workaround:
> 
>  (1) Introduce '--skewed-timestamps[=(allow|warn|reject)' to all
>      commands that create new commit objects.  If the committer
>      timestamp being used is older than any of the parent commits,
>      "warn" or "reject" depending on the setting.

I think this idea has come up before. If it's _your_ timestamp that is screwed up, this detects it, which is good. But if it's somebody else's timestamp that is screwed up, there's often not much you can do. It's already baked into the history.

I don't mind it as an extra layer of protection, I guess. But my recollection of the great skew survey[1] is that most of these problems don't come from actual clock skew, but from software bugs or bogus data in imported commits. True skew is generally less than a day, and can be handled with a fixed slop time.

[1] http://article.gmane.org/gmane.comp.version-control.git/159065

Show 6 quoted lines

>  (2) Compute a bitmap whose timestamps are suspect when we pack to
>      mark commits.  When revision.c:limit_list() tries to see if
>      there still are interesting commits, an UNINTERESTING commit
>      marked as such shouldn't be counted as "not interesting because
>      it is old enough".  Use the same hint in the walker used in
>      "describe --contains".

If you see mismatched timestamps between a parent and child commit, it's often not clear which one is suspicious. Is the parent skewed to the future, or is the child skewed to the past? Which one do you mark as suspect?

IMHO, if you are going to go to the trouble to detect and store skew, you should just go to the trouble to calculate and store reliable generation numbers.

-Peff