[PATCH] Fix "cvs log" to use UTC timezone instead of local

[PATCH] Fix "cvs log" to use UTC timezone instead of local

[Jonas Berlin]

The timestamp format used in "cvs log" output does not include a
timezone, and must thus be in UTC timezone. The timestamps from git on
the other hand contain timezone information for each commit timestamp,
but git-cvsserver discarded this information and used the timestamps
without adjusting the time accordingly. The patch adds code to apply
the timezone offset to produce a UTC timestamp.

Signed-off-by: Jonas Berlin <xkr47@outerspace.dyndns.org>
---
    Could it perhaps be that git previously reported timestamps in UTC
    instead of including a timezone?

 git-cvsserver.perl              |   11 ++++++++++-
 t/t9400-git-cvsserver-server.sh |   24 ++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 1 deletions(-)

diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 13dbd27..5ae9933 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -23,6 +23,7 @@ use Fcntl;
 use File::Temp qw/tempdir tempfile/;
 use File::Basename;
 use Getopt::Long qw(:config require_order no_ignore_case);
+use Time::Local;
 
 my $VERSION = '@@GIT_VERSION@@';
 
@@ -1686,7 +1687,15 @@ sub req_log
             print "M ----------------------------\n";
             print "M revision 1.$revision->{revision}\n";
             # reformat the date for log output
-            $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}) );
+            if ( $revision->{modified} =~ /(\d+)\s+(\w+)\s+(\d+)\s+(\d\d):(\d\d):(\d\d) ([-+])(\d\d)(\d\d)/ and defined($DATE_LIST->{$2}) )
+            {
+                my $off = $8 * 3600 + $9 * 60;
+                my $time = timegm($6, $5, $4, $1, $DATE_LIST->{$2}-1, $3 - 1900);
+                $off = -$off if ( $7 eq "-" );
+                $time -= $off;
+                my ( $sec, $min, $hour, $mday, $mon, $year ) = gmtime($time);
+                $revision->{modified} = sprintf('%04d/%02d/%02d %02d:%02d:%02d', $year + 1900, $mon + 1, $mday, $hour, $min, $sec);
+            }
             $revision->{author} =~ s/\s+.*//;
             $revision->{author} =~ s/^(.{8}).*/$1/;
             print "M date: $revision->{modified};  author: $revision->{author};  state: " . ( $revision->{filehash} eq "deleted" ? "dead" : "Exp" ) . ";  lines: +2 -3\n";
diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index 641303e..254eab7 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -405,4 +405,28 @@ test_expect_success 'cvs update (merge no-op)' \
     GIT_CONFIG="$git_config" cvs -Q update &&
     diff -q merge ../merge'
 
+#------------
+# CVS LOG
+#------------
+
+cd "$WORKDIR"
+test_expect_success 'cvs log (check that timestamps are in UTC)' \
+  'echo stamp > stamp &&
+   git add stamp &&
+   TZ=GMT-01 git commit -q -m "Add stamp" &&
+   git push gitcvs.git >/dev/null &&
+   GIT_STAMP=$(git-show --pretty=format:%ct --name-only stamp | grep -v stamp) &&
+   [ "$GIT_STAMP" ] &&
+   cd cvswork &&
+   GIT_CONFIG="$git_config" cvs -Q update &&
+   CVS_STAMP=$(GIT_CONFIG="$git_config" cvs log stamp | perl -e '\''
+      use Time::Local;
+      while(<>) {
+        last if(/^date:/);
+      }
+      my ($dummy,$y,$m,$d,$H,$M,$S) = split(m!\D+!);
+      print timegm($S,$M,$H,$d,$m-1,$y-1900);
+   '\'') &&
+   test "$CVS_STAMP" = "$GIT_STAMP"'
+
 test_done
-- 
1.5.1.6

Re: [PATCH] Fix "cvs log" to use UTC timezone instead of local

[Linus Torvalds]

On Tue, 4 Sep 2007, Jonas Berlin wrote:
>
> The timestamp format used in "cvs log" output does not include a
> timezone, and must thus be in UTC timezone. The timestamps from git on
> the other hand contain timezone information for each commit timestamp,
> but git-cvsserver discarded this information and used the timestamps
> without adjusting the time accordingly. The patch adds code to apply
> the timezone offset to produce a UTC timestamp.

I think this is wrong.

Git *internally* stores things in UTC anyway, so if there are any local 
date format things, it's because git-cvsserver.perl has read the dates 
using some format where git has turned its internal date into a local 
date.

So instead of turning it back into UTC here, I think git-cvsserver should 
be changed to ask for the date in the native git format in the first 
place.

That can be done various ways:

 - use the "raw log format" which has dates as seconds-since-UTC (and with 
   an *informational* timezone thing that should then just be ignored).

   This is likely the best approach, since anything but this will 
   almost invariably result in some potentially broken TZ conversion
   back-and-forth..

 - if it really wants to use the pretty-printing support, git-cvsserver 
   should probably be changed to do something like

	TZ=UTC git rev-list --pretty --date=local

   which will pretty-print the date in local time format rather than in 
   the timezone that the commit was done in, and then the TZ=UTC obviously 
   says that the "local" zone is UTC.

Anything else *will* be broken, or will be converting back-and-forth.

For example, I think your patch may fix "cvs log", but I'm seeing some 
suspiciously similar code in the "cvs annotate" handling, so I suspect 
that would need it too.

If instead of trying to convert things to UTC on demand, git-cvsserver 
just asks for the git date stamps in UTC in the first place, none of the 
places should ever need any timezone conversion.

		Linus

Re: [PATCH] Fix "cvs log" to use UTC timezone instead of local

[Jonas Berlin]

Quoting Linus Torvalds on 09/04/2007 01:22 PM UTC:
> So instead of turning it back into UTC here, I think git-cvsserver should 
> be changed to ask for the date in the native git format in the first 
> place.

I agree.

My first patch was a minimal-intrusion one to avoid unnecessarily breaking stuff.

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 :)

> That can be done various ways:
> 
>  - use the "raw log format" which has dates as seconds-since-UTC (and with 
>    an *informational* timezone thing that should then just be ignored).
> 
>    This is likely the best approach, since anything but this will 

This seems straightforward to implement, so I will go with this.

> For example, I think your patch may fix "cvs log", but I'm seeing some 
> suspiciously similar code in the "cvs annotate" handling, so I suspect 
> that would need it too.

I will make sure this works as well.

-- 
- xkr47

Re: [PATCH] Fix "cvs log" to use UTC timezone instead of local

[Jonas Berlin]

Quoting Jonas Berlin on 09/05/2007 09:32 PM UTC:
> Quoting Linus Torvalds on 09/04/2007 01:22 PM UTC:
>> That can be done various ways:
>>
>>  - use the "raw log format" which has dates as seconds-since-UTC (and with 
>>    an *informational* timezone thing that should then just be ignored).
>>
>>    This is likely the best approach, since anything but this will 
> 
> This seems straightforward to implement, so I will go with this.

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

> 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 :)

Another option would be to scrap support for old cvs clients.. I could investigate when the new format was introduced..

-- 
- xkr47