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

[Junio C Hamano <gitster@pobox.com>]

"Dmitry V. Levin" <ldv@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 <glebfm@altlinux.org>
> Signed-off-by: Dmitry V. Levin <ldv@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.

This alternative might be theoretically more correct if we cared
about the error and other messages from this Perl invocation, but it
requires that everybody's Perl implementation correctly supports the
additional -M'POSIX ":locale_h"' and "setlocale(LC_TIME, ...)".

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

Thanks.

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