Strange date format in git-send-email

Strange date format in git-send-email

[Martijn Kuipers]

Dear list,

My email is sorted by date and a number of email-patches list the date format 
as unknown (in kmail, that is).

When checking the headers of these mails I see the following pattern:

Date: mer, 05 jui 2006 00:36:08 +0200
X-Mailer: git-send-email 1.4.1

Date: wto, 20 cze 2006 17:59:19 +0200
X-Mailer: git-send-email 1.3.0

Date: Tue, Mar 14 12:12:35 2006 -0500
User-Agent: send_patch 0.1

Date: Tue Feb 7 18:21:02 2006 +0100


Some of these are quite old, and it has been sometime since I last noticed it 
(I actually thought it was fixed). But today I received another patch-email 
from the list ([PATCH] Beautifulise git-show output), which showed the same 
problem.

Is this my problem (or kmail), or is there something funny with 
git-send-email? I just don't git it.

If you need more info, just holler.

Kind regards,
Martijn
----------------------------------------------------------------------------------------------
Actually, did actually notice actually how many times non-native English 
speaker actually use actually?

Re: Strange date format in git-send-email

[Jakub Narebski]

Martijn Kuipers wrote:

> My email is sorted by date and a number of email-patches list the date format 
> as unknown (in kmail, that is).
> 
> When checking the headers of these mails I see the following pattern:
> 
> Date: mer, 05 jui 2006 00:36:08 +0200
> X-Mailer: git-send-email 1.4.1
> 
> Date: wto, 20 cze 2006 17:59:19 +0200
> X-Mailer: git-send-email 1.3.0
> 
> Date: Tue, Mar 14 12:12:35 2006 -0500
> User-Agent: send_patch 0.1
> 
> Date: Tue Feb 7 18:21:02 2006 +0100
[...]
> Is this my problem (or kmail), or is there something funny with 
> git-send-email? I just don't git it.

Yes, there was a problem with git-sen-email, namely it used strftime 
to print "Date:" header, but strftime is locale specific setting 
$ENV{LC_ALL} = 'C' is not enought.

There were two patches on the list: one hacky adding setlocale 
call (not applied), second implementing rfc-822 date in Perl.
Should be in current.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: Strange date format in git-send-email

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> There were two patches on the list: one hacky adding setlocale 
> call (not applied), second implementing rfc-822 date in Perl.
> Should be in current.

What do you mean by "should be in current"?

I specifically asked you about this issue a few days ago, even
with a patch to test out for people who are having problems, but
haven't heard back anything yet.  The issue is still on hold and
not in "current" as far as I am concerned.

    To: Jakub Narebski <jnareb@gmail.com>
    Cc: git@vger.kernel.org, Eric Wong <normalperson@yhbt.net>
    From: Junio C Hamano <junkio@cox.net>
    Subject: Re: [PATCH] send-email: Use setlocale in addition to
        $ENV{LC_ALL} to set locale
    Date: Sun, 02 Jul 2006 19:49:50 -0700
    Message-ID: <7vd5cnv1v5.fsf@assigned-by-dhcp.cox.net>

    I was reviewing old log and noticed this topic has never been
    resolved.  Your proposal was to use POSIX::setlocale(), and
    Eric's counter-proposal was to mimic what 822-date script does,
    doing it by hand without mucking with locales, and the
    discussion seemed to have died there.  Does this still need to
    be addressed?

    My gut feeling is that it would probably be less problematic if
    we do not muck with locales at all (so drop POSIX::strftime as
    well).

    So maybe something like this (totally untested)?
    ...

[PATCH] do not use locale specific strftime when preparing 2822 date

[Jakub Narebski]

Signed-off-by: Junio C Hamano <junkio@cox.net>
Acked-by: Jakub Narebski <jnareb@gmail.com>
---
This patch is sent from patched version of git-send-email.perl
with git tools 1.3.0. This patch is generated from current master
after Junio's patch 
  Message-ID: <7vd5cnv1v5.fsf@assigned-by-dhcp.cox.net>
applied.

 git-send-email.perl |   41 ++++++++++++++++++++++++++++++++++++++---
 1 files changed, 38 insertions(+), 3 deletions(-)

