Skip to contentSkip to conversation

t9129: fix UTF-8 locale detection

PATCH In review Yann Droneaud 6 participants 23 messages Jun 15, 2016

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 t9129: fix UTF-8 locale detection

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.

---
 t/t9129-git-svn-i18n-commitencoding.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
t/t9129-git-svn-i18n-commitencoding.sh +1 -1
index b9224bd..ec6ed4f 100755
@@ -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"
Signature · 1 line
1.6.4.4
#1 lore raw

Yann Droneaud venit, vidit, dixit 18.05.2010 16:41:

Quoted 7 lines

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

Quoted 6 lines

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?

Quoted 17 lines

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.

---
 t/t9129-git-svn-i18n-commitencoding.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
t/t9129-git-svn-i18n-commitencoding.sh +0 -0
index b9224bd..ec6ed4f 100755

t/t9129-git-svn-i18n-commitencoding.sh do · line 75

Quoted 8 lines
t/t9129-git-svn-i18n-commitencoding.sh +1 -1
@@ -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

#2 lore raw
Quoted 11 lines · Michael J Gruber

Yann Droneaud venit, vidit, dixit 18.05.2010 16:41:

Quoted 9 lines

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
Quoted 8 lines

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

Quoted 4 lines
Quoted 4 lines

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.

Quoted 15 lines

t/t9129-git-svn-i18n-commitencoding.sh do · line 75

Quoted 6 lines
t/t9129-git-svn-i18n-commitencoding.sh +1 -1
-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

(quoted diff) +1 -1
-if locale -a |grep -q en_US.utf8; then
+if locale -a |egrep -q 'en_US.utf8|en_US.UTF-8'; then
Quoted 2 lines

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.

Signature · 1 line
Yann Droneaud
#3 lore raw

PATCH 1/1 t9129: fix UTF-8 locale detection

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.

---
 t/t9129-git-svn-i18n-commitencoding.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
t/t9129-git-svn-i18n-commitencoding.sh +1 -1
index b9224bd..00a273b 100755
@@ -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"
Signature · 1 line
1.6.4.4
#4 lore raw

t/t9129-git-svn-i18n-commitencoding.sh do · line 72

Quoted 3 lines · Yann Droneaud
t/t9129-git-svn-i18n-commitencoding.sh +1 -1
-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

#5 lore raw
Quoted 9 lines · Linus Torvalds

t/t9129-git-svn-i18n-commitencoding.sh do · line 72

Quoted 3 lines · Yann Droneaud
t/t9129-git-svn-i18n-commitencoding.sh +1 -1
-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).

[...]

Quoted 4 lines

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.

Signature · 1 line
Yann Droneaud
#6 lore raw
Quoted 11 lines · Yann Droneaud
Quoted 9 lines · Linus Torvalds

t/t9129-git-svn-i18n-commitencoding.sh do · line 72

Quoted 3 lines · Yann Droneaud
t/t9129-git-svn-i18n-commitencoding.sh +1 -1
-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.

Quoted 8 lines

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.

Signature · 1 line
Yann Droneaud
#7 lore raw
In reply to Linus Torvalds
Quoted 3 lines · Linus Torvalds

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.

Signature · 3 lines
Andreas Schwab, [email protected]
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."
#8 lore raw
Quoted 7 lines · Andreas Schwab
Quoted 3 lines · Linus Torvalds

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

#9 lore raw

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

In reply to Linus Torvalds
Quoted 9 lines · Linus Torvalds

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(-)
t/t9129-git-svn-i18n-commitencoding.sh +13 -7
index b9224bd..1e9a2eb 100755
@@ -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 &&
#10 lore raw
In reply to Yann Droneaud
Quoted 12 lines · Yann Droneaud
Quoted 11 lines · Michael J Gruber

Yann Droneaud venit, vidit, dixit 18.05.2010 16:41:

Quoted 9 lines

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
Quoted 9 lines
Quoted 8 lines

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.

Signature · 1 line
Yann Droneaud
#11 lore raw

Yann Droneaud venit, vidit, dixit 24.05.2010 19:08:

Quoted 43 lines · Yann Droneaud
Quoted 12 lines · Michael J Gruber
Quoted 11 lines

Yann Droneaud venit, vidit, dixit 18.05.2010 16:41:

Quoted 9 lines

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
Quoted 9 lines
Quoted 8 lines

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.

Quoted 5 lines

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?

Quoted 6 lines

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

Regards.

Thanks,
Michael

#12 lore raw

cover en_US.UTF-8 locale detection

In reply to 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
Signature · 1 line
1.7.3.4
#13 lore raw

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

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

---
 t/lib-locale.pl |  167 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 167 insertions(+), 0 deletions(-)
 create mode 100755 t/lib-locale.pl
t/lib-locale.pl new file +167 -0
new file mode 100755
index 0000000..33d47a5
@@ -0,0 +1,167 @@
+#!/usr/bin/env perl
+
+#
+# Copyright (c) 2011 Yann Droneaud <[email protected]>
+#
+# 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;
Signature · 1 line
1.7.3.4
#14 lore raw

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

Yann Droneaud <[email protected]> writes:

Quoted 9 lines · 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

Quoted 5 lines
---
 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.

Quoted 13 lines
t/lib-locale.pl new file +13 -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>) {

t/lib-locale.pl line 149

Quoted 10 lines
t/lib-locale.pl new file +10 -0
+     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).

#15 lore raw

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

In reply to 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.

---
 t/test-lib.sh |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)
t/test-lib.sh +14 -0
index cb1ca97..3e92360 100644
@@ -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
 
Signature · 1 line
1.7.3.4
#16 lore raw

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

t/test-lib.sh case $(uname -s) in · line 1081

Quoted 13 lines · Yann Droneaud
t/test-lib.sh +13 -0
+# 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?

#17 lore raw

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

In reply to Yann Droneaud
---
 t/t9100-git-svn-basic.sh               |   25 ++++++++-----------------
 t/t9129-git-svn-i18n-commitencoding.sh |   13 +++----------
 2 files changed, 11 insertions(+), 27 deletions(-)
t/t9100-git-svn-basic.sh +8 -17
index b041516..fcdffef 100755
@@ -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
t/t9129-git-svn-i18n-commitencoding.sh +3 -10
index 8cfdfe7..f3bbde4 100755
@@ -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>;
Signature · 1 line
1.7.3.4
#18 lore raw

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

Quoted 5 lines · Yann Droneaud
---
 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.

t/t9129-git-svn-i18n-commitencoding.sh compare_git_head_with () { · line 25

Quoted 6 lines

compare_svn_head_with () {

t/t9129-git-svn-i18n-commitencoding.sh +2 -1
 	# 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.

#19 lore raw

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

In reply to Yann Droneaud
---
 t/t9129-git-svn-i18n-commitencoding.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
t/t9129-git-svn-i18n-commitencoding.sh +1 -1
index f3bbde4..6dda569 100755
@@ -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>;
Signature · 1 line
1.7.3.4
#20 lore raw

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

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

#21 lore raw
In reply to Michael J Gruber
Quoted 3 lines · Michael J Gruber

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

Signature · 1 line
Yann
#22 lore raw
Quoted 7 lines · Yann Droneaud

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

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