Re: topological index field for commit objects
- From
Linus Torvalds <torvalds@linux-foundation.org>
- Date
- Jun 30, 2016, 18:12 UTC
- Message-ID
- <CA+55aFy2AEe7ew5Px=2Uit6hraGV9zFr=JZ57rSYXWMQ4nMjeg@mail.gmail.com>
- In-Reply-To
- <5774F4C7.805@gmail.com>
On Thu, Jun 30, 2016 at 3:30 AM, Jakub Narębski <jnareb@gmail.com> wrote:
Show 6 quoted lines
> > P.S. Having Git ensure that committerdate (as an epoch) is greater > than committerdates of its parents at the commit creation time (with > providing warning about time skew, perhaps not doing it if skew is > too large) would be not costly. No need to add anything, and it would > help future queries to be faster, I think.
So I think git should check the committer date (ie verify that the commitetr date of a new commit is always equal to or more recent than all the parents).
But we should probably allow some slop for when machines are out of sync (think build farms etc automation that may do automated commits, but the clocks on different machines may be off by a few seconds). We already do things like that in some of the use of committer dates - see for example CUTOFF_DATE_SLOP)
And we should probably allow the user to override the refusal to create new commits with bogus dates - think importing repositories etc where you may have reasons to just say "that's just how it was".
And it will take years for that to percolate out to all users, so it's not like it will fix things immediately, but then some time from now, we can consider commit dates to be more reliably generation numbers.
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.
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?
Linus
fsck.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/fsck.c b/fsck.c index 05315451c56f..722b65371d38 100644 --- a/fsck.c +++ b/fsck.c @@ -60,6 +60,7 @@ FUNC(NULL_SHA1, WARN) \ FUNC(ZERO_PADDED_FILEMODE, WARN) \ FUNC(NUL_IN_COMMIT, WARN) \ + FUNC(DATE_ORDERING, WARN) \ /* infos (reported as warnings, but ignored by default) */ \ FUNC(BAD_TAG_NAME, INFO) \ FUNC(MISSING_TAGGER_ENTRY, INFO) @@ -604,6 +605,17 @@ static int fsck_ident(const char **ident, struct object *obj, struct fsck_option return 0; } +static void fsck_commit_date(struct fsck_options *options, struct commit *commit) +{ + struct commit_list *p; + + for (p = commit->parents; p; p = p->next) { + struct commit *parent = p->item; + if (commit->date < parent->date) + report(options, &commit->object, FSCK_MSG_DATE_ORDERING, "Bad commit date ordering with parent"); + } +} + static int fsck_commit_buffer(struct commit *commit, const char *buffer, unsigned long size, struct fsck_options *options) { @@ -650,6 +662,7 @@ static int fsck_commit_buffer(struct commit *commit, const char *buffer, return err; } } + fsck_commit_date(options, commit); author_count = 0; while (skip_prefix(buffer, "author ", &buffer)) { author_count++;