am: invoke perl's strftime in C locale

This fixes "hg" patch format support for locales other than C and en_*, see https://bugzilla.altlinux.org/show_bug.cgi?id=28248

Signed-off-by Dmitry V. Levin <[email protected]>

---

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

git-am.sh +1 -1

index c682d34..64b88e4 100755
@@ -334,7 +334,7 @@ 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 }
+			LC_ALL=C perl -M'POSIX qw(strftime)' -ne 'BEGIN { $subject = 0 }
 				if ($subject) { print ; }
 				elsif (/^\# User /) { s/\# User/From:/ ; print ; }
 				elsif (/^\# Date /) {

This puts all of perl into the C locale, which would mean error messages from perl would be in English rather than the user's language. It probably isn't a big deal, because that snippet of perl is short and not likely to produce problems, but I wonder how hard it would be to set the locale just for the strftime call.

From locale(7) man page:

Quoted 5 lines

This puts all of perl into the C locale, which would mean error messages from perl would be in English rather than the user's language. It probably isn't a big deal, because that snippet of perl is short and not likely to produce problems, but I wonder how hard it would be to set the locale just for the strftime call.

Maybe just setting LC_TIME to C would do ...

Quoted 6 lines · Antoine Pelisse

Quoted 5 lines

This puts all of perl into the C locale, which would mean error messages from perl would be in English rather than the user's language. It probably isn't a big deal, because that snippet of perl is short and not likely to produce problems, but I wonder how hard it would be to set the locale just for the strftime call.

Quoted 1 line

Maybe just setting LC_TIME to C would do ...

Yeah, that is a nice simple solution. Dmitry, does just setting LC_TIME fix the problem for you?

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 <[email protected]>

Signed-off-by Dmitry V. Levin <[email protected]>

---

v3: alternative implementation using setlocale(LC_TIME, "C")

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

git-am.sh +0 -0

index c682d34..8677d8c 100755

git-am.sh +0 -0

 				if ($subject) { print ; }
 				elsif (/^\# User /) { s/\# User/From:/ ; print ; }
 				elsif (/^\# Date /) {

Quoted 6 lines

git-am.sh +0 -0

index c682d34..8677d8c 100755
@@ -334,7 +334,8 @@ split_patches () {
 			# Since we cannot guarantee that the commit message is in

git-am.sh split_patches () { · line 338

Quoted 5 lines

git-am.sh +2 -1

 			# 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.

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".

Quoted 15 lines · 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 <[email protected]>

Signed-off-by Dmitry V. Levin <[email protected]>

---

v3: alternative implementation using setlocale(LC_TIME, "C")

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

git-am.sh split_patches () { · line 338

Quoted 11 lines

git-am.sh +2 -1

index c682d34..8677d8c 100755
@@ -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).

Quoted 8 lines · Jeff King

Quoted 6 lines · Antoine Pelisse

Quoted 5 lines

This puts all of perl into the C locale, which would mean error messages from perl would be in English rather than the user's language. It probably isn't a big deal, because that snippet of perl is short and not likely to produce problems, but I wonder how hard it would be to set the locale just for the strftime call.

Quoted 4 lines

Quoted 1 line

Maybe just setting LC_TIME to C would do ...

Yeah, that is a nice simple solution. Dmitry, does just setting LC_TIME fix the problem for you?

Just setting LC_TIME environment variable instead of LC_ALL would end up with unreliable solution because LC_ALL has the highest priority.

If keeping error messages from perl has the utmost importance, it could be achieved by

(quoted diff) +2 -1

-			perl -M'POSIX qw(strftime)' -ne 'BEGIN { $subject = 0 }
+			perl -M'POSIX qw(strftime :locale_h)' -ne '
+				BEGIN { setlocale(LC_TIME, "C"); $subject = 0 }

but the little perl helper script we are talking about hardly worths so much efforts.