* [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: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: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: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 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 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 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-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-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).