{"thread":{"id":"9772","subject":"[PATCH] Fix \"cvs log\" to use UTC timezone instead of local","startedAt":"2007-09-04T12:31:33Z","lastAt":"2007-09-05T21:45:21Z","messageCount":4,"participants":["Jonas Berlin","Linus Torvalds"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"52429","messageId":"11889090932256-git-send-email-xkr47@outerspace.dyndns.org","threadId":"9772","inReplyTo":null,"subject":"[PATCH] Fix \"cvs log\" to use UTC timezone instead of local","fromName":"Jonas Berlin","fromEmail":"xkr47@outerspace.dyndns.org","sentAt":"2007-09-04T12:31:33Z","receivedAt":"2007-09-04T12:31:33Z","isPatch":true,"sender":{"key":"xkr47@outerspace.dyndns.org","avatar":null},"body":"The timestamp format used in \"cvs log\" output does not include a\ntimezone, and must thus be in UTC timezone. The timestamps from git on\nthe other hand contain timezone information for each commit timestamp,\nbut git-cvsserver discarded this information and used the timestamps\nwithout adjusting the time accordingly. The patch adds code to apply\nthe timezone offset to produce a UTC timestamp.\n\nSigned-off-by: Jonas Berlin <xkr47@outerspace.dyndns.org>\n---\n    Could it perhaps be that git previously reported timestamps in UTC\n    instead of including a timezone?\n\n git-cvsserver.perl              |   11 ++++++++++-\n t/t9400-git-cvsserver-server.sh |   24 ++++++++++++++++++++++++\n 2 files changed, 34 insertions(+), 1 deletions(-)\n\ndiff --git a/git-cvsserver.perl b/git-cvsserver.perl\nindex 13dbd27..5ae9933 100755\n--- a/git-cvsserver.perl\n+++ b/git-cvsserver.perl\n@@ -23,6 +23,7 @@ use Fcntl;\n use File::Temp qw/tempdir tempfile/;\n use File::Basename;\n use Getopt::Long qw(:config require_order no_ignore_case);\n+use Time::Local;\n \n my $VERSION = '@@GIT_VERSION@@';\n \n@@ -1686,7 +1687,15 @@ sub req_log\n             print \"M ----------------------------\\n\";\n             print \"M revision 1.$revision->{revision}\\n\";\n             # reformat the date for log output\n-            $revision->{modified} = sprintf('%04d/%02d/%02d %s', $3, $DATE_LIST->{$2}, $1, $4 ) if ( $revision->{modified} =~ /(\\d+)\\s+(\\w+)\\s+(\\d+)\\s+(\\S+)/ and defined($DATE_LIST->{$2}) );\n+            if ( $revision->{modified} =~ /(\\d+)\\s+(\\w+)\\s+(\\d+)\\s+(\\d\\d):(\\d\\d):(\\d\\d) ([-+])(\\d\\d)(\\d\\d)/ and defined($DATE_LIST->{$2}) )\n+            {\n+                my $off = $8 * 3600 + $9 * 60;\n+                my $time = timegm($6, $5, $4, $1, $DATE_LIST->{$2}-1, $3 - 1900);\n+                $off = -$off if ( $7 eq \"-\" );\n+                $time -= $off;\n+                my ( $sec, $min, $hour, $mday, $mon, $year ) = gmtime($time);\n+                $revision->{modified} = sprintf('%04d/%02d/%02d %02d:%02d:%02d', $year + 1900, $mon + 1, $mday, $hour, $min, $sec);\n+            }\n             $revision->{author} =~ s/\\s+.*//;\n             $revision->{author} =~ s/^(.{8}).*/$1/;\n             print \"M date: $revision->{modified};  author: $revision->{author};  state: \" . ( $revision->{filehash} eq \"deleted\" ? \"dead\" : \"Exp\" ) . \";  lines: +2 -3\\n\";\ndiff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh\nindex 641303e..254eab7 100755\n--- a/t/t9400-git-cvsserver-server.sh\n+++ b/t/t9400-git-cvsserver-server.sh\n@@ -405,4 +405,28 @@ test_expect_success 'cvs update (merge no-op)' \\\n     GIT_CONFIG=\"$git_config\" cvs -Q update &&\n     diff -q merge ../merge'\n \n+#------------\n+# CVS LOG\n+#------------\n+\n+cd \"$WORKDIR\"\n+test_expect_success 'cvs log (check that timestamps are in UTC)' \\\n+  'echo stamp > stamp &&\n+   git add stamp &&\n+   TZ=GMT-01 git commit -q -m \"Add stamp\" &&\n+   git push gitcvs.git >/dev/null &&\n+   GIT_STAMP=$(git-show --pretty=format:%ct --name-only stamp | grep -v stamp) &&\n+   [ \"$GIT_STAMP\" ] &&\n+   cd cvswork &&\n+   GIT_CONFIG=\"$git_config\" cvs -Q update &&\n+   CVS_STAMP=$(GIT_CONFIG=\"$git_config\" cvs log stamp | perl -e '\\''\n+      use Time::Local;\n+      while(<>) {\n+        last if(/^date:/);\n+      }\n+      my ($dummy,$y,$m,$d,$H,$M,$S) = split(m!\\D+!);\n+      print timegm($S,$M,$H,$d,$m-1,$y-1900);\n+   '\\'') &&\n+   test \"$CVS_STAMP\" = \"$GIT_STAMP\"'\n+\n test_done\n-- \n1.5.1.6\n"},{"id":"52435","messageId":"alpine.LFD.0.999.0709040612260.3088@evo.linux-foundation.org","threadId":"9772","inReplyTo":"11889090932256-git-send-email-xkr47@outerspace.dyndns.org","subject":"Re: [PATCH] Fix \"cvs log\" to use UTC timezone instead of local","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-09-04T13:22:25Z","receivedAt":"2007-09-04T13:22:25Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 4 Sep 2007, Jonas Berlin wrote:\n>\n> The timestamp format used in \"cvs log\" output does not include a\n> timezone, and must thus be in UTC timezone. The timestamps from git on\n> the other hand contain timezone information for each commit timestamp,\n> but git-cvsserver discarded this information and used the timestamps\n> without adjusting the time accordingly. The patch adds code to apply\n> the timezone offset to produce a UTC timestamp.\n\nI think this is wrong.\n\nGit *internally* stores things in UTC anyway, so if there are any local \ndate format things, it's because git-cvsserver.perl has read the dates \nusing some format where git has turned its internal date into a local \ndate.\n\nSo instead of turning it back into UTC here, I think git-cvsserver should \nbe changed to ask for the date in the native git format in the first \nplace.\n\nThat can be done various ways:\n\n - use the \"raw log format\" which has dates as seconds-since-UTC (and with \n   an *informational* timezone thing that should then just be ignored).\n\n   This is likely the best approach, since anything but this will \n   almost invariably result in some potentially broken TZ conversion\n   back-and-forth..\n\n - if it really wants to use the pretty-printing support, git-cvsserver \n   should probably be changed to do something like\n\n\tTZ=UTC git rev-list --pretty --date=local\n\n   which will pretty-print the date in local time format rather than in \n   the timezone that the commit was done in, and then the TZ=UTC obviously \n   says that the \"local\" zone is UTC.\n\nAnything else *will* be broken, or will be converting back-and-forth.\n\nFor example, I think your patch may fix \"cvs log\", but I'm seeing some \nsuspiciously similar code in the \"cvs annotate\" handling, so I suspect \nthat would need it too.\n\nIf instead of trying to convert things to UTC on demand, git-cvsserver \njust asks for the git date stamps in UTC in the first place, none of the \nplaces should ever need any timezone conversion.\n\n\t\tLinus\n"},{"id":"52638","messageId":"46DF2058.7060405@outerspace.dyndns.org","threadId":"9772","inReplyTo":"alpine.LFD.0.999.0709040612260.3088@evo.linux-foundation.org","subject":"Re: [PATCH] Fix \"cvs log\" to use UTC timezone instead of local","fromName":"Jonas Berlin","fromEmail":"xkr47@outerspace.dyndns.org","sentAt":"2007-09-05T21:32:08Z","receivedAt":"2007-09-05T21:32:08Z","isPatch":true,"sender":{"key":"xkr47@outerspace.dyndns.org","avatar":null},"body":"Quoting Linus Torvalds on 09/04/2007 01:22 PM UTC:\n> So instead of turning it back into UTC here, I think git-cvsserver should \n> be changed to ask for the date in the native git format in the first \n> place.\n\nI agree.\n\nMy first patch was a minimal-intrusion one to avoid unnecessarily breaking stuff.\n\nI guess at this point it's good to mention that current cvs implementations (at least 1.12.12) produce timestamps of format \"yyyy-mm-dd HH:MM:SS +ZZZZ\" (i.e. they do include timezone information) while older versions (at least 1.11.22) produce the UTC-only format \"yyyy/mm/dd HH:MM:SS\" which is currently used by git-cvsserver. Backwards compatibility generally being a good thing, while at the expense of timezone information, I chose to keep the older UTC-only format. Should you prefer to keep the timezone information, I'll update the cvs log format instead. Heck, I could even support both through some configuration option if you really wanted :)\n\n> That can be done various ways:\n> \n>  - use the \"raw log format\" which has dates as seconds-since-UTC (and with \n>    an *informational* timezone thing that should then just be ignored).\n> \n>    This is likely the best approach, since anything but this will \n\nThis seems straightforward to implement, so I will go with this.\n\n> For example, I think your patch may fix \"cvs log\", but I'm seeing some \n> suspiciously similar code in the \"cvs annotate\" handling, so I suspect \n> that would need it too.\n\nI will make sure this works as well.\n\n-- \n- xkr47\n"},{"id":"52639","messageId":"46DF2371.5030603@outerspace.dyndns.org","threadId":"9772","inReplyTo":"46DF2058.7060405@outerspace.dyndns.org","subject":"Re: [PATCH] Fix \"cvs log\" to use UTC timezone instead of local","fromName":"Jonas Berlin","fromEmail":"xkr47@outerspace.dyndns.org","sentAt":"2007-09-05T21:45:21Z","receivedAt":"2007-09-05T21:45:21Z","isPatch":true,"sender":{"key":"xkr47@outerspace.dyndns.org","avatar":null},"body":"Quoting Jonas Berlin on 09/05/2007 09:32 PM UTC:\n> Quoting Linus Torvalds on 09/04/2007 01:22 PM UTC:\n>> That can be done various ways:\n>>\n>>  - use the \"raw log format\" which has dates as seconds-since-UTC (and with \n>>    an *informational* timezone thing that should then just be ignored).\n>>\n>>    This is likely the best approach, since anything but this will \n> \n> This seems straightforward to implement, so I will go with this.\n\nI just realized that since git-cvsserver creates a SQLite database (assumably for keeping track of what cvs revision numbers map to which git commits) AND the \"timezonized\" timestamps are stored as strings in there, switching to UTC timestamps would either break current SQLite databases or then require backwards compatibility code to handle the pretty-printed timestamp (with the UTC unrolling code from the patch I sent).\n\n> I guess at this point it's good to mention that current cvs implementations (at least 1.12.12) produce timestamps of format \"yyyy-mm-dd HH:MM:SS +ZZZZ\" (i.e. they do include timezone information) while older versions (at least 1.11.22) produce the UTC-only format \"yyyy/mm/dd HH:MM:SS\" which is currently used by git-cvsserver. Backwards compatibility generally being a good thing, while at the expense of timezone information, I chose to keep the older UTC-only format. Should you prefer to keep the timezone information, I'll update the cvs log format instead. Heck, I could even support both through some configuration option if you really wanted :)\n\nAnother option would be to scrap support for old cvs clients.. I could investigate when the new format was introduced..\n\n-- \n- xkr47\n"}]}