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

From: jeffpc@josefsipek.net <jeffpc@josefsipek.net>
Date: Jul 5, 2010, 18:52 UTC
Message-ID: <20100705185238.GS22659@josefsipek.net>
In-Reply-To: <67D0ABD4-BD1A-4B7A-B3EC-F48F21B5DD01@mit.edu>

On Mon, Jul 05, 2010 at 07:06:45AM -0400, Theodore Tso wrote:

Show 10 quoted lines

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

Show 6 quoted lines

> > 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?

Show 14 quoted lines

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