Re: [PATCH] guilt: Make sure the commit time is increasing

[jeffpc@josefsipek.net]

On Mon, Jul 05, 2010 at 07:06:45AM -0400, Theodore Tso wrote:
> On Jul 4, 2010, at 10:59 PM, jeffpc@josefsipek.net wrote:
> > 
> > Am I understanding this right?  You want the timestamps to be monotonically
> > increasing?  
> 
> Yup, that's correct.  In more modern versions of git most (all?) of the places
> that depend on the committer time of the child commit to be greater than the
> committer time of its parents have been relaxed to accept up to a day's worth
> of clock skew, but in the interests of "be conservative in what you send",
> strictly increasing seemed like the best thing to do.

Alright, makes sense.

> > Is the +60 the most obvious choice?
> 
> It's somewhat arbitrary.  I figured a minute increase between commits was
> more aesthetically pleasing than a second, 5 minutes, or an hour, which
> were some other deltas that previous versions of my patch used while I
> was experimenting.

I think we might need a little bit more logic in this patch...

if I commit, and immediately after push 10 patches, wouldn't the HEAD end up
with a commit that's ~10 minutes in the future?

> > Can I get an example of how git can get confused?
> 
> This first one is explicitly my/guilt's fault (and it's when I learned that I
> was causing problems by how I was using guilt in the ext4 tree):
> 
> http://kerneltrap.org/mailarchive/git/2010/4/22/28928/thread
> 
> In this thread we see how the clock skew gets in the way of an optimization
> that speeds up "git tag --contains" by over two orders of magnitude, but it
> gets screwed over by extreme clock skew.  I suggested in that thread that 
> if git is going to depend on it, then maybe "git commit" should either warn
> or error out if the git committer timestamp goes backwards --- and that's when
> I decided maybe I should offer up a patch to guilt to fix this, either before or
> instead of fixing up "git commit" to throw a warning/error:

I do like the idea of git-commit warning/erroring, but I don't think that
guilt issuing a warning is necessary.  Afterall, it's only a timestamp
change.  It might be a bit of a shock for anyone looking at the timestamps
expecting them to be out of order (based on the patch times), but I think
it's better than warning all the time.

Jeff.

-- 
What is the difference between Mechanical Engineers and Civil Engineers?
Mechanical Engineers build weapons, Civil Engineers build targets.