{"thread":{"id":"24287","subject":"[PATCH] guilt: Make sure the commit time is increasing","startedAt":"2010-07-05T02:23:59Z","lastAt":"2010-07-14T03:01:17Z","messageCount":19,"participants":["Theodore Ts'o","tytso@mit.edu","jeffpc@josefsipek.net","Theodore Tso","Jonathan Nieder","Erik Faye-Lund","Jeff King","Josef 'Jeff' Sipek"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"144789","messageId":"1278296639-25024-1-git-send-email-tytso@mit.edu","threadId":"24287","inReplyTo":null,"subject":"[PATCH] guilt: Make sure the commit time is increasing","fromName":"Theodore Ts'o","fromEmail":"tytso@mit.edu","sentAt":"2010-07-05T02:23:59Z","receivedAt":"2010-07-05T02:23:59Z","isPatch":true,"sender":{"key":"tytso@mit.edu","avatar":"https://avatars.githubusercontent.com/u/51416?v=4"},"body":"Git has various algorithms, most notably in git rev-list, git\nname-rev, and others, which depend on the commit time increasing.  We\nwant to keep the commit time the same as much as possible, but if\nnecessary, adjust the time stamps of the patch files to obey this\nconstraint.\n\nSigned-off-by: \"Theodore Ts'o\" <tytso@mit.edu>\n---\n guilt |    7 +++++++\n 1 files changed, 7 insertions(+), 0 deletions(-)\n\ndiff --git a/guilt b/guilt\nindex b6e2a6c..2371e98 100755\n--- a/guilt\n+++ b/guilt\n@@ -535,6 +535,13 @@ commit()\n                         export GIT_AUTHOR_EMAIL=\"`echo $author_str | sed -e 's/[^<]*//'`\"\n \t\tfi\n \n+\t\tct=$(git log -1 --pretty=%ct)\n+\t\tif [ $ct -gt $(stat -c %Y \"$p\") ]; then\n+\t\t    echo \"Warning time went backwards, adjusting mod time of\" \\\n+\t\t\t$(basename \"$p\")\n+\t\t    touch -d @$(expr $ct + 60) \"$p\" || touch \"$p\"\n+\t\tfi\n+\n \t\t# must strip nano-second part otherwise git gets very\n \t\t# confused, and makes up strange timestamps from the past\n \t\t# (chances are it decides to interpret it as a unix\n-- \n1.7.0.4\n"},{"id":"144790","messageId":"20100705025117.GC6384@thunk.org","threadId":"24287","inReplyTo":"1278296639-25024-1-git-send-email-tytso@mit.edu","subject":"Re: [PATCH] guilt: Make sure the commit time is increasing","fromName":"","fromEmail":"tytso@mit.edu","sentAt":"2010-07-05T02:51:17Z","receivedAt":"2010-07-05T02:51:17Z","isPatch":true,"sender":{"key":"tytso@mit.edu","avatar":"https://avatars.githubusercontent.com/u/51416?v=4"},"body":"On Sun, Jul 04, 2010 at 10:23:59PM -0400, Theodore Ts'o wrote:\n> +\t\tct=$(git log -1 --pretty=%ct)\n> +\t\tif [ $ct -gt $(stat -c %Y \"$p\") ]; then\n> +\t\t    echo \"Warning time went backwards, adjusting mod time of\" \\\n> +\t\t\t$(basename \"$p\")\n> +\t\t    touch -d @$(expr $ct + 60) \"$p\" || touch \"$p\"\n                                                    ^^^^^^^^^^^^^\n\nhmm, I just realized, this is strictly speaking not necessary.\n\n\"stat -c %Y\" means that guilt only works if GNU coreutils is\ninstalled, which means that \"touch -d @secs-since-epoch\" should also\nwork.\n\nThis will be a problem on legacy systems such as Solaris (unless their\npath puts the GNU utilities head of their System V-style utilities),\nbut that's going to be true of guilt in general, it looks like.\n\n      \t    \t    \t     \t      - Ted\n"},{"id":"144791","messageId":"20100705025900.GQ22659@josefsipek.net","threadId":"24287","inReplyTo":"1278296639-25024-1-git-send-email-tytso@mit.edu","subject":"Re: [PATCH] guilt: Make sure the commit time is increasing","fromName":"","fromEmail":"jeffpc@josefsipek.net","sentAt":"2010-07-05T02:59:00Z","receivedAt":"2010-07-05T02:59:00Z","isPatch":true,"sender":{"key":"jeffpc@josefsipek.net","avatar":null},"body":"On Sun, Jul 04, 2010 at 10:23:59PM -0400, Theodore Ts'o wrote:\n> Git has various algorithms, most notably in git rev-list, git\n> name-rev, and others, which depend on the commit time increasing.  We\n> want to keep the commit time the same as much as possible, but if\n> necessary, adjust the time stamps of the patch files to obey this\n> constraint.\n\nAm I understanding this right?  You want the timestamps to be monotonically\nincreasing?  Is the +60 the most obvious choice?\n\nCan I get an example of how git can get confused?\n\nJosef 'Jeff' Sipek.\n\n> Signed-off-by: \"Theodore Ts'o\" <tytso@mit.edu>\n> ---\n>  guilt |    7 +++++++\n>  1 files changed, 7 insertions(+), 0 deletions(-)\n> \n> diff --git a/guilt b/guilt\n> index b6e2a6c..2371e98 100755\n> --- a/guilt\n> +++ b/guilt\n> @@ -535,6 +535,13 @@ commit()\n>                          export GIT_AUTHOR_EMAIL=\"`echo $author_str | sed -e 's/[^<]*//'`\"\n>  \t\tfi\n>  \n> +\t\tct=$(git log -1 --pretty=%ct)\n> +\t\tif [ $ct -gt $(stat -c %Y \"$p\") ]; then\n> +\t\t    echo \"Warning time went backwards, adjusting mod time of\" \\\n> +\t\t\t$(basename \"$p\")\n> +\t\t    touch -d @$(expr $ct + 60) \"$p\" || touch \"$p\"\n> +\t\tfi\n> +\n>  \t\t# must strip nano-second part otherwise git gets very\n>  \t\t# confused, and makes up strange timestamps from the past\n>  \t\t# (chances are it decides to interpret it as a unix\n> -- \n> 1.7.0.4\n> \n\n-- \nUNIX is user-friendly ... it's just selective about who its friends are\n"},{"id":"144792","messageId":"20100705030107.GR22659@josefsipek.net","threadId":"24287","inReplyTo":"20100705025117.GC6384@thunk.org","subject":"Re: [PATCH] guilt: Make sure the commit time is increasing","fromName":"","fromEmail":"jeffpc@josefsipek.net","sentAt":"2010-07-05T03:01:08Z","receivedAt":"2010-07-05T03:01:08Z","isPatch":true,"sender":{"key":"jeffpc@josefsipek.net","avatar":null},"body":"On Sun, Jul 04, 2010 at 10:51:17PM -0400, tytso@mit.edu wrote:\n> On Sun, Jul 04, 2010 at 10:23:59PM -0400, Theodore Ts'o wrote:\n> > +\t\tct=$(git log -1 --pretty=%ct)\n> > +\t\tif [ $ct -gt $(stat -c %Y \"$p\") ]; then\n> > +\t\t    echo \"Warning time went backwards, adjusting mod time of\" \\\n> > +\t\t\t$(basename \"$p\")\n> > +\t\t    touch -d @$(expr $ct + 60) \"$p\" || touch \"$p\"\n>                                                     ^^^^^^^^^^^^^\n> \n> hmm, I just realized, this is strictly speaking not necessary.\n> \n> \"stat -c %Y\" means that guilt only works if GNU coreutils is\n> installed, which means that \"touch -d @secs-since-epoch\" should also\n> work.\n> \n> This will be a problem on legacy systems such as Solaris (unless their\n> path puts the GNU utilities head of their System V-style utilities),\n> but that's going to be true of guilt in general, it looks like.\n\nI've been meaning to make guilt less GNU-dependant, but I just don't use\nnon-Linux systems enough (read: almost never) to do it myself.\n\nJeff.\n\n-- \nThe box said \"Windows XP or better required\". So I installed Linux.\n"},{"id":"144809","messageId":"67D0ABD4-BD1A-4B7A-B3EC-F48F21B5DD01@mit.edu","threadId":"24287","inReplyTo":"20100705025900.GQ22659@josefsipek.net","subject":"Re: [PATCH] guilt: Make sure the commit time is increasing","fromName":"Theodore Tso","fromEmail":"tytso@mit.edu","sentAt":"2010-07-05T11:06:45Z","receivedAt":"2010-07-05T11:06:45Z","isPatch":true,"sender":{"key":"tytso@mit.edu","avatar":"https://avatars.githubusercontent.com/u/51416?v=4"},"body":"\nOn Jul 4, 2010, at 10:59 PM, jeffpc@josefsipek.net wrote:\n> \n> Am I understanding this right?  You want the timestamps to be monotonically\n> increasing?  \n\nYup, that's correct.  In more modern versions of git most (all?) of the places\nthat depend on the committer time of the child commit to be greater than the\ncommitter time of its parents have been relaxed to accept up to a day's worth\nof clock skew, but in the interests of \"be conservative in what you send\",\nstrictly increasing seemed like the best thing to do.\n\n> Is the +60 the most obvious choice?\n\nIt's somewhat arbitrary.  I figured a minute increase between commits was\nmore aesthetically pleasing than a second, 5 minutes, or an hour, which\nwere some other deltas that previous versions of my patch used while I\nwas experimenting.\n\n> \n> Can I get an example of how git can get confused?\n\nThis first one is explicitly my/guilt's fault (and it's when I learned that I\nwas causing problems by how I was using guilt in the ext4 tree):\n\nhttp://kerneltrap.org/mailarchive/git/2010/4/22/28928/thread\n\nIn this thread we see how the clock skew gets in the way of an optimization\nthat speeds up \"git tag --contains\" by over two orders of magnitude, but it\ngets screwed over by extreme clock skew.  I suggested in that thread that \nif git is going to depend on it, then maybe \"git commit\" should either warn\nor error out if the git committer timestamp goes backwards --- and that's when\nI decided maybe I should offer up a patch to guilt to fix this, either before or\ninstead of fixing up \"git commit\" to throw a warning/error:\n\nhttp://www.spinics.net/lists/git/msg134307.html\n\nOther threads:\n\nhttp://kerneltrap.org/mailarchive/git/2010/4/8/27731/thread\nhttp://www.kerneltrap.com/mailarchive/git/2007/5/24/247375\n\n-- Ted\n"},{"id":"144838","messageId":"20100705185238.GS22659@josefsipek.net","threadId":"24287","inReplyTo":"67D0ABD4-BD1A-4B7A-B3EC-F48F21B5DD01@mit.edu","subject":"Re: [PATCH] guilt: Make sure the commit time is increasing","fromName":"","fromEmail":"jeffpc@josefsipek.net","sentAt":"2010-07-05T18:52:38Z","receivedAt":"2010-07-05T18:52:38Z","isPatch":true,"sender":{"key":"jeffpc@josefsipek.net","avatar":null},"body":"On Mon, Jul 05, 2010 at 07:06:45AM -0400, Theodore Tso wrote:\n> On Jul 4, 2010, at 10:59 PM, jeffpc@josefsipek.net wrote:\n> > \n> > Am I understanding this right?  You want the timestamps to be monotonically\n> > increasing?  \n> \n> Yup, that's correct.  In more modern versions of git most (all?) of the places\n> that depend on the committer time of the child commit to be greater than the\n> committer time of its parents have been relaxed to accept up to a day's worth\n> of clock skew, but in the interests of \"be conservative in what you send\",\n> strictly increasing seemed like the best thing to do.\n\nAlright, makes sense.\n\n> > Is the +60 the most obvious choice?\n> \n> It's somewhat arbitrary.  I figured a minute increase between commits was\n> more aesthetically pleasing than a second, 5 minutes, or an hour, which\n> were some other deltas that previous versions of my patch used while I\n> was experimenting.\n\nI think we might need a little bit more logic in this patch...\n\nif I commit, and immediately after push 10 patches, wouldn't the HEAD end up\nwith a commit that's ~10 minutes in the future?\n\n> > Can I get an example of how git can get confused?\n> \n> This first one is explicitly my/guilt's fault (and it's when I learned that I\n> was causing problems by how I was using guilt in the ext4 tree):\n> \n> http://kerneltrap.org/mailarchive/git/2010/4/22/28928/thread\n> \n> In this thread we see how the clock skew gets in the way of an optimization\n> that speeds up \"git tag --contains\" by over two orders of magnitude, but it\n> gets screwed over by extreme clock skew.  I suggested in that thread that \n> if git is going to depend on it, then maybe \"git commit\" should either warn\n> or error out if the git committer timestamp goes backwards --- and that's when\n> I decided maybe I should offer up a patch to guilt to fix this, either before or\n> instead of fixing up \"git commit\" to throw a warning/error:\n\nI do like the idea of git-commit warning/erroring, but I don't think that\nguilt issuing a warning is necessary.  Afterall, it's only a timestamp\nchange.  It might be a bit of a shock for anyone looking at the timestamps\nexpecting them to be out of order (based on the patch times), but I think\nit's better than warning all the time.\n\nJeff.\n\n-- \nWhat is the difference between Mechanical Engineers and Civil Engineers?\nMechanical Engineers build weapons, Civil Engineers build targets.\n"},{"id":"144841","messageId":"20100705192201.GI25518@thunk.org","threadId":"24287","inReplyTo":"20100705185238.GS22659@josefsipek.net","subject":"Re: [PATCH] guilt: Make sure the commit time is increasing","fromName":"","fromEmail":"tytso@mit.edu","sentAt":"2010-07-05T19:22:01Z","receivedAt":"2010-07-05T19:22:01Z","isPatch":true,"sender":{"key":"tytso@mit.edu","avatar":"https://avatars.githubusercontent.com/u/51416?v=4"},"body":"On Mon, Jul 05, 2010 at 02:52:38PM -0400, jeffpc@josefsipek.net wrote:\n> \n> I think we might need a little bit more logic in this patch...\n> \n> if I commit, and immediately after push 10 patches, wouldn't the HEAD end up\n> with a commit that's ~10 minutes in the future?\n\nHmm, good point.  I hadn't considered that case.  The most common case\nhappens when I rebase to a new release, and then do a \"guilt push -a\".\nIn that case the time that start with isn't \"now\", but whenver the\nlast release happened, which is typically far enough in the past that\nwe don't end up in the future.  However, I agree that's a concern.\n\nHow about this?\n\n> I do like the idea of git-commit warning/erroring, but I don't think that\n> guilt issuing a warning is necessary.  Afterall, it's only a timestamp\n> change.  It might be a bit of a shock for anyone looking at the timestamps\n> expecting them to be out of order (based on the patch times), but I think\n> it's better than warning all the time.\n\nI guess I didn't worry too much since \"guilt push -a\" is pretty noisy\nanyway.  I've shortened the message, but if you think it's better to\npull the message altogether feel free...\n\n\t\t\t\t\t\t- Ted\n\n>From d5659084435a885e05a8fc9afbffe8cdd9535828 Mon Sep 17 00:00:00 2001\nFrom: Theodore Ts'o <tytso@mit.edu>\nDate: Sun, 4 Jul 2010 22:06:08 -0400\nSubject: [PATCH] guilt: Make sure the commit time is increasing\n\nGit has various algorithms, most notably in git rev-list, git\nname-rev, and others, which depend on the commit time increasing.  We\nwant to keep the commit time the same as much as possible, but if\nnecessary, adjust the time stamps of the patch files to obey this\nconstraint.\n\nSigned-off-by: \"Theodore Ts'o\" <tytso@mit.edu>\n---\n guilt |   11 +++++++++++\n 1 files changed, 11 insertions(+), 0 deletions(-)\n\ndiff --git a/guilt b/guilt\nindex b6e2a6c..edcfb34 100755\n--- a/guilt\n+++ b/guilt\n@@ -535,6 +535,17 @@ commit()\n                         export GIT_AUTHOR_EMAIL=\"`echo $author_str | sed -e 's/[^<]*//'`\"\n \t\tfi\n \n+\t\tct=$(git log -1 --pretty=%ct)\n+\t\tif [ $ct -gt $(stat -c %Y \"$p\") ]; then\n+\t\t    echo \"Adjusting mod time of\" $(basename \"$p\")\n+\t\t    ct=$(expr $ct + 60)\n+\t\t    if [ $ct -gt $(date +%s) ]; then\n+\t\t\ttouch \"$p\"\n+\t\t    else\n+\t\t\ttouch -d @$(expr $ct + 60) \"$p\"\n+\t\t    fi\n+\t\tfi\n+\n \t\t# must strip nano-second part otherwise git gets very\n \t\t# confused, and makes up strange timestamps from the past\n \t\t# (chances are it decides to interpret it as a unix\n-- \n1.7.0.4\n"},{"id":"144887","messageId":"20100706080322.GA2856@burratino","threadId":"24287","inReplyTo":"20100705192201.GI25518@thunk.org","subject":"Re: [PATCH] guilt: Make sure the commit time is increasing","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-06T08:03:22Z","receivedAt":"2010-07-06T08:03:22Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"tytso@mit.edu wrote:\n> On Mon, Jul 05, 2010 at 02:52:38PM -0400, jeffpc@josefsipek.net wrote:\n\n>> if I commit, and immediately after push 10 patches, wouldn't the HEAD end up\n>> with a commit that's ~10 minutes in the future?\n\nI don’t think git has ever required commit dates to be _strictly_\nmonotonic.\n\nAt one point rev-list did require monotonic --- i.e., the committer\ndate of each commit had to be equal to or later than that of each of\nits parents) with no clock skew but that was considered a bug and\nfixed by v1.5.5-rc1~16 (Make revision limiting more robust against\noccasional bad commit dates, 2008-03-17)\n\n> diff --git a/guilt b/guilt\n> index b6e2a6c..edcfb34 100755\n> --- a/guilt\n> +++ b/guilt\n> @@ -535,6 +535,17 @@ commit()\n>                          export GIT_AUTHOR_EMAIL=\"`echo $author_str | sed -e 's/[^<]*//'`\"\n>  \t\tfi\n>  \n> +\t\tct=$(git log -1 --pretty=%ct)\n> +\t\tif [ $ct -gt $(stat -c %Y \"$p\") ]; then\n> +\t\t    echo \"Adjusting mod time of\" $(basename \"$p\")\n> +\t\t    ct=$(expr $ct + 60)\n> +\t\t    if [ $ct -gt $(date +%s) ]; then\n> +\t\t\ttouch \"$p\"\n> +\t\t    else\n> +\t\t\ttouch -d @$(expr $ct + 60) \"$p\"\n\nSo I would suggest\n\n echo \"Adjusting mod time of $(basename \"$p\")\"\n touch -d \"$ct\" \"$p\"\n\nIf the parent commit time happens to be in the future, well, at\nleast we’re not making it worse.\n\nBy the way, I think your idea to have commit warn about nonmonotonic\ncommit dates is a good one.  We should also decide on a rule,\nhopefully one the kernel repo obeys (30 days max skew? *crosses\nfingers*) and make git fsck warn loudly about violations.\n"},{"id":"144895","messageId":"DD1E6EE4-1196-4FCA-87DA-EB9EBCA3AC83@mit.edu","threadId":"24287","inReplyTo":"20100706080322.GA2856@burratino","subject":"Re: [PATCH] guilt: Make sure the commit time is increasing","fromName":"Theodore Tso","fromEmail":"tytso@mit.edu","sentAt":"2010-07-06T10:56:36Z","receivedAt":"2010-07-06T10:56:36Z","isPatch":true,"sender":{"key":"tytso@mit.edu","avatar":"https://avatars.githubusercontent.com/u/51416?v=4"},"body":"\nOn Jul 6, 2010, at 4:03 AM, Jonathan Nieder wrote:\n> At one point rev-list did require monotonic --- i.e., the committer\n> date of each commit had to be equal to or later than that of each of\n> its parents) with no clock skew but that was considered a bug and\n> fixed by v1.5.5-rc1~16 (Make revision limiting more robust against\n> occasional bad commit dates, 2008-03-17)\n\nYou're right that it's been a while since git has run into problems with \nmild forms of clock skew (even Debian Stable is shipping v1.5.6) but\nI think it's better to times in the future if we can at all help it, and it's not\nlike we're talking about a lot of extra complexity to guilt to test for this.\n\n> By the way, I think your idea to have commit warn about nonmonotonic\n> commit dates is a good one.  We should also decide on a rule,\n> hopefully one the kernel repo obeys (30 days max skew? *crosses\n> fingers*) and make git fsck warn loudly about violations.\n\nHaving git commit warn, absolutely.\n\nUnfortunately, there is already ~100 days of skew (see the earlier\ndiscusion on this thread by Jeff King) in the Linux kernel repo\nalready...\n\n-- Ted\n"},{"id":"144905","messageId":"AANLkTinZ4UV9in60Y4myfUWv08Vx3OMvh-_YQl2BXSjC@mail.gmail.com","threadId":"24287","inReplyTo":"20100706080322.GA2856@burratino","subject":"Re: [PATCH] guilt: Make sure the commit time is increasing","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@googlemail.com","sentAt":"2010-07-06T13:53:56Z","receivedAt":"2010-07-06T13:53:56Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Tue, Jul 6, 2010 at 10:03 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> tytso@mit.edu wrote:\n>> On Mon, Jul 05, 2010 at 02:52:38PM -0400, jeffpc@josefsipek.net wrote:\n>\n>>> if I commit, and immediately after push 10 patches, wouldn't the HEAD end up\n>>> with a commit that's ~10 minutes in the future?\n>\n> I don’t think git has ever required commit dates to be _strictly_\n> monotonic.\n>\n> At one point rev-list did require monotonic --- i.e., the committer\n> date of each commit had to be equal to or later than that of each of\n> its parents) with no clock skew but that was considered a bug and\n> fixed by v1.5.5-rc1~16 (Make revision limiting more robust against\n> occasional bad commit dates, 2008-03-17)\n>\n\nThis might be a stupid question, but I'm not entirely clear on why\nit's not a strict requirement; surely it would be easy to ensure that\nthe commit-time is at least as big as the parents when generating the\ncommit...?\n\nIs it to avoid the case where a user commits with the clock set to\nsome point (potentially far) in the future, so all subsequent commits\nwould have the same, artificially high commit time? Or is there some\nother reason I can't think of?\n\n-- \nErik \"kusma\" Faye-Lund\n"},{"id":"144906","messageId":"20100706142921.GB6666@sigill.intra.peff.net","threadId":"24287","inReplyTo":"AANLkTinZ4UV9in60Y4myfUWv08Vx3OMvh-_YQl2BXSjC@mail.gmail.com","subject":"Re: [PATCH] guilt: Make sure the commit time is increasing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-07-06T14:29:22Z","receivedAt":"2010-07-06T14:29:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jul 06, 2010 at 03:53:56PM +0200, Erik Faye-Lund wrote:\n\n> > At one point rev-list did require monotonic --- i.e., the committer\n> > date of each commit had to be equal to or later than that of each of\n> > its parents) with no clock skew but that was considered a bug and\n> > fixed by v1.5.5-rc1~16 (Make revision limiting more robust against\n> > occasional bad commit dates, 2008-03-17)\n> >\n> \n> This might be a stupid question, but I'm not entirely clear on why\n> it's not a strict requirement; surely it would be easy to ensure that\n> the commit-time is at least as big as the parents when generating the\n> commit...?\n> \n> Is it to avoid the case where a user commits with the clock set to\n> some point (potentially far) in the future, so all subsequent commits\n> would have the same, artificially high commit time? Or is there some\n> other reason I can't think of?\n\nYou can have clock skew between distributed developers. So imagine you\ncommit at 5:00pm, then I pull at 5:01pm, but it turns out your clock is\ntwo minutes fast, so it's actually 4:59pm.\n\nWhat should my commit do? If I insist on monotonic increases, then my\nclock gets pushed forward artificially by your fast, broken clock (which\nis probably not the end of the world; in practice, if your clock is N\nseconds fast, there will presumably be some N second period where I'm\nnot making a commit, and the clocks can \"catch up\" with each other).\n\n-Peff\n"},{"id":"144914","messageId":"AANLkTikWGzEq8wiVyu_xJ-tK92N1oRFOrawjOe9UQXkr@mail.gmail.com","threadId":"24287","inReplyTo":"20100706142921.GB6666@sigill.intra.peff.net","subject":"Re: [PATCH] guilt: Make sure the commit time is increasing","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@googlemail.com","sentAt":"2010-07-06T15:02:51Z","receivedAt":"2010-07-06T15:02:51Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Tue, Jul 6, 2010 at 4:29 PM, Jeff King <peff@peff.net> wrote:\n> On Tue, Jul 06, 2010 at 03:53:56PM +0200, Erik Faye-Lund wrote:\n>\n>> > At one point rev-list did require monotonic --- i.e., the committer\n>> > date of each commit had to be equal to or later than that of each of\n>> > its parents) with no clock skew but that was considered a bug and\n>> > fixed by v1.5.5-rc1~16 (Make revision limiting more robust against\n>> > occasional bad commit dates, 2008-03-17)\n>> >\n>>\n>> This might be a stupid question, but I'm not entirely clear on why\n>> it's not a strict requirement; surely it would be easy to ensure that\n>> the commit-time is at least as big as the parents when generating the\n>> commit...?\n>>\n>> Is it to avoid the case where a user commits with the clock set to\n>> some point (potentially far) in the future, so all subsequent commits\n>> would have the same, artificially high commit time? Or is there some\n>> other reason I can't think of?\n>\n> You can have clock skew between distributed developers. So imagine you\n> commit at 5:00pm, then I pull at 5:01pm, but it turns out your clock is\n> two minutes fast, so it's actually 4:59pm.\n>\n> What should my commit do? If I insist on monotonic increases, then my\n> clock gets pushed forward artificially by your fast, broken clock (which\n> is probably not the end of the world; in practice, if your clock is N\n> seconds fast, there will presumably be some N second period where I'm\n> not making a commit, and the clocks can \"catch up\" with each other).\n>\n\nYeah, but this doesn't really answer my question; as you're saying,\nit's probably not the end of the world, at least when the skew is low.\n\nBut I can imagine it becoming a big deal when the skew is high. The\nagain, perhaps this should constitute a \"bad commit\" and commit should\nerror out if a parent commit was more than some number of minutes\nnewer than the current time (or whatever)? That way, skewed commits\nwould be caught early if a developer is working with other people, and\na lot of the traversal could perhaps be faster (or more robust). If\nthe developer with the skewed clock doesn't work with anyone, skew\nisn't really a problem, but perhaps he'd have to do some\nbranch-filtering to un-skew commits when starting to work with others.\nAnd only if the skew is really high... like, multiple days... Which\ncan't really be THAT common?\n\nHowever, turning a technical problem that already have a solution that\nseems to work for everyone into a social one might be a bad idea. I'm\nreally just thinking out loud here :)\n\n-- \nErik \"kusma\" Faye-Lund\n"},{"id":"144915","messageId":"20100706150917.GA1558@burratino","threadId":"24287","inReplyTo":"DD1E6EE4-1196-4FCA-87DA-EB9EBCA3AC83@mit.edu","subject":"Re: [PATCH] guilt: Make sure the commit time is increasing","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-06T15:09:17Z","receivedAt":"2010-07-06T15:09:17Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Theodore Tso wrote:\n\n> You're right that it's been a while since git has run into problems with \n> mild forms of clock skew (even Debian Stable is shipping v1.5.6) but\n> I think it's better to times in the future if we can at all help it, and it's not\n> like we're talking about a lot of extra complexity to guilt to test for this.\n\nSorry, I’m a little lost.  There are five phenomena one could\nforbid:\n\n 1. Commits with timestamp equal to or before a parent\n 2. Commits with timestamp before a parent\n 3. Commits with timestamp unreasonably long before a parent\n 4. Commits with timestamp unreasonably long before an ancestor\n 5. Commits with timestamp in the future\n\nGit has always been able to cope with #5 (timestamps in the future).\nI see no reason to avoid it, except that it is hard to assign a\ntimestamp for commits on top of that one.\n\nGit’s problem today is #4 (long-term slop).  Maybe as Jeff suggested\n\"unreasonably long\" should defined per repository.  Or we could\nmeasure the kernel’s maximum (something like 120 days?) and make that\na hard limit.\n\nDo #3 a few times, and you get #4.  So ‘commit’ should warn\nabout it (where ‘unreasonably long’ could be as short as 0 or\n1 days).\n\n#2 (nonmonotonic commits) was broken in ancient git; I think it’s too\nrigid of a rule to worry about it on that account.  But a variant of\nthe rationale for avoiding #3 applies to it.\n\nI have never heard of any version of Git copying poorly with #1\n(commits with the same timestamp).  Avoiding it artificially leads\ninevitably to timestamps in the future when you somehow try to assign\n100 timestamps for the series you have rebased on top of a patch\ncommitted a few seconds ago.\n\nIncrementing the timestamp to ensure strictly monotonic commits seems\nlike a recipe for trouble to me.\n\nFor guilt, I think the best thing to do would to save a Date: line\nfor the author date with the From: and Subject: and then touch\npatches with the _current_ date when appropriate to avoid skew.\n\nHTH,\nJonathan\n"},{"id":"144921","messageId":"20100706171231.GL25518@thunk.org","threadId":"24287","inReplyTo":"20100706150917.GA1558@burratino","subject":"Re: [PATCH] guilt: Make sure the commit time is increasing","fromName":"","fromEmail":"tytso@mit.edu","sentAt":"2010-07-06T17:12:31Z","receivedAt":"2010-07-06T17:12:31Z","isPatch":true,"sender":{"key":"tytso@mit.edu","avatar":"https://avatars.githubusercontent.com/u/51416?v=4"},"body":"On Tue, Jul 06, 2010 at 10:09:17AM -0500, Jonathan Nieder wrote:\n> \n> I have never heard of any version of Git copying poorly with #1\n> (commits with the same timestamp).  Avoiding it artificially leads\n> inevitably to timestamps in the future when you somehow try to assign\n> 100 timestamps for the series you have rebased on top of a patch\n> committed a few seconds ago.\n> \n> Incrementing the timestamp to ensure strictly monotonic commits seems\n> like a recipe for trouble to me.\n\nUm, I'm guessing you spent a lot of time typing your note, but not a\nlot of time looking at my most recent patch?  My most recent patch for\nguilt simply will set the time of the patch to the current time to\navoid setting it into the future.\n\n      \t      \t\t\t\t- Ted\n"},{"id":"144922","messageId":"20100706172109.GM25518@thunk.org","threadId":"24287","inReplyTo":"AANLkTikWGzEq8wiVyu_xJ-tK92N1oRFOrawjOe9UQXkr@mail.gmail.com","subject":"Re: [PATCH] guilt: Make sure the commit time is increasing","fromName":"","fromEmail":"tytso@mit.edu","sentAt":"2010-07-06T17:21:09Z","receivedAt":"2010-07-06T17:21:09Z","isPatch":true,"sender":{"key":"tytso@mit.edu","avatar":"https://avatars.githubusercontent.com/u/51416?v=4"},"body":"On Tue, Jul 06, 2010 at 05:02:51PM +0200, Erik Faye-Lund wrote:\n> But I can imagine it becoming a big deal when the skew is high. The\n> again, perhaps this should constitute a \"bad commit\" and commit should\n> error out if a parent commit was more than some number of minutes\n> newer than the current time (or whatever)? That way, skewed commits\n> would be caught early if a developer is working with other people, and\n> a lot of the traversal could perhaps be faster (or more robust). If\n> the developer with the skewed clock doesn't work with anyone, skew\n> isn't really a problem, but perhaps he'd have to do some\n> branch-filtering to un-skew commits when starting to work with others.\n> And only if the skew is really high... like, multiple days... Which\n> can't really be THAT common?\n\nGuilt uses the modtime of the patch in a patch series for the\ncommitter time and the author time.  The reasoning behind it doing\nthis is so that you can do \"git pop -a\" followed by \"git push -a\" and\nif the patch files haven't changed, the commit id's don't change\neither.\n\nBut if you change a commit in the middle of the series, you can end up\nwith clock skews that could be several days or weeks.  Becuase of my\next4 workflow, the Linux kernel has a maximum skew of 100 days.  Mea\nculpa; I stopped doing this as soon as I was told that git was\ndepending on committer time being roughly increasing, and so I at\nleast haven't introduced any such time skews since v2.6.34.  And part\nof my making up for this has been to submit a patch to guilt to\nprevent this from happening again in the future, by fixing up guilt so\nthat it won't request \"git commit\" to create timestamps that show very\nwild clock skews within a single linear branch.\n\nWe could still get potentially screwed though.  Every so often I will\nsee someone sending e-mail from a client host whose time is years if\nnot decades in the past or in the future.  If they were to do a \"git\ncommit\", and then push that commit to a public repository, we could\neasily introduce a large clock skew into a git repo.  Has that ever\nhappened to date?  Not to my knowledge.  Could it happen?  Very\nclearly, yes.  Should we try to put in some safety checks to prevent\nit, or at least issue warnings?  Maybe.\n\n\t\t\t\t\t\t- Ted\n"},{"id":"144925","messageId":"20100706172950.GA2671@burratino","threadId":"24287","inReplyTo":"20100706171231.GL25518@thunk.org","subject":"Re: [PATCH] guilt: Make sure the commit time is increasing","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-06T17:29:50Z","receivedAt":"2010-07-06T17:29:50Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"tytso@mit.edu wrote:\n> On Tue, Jul 06, 2010 at 10:09:17AM -0500, Jonathan Nieder wrote:\n\n>> I have never heard of any version of Git copying poorly with #1\n>> (commits with the same timestamp).  Avoiding it artificially leads\n>> inevitably to timestamps in the future when you somehow try to assign\n>> 100 timestamps for the series you have rebased on top of a patch\n>> committed a few seconds ago.\n>> \n>> Incrementing the timestamp to ensure strictly monotonic commits seems\n>> like a recipe for trouble to me.\n>\n> Um, I'm guessing you spent a lot of time typing your note, but not a\n> lot of time looking at my most recent patch?  My most recent patch for\n> guilt simply will set the time of the patch to the current time to\n> avoid setting it into the future.\n\nI read it, and I did not like that specific part.  I even responded to it.\n\nI guess \"leads inevitably to timestamps in the future\" was a poor\nchoice of words, though.\n"},{"id":"144924","messageId":"20100706172954.GD18795@josefsipek.net","threadId":"24287","inReplyTo":"20100705192201.GI25518@thunk.org","subject":"Re: [PATCH] guilt: Make sure the commit time is increasing","fromName":"","fromEmail":"jeffpc@josefsipek.net","sentAt":"2010-07-06T17:29:54Z","receivedAt":"2010-07-06T17:29:54Z","isPatch":true,"sender":{"key":"jeffpc@josefsipek.net","avatar":null},"body":"On Mon, Jul 05, 2010 at 03:22:01PM -0400, tytso@mit.edu wrote:\n...\n\nI'm going to play with this patch locally a little bit, but I'm all for it.\n\n> From d5659084435a885e05a8fc9afbffe8cdd9535828 Mon Sep 17 00:00:00 2001\n\nSpeaking of weird timestamps...2001?  Where did that come from? :)\n\n> From: Theodore Ts'o <tytso@mit.edu>\n> Date: Sun, 4 Jul 2010 22:06:08 -0400\n> Subject: [PATCH] guilt: Make sure the commit time is increasing\n> \n> Git has various algorithms, most notably in git rev-list, git\n> name-rev, and others, which depend on the commit time increasing.  We\n> want to keep the commit time the same as much as possible, but if\n> necessary, adjust the time stamps of the patch files to obey this\n> constraint.\n> \n> Signed-off-by: \"Theodore Ts'o\" <tytso@mit.edu>\n> ---\n>  guilt |   11 +++++++++++\n>  1 files changed, 11 insertions(+), 0 deletions(-)\n> \n> diff --git a/guilt b/guilt\n> index b6e2a6c..edcfb34 100755\n> --- a/guilt\n> +++ b/guilt\n> @@ -535,6 +535,17 @@ commit()\n>                          export GIT_AUTHOR_EMAIL=\"`echo $author_str | sed -e 's/[^<]*//'`\"\n>  \t\tfi\n>  \n> +\t\tct=$(git log -1 --pretty=%ct)\n> +\t\tif [ $ct -gt $(stat -c %Y \"$p\") ]; then\n> +\t\t    echo \"Adjusting mod time of\" $(basename \"$p\")\n\nDepending on how my playing goes, I might remove the echo.\n\n> +\t\t    ct=$(expr $ct + 60)\n\nSo, ct is now the +1min time.\n\n> +\t\t    if [ $ct -gt $(date +%s) ]; then\n> +\t\t\ttouch \"$p\"\n> +\t\t    else\n> +\t\t\ttouch -d @$(expr $ct + 60) \"$p\"\n\nAnd we're touching +1+1min.  I'll fix it up before applying.\n\nThanks,\n\nJeff.\n\n-- \nNT is to UNIX what a doughnut is to a particle accelerator.\n"},{"id":"144946","messageId":"20100706185745.GB26677@thunk.org","threadId":"24287","inReplyTo":"20100706172954.GD18795@josefsipek.net","subject":"Re: [PATCH] guilt: Make sure the commit time is increasing","fromName":"","fromEmail":"tytso@mit.edu","sentAt":"2010-07-06T18:57:45Z","receivedAt":"2010-07-06T18:57:45Z","isPatch":true,"sender":{"key":"tytso@mit.edu","avatar":"https://avatars.githubusercontent.com/u/51416?v=4"},"body":"On Tue, Jul 06, 2010 at 01:29:54PM -0400, jeffpc@josefsipek.net wrote:\n> \n> And we're touching +1+1min.  I'll fix it up before applying.\n> \n\nYup, I forgot to take the \"+ 60\" out.  Thanks for catching that.\n\n\t\t\t\t\t- Ted\n"},{"id":"145504","messageId":"20100714030117.GA8658@maat.home","threadId":"24287","inReplyTo":"20100705192201.GI25518@thunk.org","subject":"Re: [PATCH] guilt: Make sure the commit time is increasing","fromName":"Josef 'Jeff' Sipek","fromEmail":"jeffpc@josefsipek.net","sentAt":"2010-07-14T03:01:17Z","receivedAt":"2010-07-14T03:01:17Z","isPatch":true,"sender":{"key":"jeffpc@josefsipek.net","avatar":null},"body":"FWIW, I pushed the change out.  I think this is a major enough fix that I'll\ncut a new release soon.\n\nThanks!\n\nJeff.\n\n-- \nThe obvious mathematical breakthrough would be development of an easy way to\nfactor large prime numbers.\n\t\t- Bill Gates, The Road Ahead, pg. 265\n"}]}