[PATCH] t9129: fix UTF-8 locale detection

[PATCH] t9129: fix UTF-8 locale detection

[Yann Droneaud]

Since I don't have en_US.utf8, some tests failed:

  * UTF-8 locale not available, test skipped
  * skip 10: ISO-8859-1 should match UTF-8 in svn
  * skip 11: eucJP should match UTF-8 in svn
  * skip 12: ISO-2022-JP should match UTF-8 in svn

On my system locale -a reports:

   en_US
   en_US.ISO-8859-1
   en_US.UTF-8

According to Wikipedia utf8 is not a correct name
for the UTF-8 encoding:
http://en.wikipedia.org/wiki/UTF-8#Official_name_and_incorrect_variants

And compare_svn_head_with() is explicitly using en_US.UTF-8
locale.

Signed-off-by: Yann Droneaud <yann@droneaud.fr>
---
 t/t9129-git-svn-i18n-commitencoding.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/t/t9129-git-svn-i18n-commitencoding.sh b/t/t9129-git-svn-i18n-commitencoding.sh
index b9224bd..ec6ed4f 100755
--- a/t/t9129-git-svn-i18n-commitencoding.sh
+++ b/t/t9129-git-svn-i18n-commitencoding.sh
@@ -69,7 +69,7 @@ do
 	'
 done
 
-if locale -a |grep -q en_US.utf8; then
+if locale -a |grep -q en_US.UTF-8; then
 	test_set_prereq UTF8
 else
 	say "UTF-8 locale not available, test skipped"
-- 
1.6.4.4

Re: [PATCH] t9129: fix UTF-8 locale detection

[Michael J Gruber]

Yann Droneaud venit, vidit, dixit 18.05.2010 16:41:
> Since I don't have en_US.utf8, some tests failed:
> 
>   * UTF-8 locale not available, test skipped
>   * skip 10: ISO-8859-1 should match UTF-8 in svn
>   * skip 11: eucJP should match UTF-8 in svn
>   * skip 12: ISO-2022-JP should match UTF-8 in svn
> 
> On my system locale -a reports:
> 
>    en_US
>    en_US.ISO-8859-1
>    en_US.UTF-8
> 

locale -a|grep en_US
en_US
en_US.iso88591
en_US.iso885915
en_US.utf8

This is on Fedora 13, which is not exactly exotic. What is your system?

> According to Wikipedia utf8 is not a correct name
> for the UTF-8 encoding:
> http://en.wikipedia.org/wiki/UTF-8#Official_name_and_incorrect_variants
> 
> And compare_svn_head_with() is explicitly using en_US.UTF-8
> locale.
> 
> Signed-off-by: Yann Droneaud <yann@droneaud.fr>
> ---
>  t/t9129-git-svn-i18n-commitencoding.sh |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/t/t9129-git-svn-i18n-commitencoding.sh b/t/t9129-git-svn-i18n-commitencoding.sh
> index b9224bd..ec6ed4f 100755
> --- a/t/t9129-git-svn-i18n-commitencoding.sh
> +++ b/t/t9129-git-svn-i18n-commitencoding.sh
> @@ -69,7 +69,7 @@ do
>  	'
>  done
>  
> -if locale -a |grep -q en_US.utf8; then
> +if locale -a |grep -q en_US.UTF-8; then
>  	test_set_prereq UTF8
>  else
>  	say "UTF-8 locale not available, test skipped"

Funny thing is the test succeeds for me, even when run within
LANG=en_US.iso88591.
So I'd suggest to use

-if locale -a |grep -q en_US.utf8; then
+if locale -a |egrep -q 'en_US.utf8|en_US.UTF-8'; then

and embrace for more variants to appear down the road...

Michael

Re: [PATCH] t9129: fix UTF-8 locale detection

[Yann Droneaud]

Le mardi 18 mai 2010 à 18:05 +0200, Michael J Gruber a écrit :
> Yann Droneaud venit, vidit, dixit 18.05.2010 16:41:
> > Since I don't have en_US.utf8, some tests failed:

> > 
> > On my system locale -a reports:
> > 
> >    en_US
> >    en_US.ISO-8859-1
> >    en_US.UTF-8
> > 
> 
> locale -a|grep en_US
> en_US
> en_US.iso88591
> en_US.iso885915
> en_US.utf8
> 
> This is on Fedora 13, which is not exactly exotic. What is your system?
> 

Mandriva Linux 2009.1 and 2010.0, see results of locale -a :

http://pastebin.mandriva.com/18557
http://pastebin.mandriva.com/18555

I've double check with Mandriva's developers who have

  en_US
  en_US.iso88591
  en_US.utf8
  en_US.UTF-8

