* [PATCH] guilt: Make sure the commit time is increasing @ 2010-07-05 2:23 Theodore Ts'o 2010-07-05 2:51 ` tytso 2010-07-05 2:59 ` jeffpc 0 siblings, 2 replies; 19+ messages in thread From: Theodore Ts'o @ 2010-07-05 2:23 UTC (permalink / raw) To: jeffpc; +Cc: Git Mailing List, Theodore Ts'o Git has various algorithms, most notably in git rev-list, git name-rev, and others, which depend on the commit time increasing. We want to keep the commit time the same as much as possible, but if necessary, adjust the time stamps of the patch files to obey this constraint. Signed-off-by: "Theodore Ts'o" <tytso@mit.edu> --- guilt | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/guilt b/guilt index b6e2a6c..2371e98 100755 --- a/guilt +++ b/guilt @@ -535,6 +535,13 @@ commit() export GIT_AUTHOR_EMAIL="`echo $author_str | sed -e 's/[^<]*//'`" fi + ct=$(git log -1 --pretty=%ct) + if [ $ct -gt $(stat -c %Y "$p") ]; then + echo "Warning time went backwards, adjusting mod time of" \ + $(basename "$p") + touch -d @$(expr $ct + 60) "$p" || touch "$p" + fi + # must strip nano-second part otherwise git gets very # confused, and makes up strange timestamps from the past # (chances are it decides to interpret it as a unix -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] guilt: Make sure the commit time is increasing 2010-07-05 2:23 [PATCH] guilt: Make sure the commit time is increasing Theodore Ts'o @ 2010-07-05 2:51 ` tytso 2010-07-05 3:01 ` jeffpc 2010-07-05 2:59 ` jeffpc 1 sibling, 1 reply; 19+ messages in thread From: tytso @ 2010-07-05 2:51 UTC (permalink / raw) To: jeffpc; +Cc: Git Mailing List On Sun, Jul 04, 2010 at 10:23:59PM -0400, Theodore Ts'o wrote: > + ct=$(git log -1 --pretty=%ct) > + if [ $ct -gt $(stat -c %Y "$p") ]; then > + echo "Warning time went backwards, adjusting mod time of" \ > + $(basename "$p") > + touch -d @$(expr $ct + 60) "$p" || touch "$p" ^^^^^^^^^^^^^ hmm, I just realized, this is strictly speaking not necessary. "stat -c %Y" means that guilt only works if GNU coreutils is installed, which means that "touch -d @secs-since-epoch" should also work. This will be a problem on legacy systems such as Solaris (unless their path puts the GNU utilities head of their System V-style utilities), but that's going to be true of guilt in general, it looks like. - Ted ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] guilt: Make sure the commit time is increasing 2010-07-05 2:51 ` tytso @ 2010-07-05 3:01 ` jeffpc 0 siblings, 0 replies; 19+ messages in thread From: jeffpc @ 2010-07-05 3:01 UTC (permalink / raw) To: tytso; +Cc: Git Mailing List On Sun, Jul 04, 2010 at 10:51:17PM -0400, tytso@mit.edu wrote: > On Sun, Jul 04, 2010 at 10:23:59PM -0400, Theodore Ts'o wrote: > > + ct=$(git log -1 --pretty=%ct) > > + if [ $ct -gt $(stat -c %Y "$p") ]; then > > + echo "Warning time went backwards, adjusting mod time of" \ > > + $(basename "$p") > > + touch -d @$(expr $ct + 60) "$p" || touch "$p" > ^^^^^^^^^^^^^ > > hmm, I just realized, this is strictly speaking not necessary. > > "stat -c %Y" means that guilt only works if GNU coreutils is > installed, which means that "touch -d @secs-since-epoch" should also > work. > > This will be a problem on legacy systems such as Solaris (unless their > path puts the GNU utilities head of their System V-style utilities), > but that's going to be true of guilt in general, it looks like. I've been meaning to make guilt less GNU-dependant, but I just don't use non-Linux systems enough (read: almost never) to do it myself. Jeff. -- The box said "Windows XP or better required". So I installed Linux. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] guilt: Make sure the commit time is increasing 2010-07-05 2:23 [PATCH] guilt: Make sure the commit time is increasing Theodore Ts'o 2010-07-05 2:51 ` tytso @ 2010-07-05 2:59 ` jeffpc 2010-07-05 11:06 ` Theodore Tso 1 sibling, 1 reply; 19+ messages in thread From: jeffpc @ 2010-07-05 2:59 UTC (permalink / raw) To: Theodore Ts'o; +Cc: Git Mailing List On Sun, Jul 04, 2010 at 10:23:59PM -0400, Theodore Ts'o wrote: > Git has various algorithms, most notably in git rev-list, git > name-rev, and others, which depend on the commit time increasing. We > want to keep the commit time the same as much as possible, but if > necessary, adjust the time stamps of the patch files to obey this > constraint. Am I understanding this right? You want the timestamps to be monotonically increasing? Is the +60 the most obvious choice? Can I get an example of how git can get confused? Josef 'Jeff' Sipek. > Signed-off-by: "Theodore Ts'o" <tytso@mit.edu> > --- > guilt | 7 +++++++ > 1 files changed, 7 insertions(+), 0 deletions(-) > > diff --git a/guilt b/guilt > index b6e2a6c..2371e98 100755 > --- a/guilt > +++ b/guilt > @@ -535,6 +535,13 @@ commit() > export GIT_AUTHOR_EMAIL="`echo $author_str | sed -e 's/[^<]*//'`" > fi > > + ct=$(git log -1 --pretty=%ct) > + if [ $ct -gt $(stat -c %Y "$p") ]; then > + echo "Warning time went backwards, adjusting mod time of" \ > + $(basename "$p") > + touch -d @$(expr $ct + 60) "$p" || touch "$p" > + fi > + > # must strip nano-second part otherwise git gets very > # confused, and makes up strange timestamps from the past > # (chances are it decides to interpret it as a unix > -- > 1.7.0.4 > -- UNIX is user-friendly ... it's just selective about who its friends are ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] guilt: Make sure the commit time is increasing 2010-07-05 2:59 ` jeffpc @ 2010-07-05 11:06 ` Theodore Tso 2010-07-05 18:52 ` jeffpc 0 siblings, 1 reply; 19+ messages in thread From: Theodore Tso @ 2010-07-05 11:06 UTC (permalink / raw) To: jeffpc; +Cc: Git Mailing List 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. > 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. > > 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: http://www.spinics.net/lists/git/msg134307.html Other threads: http://kerneltrap.org/mailarchive/git/2010/4/8/27731/thread http://www.kerneltrap.com/mailarchive/git/2007/5/24/247375 -- Ted ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] guilt: Make sure the commit time is increasing 2010-07-05 11:06 ` Theodore Tso @ 2010-07-05 18:52 ` jeffpc 2010-07-05 19:22 ` tytso 0 siblings, 1 reply; 19+ messages in thread From: jeffpc @ 2010-07-05 18:52 UTC (permalink / raw) To: Theodore Tso; +Cc: Git Mailing List 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. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] guilt: Make sure the commit time is increasing 2010-07-05 18:52 ` jeffpc @ 2010-07-05 19:22 ` tytso 2010-07-06 8:03 ` Jonathan Nieder ` (2 more replies) 0 siblings, 3 replies; 19+ messages in thread From: tytso @ 2010-07-05 19:22 UTC (permalink / raw) To: jeffpc; +Cc: Git Mailing List On Mon, Jul 05, 2010 at 02:52:38PM -0400, jeffpc@josefsipek.net wrote: > > 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? Hmm, good point. I hadn't considered that case. The most common case happens when I rebase to a new release, and then do a "guilt push -a". In that case the time that start with isn't "now", but whenver the last release happened, which is typically far enough in the past that we don't end up in the future. However, I agree that's a concern. How about this? > 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. I guess I didn't worry too much since "guilt push -a" is pretty noisy anyway. I've shortened the message, but if you think it's better to pull the message altogether feel free... - Ted >From d5659084435a885e05a8fc9afbffe8cdd9535828 Mon Sep 17 00:00:00 2001 From: Theodore Ts'o <tytso@mit.edu> Date: Sun, 4 Jul 2010 22:06:08 -0400 Subject: [PATCH] guilt: Make sure the commit time is increasing Git has various algorithms, most notably in git rev-list, git name-rev, and others, which depend on the commit time increasing. We want to keep the commit time the same as much as possible, but if necessary, adjust the time stamps of the patch files to obey this constraint. Signed-off-by: "Theodore Ts'o" <tytso@mit.edu> --- guilt | 11 +++++++++++ 1 files changed, 11 insertions(+), 0 deletions(-) diff --git a/guilt b/guilt index b6e2a6c..edcfb34 100755 --- a/guilt +++ b/guilt @@ -535,6 +535,17 @@ commit() export GIT_AUTHOR_EMAIL="`echo $author_str | sed -e 's/[^<]*//'`" fi + ct=$(git log -1 --pretty=%ct) + if [ $ct -gt $(stat -c %Y "$p") ]; then + echo "Adjusting mod time of" $(basename "$p") + ct=$(expr $ct + 60) + if [ $ct -gt $(date +%s) ]; then + touch "$p" + else + touch -d @$(expr $ct + 60) "$p" + fi + fi + # must strip nano-second part otherwise git gets very # confused, and makes up strange timestamps from the past # (chances are it decides to interpret it as a unix -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] guilt: Make sure the commit time is increasing 2010-07-05 19:22 ` tytso @ 2010-07-06 8:03 ` Jonathan Nieder 2010-07-06 10:56 ` Theodore Tso 2010-07-06 13:53 ` Erik Faye-Lund 2010-07-06 17:29 ` jeffpc 2010-07-14 3:01 ` Josef 'Jeff' Sipek 2 siblings, 2 replies; 19+ messages in thread From: Jonathan Nieder @ 2010-07-06 8:03 UTC (permalink / raw) To: tytso; +Cc: jeffpc, Git Mailing List, Jeff King tytso@mit.edu wrote: > On Mon, Jul 05, 2010 at 02:52:38PM -0400, jeffpc@josefsipek.net wrote: >> 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? I don’t think git has ever required commit dates to be _strictly_ monotonic. At one point rev-list did require monotonic --- i.e., the committer date of each commit had to be equal to or later than that of each of its parents) with no clock skew but that was considered a bug and fixed by v1.5.5-rc1~16 (Make revision limiting more robust against occasional bad commit dates, 2008-03-17) > diff --git a/guilt b/guilt > index b6e2a6c..edcfb34 100755 > --- a/guilt > +++ b/guilt > @@ -535,6 +535,17 @@ commit() > export GIT_AUTHOR_EMAIL="`echo $author_str | sed -e 's/[^<]*//'`" > fi > > + ct=$(git log -1 --pretty=%ct) > + if [ $ct -gt $(stat -c %Y "$p") ]; then > + echo "Adjusting mod time of" $(basename "$p") > + ct=$(expr $ct + 60) > + if [ $ct -gt $(date +%s) ]; then > + touch "$p" > + else > + touch -d @$(expr $ct + 60) "$p" So I would suggest echo "Adjusting mod time of $(basename "$p")" touch -d "$ct" "$p" If the parent commit time happens to be in the future, well, at least we’re not making it worse. By the way, I think your idea to have commit warn about nonmonotonic commit dates is a good one. We should also decide on a rule, hopefully one the kernel repo obeys (30 days max skew? *crosses fingers*) and make git fsck warn loudly about violations. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] guilt: Make sure the commit time is increasing 2010-07-06 8:03 ` Jonathan Nieder @ 2010-07-06 10:56 ` Theodore Tso 2010-07-06 15:09 ` Jonathan Nieder 2010-07-06 13:53 ` Erik Faye-Lund 1 sibling, 1 reply; 19+ messages in thread From: Theodore Tso @ 2010-07-06 10:56 UTC (permalink / raw) To: Jonathan Nieder; +Cc: jeffpc, Git Mailing List, Jeff King On Jul 6, 2010, at 4:03 AM, Jonathan Nieder wrote: > At one point rev-list did require monotonic --- i.e., the committer > date of each commit had to be equal to or later than that of each of > its parents) with no clock skew but that was considered a bug and > fixed by v1.5.5-rc1~16 (Make revision limiting more robust against > occasional bad commit dates, 2008-03-17) You're right that it's been a while since git has run into problems with mild forms of clock skew (even Debian Stable is shipping v1.5.6) but I think it's better to times in the future if we can at all help it, and it's not like we're talking about a lot of extra complexity to guilt to test for this. > By the way, I think your idea to have commit warn about nonmonotonic > commit dates is a good one. We should also decide on a rule, > hopefully one the kernel repo obeys (30 days max skew? *crosses > fingers*) and make git fsck warn loudly about violations. Having git commit warn, absolutely. Unfortunately, there is already ~100 days of skew (see the earlier discusion on this thread by Jeff King) in the Linux kernel repo already... -- Ted ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] guilt: Make sure the commit time is increasing 2010-07-06 10:56 ` Theodore Tso @ 2010-07-06 15:09 ` Jonathan Nieder 2010-07-06 17:12 ` tytso 0 siblings, 1 reply; 19+ messages in thread From: Jonathan Nieder @ 2010-07-06 15:09 UTC (permalink / raw) To: Theodore Tso; +Cc: jeffpc, Git Mailing List, Jeff King Theodore Tso wrote: > You're right that it's been a while since git has run into problems with > mild forms of clock skew (even Debian Stable is shipping v1.5.6) but > I think it's better to times in the future if we can at all help it, and it's not > like we're talking about a lot of extra complexity to guilt to test for this. Sorry, I’m a little lost. There are five phenomena one could forbid: 1. Commits with timestamp equal to or before a parent 2. Commits with timestamp before a parent 3. Commits with timestamp unreasonably long before a parent 4. Commits with timestamp unreasonably long before an ancestor 5. Commits with timestamp in the future Git has always been able to cope with #5 (timestamps in the future). I see no reason to avoid it, except that it is hard to assign a timestamp for commits on top of that one. Git’s problem today is #4 (long-term slop). Maybe as Jeff suggested "unreasonably long" should defined per repository. Or we could measure the kernel’s maximum (something like 120 days?) and make that a hard limit. Do #3 a few times, and you get #4. So ‘commit’ should warn about it (where ‘unreasonably long’ could be as short as 0 or 1 days). #2 (nonmonotonic commits) was broken in ancient git; I think it’s too rigid of a rule to worry about it on that account. But a variant of the rationale for avoiding #3 applies to it. I have never heard of any version of Git copying poorly with #1 (commits with the same timestamp). Avoiding it artificially leads inevitably to timestamps in the future when you somehow try to assign 100 timestamps for the series you have rebased on top of a patch committed a few seconds ago. Incrementing the timestamp to ensure strictly monotonic commits seems like a recipe for trouble to me. For guilt, I think the best thing to do would to save a Date: line for the author date with the From: and Subject: and then touch patches with the _current_ date when appropriate to avoid skew. HTH, Jonathan ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] guilt: Make sure the commit time is increasing 2010-07-06 15:09 ` Jonathan Nieder @ 2010-07-06 17:12 ` tytso 2010-07-06 17:29 ` Jonathan Nieder 0 siblings, 1 reply; 19+ messages in thread From: tytso @ 2010-07-06 17:12 UTC (permalink / raw) To: Jonathan Nieder; +Cc: jeffpc, Git Mailing List, Jeff King On Tue, Jul 06, 2010 at 10:09:17AM -0500, Jonathan Nieder wrote: > > I have never heard of any version of Git copying poorly with #1 > (commits with the same timestamp). Avoiding it artificially leads > inevitably to timestamps in the future when you somehow try to assign > 100 timestamps for the series you have rebased on top of a patch > committed a few seconds ago. > > Incrementing the timestamp to ensure strictly monotonic commits seems > like a recipe for trouble to me. Um, I'm guessing you spent a lot of time typing your note, but not a lot of time looking at my most recent patch? My most recent patch for guilt simply will set the time of the patch to the current time to avoid setting it into the future. - Ted ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] guilt: Make sure the commit time is increasing 2010-07-06 17:12 ` tytso @ 2010-07-06 17:29 ` Jonathan Nieder 0 siblings, 0 replies; 19+ messages in thread From: Jonathan Nieder @ 2010-07-06 17:29 UTC (permalink / raw) To: tytso; +Cc: jeffpc, Git Mailing List, Jeff King tytso@mit.edu wrote: > On Tue, Jul 06, 2010 at 10:09:17AM -0500, Jonathan Nieder wrote: >> I have never heard of any version of Git copying poorly with #1 >> (commits with the same timestamp). Avoiding it artificially leads >> inevitably to timestamps in the future when you somehow try to assign >> 100 timestamps for the series you have rebased on top of a patch >> committed a few seconds ago. >> >> Incrementing the timestamp to ensure strictly monotonic commits seems >> like a recipe for trouble to me. > > Um, I'm guessing you spent a lot of time typing your note, but not a > lot of time looking at my most recent patch? My most recent patch for > guilt simply will set the time of the patch to the current time to > avoid setting it into the future. I read it, and I did not like that specific part. I even responded to it. I guess "leads inevitably to timestamps in the future" was a poor choice of words, though. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] guilt: Make sure the commit time is increasing 2010-07-06 8:03 ` Jonathan Nieder 2010-07-06 10:56 ` Theodore Tso @ 2010-07-06 13:53 ` Erik Faye-Lund 2010-07-06 14:29 ` Jeff King 1 sibling, 1 reply; 19+ messages in thread From: Erik Faye-Lund @ 2010-07-06 13:53 UTC (permalink / raw) To: Jonathan Nieder; +Cc: tytso, jeffpc, Git Mailing List, Jeff King On Tue, Jul 6, 2010 at 10:03 AM, Jonathan Nieder <jrnieder@gmail.com> wrote: > tytso@mit.edu wrote: >> On Mon, Jul 05, 2010 at 02:52:38PM -0400, jeffpc@josefsipek.net wrote: > >>> 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? > > I don’t think git has ever required commit dates to be _strictly_ > monotonic. > > At one point rev-list did require monotonic --- i.e., the committer > date of each commit had to be equal to or later than that of each of > its parents) with no clock skew but that was considered a bug and > fixed by v1.5.5-rc1~16 (Make revision limiting more robust against > occasional bad commit dates, 2008-03-17) > This might be a stupid question, but I'm not entirely clear on why it's not a strict requirement; surely it would be easy to ensure that the commit-time is at least as big as the parents when generating the commit...? Is it to avoid the case where a user commits with the clock set to some point (potentially far) in the future, so all subsequent commits would have the same, artificially high commit time? Or is there some other reason I can't think of? -- Erik "kusma" Faye-Lund ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] guilt: Make sure the commit time is increasing 2010-07-06 13:53 ` Erik Faye-Lund @ 2010-07-06 14:29 ` Jeff King 2010-07-06 15:02 ` Erik Faye-Lund 0 siblings, 1 reply; 19+ messages in thread From: Jeff King @ 2010-07-06 14:29 UTC (permalink / raw) To: kusmabite; +Cc: Jonathan Nieder, tytso, jeffpc, Git Mailing List On Tue, Jul 06, 2010 at 03:53:56PM +0200, Erik Faye-Lund wrote: > > At one point rev-list did require monotonic --- i.e., the committer > > date of each commit had to be equal to or later than that of each of > > its parents) with no clock skew but that was considered a bug and > > fixed by v1.5.5-rc1~16 (Make revision limiting more robust against > > occasional bad commit dates, 2008-03-17) > > > > This might be a stupid question, but I'm not entirely clear on why > it's not a strict requirement; surely it would be easy to ensure that > the commit-time is at least as big as the parents when generating the > commit...? > > Is it to avoid the case where a user commits with the clock set to > some point (potentially far) in the future, so all subsequent commits > would have the same, artificially high commit time? Or is there some > other reason I can't think of? You can have clock skew between distributed developers. So imagine you commit at 5:00pm, then I pull at 5:01pm, but it turns out your clock is two minutes fast, so it's actually 4:59pm. What should my commit do? If I insist on monotonic increases, then my clock gets pushed forward artificially by your fast, broken clock (which is probably not the end of the world; in practice, if your clock is N seconds fast, there will presumably be some N second period where I'm not making a commit, and the clocks can "catch up" with each other). -Peff ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] guilt: Make sure the commit time is increasing 2010-07-06 14:29 ` Jeff King @ 2010-07-06 15:02 ` Erik Faye-Lund 2010-07-06 17:21 ` tytso 0 siblings, 1 reply; 19+ messages in thread From: Erik Faye-Lund @ 2010-07-06 15:02 UTC (permalink / raw) To: Jeff King; +Cc: Jonathan Nieder, tytso, jeffpc, Git Mailing List On Tue, Jul 6, 2010 at 4:29 PM, Jeff King <peff@peff.net> wrote: > On Tue, Jul 06, 2010 at 03:53:56PM +0200, Erik Faye-Lund wrote: > >> > At one point rev-list did require monotonic --- i.e., the committer >> > date of each commit had to be equal to or later than that of each of >> > its parents) with no clock skew but that was considered a bug and >> > fixed by v1.5.5-rc1~16 (Make revision limiting more robust against >> > occasional bad commit dates, 2008-03-17) >> > >> >> This might be a stupid question, but I'm not entirely clear on why >> it's not a strict requirement; surely it would be easy to ensure that >> the commit-time is at least as big as the parents when generating the >> commit...? >> >> Is it to avoid the case where a user commits with the clock set to >> some point (potentially far) in the future, so all subsequent commits >> would have the same, artificially high commit time? Or is there some >> other reason I can't think of? > > You can have clock skew between distributed developers. So imagine you > commit at 5:00pm, then I pull at 5:01pm, but it turns out your clock is > two minutes fast, so it's actually 4:59pm. > > What should my commit do? If I insist on monotonic increases, then my > clock gets pushed forward artificially by your fast, broken clock (which > is probably not the end of the world; in practice, if your clock is N > seconds fast, there will presumably be some N second period where I'm > not making a commit, and the clocks can "catch up" with each other). > Yeah, but this doesn't really answer my question; as you're saying, it's probably not the end of the world, at least when the skew is low. But I can imagine it becoming a big deal when the skew is high. The again, perhaps this should constitute a "bad commit" and commit should error out if a parent commit was more than some number of minutes newer than the current time (or whatever)? That way, skewed commits would be caught early if a developer is working with other people, and a lot of the traversal could perhaps be faster (or more robust). If the developer with the skewed clock doesn't work with anyone, skew isn't really a problem, but perhaps he'd have to do some branch-filtering to un-skew commits when starting to work with others. And only if the skew is really high... like, multiple days... Which can't really be THAT common? However, turning a technical problem that already have a solution that seems to work for everyone into a social one might be a bad idea. I'm really just thinking out loud here :) -- Erik "kusma" Faye-Lund ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] guilt: Make sure the commit time is increasing 2010-07-06 15:02 ` Erik Faye-Lund @ 2010-07-06 17:21 ` tytso 0 siblings, 0 replies; 19+ messages in thread From: tytso @ 2010-07-06 17:21 UTC (permalink / raw) To: kusmabite; +Cc: Jeff King, Jonathan Nieder, jeffpc, Git Mailing List On Tue, Jul 06, 2010 at 05:02:51PM +0200, Erik Faye-Lund wrote: > But I can imagine it becoming a big deal when the skew is high. The > again, perhaps this should constitute a "bad commit" and commit should > error out if a parent commit was more than some number of minutes > newer than the current time (or whatever)? That way, skewed commits > would be caught early if a developer is working with other people, and > a lot of the traversal could perhaps be faster (or more robust). If > the developer with the skewed clock doesn't work with anyone, skew > isn't really a problem, but perhaps he'd have to do some > branch-filtering to un-skew commits when starting to work with others. > And only if the skew is really high... like, multiple days... Which > can't really be THAT common? Guilt uses the modtime of the patch in a patch series for the committer time and the author time. The reasoning behind it doing this is so that you can do "git pop -a" followed by "git push -a" and if the patch files haven't changed, the commit id's don't change either. But if you change a commit in the middle of the series, you can end up with clock skews that could be several days or weeks. Becuase of my ext4 workflow, the Linux kernel has a maximum skew of 100 days. Mea culpa; I stopped doing this as soon as I was told that git was depending on committer time being roughly increasing, and so I at least haven't introduced any such time skews since v2.6.34. And part of my making up for this has been to submit a patch to guilt to prevent this from happening again in the future, by fixing up guilt so that it won't request "git commit" to create timestamps that show very wild clock skews within a single linear branch. We could still get potentially screwed though. Every so often I will see someone sending e-mail from a client host whose time is years if not decades in the past or in the future. If they were to do a "git commit", and then push that commit to a public repository, we could easily introduce a large clock skew into a git repo. Has that ever happened to date? Not to my knowledge. Could it happen? Very clearly, yes. Should we try to put in some safety checks to prevent it, or at least issue warnings? Maybe. - Ted ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] guilt: Make sure the commit time is increasing 2010-07-05 19:22 ` tytso 2010-07-06 8:03 ` Jonathan Nieder @ 2010-07-06 17:29 ` jeffpc 2010-07-06 18:57 ` tytso 2010-07-14 3:01 ` Josef 'Jeff' Sipek 2 siblings, 1 reply; 19+ messages in thread From: jeffpc @ 2010-07-06 17:29 UTC (permalink / raw) To: tytso; +Cc: Git Mailing List On Mon, Jul 05, 2010 at 03:22:01PM -0400, tytso@mit.edu wrote: ... I'm going to play with this patch locally a little bit, but I'm all for it. > From d5659084435a885e05a8fc9afbffe8cdd9535828 Mon Sep 17 00:00:00 2001 Speaking of weird timestamps...2001? Where did that come from? :) > From: Theodore Ts'o <tytso@mit.edu> > Date: Sun, 4 Jul 2010 22:06:08 -0400 > Subject: [PATCH] guilt: Make sure the commit time is increasing > > Git has various algorithms, most notably in git rev-list, git > name-rev, and others, which depend on the commit time increasing. We > want to keep the commit time the same as much as possible, but if > necessary, adjust the time stamps of the patch files to obey this > constraint. > > Signed-off-by: "Theodore Ts'o" <tytso@mit.edu> > --- > guilt | 11 +++++++++++ > 1 files changed, 11 insertions(+), 0 deletions(-) > > diff --git a/guilt b/guilt > index b6e2a6c..edcfb34 100755 > --- a/guilt > +++ b/guilt > @@ -535,6 +535,17 @@ commit() > export GIT_AUTHOR_EMAIL="`echo $author_str | sed -e 's/[^<]*//'`" > fi > > + ct=$(git log -1 --pretty=%ct) > + if [ $ct -gt $(stat -c %Y "$p") ]; then > + echo "Adjusting mod time of" $(basename "$p") Depending on how my playing goes, I might remove the echo. > + ct=$(expr $ct + 60) So, ct is now the +1min time. > + if [ $ct -gt $(date +%s) ]; then > + touch "$p" > + else > + touch -d @$(expr $ct + 60) "$p" And we're touching +1+1min. I'll fix it up before applying. Thanks, Jeff. -- NT is to UNIX what a doughnut is to a particle accelerator. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] guilt: Make sure the commit time is increasing 2010-07-06 17:29 ` jeffpc @ 2010-07-06 18:57 ` tytso 0 siblings, 0 replies; 19+ messages in thread From: tytso @ 2010-07-06 18:57 UTC (permalink / raw) To: jeffpc; +Cc: Git Mailing List On Tue, Jul 06, 2010 at 01:29:54PM -0400, jeffpc@josefsipek.net wrote: > > And we're touching +1+1min. I'll fix it up before applying. > Yup, I forgot to take the "+ 60" out. Thanks for catching that. - Ted ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] guilt: Make sure the commit time is increasing 2010-07-05 19:22 ` tytso 2010-07-06 8:03 ` Jonathan Nieder 2010-07-06 17:29 ` jeffpc @ 2010-07-14 3:01 ` Josef 'Jeff' Sipek 2 siblings, 0 replies; 19+ messages in thread From: Josef 'Jeff' Sipek @ 2010-07-14 3:01 UTC (permalink / raw) To: tytso; +Cc: Git Mailing List FWIW, I pushed the change out. I think this is a major enough fix that I'll cut a new release soon. Thanks! Jeff. -- The obvious mathematical breakthrough would be development of an easy way to factor large prime numbers. - Bill Gates, The Road Ahead, pg. 265 ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2010-07-14 3:01 UTC | newest] Thread overview: 19+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-07-05 2:23 [PATCH] guilt: Make sure the commit time is increasing Theodore Ts'o 2010-07-05 2:51 ` tytso 2010-07-05 3:01 ` jeffpc 2010-07-05 2:59 ` jeffpc 2010-07-05 11:06 ` Theodore Tso 2010-07-05 18:52 ` jeffpc 2010-07-05 19:22 ` tytso 2010-07-06 8:03 ` Jonathan Nieder 2010-07-06 10:56 ` Theodore Tso 2010-07-06 15:09 ` Jonathan Nieder 2010-07-06 17:12 ` tytso 2010-07-06 17:29 ` Jonathan Nieder 2010-07-06 13:53 ` Erik Faye-Lund 2010-07-06 14:29 ` Jeff King 2010-07-06 15:02 ` Erik Faye-Lund 2010-07-06 17:21 ` tytso 2010-07-06 17:29 ` jeffpc 2010-07-06 18:57 ` tytso 2010-07-14 3:01 ` Josef 'Jeff' Sipek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).