[PATCH] git-svn: Fix time zone in --localtime

[PATCH] git-svn: Fix time zone in --localtime

["Wei-Yin Chen (陳威尹)"]

Use numerical form of time zone to replace alphabetic time zone
abbreviation generated by "%Z". "%Z" is not portable and contain
ambiguity for many areas. For example, CST could be "Central
Standard Time" (GMT-0600) and "China Standard Time" (GMT+0800).
Alphabetic time zone abbreviation is meant for human readability,
not for specifying a time zone for machines.

Failed case can be illustrated like this in linux shell:
  > echo $TZ
  Asia/Taipei
  > date +%Z
  CST
  > env TZ=`date +%Z` date
  Mon Dec 19 06:03:04 CST 2011
  > date
  Mon Dec 19 14:03:04 CST 2011

Signed-off-by: Wei-Yin Chen (陳威尹) <chen.weiyin@gmail.com>
---
 git-svn.perl |   18 +++++++++++-------
 1 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index e30df22..f0b6340 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -2029,6 +2029,7 @@ use Carp qw/croak/;
 use File::Path qw/mkpath/;
 use File::Copy qw/copy/;
 use IPC::Open3;
+use Time::Local;
 use Memoize;  # core since 5.8.0, Jul 2002
 use Memoize::Storable;

@@ -3287,6 +3288,14 @@ sub get_untracked {
 	\@out;
 }

+sub get_tz {
+	# some systmes don't handle or mishandle %z, so be creative.
+	my $t = shift || time;
+	my $gm = timelocal(gmtime($t));
+	my $sign = qw( + + - )[ $t <=> $gm ];
+	return sprintf("%s%02d%02d", $sign, (gmtime(abs($t - $gm)))[2,1]);
+}
+
 # parse_svn_date(DATE)
 # --------------------
 # Given a date (in UTC) from Subversion, return a string in the format
@@ -3319,8 +3328,7 @@ sub parse_svn_date {
 			delete $ENV{TZ};
 		}

-		my $our_TZ =
-		    POSIX::strftime('%Z', $S, $M, $H, $d, $m - 1, $Y - 1900);
+		my $our_TZ = get_tz();

 		# This converts $epoch_in_UTC into our local timezone.
 		my ($sec, $min, $hour, $mday, $mon, $year,
@@ -5994,7 +6002,6 @@ package Git::SVN::Log;
 use strict;
 use warnings;
 use POSIX qw/strftime/;
-use Time::Local;
 use constant commit_log_separator => ('-' x 72) . "\n";
 use vars qw/$TZ $limit $color $pager $non_recursive $verbose $oneline
             %rusers $show_commit $incremental/;
@@ -6104,11 +6111,8 @@ sub run_pager {
 }

 sub format_svn_date {
-	# some systmes don't handle or mishandle %z, so be creative.
 	my $t = shift || time;
-	my $gm = timelocal(gmtime($t));
-	my $sign = qw( + + - )[ $t <=> $gm ];
-	my $gmoff = sprintf("%s%02d%02d", $sign, (gmtime(abs($t - $gm)))[2,1]);
+	my $gmoff = get_tz($t);
 	return strftime("%Y-%m-%d %H:%M:%S $gmoff (%a, %d %b %Y)", localtime($t));
 }

Re: [PATCH] git-svn: Fix time zone in --localtime

[Eric Wong]

"\"Wei-Yin Chen (陳威尹)\"" <chen.weiyin@gmail.com> wrote:
> Use numerical form of time zone to replace alphabetic time zone
> abbreviation generated by "%Z". "%Z" is not portable and contain
> ambiguity for many areas. For example, CST could be "Central
> Standard Time" (GMT-0600) and "China Standard Time" (GMT+0800).
> Alphabetic time zone abbreviation is meant for human readability,
> not for specifying a time zone for machines.
> 
> Failed case can be illustrated like this in linux shell:
>   > echo $TZ
>   Asia/Taipei
>   > date +%Z
>   CST
>   > env TZ=`date +%Z` date
>   Mon Dec 19 06:03:04 CST 2011
>   > date
>   Mon Dec 19 14:03:04 CST 2011
> 
> Signed-off-by: Wei-Yin Chen (陳威尹) <chen.weiyin@gmail.com>
> ---

Hi, sorry for the late reply, this got lost in the shuffle.

This issue seems reasonable, but did you test this change at all?

>  sub format_svn_date {
> -	# some systmes don't handle or mishandle %z, so be creative.
>  	my $t = shift || time;
> -	my $gm = timelocal(gmtime($t));
> -	my $sign = qw( + + - )[ $t <=> $gm ];
> -	my $gmoff = sprintf("%s%02d%02d", $sign, (gmtime(abs($t - $gm)))[2,1]);
> +	my $gmoff = get_tz($t);
>  	return strftime("%Y-%m-%d %H:%M:%S $gmoff (%a, %d %b %Y)", localtime($t));
>  }

I needed the following additional change to get things working:

--- a/git-svn.perl
+++ b/git-svn.perl
@@ -6111,7 +6111,7 @@ sub run_pager {
 
 sub format_svn_date {
 	my $t = shift || time;
-	my $gmoff = get_tz($t);
+	my $gmoff = Git::SVN::get_tz($t);
 	return strftime("%Y-%m-%d %H:%M:%S $gmoff (%a, %d %b %Y)", localtime($t));
 }

If there are no objections, I'll squash this and push for Junio
tomorrow.

Re: [PATCH] git-svn: Fix time zone in --localtime

["Wei-Yin Chen (陳威尹)"]

On 2012-02-12 16:15, Eric Wong wrote:
> I needed the following additional change to get things working:
>
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -6111,7 +6111,7 @@ sub run_pager {
>
>   sub format_svn_date {
>   	my $t = shift || time;
> -	my $gmoff = get_tz($t);
> +	my $gmoff = Git::SVN::get_tz($t);
>   	return strftime("%Y-%m-%d %H:%M:%S $gmoff (%a, %d %b %Y)", localtime($t));
>   }

Sorry, my bad. This bug of missing package reference can be triggered by 
"git svn log", but unfortunately I didn't test this part manually. I'll 
learn to use the git test suite next time.

>
> If there are no objections, I'll squash this and push for Junio
> tomorrow.

Excellent. Thank you for the testing and fixing effort!