> > According to Wikipedia utf8 is not a correct name
> > for the UTF-8 encoding:
> > http://en.wikipedia.org/wiki/UTF-8#Official_name_and_incorrect_variants
> > 

UTF-8 seems to be the correct name.

> >  
> > -if locale -a |grep -q en_US.utf8; then
> > +if locale -a |grep -q en_US.UTF-8; then
> >  	test_set_prereq UTF8
> >  else
> >  	say "UTF-8 locale not available, test skipped"
> 
> Funny thing is the test succeeds for me, even when run within
> LANG=en_US.iso88591.

> So I'd suggest to use
> 
> -if locale -a |grep -q en_US.utf8; then
> +if locale -a |egrep -q 'en_US.utf8|en_US.UTF-8'; then
> 
> and embrace for more variants to appear down the road...
> 

Using en_US.UTF-8 seems more accurate when I wrote the patch since, as I
wrote before, compare_svn_head_with() is using LC_ALL=en_US.UTF-8.
So en_US.UTF-8 is an alias for en_US.utf8, whatever the canonical
version is.

So let's go for another version.

-- 
Yann Droneaud

[PATCH] t9129: fix UTF-8 locale detection

[Yann Droneaud]

Since I don't have en_US.utf8, some tests failed:

  * UTF-8 locale not available, test skipped
  * skip 10: ISO-8859-1 should match UTF-8 in svn
  * skip 11: eucJP should match UTF-8 in svn
  * skip 12: ISO-2022-JP should match UTF-8 in svn

On my system locale -a reports:

   en_US
   en_US.ISO-8859-1
   en_US.UTF-8

Tests available locales against en_US\.(utf|UTF)-?8 regexp.

Signed-off-by: Yann Droneaud <yann@droneaud.fr>
---
 t/t9129-git-svn-i18n-commitencoding.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/t/t9129-git-svn-i18n-commitencoding.sh b/t/t9129-git-svn-i18n-commitencoding.sh
index b9224bd..00a273b 100755
--- a/t/t9129-git-svn-i18n-commitencoding.sh
+++ b/t/t9129-git-svn-i18n-commitencoding.sh
@@ -69,7 +69,7 @@ do
 	'
 done
 
-if locale -a |grep -q en_US.utf8; then
+if locale -a |grep -qE '^en_US\.(utf|UTF)-?8$'; then
 	test_set_prereq UTF8
 else
 	say "UTF-8 locale not available, test skipped"
-- 
1.6.4.4

Re: [PATCH] t9129: fix UTF-8 locale detection

[Linus Torvalds]

On Tue, 18 May 2010, Yann Droneaud wrote:
>  
> -if locale -a |grep -q en_US.utf8; then
> +if locale -a |grep -qE '^en_US\.(utf|UTF)-?8$'; then

While -E is POSIX, I suspect that it's not universal. iirc, you still have 
some really crap fileutils tools coming with Solaris, for example. 

Wouldn't it be easier to just make it ignore case, and do

	grep -qi '^en_US\.utf-?8$'

instead?

I'm also not entirely sure you want to make that pattern stricter - the 
whole problem with the old pattern was that it was too exact, so why add 
the beginning/end requirement?

		Linus

Re: [PATCH] t9129: fix UTF-8 locale detection

[Andreas Schwab]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> Wouldn't it be easier to just make it ignore case, and do
>
> 	grep -qi '^en_US\.utf-?8$'

You'll need ERE's for the ? operator.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] t9129: fix UTF-8 locale detection

[Linus Torvalds]

On Tue, 18 May 2010, Andreas Schwab wrote:

> Linus Torvalds <torvalds@linux-foundation.org> writes:
> 
> > Wouldn't it be easier to just make it ignore case, and do
> >
> > 	grep -qi '^en_US\.utf-?8$'
> 
> You'll need ERE's for the ? operator.

Oh, just replace it with '*' then. It's not like anybody cares.

		Linus

Re: [PATCH] t9129: fix UTF-8 locale detection

[Yann Droneaud]

Le mardi 18 mai 2010 à 10:45 -0700, Linus Torvalds a écrit :
> 
> On Tue, 18 May 2010, Yann Droneaud wrote:
> >  
> > -if locale -a |grep -q en_US.utf8; then
> > +if locale -a |grep -qE '^en_US\.(utf|UTF)-?8$'; then
> 
> While -E is POSIX, I suspect that it's not universal. iirc, you still have 
> some really crap fileutils tools coming with Solaris, for example. 
> 

You're right, Solaris's own grep doesn't known about -E nor -e.

And it even doesn't know about -q : 

 $ grep -q                                                     
 grep: illegal option -- q
 Usage: grep -hblcnsviw pattern file . . .

