Re: topological index field for commit objects

From: Jakub Narębski <jnareb@gmail.com>
Date: Jul 1, 2016, 09:48 UTC
Message-ID: <57763C78.3020509@gmail.com>
In-Reply-To: <20160701031711.GA4832@sigill.intra.peff.net>

W dniu 2016-07-01 o 05:17, Jeff King pisze:

Show 12 quoted lines

> On Thu, Jun 30, 2016 at 11:12:52AM -0700, Linus Torvalds wrote:
> 
>> I do think that it's ok to cache generation numbers somewhere if there
>> is an algorithm that can make use of them, but every time this comes
>> up, it's just not been important enough to make a big deal and a new
>> incompatible object format for it. The committer date is preexisting
>> and has existing pseudo-generation-number usage, so..improving on the
>> quality of it sounds like a good idea.
> 
> If you are OK with a cache, I don't think one needs to change the object
> format at all. It can be computed on the fly, and is purely a local
> optimization.

Note that if cache is generated during (re)packing, it could be optimized for reading. If we want a cache that follow replacements, it would need to be adjusted after each new or modified replacement; assuming that one doesn't modify replacements by hand (then Git would need to detect changes, and modify cache if needed, perhaps on query, e.g. "git log").

Show 7 quoted lines

>> The first step should probably be to make fsck warn about the existing
>> cases of "commit has older date than parents". Something like the
>> attached patch, perhaps?
> 
> I have mixed feelings on this, because it forces the user to confront
> the issue at a time that's potentially very far from when it actually
> happened (and often when it is not their fault).

It would be nice to have as an optional check, like e.g. unreachable objects, or use warning instead of error, like e.g. dangling objects warning. This would be the default; it can always be changed using the `fsck.<msg-id>` and `receive.fsck.<msg-id>` configuration variables.

This way we could use this mode to adjust clock-skew heuristics that Git uses to speed up reachability queries (including time slop), be it automatically or via a configuration variable.

[...]

Show 11 quoted lines

> E.g., imagine somebody else has their clock set forward, and you fetch
> from them. Their object by itself is not broken. It is only when you
> want to commit on top of it, with the correct clock, that the broken
> state is created (and then, we cannot know whether it is your clock or
> the original committer's clock that is bad).
> 
> So I think it would be more productive to put a check like this in "git
> commit" rather than (or perhaps in addition to) fsck. That prevents
> us creating the broken relationship, but it does still mean the user may
> have to to go back and tell the original committer that their clock was
> broken.

Well, check at "git commit" time (perhaps with automatic fixing to used committerdate for the commit being created, within specified conditions) cannot distinguish between the two cases:

 * committerdate in parent commit(s) is correct,
   but the clock is in the past

 * committerdate in parent commit(s) is incorrect, and in the future,
   while the clock is correct

Show 5 quoted lines

> You could also have the fsck check look not only for out-of-order
> commits, but also commits in the future (from the perspective of the
> receiver). That would reject such broken commits before they even hit
> your repository (though again, it is unclear in such a case if the
> commit is broken or the clock of the checker).

Well, one heuristics is that if commits in the future, and/or commits with committerdate out of order have all the same committer, then probably commits are broken. Another "heuristics" would be to assume that if you invoked date-checking functionality, you have ensured that your clock is correct.

-- 
Jakub Narębski