login
Header Space

 
 

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

Previous thread: HFS+ Unicode weirdness by Wincent Colaiuta on Tuesday, September 4, 2007 - 8:30 am. (4 messages)

Next thread: [PATCH] Functions for updating refs. by Carlos Rica on Tuesday, September 4, 2007 - 9:39 am. (10 messages)
To: <git@...>
Cc: Jonas Berlin <xkr47@...>
Date: Tuesday, September 4, 2007 - 8:31 am

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 &lt;xkr47@outerspace.dyndns.org&gt;
---
    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-&gt;{revision}\n";
             # reformat the date for log output
-            $revision-&gt;{modified} = sprintf('%04d/%02d/%02d %s', $3, $DATE_LIST-&gt;{$2}, $1, $4 ) if ( $revision-&gt;{modified} =~ /(\d+)\s+(\w+)\s+(\d+)\s+(\S+)/ and defined($DATE_LIST-&gt;{$2}) );
+            if ( $revision-&gt;{modified} =~ /(\d+)\s+(\w+)\s+(\d+)\s+(\d\d):(\d\d):(\d\d) ([-+])(\d\d)(\d\d)/ and defined($DATE_LIST-&gt;{$2}) )
+            {
+                my $off = $8 * 3600 + $9 * 60;
+                my $time = timegm($6, $5, $4, $1, $DATE_LIST-&gt;{$2}-1, $3 - 1900);
+                $off = -$off if ( $7 eq "-" );
+                $time -= $off;
+                my ( $sec, $min, $hour, $mday, $mon, $year ) = gmtime($time);
+                $revision-&gt;{modified} = sprintf('%04d/%02d/%02d %02d...
To: Jonas Berlin <xkr47@...>
Cc: <git@...>
Date: Tuesday, September 4, 2007 - 9:22 am

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
-
To: Linus Torvalds <torvalds@...>
Cc: <git@...>
Date: Wednesday, September 5, 2007 - 5:32 pm

I agree.

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



I will make sure this works as well.

-- 
- xkr47
-
To: Linus Torvalds <torvalds@...>
Cc: <git@...>
Date: Wednesday, September 5, 2007 - 5:45 pm

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

-- 
- xkr47
-
Previous thread: HFS+ Unicode weirdness by Wincent Colaiuta on Tuesday, September 4, 2007 - 8:30 am. (4 messages)

Next thread: [PATCH] Functions for updating refs. by Carlos Rica on Tuesday, September 4, 2007 - 9:39 am. (10 messages)
speck-geostationary