So the whole test won't work for older Solaris.

Solaris don't support grep -e, but has the non POSIX egrep instead
(which doesn't support -q too).

[...]

> I'm also not entirely sure you want to make that pattern stricter - the 
> whole problem with the old pattern was that it was too exact, so why add 
> the beginning/end requirement?
> 

Just to be sure it doesn't match "garbage". 
Initial regexp was using a straight '.' operator, so while fixing it, I
thought it would be better to achieve "perfect match".

Regards.

-- 
Yann Droneaud

Re: [PATCH] t9129: fix UTF-8 locale detection

[Yann Droneaud]

Le mardi 18 mai 2010 à 22:49 +0200, Yann Droneaud a écrit :
> Le mardi 18 mai 2010 à 10:45 -0700, Linus Torvalds a écrit :
> > 
> > On Tue, 18 May 2010, Yann Droneaud wrote:
> > >  
> > > -if locale -a |grep -q en_US.utf8; then
> > > +if locale -a |grep -qE '^en_US\.(utf|UTF)-?8$'; then
> > 
> > While -E is POSIX, I suspect that it's not universal. iirc, you still have 
> > some really crap fileutils tools coming with Solaris, for example. 
> > 
> 
> You're right, Solaris's own grep doesn't known about -E nor -e.
> 
> And it even doesn't know about -q : 
> 
>  $ grep -q                                                     
>  grep: illegal option -- q
>  Usage: grep -hblcnsviw pattern file . . .
> 

Integrating Linus's remarks and some autoconf[1][2] hints, here is a
proposal for a portable test:

   if locale -a |grep -i 'en_US\.utf-*8' > /dev/null ; then

It should be as portable as possible without too much work.


[1] egrep
<http://www.gnu.org/software/autoconf/manual/html_node/Limitations-of-Usual-Tools.html#index-g_t_0040command_007begrep_007d-1706>

[2] grep
<http://www.gnu.org/software/autoconf/manual/html_node/Limitations-of-Usual-Tools.html#index-g_t_0040command_007bgrep_007d-1712>

Regards.

-- 
Yann Droneaud

Re* [PATCH] t9129: fix UTF-8 locale detection

[Junio C Hamano]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> Wouldn't it be easier to just make it ignore case, and do
>
> 	grep -qi '^en_US\.utf-?8$'
>
> instead?
>
> I'm also not entirely sure you want to make that pattern stricter - the 
> whole problem with the old pattern was that it was too exact, so why add 
> the beginning/end requirement?

Sorry for being late to the party...

The prerequisite test is supposed to protect a real test that does this:

	LC_ALL=en_US.UTF-8 svn log `git svn info --url` | perl -w -e '...'

and the original patch at least matches what we check with what we
actually ask for from the system.

I don't know if the above "svn log" test would still work if we run it
under any locale with UTF-8 (I checked with ja_JP.UTF-8 and it seems to be
Ok), but if it does, then a patch like this might be a better alternative.

 t/t9129-git-svn-i18n-commitencoding.sh |   20 +++++++++++++-------
 1 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/t/t9129-git-svn-i18n-commitencoding.sh b/t/t9129-git-svn-i18n-commitencoding.sh
index b9224bd..1e9a2eb 100755
--- a/t/t9129-git-svn-i18n-commitencoding.sh
+++ b/t/t9129-git-svn-i18n-commitencoding.sh
@@ -14,10 +14,22 @@ compare_git_head_with () {
 	test_cmp current "$1"
 }
 
+a_utf8_locale=$(locale -a | sed -n '/\.[uU][tT][fF]-*8$/{
+	p
+	q
+}')
+
+if test -n "$a_utf8_locale"
+then
+	test_set_prereq UTF8
+else
+	say "UTF-8 locale not available, some tests are skipped"
+fi
+
 compare_svn_head_with () {
 	# extract just the log message and strip out committer info.
 	# don't use --limit here since svn 1.1.x doesn't have it,
-	LC_ALL=en_US.UTF-8 svn log `git svn info --url` | perl -w -e '
+	LC_ALL="$a_utf8_locale" svn log `git svn info --url` | perl -w -e '
 		use bytes;
 		$/ = ("-"x72) . "\n";
 		my @x = <STDIN>;
@@ -69,12 +81,6 @@ do
 	'
 done
 
-if locale -a |grep -q en_US.utf8; then
-	test_set_prereq UTF8
-else
-	say "UTF-8 locale not available, test skipped"
-fi
-
 test_expect_success UTF8 'ISO-8859-1 should match UTF-8 in svn' '
 	(
 		cd ISO8859-1 &&

Re: [PATCH] t9129: fix UTF-8 locale detection

[Yann Droneaud]

Le mardi 18 mai 2010 à 19:08 +0200, Yann Droneaud a écrit :
> Le mardi 18 mai 2010 à 18:05 +0200, Michael J Gruber a écrit :
> > Yann Droneaud venit, vidit, dixit 18.05.2010 16:41:
> > > Since I don't have en_US.utf8, some tests failed:
> 
> > > 
> > > On my system locale -a reports:
> > > 
> > >    en_US
> > >    en_US.ISO-8859-1
> > >    en_US.UTF-8
> > > 
> > 
> > locale -a|grep en_US
> > en_US
> > en_US.iso88591
> > en_US.iso885915
> > en_US.utf8
> > 
> > This is on Fedora 13, which is not exactly exotic. What is your system?
> > 
> 

I've checked carefully multiple system and configuration, and found why
we have some little locale problem here.

Since glibc 2.3, a file can hold all locales in file "locale-archive"
instead of having a tons of directory. To store all the locales in this
file, it uses an index based on a "normalized" codeset, e.g. it converts
codeset to lowercase, removes dash and minus. 
So when one ask for the locale list, locale first go through the
"locale-archive" content and report normalized codeset (utf8)  instead
of canonical codeset (UTF-8), then it proceed with the legacy locales
directories, using for them the canonical codeset.

Until recently, Mandriva Linux doesn't make use of "locale-archive", so
UTF-8 locales were reported. Version in development uses
"locale-archive" + legacy locale directories, hence the mix I've
reported. Other Linux distributions like Fedora and Ubuntu uses only
"locale-archive" and so, have only "normalized" codeset.
POSIX doesn't specify the output of locale -a, so it's not really a bug
to show "normalized" codeset name.

But all others "POSIX" system I've found report "canonical" codeset,
e.g. UTF-8 (all but latest cygwin). 

Here's the bug report:
http://sourceware.org/bugzilla/show_bug.cgi?id=11629

BTW, I will shortly provided a fix for the testcase, which will handle
all cases.

Regards.

-- 
Yann Droneaud

Re: [PATCH] t9129: fix UTF-8 locale detection

[Michael J Gruber]

Yann Droneaud venit, vidit, dixit 24.05.2010 19:08:
> Le mardi 18 mai 2010 à 19:08 +0200, Yann Droneaud a écrit :
>> Le mardi 18 mai 2010 à 18:05 +0200, Michael J Gruber a écrit :
>>> Yann Droneaud venit, vidit, dixit 18.05.2010 16:41:
>>>> Since I don't have en_US.utf8, some tests failed:
>>
>>>>
>>>> On my system locale -a reports:
>>>>
>>>>    en_US
>>>>    en_US.ISO-8859-1
>>>>    en_US.UTF-8
>>>>
>>>
>>> locale -a|grep en_US
>>> en_US
>>> en_US.iso88591
>>> en_US.iso885915
>>> en_US.utf8
>>>
>>> This is on Fedora 13, which is not exactly exotic. What is your system?
>>>
>>
> 
> I've checked carefully multiple system and configuration, and found why
> we have some little locale problem here.
> 
> Since glibc 2.3, a file can hold all locales in file "locale-archive"
> instead of having a tons of directory. To store all the locales in this
> file, it uses an index based on a "normalized" codeset, e.g. it converts
> codeset to lowercase, removes dash and minus. 
> So when one ask for the locale list, locale first go through the
> "locale-archive" content and report normalized codeset (utf8)  instead
> of canonical codeset (UTF-8), then it proceed with the legacy locales
> directories, using for them the canonical codeset.
> 
> Until recently, Mandriva Linux doesn't make use of "locale-archive", so
> UTF-8 locales were reported. Version in development uses
> "locale-archive" + legacy locale directories, hence the mix I've
> reported. Other Linux distributions like Fedora and Ubuntu uses only
> "locale-archive" and so, have only "normalized" codeset.
> POSIX doesn't specify the output of locale -a, so it's not really a bug
> to show "normalized" codeset name.
> 
> But all others "POSIX" system I've found report "canonical" codeset,
> e.g. UTF-8 (all but latest cygwin). 
> 
> Here's the bug report:
> http://sourceware.org/bugzilla/show_bug.cgi?id=11629

Thanks a lot for doing the leg work! Is there any way to, say,
set_local(a) and check whether get_locale() == a up to equivalence?

> 
> BTW, I will shortly provided a fix for the testcase, which will handle
> all cases.
> 
> Regards.
> 

Thanks,
Michael

[PATCH/RFC 0/4] en_US.UTF-8 locale detection

[Yann Droneaud]

Following discussions[1] about test t9129 UTF-8 locale detection,
here's a generic rewrite of the en_US.UTF-8 locale detection
to be used with all other tests.

[1] http://thread.gmane.org/gmane.comp.version-control.git/147283

The proposed mechanism could work for system without "locale" command,
and would detect en_US.UTF-8 locale when named en_US.utf8 or some other
variations.

It must be tested on a wider range of systems (especially non-Linux).

Yann Droneaud (4):
  test: added a library to detect an en_US.UTF-8 locale
  test-lib.sh: added test_utf8() function
  test: use test_utf8 and GIT_LC_UTF8 where an en_US.UTF-8 locale is
    required
  t9129: use "$PERL_PATH" instead of "perl"

 t/lib-locale.pl                        |  167 ++++++++++++++++++++++++++++++++
 t/t9100-git-svn-basic.sh               |   25 ++---
 t/t9129-git-svn-i18n-commitencoding.sh |   13 +--
 t/test-lib.sh                          |   14 +++
 4 files changed, 192 insertions(+), 27 deletions(-)
 create mode 100755 t/lib-locale.pl

-- 
1.7.3.4

[PATCH 1/4] test: add a library to detect an en_US.UTF-8 locale

[Yann Droneaud]

Since one can't rely on "locale" command availability nor its -a output,
perl script t/lib-locale.pl first use setlocale() and langinfo(CODESET)
to search for a working en_US.UTF-8 locale among many name variants.

If this fail, the script fallback to "locale" usage with two steps:
- try the "charmap" keyword, for example LC_ALL=en_US locale charmap
- then try "-a" option and match a pattern looking to the
  locale names

Signed-off-by: Yann Droneaud <yann@droneaud.fr>
---
 t/lib-locale.pl |  167 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 167 insertions(+), 0 deletions(-)
 create mode 100755 t/lib-locale.pl

diff --git a/t/lib-locale.pl b/t/lib-locale.pl
new file mode 100755
index 0000000..33d47a5
--- /dev/null
+++ b/t/lib-locale.pl
@@ -0,0 +1,167 @@
+#!/usr/bin/env perl
+
+#
+# Copyright (c) 2011 Yann Droneaud <yann@droneaud.fr>
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see http://www.gnu.org/licenses/ .
+#
+
+use strict;
+if ($] > 5.004) {
+    use warnings; # unless $] < 5.0004;
+}
+use POSIX;
+
+# according to manpage: setlocale can be used starting from perl 5.004
+# starting from May 1997.
+require 5.004;
+
+# it can also be tested with  perl -V:d_setlocale
+# which must return something like "d_setlocale='define';"
+
+my $default_locale = setlocale(LC_ALL);
+
+# try a locale and return its codeset if available
+sub locale_get_codeset
+{
+    my ($locale) = @_;
+    my $codeset;
+    my $ret;
+
+    $ret = setlocale(LC_ALL, $locale);
+    if (!defined($ret)) {
+	# print "can't set locale $locale\n";
+	return undef;
+    }
+
+    #
+    # I18N::Langinfo is not available everywhere
+    # use the trick from perldoc I18N::Langinfo
+    #
+    $codeset = eval {
+	require I18N::Langinfo;
+	I18N::Langinfo->import(qw(langinfo CODESET));
+	langinfo(CODESET()); # note the ()
+    };
+
+    if (defined($@) && $@) {
+	print STDERR "error while testing codeset:\n $@";
+	return undef;
+    }
+
+    # restore locale
+    setlocale(LC_ALL, $default_locale);
+
+    return $codeset;
+}
+
+# check that given codeset match UTF-8
+sub codeset_check
+{
+    my ($codeset) = @_;
+
+    if (defined($codeset) && $codeset =~ /^UTF[_-]?8$/i) {
+	return 1;
+    }
+
+    return 0;
+}
+
+# try all those locales
+# require en_US with UTF-8
+my @locales = ( "en_US.UTF-8",
+		"en_US.utf8",
+		"en_US.UTF8",
+		"en_US.utf-8",
+		"en_US.UTF_8",
+		"en_US.utf_8",
+		"en_US" );
+
+my $locale;
+
+my $codeset;
+
+#
+# Check locale from within perl
+#
+foreach $locale (@locales) {
+
+    $codeset = locale_get_codeset($locale);
+
+    if (codeset_check($codeset)) {
+	print $locale, "\n";
+	exit 0;
+    }
+}
+
+#
+# if 'locale' command is available, test 'locale charmap'
+#
+foreach $locale (@locales) {
+
+    $codeset = `LC_ALL=$locale locale charmap 2>/dev/null`;
+
+    # if any error, skip
+    if ($!) {
+	last;
+    }
+
+    if (codeset_check($codeset)) {
+	print $locale, "\n";
+	exit 0;
+    }
+}
+
+#
+# try to execute "locale -a" command
+# this command is not always available and
+# output format is not normalized
+#
+use IPC::Open3;
+use File::Spec;
+
+open(NULLR, "<", File::Spec->devnull) || die "Can't open devnull: $!";
+open(NULLW, ">", File::Spec->devnull) || die "Can't open devnull: $!";
+
+my $pid = open3("<&NULLR", \*LOCALES, ">&NULLW" , "locale", "-a") || die "Can't launch locale -a: $!";
+
+while(<LOCALES>) {
+     chomp;
+     if (/(en_US\.([\w-]+))/i) {
+	 if (codeset_check($2)) {
+	     $locale = $1;
+	     last;
+	 }
+     }
+}
+
+waitpid($pid, 0);
+
+my $retcode = $?;
+
+close(LOCALES);
+
+if ($retcode == 0) {
+    if (defined $locale) {
+	print "$locale\n";
+	exit 0;
+    }
+}
+
+#
+# Nothing available,
+# at last, return an error
+#
+
+exit 1;
-- 
1.7.3.4

Re: [PATCH 1/4] test: add a library to detect an en_US.UTF-8 locale

[Junio C Hamano]

Yann Droneaud <yann@droneaud.fr> writes:

Yann Droneaud <yann@droneaud.fr> writes:

> Since one can't rely on "locale" command availability nor its -a output,
> perl script t/lib-locale.pl first use setlocale() and langinfo(CODESET)
> to search for a working en_US.UTF-8 locale among many name variants.
>
> If this fail, the script fallback to "locale" usage with two steps:
> - try the "charmap" keyword, for example LC_ALL=en_US locale charmap
> - then try "-a" option and match a pattern looking to the
>   locale names
>
> Signed-off-by: Yann Droneaud <yann@droneaud.fr>
> ---
>  t/lib-locale.pl |  167 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 167 insertions(+), 0 deletions(-)
>  create mode 100755 t/lib-locale.pl

The remainder of the series looks more or less sane, but a new 170-line
script somehow feels a bit overengineered solution to a minor problem that
has already been solved in a much simpler way.

> +# try to execute "locale -a" command
> +# this command is not always available and
> +# output format is not normalized
> +#
> +use IPC::Open3;
> +use File::Spec;
> +
> +open(NULLR, "<", File::Spec->devnull) || die "Can't open devnull: $!";
> +open(NULLW, ">", File::Spec->devnull) || die "Can't open devnull: $!";
> +
> +my $pid = open3("<&NULLR", \*LOCALES, ">&NULLW" , "locale", "-a") || die "Can't launch locale -a: $!";
> +
> +while(<LOCALES>) {
> +     chomp;
> +     if (/(en_US\.([\w-]+))/i) {
> +	 if (codeset_check($2)) {
> +	     $locale = $1;
> +	     last;
> +	 }
> +     }
> +}
> +
> +waitpid($pid, 0);

You are trying to buy something with the complexity of Open3 for doing
this logic, over a bog-naive "for (`locale -a`) { ... }", but is that
something really worth the money?

"lib-locale.pl" is a gross misnomer for this script.

It may be a good helper to be used in 2/4 (test_utf8), but later people
may want to have more helper feature in something called "lib-locale", not
just "pick a single UTF-8 locale randomly from available ones on the
system" (especially when ab/i18n starts moving again).  I'd suggest either
to (1) rename it "pick-utf8-locale.pl" (i.e. honest naming) or (2) prepare
the helper command to be extensible from the beginning, i.e. require a
command word e.g. "lib-locale.pl pick-utf8-locale" to trigger the
currently implemented feature (i.e. forward looking naming).

[PATCH 2/4] test-lib.sh: add test_utf8() function

[Yann Droneaud]

test_utf8() checks for en_US.UTF-8 locale availability using lib-locale.pl.
The function returns 1 if a locale was not found, otherwise it returns 0,
set prereq UTF8 and export GIT_LC_UTF8 with the locale name.

If a test needs to use an en_US.UTF-8 locale, it has to call test_utf8() first.
Then it can do tests based on prereq UTF8 availability and use LC_ALL=$GIT_LC_UTF8.

Signed-off-by: Yann Droneaud <yann@droneaud.fr>
---
 t/test-lib.sh |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index cb1ca97..3e92360 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1066,6 +1066,20 @@ case $(uname -s) in
 	;;
 esac
 
+# check UTF-8 locale availability
+test_utf8 () {
+    if test_have_prereq PERL ; then
+	# output an en_US.UTF-8 locale compatible name
+	GIT_LC_UTF8=`$PERL_PATH $GIT_BUILD_DIR/t/lib-locale.pl`
+    fi
+    if test -z "$GIT_LC_UTF8" ; then
+	return 1
+    else
+	test_set_prereq UTF8
+	return 0
+    fi
+}
+
 test -z "$NO_PERL" && test_set_prereq PERL
 test -z "$NO_PYTHON" && test_set_prereq PYTHON
 
-- 
1.7.3.4

Re: [PATCH 2/4] test-lib.sh: add test_utf8() function

[Junio C Hamano]

Yann Droneaud <yann@droneaud.fr> writes:

> +# check UTF-8 locale availability
> +test_utf8 () {
> +    if test_have_prereq PERL ; then
> +	# output an en_US.UTF-8 locale compatible name
> +	GIT_LC_UTF8=`$PERL_PATH $GIT_BUILD_DIR/t/lib-locale.pl`
> +    fi
> +    if test -z "$GIT_LC_UTF8" ; then
> +	return 1
> +    else
> +	test_set_prereq UTF8
> +	return 0
> +    fi
> +}

Nice abstraction to have a helper function that picks a locale to be used
when we want to test UTF-8 thingy.  Perhaps pick_utf8_locale might be a
better name, though. 

The comment in the function is not wrong per-se, but it and the
implementation in 1/4 may be too restrictive---all it needs to do is to
pick a locale that is UTF-8, and it does not necessarily have to be en_US,
no?

[PATCH 3/4] test: use test_utf8 and GIT_LC_UTF8 where an en_US.UTF-8 locale is required

[Yann Droneaud]

Signed-off-by: Yann Droneaud <yann@droneaud.fr>
---
 t/t9100-git-svn-basic.sh               |   25 ++++++++-----------------
 t/t9129-git-svn-i18n-commitencoding.sh |   13 +++----------
 2 files changed, 11 insertions(+), 27 deletions(-)

diff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh
index b041516..fcdffef 100755
--- a/t/t9100-git-svn-basic.sh
+++ b/t/t9100-git-svn-basic.sh
@@ -4,20 +4,14 @@
 #
 
 test_description='git svn basic tests'
-GIT_SVN_LC_ALL=${LC_ALL:-$LANG}
 
 . ./lib-git-svn.sh
 
 say 'define NO_SVN_TESTS to skip git svn tests'
 
-case "$GIT_SVN_LC_ALL" in
-*.UTF-8)
-	test_set_prereq UTF8
-	;;
-*)
-	say "# UTF-8 locale not set, some tests skipped ($GIT_SVN_LC_ALL)"
-	;;
-esac
+if ! test_utf8 ; then
+	say "# UTF-8 locale not set, some tests skipped"
+fi
 
 test_expect_success \
     'initialize git svn' '