f00ff14faf86c376d0ffb3cef24d2e5a5437dfcf
diff --git a/git-send-email.perl b/git-send-email.perl
index b04b8f4..c9c1975 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -34,8 +34,43 @@ sub readline {
 package main;
 
 # most mail servers generate the Date: header, but not all...
-$ENV{LC_ALL} = 'C';
-use POSIX qw/strftime/;
+sub format_2822_time {
+	my ($time) = @_;
+	my @localtm = localtime($time);
+	my @gmttm = gmtime($time);
+	my $localmin = $localtm[1] + $localtm[2] * 60;
+	my $gmtmin = $gmttm[1] + $gmttm[2] * 60;
+	if ($localtm[0] != $gmttm[0]) {
+		die "local zone differs from GMT by a non-minute interval\n";
+	}
+	if ((($gmttm[6] + 1) % 7) == $localtm[6]) {
+		$localmin += 1440;
+	} elsif ((($gmttm[6] - 1) % 7) == $localtm[6]) {
+		$localmin -= 1440;
+	} elsif ($gmttm[6] != $localtm[6]) {
+		die "local time offset greater than or equal to 24 hours\n";
+	}
+	my $offset = $localmin - $gmtmin;
+	my $offhour = $offset / 60;
+	my $offmin = abs($offset % 60);
+	if (abs($offhour) >= 24) {
+		die ("local time offset greater than or equal to 24 hours\n");
+	}
+
+	return sprintf("%s, %2d %s %d %02d:%02d:%02d %s%02d%02d",
+		       qw(Sun Mon Tue Wed Thu Fri Sat)[$localtm[6]],
+		       $localtm[3],
+		       qw(Jan Feb Mar Apr May Jun
+			  Jul Aug Sep Oct Nov Dec)[$localtm[4]],
+		       $localtm[5]+1900,
+		       $localtm[2],
+		       $localtm[1],
+		       $localtm[0],
+		       ($offset >= 0) ? '+' : '-',
+		       abs($offhour),
+		       $offmin,
+		       );
+}
 
 my $have_email_valid = eval { require Email::Valid; 1 };
 my $smtp;
@@ -387,7 +422,7 @@ sub send_message
 	my @recipients = unique_email_list(@to);
 	my $to = join (",\n\t", @recipients);
 	@recipients = unique_email_list(@recipients,@cc,@bcclist);
-	my $date = strftime('%a, %d %b %Y %H:%M:%S %z', localtime($time++));
+	my $date = format_2822_time($time++);
 	my $gitversion = '@@GIT_VERSION@@';
 	if ($gitversion =~ m/..GIT_VERSION../) {
 	    $gitversion = `git --version`;
-- 
1.3.0

Re: [PATCH] do not use locale specific strftime when preparing 2822 date

[Jakub Narebski]

It looks like it is _almost_ correct. It should be
  Date: Fri, 07 Jul 2006 20:57:55 +0200
instead of
  Date: Fri,  7 Jul 2006 20:57:55 +0200

It is "day = ([FWS] 1*2DIGIT) / obs-day" in RFC2822.

-- 
Jakub Narebski

Re: [PATCH] do not use locale specific strftime when preparing 2822 date

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> It looks like it is _almost_ correct. It should be
>   Date: Fri, 07 Jul 2006 20:57:55 +0200
> instead of
>   Date: Fri,  7 Jul 2006 20:57:55 +0200
>
> It is "day = ([FWS] 1*2DIGIT) / obs-day" in RFC2822.

I think you are reading ABNF wrong. <a>*<b>element means at least
<a> times and at most <b> times occurrences of element.  Exact
number of repetition is written as <n>element (which is a
short-and for <n>*<n>element).

See the definition of "hour" and friends a few lines below what
you quoted.  It is defined as "2DIGIT / obs-hour" and that is
why we say "01:23:45" not "1:23:45"

Re: [PATCH] do not use locale specific strftime when preparing 2822 date

[Jakub Narebski]

Junio C Hamano wrote:

> Jakub Narebski <jnareb@gmail.com> writes:
> 
>> It looks like it is _almost_ correct. It should be
>>   Date: Fri, 07 Jul 2006 20:57:55 +0200
>> instead of
>>   Date: Fri,  7 Jul 2006 20:57:55 +0200
>>
>> It is "day = ([FWS] 1*2DIGIT) / obs-day" in RFC2822.
> 
> I think you are reading ABNF wrong. [...]
> 
> See the definition of "hour" and friends a few lines below what
> you quoted.  It is defined as "2DIGIT / obs-hour" and that is
> why we say "01:23:45" not "1:23:45"

But it is "day = [...] 2DIGIT [...]"! 

Besides, that what other mailers do (I checked the post I replied via
git-send-email to, i.e. your post). Although I don't think that it can
cause any problems, like using locale date with non US-ASCII characters
did...

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [PATCH] do not use locale specific strftime when preparing 2822 date

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> Signed-off-by: Junio C Hamano <junkio@cox.net>
> Acked-by: Jakub Narebski <jnareb@gmail.com>
> ---
> This patch is sent from patched version of git-send-email.perl
> with git tools 1.3.0. This patch is generated from current master
> after Junio's patch 
>   Message-ID: <7vd5cnv1v5.fsf@assigned-by-dhcp.cox.net>
> applied.

Good test.  Thanks.

Re: [PATCH] do not use locale specific strftime when preparing 2822 date

[Jakub Narebski]

Junio C Hamano wrote:

> Jakub Narebski <jnareb@gmail.com> writes:
> 
>> Signed-off-by: Junio C Hamano <junkio@cox.net>
>> Acked-by: Jakub Narebski <jnareb@gmail.com>
>> ---
>> This patch is sent from patched version of git-send-email.perl
>> with git tools 1.3.0. This patch is generated from current master
>> after Junio's patch 
>>   Message-ID: <7vd5cnv1v5.fsf@assigned-by-dhcp.cox.net>
>> applied.
> 
> Good test.  Thanks.

By the way, patch didn't apply cleanly (some fuzz was used).
The patch I send was from _applied_ (to master) patch.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

[PATCH] do not use locale specific strftime when preparing 2822 date

[Jakub Narebski]

---

 git-send-email.perl |   41 ++++++++++++++++++++++++++++++++++++++---
 1 files changed, 38 insertions(+), 3 deletions(-)

f00ff14faf86c376d0ffb3cef24d2e5a5437dfcf
diff --git a/git-send-email.perl b/git-send-email.perl
index b04b8f4..c9c1975 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -34,8 +34,43 @@ sub readline {
 package main;
 
 # most mail servers generate the Date: header, but not all...
-$ENV{LC_ALL} = 'C';
-use POSIX qw/strftime/;
+sub format_2822_time {
+	my ($time) = @_;
+	my @localtm = localtime($time);
+	my @gmttm = gmtime($time);
+	my $localmin = $localtm[1] + $localtm[2] * 60;
+	my $gmtmin = $gmttm[1] + $gmttm[2] * 60;
+	if ($localtm[0] != $gmttm[0]) {
+		die "local zone differs from GMT by a non-minute interval\n";
+	}
+	if ((($gmttm[6] + 1) % 7) == $localtm[6]) {
+		$localmin += 1440;
+	} elsif ((($gmttm[6] - 1) % 7) == $localtm[6]) {
+		$localmin -= 1440;
+	} elsif ($gmttm[6] != $localtm[6]) {
+		die "local time offset greater than or equal to 24 hours\n";
+	}
+	my $offset = $localmin - $gmtmin;
+	my $offhour = $offset / 60;
+	my $offmin = abs($offset % 60);
+	if (abs($offhour) >= 24) {
+		die ("local time offset greater than or equal to 24 hours\n");
+	}
+
+	return sprintf("%s, %2d %s %d %02d:%02d:%02d %s%02d%02d",
+		       qw(Sun Mon Tue Wed Thu Fri Sat)[$localtm[6]],
+		       $localtm[3],
+		       qw(Jan Feb Mar Apr May Jun
+			  Jul Aug Sep Oct Nov Dec)[$localtm[4]],
+		       $localtm[5]+1900,
+		       $localtm[2],
+		       $localtm[1],
+		       $localtm[0],
+		       ($offset >= 0) ? '+' : '-',
+		       abs($offhour),
+		       $offmin,
+		       );
+}
 
 my $have_email_valid = eval { require Email::Valid; 1 };
 my $smtp;
@@ -387,7 +422,7 @@ sub send_message
 	my @recipients = unique_email_list(@to);
 	my $to = join (",\n\t", @recipients);
 	@recipients = unique_email_list(@recipients,@cc,@bcclist);
-	my $date = strftime('%a, %d %b %Y %H:%M:%S %z', localtime($time++));
+	my $date = format_2822_time($time++);
 	my $gitversion = '@@GIT_VERSION@@';
 	if ($gitversion =~ m/..GIT_VERSION../) {
 	    $gitversion = `git --version`;
-- 
1.3.0

Re: [PATCH] do not use locale specific strftime when preparing 2822 date

[Jakub Narebski]

Disregard this email. It was send by mistake by git-send-spam^Wemail.perl

-- 
Jakub Narebski