Re: [PATCH v3] am: invoke perl's strftime in C locale

2013-01-20 Thread Junio C Hamano 
Dmitry V. Levin l...@altlinux.org writes:

 Personally I prefer 2nd edition that is simpler and does the right thing
 (not that LC_ALL=C is necessary and sufficient, you neither need to add
 things like LANG=C nor can relax it to LC_TIME=C).

I guess everybody involved is in agreement, then.

Just FYI, LC_ALL=C LANG=C comes from the inertia dating back when
not everybody understood LC_*; I do not personally know of a system
that will be helped by the extra LANG=C these days, but I know it
will not hurt anybody, so...
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] am: invoke perl's strftime in C locale

2013-01-19 Thread Jeff King 
On Fri, Jan 18, 2013 at 12:36:46PM -0800, Junio C Hamano wrote:

  diff --git a/git-am.sh b/git-am.sh
  index c682d34..8677d8c 100755
  --- a/git-am.sh
  +++ b/git-am.sh
  @@ -334,7 +334,8 @@ split_patches () {
  # Since we cannot guarantee that the commit message is 
  in
  # git-friendly format, we put no Subject: line and just 
  consume
  # all of the message as the body
  -   perl -M'POSIX qw(strftime)' -ne 'BEGIN { $subject = 0 }
  +   perl -M'POSIX qw(strftime :locale_h)' -ne '
  +   BEGIN { setlocale(LC_TIME, C); $subject = 0 }
 
 I still haven't convinced myself that this is an improvement over
 the simple LC_ALL=C LANG=C perl ... approach.

Yeah, I was the one who brought it up, but I think I was probably being
too nit-picky. It almost certainly doesn't matter, and the alternatives
are just as likely to cause problems.

 I am tempted to use the previous one that puts the whole process
 under LC_ALL=C instead, unless I hear a we already depend on that
 elsewhere, look at $that_code.

I'm fine with that.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] am: invoke perl's strftime in C locale

2013-01-19 Thread Dmitry V. Levin 
On Fri, Jan 18, 2013 at 12:36:46PM -0800, Junio C Hamano wrote:
 Dmitry V. Levin l...@altlinux.org writes:
 
  This fixes hg patch format support for locales other than C and en_*.
  Before the change, git-am was making Date: line from hg changeset
  metadata according to the current locale, and this line was rejected
  later with invalid date format diagnostics because localized date
  strings are not supported.
 
  Reported-by: Gleb Fotengauer-Malinovskiy gle...@altlinux.org
  Signed-off-by: Dmitry V. Levin l...@altlinux.org
  ---
 
   v3: alternative implementation using setlocale(LC_TIME, C)
 
   git-am.sh | 3 ++-
   1 file changed, 2 insertions(+), 1 deletion(-)
 
  diff --git a/git-am.sh b/git-am.sh
  index c682d34..8677d8c 100755
  --- a/git-am.sh
  +++ b/git-am.sh
  @@ -334,7 +334,8 @@ split_patches () {
  # Since we cannot guarantee that the commit message is 
  in
  # git-friendly format, we put no Subject: line and just 
  consume
  # all of the message as the body
  -   perl -M'POSIX qw(strftime)' -ne 'BEGIN { $subject = 0 }
  +   perl -M'POSIX qw(strftime :locale_h)' -ne '
  +   BEGIN { setlocale(LC_TIME, C); $subject = 0 }
 
 I still haven't convinced myself that this is an improvement over
 the simple LC_ALL=C LANG=C perl ... approach.

Personally I prefer 2nd edition that is simpler and does the right thing
(not that LC_ALL=C is necessary and sufficient, you neither need to add
things like LANG=C nor can relax it to LC_TIME=C).


-- 
ldv


pgp3AH6oqBPnI.pgp
Description: PGP signature

[PATCH v3] am: invoke perl's strftime in C locale

2013-01-15 Thread Dmitry V. Levin 
This fixes hg patch format support for locales other than C and en_*.
Before the change, git-am was making Date: line from hg changeset
metadata according to the current locale, and this line was rejected
later with invalid date format diagnostics because localized date
strings are not supported.

Reported-by: Gleb Fotengauer-Malinovskiy gle...@altlinux.org
Signed-off-by: Dmitry V. Levin l...@altlinux.org
---

 v3: alternative implementation using setlocale(LC_TIME, C)

 git-am.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/git-am.sh b/git-am.sh
index c682d34..8677d8c 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -334,7 +334,8 @@ split_patches () {
# Since we cannot guarantee that the commit message is 
in
# git-friendly format, we put no Subject: line and just 
consume
# all of the message as the body
-   perl -M'POSIX qw(strftime)' -ne 'BEGIN { $subject = 0 }
+   perl -M'POSIX qw(strftime :locale_h)' -ne '
+   BEGIN { setlocale(LC_TIME, C); $subject = 0 }
if ($subject) { print ; }
elsif (/^\# User /) { s/\# User/From:/ ; print 
; }
elsif (/^\# Date /) {

-- 
ldv
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html