@@ -172,15 +166,12 @@ test_expect_success "$name" '
 	test ! -h "$SVN_TREE"/exec-2.sh &&
 	test_cmp help "$SVN_TREE"/exec-2.sh'
 
-name="commit with UTF-8 message: locale: $GIT_SVN_LC_ALL"
-LC_ALL="$GIT_SVN_LC_ALL"
-export LC_ALL
+name="commit with UTF-8 message: locale: $GIT_LC_UTF8"
 test_expect_success UTF8 "$name" "
-	echo '# hello' >> exec-2.sh &&
-	git update-index exec-2.sh &&
-	git commit -m 'éï∏' &&
-	git svn set-tree HEAD"
-unset LC_ALL
+	LC_ALL=$GIT_LC_UTF8 echo '# hello' >> exec-2.sh &&
+	LC_ALL=$GIT_LC_UTF8 git update-index exec-2.sh &&
+	LC_ALL=$GIT_LC_UTF8 git commit -m 'éï∏' &&
+	LC_ALL=$GIT_LC_UTF8 git svn set-tree HEAD"
 
 name='test fetch functionality (svn => git) with alternate GIT_SVN_ID'
 GIT_SVN_ID=alt
diff --git a/t/t9129-git-svn-i18n-commitencoding.sh b/t/t9129-git-svn-i18n-commitencoding.sh
index 8cfdfe7..f3bbde4 100755
--- a/t/t9129-git-svn-i18n-commitencoding.sh
+++ b/t/t9129-git-svn-i18n-commitencoding.sh
@@ -14,22 +14,15 @@ compare_git_head_with () {
 	test_cmp current "$1"
 }
 
