patch, 3 partsRe: [PATCH 1/3] gitweb: whitespace cleanup

Jakub Narebski <jnareb@gmail.com> wrote:

It should be of course

 Signed-off-by: Jakub Narebski <jnareb@gmail.com>

Somehow cover letter got lost, and patches did not appear on mailing list archives. If the mail will not appear in few hours, I'll resend it to mailing list only.

Probably because git-send-email should ensure LC_ALL=C for Date: header generation...

Jakub Narebski <jnareb@gmail.com> writes:

All three applied cleanly (finally!), but with hand munging of
Date: field.

I think git-send-email should internally do LC_ALL=C or stop using strftime() or both.

$ENV{LC_ALL} = 'C'; does not change locale used by strftime. Use setlocale( LC_ALL, 'C' ); instead.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>

---

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

7c796152570e28d3f95c17e93864c6abc8edef24

Jakub Narebski wrote:

Perhaps instead of 
  setlocale( &LC_ALL, 'C' );
we should use
  setlocale( &LC_ALL, '' );
(i.e. set the LC_ALL behaviour according to the locale environment
variables). I'm not that versed in locale, POSIX and Perl.

Jakub Narebski <jnareb@gmail.com> wrote:

I'm responsible for the $ENV{LC_ALL} = 'C' setting but I never actually tested how things would work with a non-English locale (not being well-versed in these things myself, either).

I've always wondered about why /usr/bin/822-date existed without strftime on my Debian systems but never bothered asking, I guess this could be a good reason why...

For reference, here's the /usr/bin/822-date script (trailing whitespace fixed):

------- 8< -------

#!/usr/bin/perl -- # I hereby place this in the public domain - Ian Jackson, 1995. # Changes by Klee Dienes also placed in public domain (1997).

# time structure: # [ sec min hour mday mon year wday yday isdst ]

@ARGV && die "usage: 822-date\n";

$curtime = time; @localtm = localtime ($curtime); $localtms = localtime ($curtime); @gmttm = gmtime ($curtime); $gmttms = gmtime ($curtime);

if ($localtm[0] != $gmttm[0]) {
    die (sprintf ("local timezone differs from GMT by a non-minute interval\n"
		 . "local time: %s\n"
		 . "GMT time: %s\n", $localtms, $gmttms));
}

$localmin = $localtm[1] + $localtm[2] * 60; $gmtmin = $gmttm[1] + $gmttm[2] * 60;

if ((($gmttm[6] + 1) % 7) == $localtm[6]) {
    $localmin += 1440;
} elsif ((($gmttm[6] - 1) % 7) == $localtm[6]) {
    $localmin -= 1440;
} elsif ($gmttm[6] == $localtm[6]) {
    1;
} else {
    die ("822-date: local time offset greater than or equal to 24 hours\n");
}

$offset = $localmin - $gmtmin; $offhour = $offset / 60; $offmin = abs ($offset % 60);

if (abs ($offhour) >= 24) {
    die ("822-date: local time offset greater than or equal to 24 hours\n");
}

printf
    (
     "%s, %2d %s %d %02d:%02d:%02d %s%02d%02d\n",
     (Sun,Mon,Tue,Wed,Thu,Fri,Sat)[$localtm[6]], # day of week
     $localtm[3],		# day of month
     (Jan,Feb,Mar,Apr,May,Jun,Jul,Aug,Sep,Oct,Nov,Dec)[$localtm[4]], # month
     $localtm[5]+1900,		# year
     $localtm[2],		# hour
     $localtm[1],		# minute
     $localtm[0],		# sec
     ($offset >= 0) ? '+' : '-',# TZ offset direction
     abs ($offhour),		# TZ offset hour
     $offmin,			# TZ offset minute
     ) || die "822-date: output error: $!\n";

------- 8< -------

Eric Wong wrote:

It looks like I have broken locale badly somewhat, as I yesterday sent patches successfully without the patch. I suspect that sendmail removes incorrect Date: header and adds it's own... unless Date: header contains characters outside US-ASCII.

It means that sendmail can deal with
  Date: wto, 20 cze 2006 13:14:11 +0200
instead of correct
  Date: Tue, 20 Jun 2006 13:14:11 +0200

but cannot deal with (iso-8859-2 encoded I think)
  Date: ¶ro, 21 cze 2006 09:48:02 +0200
from git-send-email.perl without patch, instead of correct
  Date: Wed, 21 Jun 2006 11:25:52 +0200
with the patch.

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

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

Signed-off-by: Junio C Hamano <junkio@cox.net>
---