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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



"Dmitry V. Levin" <ldv@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
> Signed-off-by: Dmitry V. Levin <ldv@xxxxxxxxxxxx>
> ---
>
>  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 /) {
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]