-a_utf8_locale=$(locale -a | sed -n '/\.[uU][tT][fF]-*8$/{
-	p
-	q
-}')
-
-if test -n "$a_utf8_locale"
-then
-	test_set_prereq UTF8
-else
+if ! test_utf8 ; then
 	say "# UTF-8 locale not available, some tests are skipped"
 fi
 
 compare_svn_head_with () {
 	# extract just the log message and strip out committer info.
 	# don't use --limit here since svn 1.1.x doesn't have it,
-	LC_ALL="$a_utf8_locale" svn log `git svn info --url` | perl -w -e '
+	LC_ALL=$GIT_LC_UTF8 svn log `git svn info --url` | \
+	    LC_ALL=$GIT_LC_UTF8 perl -w -e '
 		use bytes;
 		$/ = ("-"x72) . "\n";
 		my @x = <STDIN>;
-- 
1.7.3.4

Re: [PATCH 3/4] test: use test_utf8 and GIT_LC_UTF8 where an en_US.UTF-8 locale is required

[Junio C Hamano]

Yann Droneaud <yann@droneaud.fr> writes:

> Signed-off-by: Yann Droneaud <yann@droneaud.fr>
> ---
>  t/t9100-git-svn-basic.sh               |   25 ++++++++-----------------
>  t/t9129-git-svn-i18n-commitencoding.sh |   13 +++----------
>  2 files changed, 11 insertions(+), 27 deletions(-)

Both are nice changes; this patch shows the earlier abstraction in 2/4 is
the right direction to go.

>  compare_svn_head_with () {
>  	# extract just the log message and strip out committer info.
>  	# don't use --limit here since svn 1.1.x doesn't have it,
> -	LC_ALL="$a_utf8_locale" svn log `git svn info --url` | perl -w -e '
> +	LC_ALL=$GIT_LC_UTF8 svn log `git svn info --url` | \
> +	    LC_ALL=$GIT_LC_UTF8 perl -w -e '

Style.

	LC_ALL=... svn log ... |
        LC_ALL=... perl -w -e '
        	...
	'

When you end a line with '|', the shell knows that you haven't finished
talking to it, so there is no need for the trailing bs-lf there.  Indent
the downstream of the pipe to the same level as the upstream.

[PATCH 4/4] t9129: use "$PERL_PATH" instead of "perl"

[Yann Droneaud]

Signed-off-by: Yann Droneaud <yann@droneaud.fr>
---
 t/t9129-git-svn-i18n-commitencoding.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/t/t9129-git-svn-i18n-commitencoding.sh b/t/t9129-git-svn-i18n-commitencoding.sh
index f3bbde4..6dda569 100755
--- a/t/t9129-git-svn-i18n-commitencoding.sh
+++ b/t/t9129-git-svn-i18n-commitencoding.sh
@@ -22,7 +22,7 @@ compare_svn_head_with () {
 	# extract just the log message and strip out committer info.
 	# don't use --limit here since svn 1.1.x doesn't have it,
 	LC_ALL=$GIT_LC_UTF8 svn log `git svn info --url` | \
-	    LC_ALL=$GIT_LC_UTF8 perl -w -e '
+	    LC_ALL=$GIT_LC_UTF8 $PERL_PATH -w -e '
 		use bytes;
 		$/ = ("-"x72) . "\n";
 		my @x = <STDIN>;
-- 
1.7.3.4

Re: [PATCH 4/4] t9129: use "$PERL_PATH" instead of "perl"

[Junio C Hamano]

Squash this to 3/4, perhaps saying "while at it, fix $this".

Re: [PATCH] t9129: fix UTF-8 locale detection

[Yann Droneaud]

Le mardi 18 mai 2010 à 18:05 +0200, Michael J Gruber a écrit :
> Yann Droneaud venit, vidit, dixit 18.05.2010 16:41:

> This is on Fedora 13, which is not exactly exotic. What is your system?

I've tested on:
 - FreeBSD 8.0
 - NetBSD 5.0.1
 - OpenSolaris 2009.06

and none of these systems report en_US.utf8 in locale -a, they're all
reporting en_US.UTF-8

Regards

-- 
Yann

Re: [PATCH] t9129: fix UTF-8 locale detection

[Miles Bader]

Yann Droneaud <yann@droneaud.fr> writes:
> I've tested on:
>  - FreeBSD 8.0
>  - NetBSD 5.0.1
>  - OpenSolaris 2009.06
>
> and none of these systems report en_US.utf8 in locale -a, they're all
> reporting en_US.UTF-8

Still, "utf8" is common enough, so clearly it should be supported (along
with "utf-8" and "UTF-8" etc).

[On my system::

   $ locale -a
   C
   POSIX
   en_US.utf8
   ja_JP.utf8
   ko_KR
   ko_KR.euckr
   ko_KR.utf8
   korean
   korean.euc
]

-miles

-- 
Idiot, n. A member of a large and powerful tribe whose influence in human
affairs has always been dominant and controlling.