[PATCH] i18n: disable i18n for shell scripts if NO_GETTEXT defined

[PATCH] i18n: disable i18n for shell scripts if NO_GETTEXT defined

[Alex Riesen]

[-- Attachment #1: Type: text/plain, Size: 1794 bytes --]

Otherwise the i18n is used in the scripts even with NO_GETTEXT set.
It is very unexpected.

---

I usually disable i18n on my working systems as they are generally very
out-of-date and not supported by any sane developer. In particular the
gettext provided with this (very old) Cygwin distribution is fubar and
never produces any output.

 Makefile       |    1 +
 git-sh-i18n.sh |    4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index a782409..d82ea6a 100644
--- a/Makefile
+++ b/Makefile
@@ -1887,6 +1887,7 @@ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
     -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
     -e 's|@@LOCALEDIR@@|$(localedir_SQ)|g' \
     -e 's/@@NO_CURL@@/$(NO_CURL)/g' \
+    -e 's/@@NO_GETTEXT@@/$(NO_GETTEXT)/g' \
     -e $(BROKEN_PATH_FIX) \
     $@.sh >$@+
 endef
diff --git a/git-sh-i18n.sh b/git-sh-i18n.sh
index b4575fb..7f7e32b 100644
--- a/git-sh-i18n.sh
+++ b/git-sh-i18n.sh
@@ -18,7 +18,7 @@ export TEXTDOMAINDIR

 if test -z "$GIT_GETTEXT_POISON"
 then
-	if test -z "$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS" && type gettext.sh
>/dev/null 2>&1
+	if test -z "@@NO_GETTEXT@@" && test -z
"$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS" && type gettext.sh >/dev/null
2>&1
 	then
 		# This is GNU libintl's gettext.sh, we don't need to do anything
 		# else than setting up the environment and loading gettext.sh
@@ -29,7 +29,7 @@ then
 		# can't.
 		. gettext.sh

-	elif test -z "$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS" && test
"$(gettext -h 2>&1)" = "-h"
+	elif test -z "@@NO_GETTEXT@@" && test -z
"$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS" && test "$(gettext -h 2>&1)" =
"-h"
 	then
 		# We don't have gettext.sh, but there's a gettext binary in our
 		# path. This is probably Solaris or something like it which has a
-- 
1.7.8.2.388.ge40c2

[-- Attachment #2: 0001-disable-i18n-for-shell-scripts-if-NO_GETTEXT-def.diff --]
[-- Type: text/x-patch, Size: 2022 bytes --]

From 36e73fe14cbecd04512a6e8a21b9eb14d278d1dc Mon Sep 17 00:00:00 2001
From: Alex Riesen <raa.lkml@gmail.com>
Date: Tue, 17 Jan 2012 14:25:24 +0100
Subject: [PATCH] i18n: disable i18n for shell scripts if NO_GETTEXT defined

Otherwise the i18n is used in the scripts even with NO_GETTEXT set.
It is very unexpected.

I generally disable i18n on my working systems as they are generally very
out-of-date and not supported by any sane developer. In particular the
gettext provided with this (very old) Cygwin distribution is fubar and
never produces any output.
---
 Makefile       |    1 +
 git-sh-i18n.sh |    4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index a782409..d82ea6a 100644
--- a/Makefile
+++ b/Makefile
@@ -1887,6 +1887,7 @@ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
     -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
     -e 's|@@LOCALEDIR@@|$(localedir_SQ)|g' \
     -e 's/@@NO_CURL@@/$(NO_CURL)/g' \
+    -e 's/@@NO_GETTEXT@@/$(NO_GETTEXT)/g' \
     -e $(BROKEN_PATH_FIX) \
     $@.sh >$@+
 endef
diff --git a/git-sh-i18n.sh b/git-sh-i18n.sh
index b4575fb..7f7e32b 100644
--- a/git-sh-i18n.sh
+++ b/git-sh-i18n.sh
@@ -18,7 +18,7 @@ export TEXTDOMAINDIR
 
 if test -z "$GIT_GETTEXT_POISON"
 then
-	if test -z "$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS" && type gettext.sh >/dev/null 2>&1
+	if test -z "@@NO_GETTEXT@@" && test -z "$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS" && type gettext.sh >/dev/null 2>&1
 	then
 		# This is GNU libintl's gettext.sh, we don't need to do anything
 		# else than setting up the environment and loading gettext.sh
@@ -29,7 +29,7 @@ then
 		# can't.
 		. gettext.sh
 
-	elif test -z "$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS" && test "$(gettext -h 2>&1)" = "-h"
+	elif test -z "@@NO_GETTEXT@@" && test -z "$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS" && test "$(gettext -h 2>&1)" = "-h"
 	then
 		# We don't have gettext.sh, but there's a gettext binary in our
 		# path. This is probably Solaris or something like it which has a
-- 
1.7.8.2.388.ge40c2

Re: [PATCH] i18n: disable i18n for shell scripts if NO_GETTEXT defined

[Junio C Hamano]

Alex Riesen <raa.lkml@gmail.com> writes:

> From: Alex Riesen <raa.lkml@gmail.com>
> Date: Tue, 17 Jan 2012 14:25:24 +0100
> Subject: [PATCH] i18n: disable i18n for shell scripts if NO_GETTEXT defined
>
> Otherwise the i18n is used in the scripts even with NO_GETTEXT set.
> It is very unexpected.
>
> I generally disable i18n on my working systems as they are generally very
> out-of-date and not supported by any sane developer. In particular the
> gettext provided with this (very old) Cygwin distribution is fubar and
> never produces any output.
> ---

Thanks for spotting. I agree that we should honor NO_GETTEXT here.

But the result of the patch looks almost unreadable. could we restructure
the script like this instead?

        # Decide what to do...
        GIT_INTERNAL_GETTEXT_SH_SCHEME=fallthrough
	if test -n "@@NO_GETTEXT@@$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS"
	then
		: no probing necessary
        elif test -n "$GIT_GETTEXT_POISON"
        then
                GIT_INTERNAL_GETTEXT_SH_SCHEME=poison
        elif type gettext.sh >/dev/null 2>&1
        then
                GIT_INTERNAL_GETTEXT_SH_SCHEME=gnu
        elif test "$(gettext -h 2>&1)" = "-h"
        then
                GIT_INTERNAL_GETTEXT_SH_SCHEME=solaris
        fi
        export GIT_INTERNAL_GETTEXT_SH_SCHEME

        # ... and then carry out the decision
        case "$GIT_INTERNAL_GETTEXT_SH_SCHEME" in
        gnu)
                ... gnu definition here ...
                ;;
        solaris)
                ... solaris cdefinition here ...
                ;;
        poison)
                ... poison cdefinition here ...
                ;;
        *)
                ... fallthru definition here ...
                ;;
        esac

Re: [PATCH] i18n: disable i18n for shell scripts if NO_GETTEXT defined

[Alex Riesen]

On Tue, Jan 17, 2012 at 20:08, Junio C Hamano <gitster@pobox.com> wrote:
> Alex Riesen <raa.lkml@gmail.com> writes:
> Thanks for spotting. I agree that we should honor NO_GETTEXT here.
>
> But the result of the patch looks almost unreadable. could we restructure
> the script like this instead?
>
>        # Decide what to do...
>        GIT_INTERNAL_GETTEXT_SH_SCHEME=fallthrough
>        if test -n "@@NO_GETTEXT@@$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS"
>        then

Oh, it is much nicer indeed. I shall redo the patch as soon as I get off
this crappy winxp computer.

Re: [PATCH] i18n: disable i18n for shell scripts if NO_GETTEXT defined

[Ævar Arnfjörð Bjarmason]

On Tue, Jan 17, 2012 at 14:42, Alex Riesen <raa.lkml@gmail.com> wrote:
> Otherwise the i18n is used in the scripts even with NO_GETTEXT set.
> It is very unexpected.

So the reason it's like that is that I was assuming that gettext.sh
wouldn't be FUBAR anywhere, but the translations shouldn't kick in
since we haven't installed them during "make install".

But I wonder if this negatively affects some systems, now we now:

 * Don't use gettext.sh, which means that we're using our fallback
   shell function instead of the binary gettext(1), which is probably
   faster.

 * Use our own eval_gettext() instead of using the system one, which
   uses the GNU binary which is more likely to be in the FS cache
   already since other programs are probably using it.

Which is why I didn't do something like this to begin with.

Re: [PATCH] i18n: disable i18n for shell scripts if NO_GETTEXT defined

[Alex Riesen]

On Wed, Jan 18, 2012 at 16:22, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> On Tue, Jan 17, 2012 at 14:42, Alex Riesen <raa.lkml@gmail.com> wrote:
>> Otherwise the i18n is used in the scripts even with NO_GETTEXT set.
>> It is very unexpected.
>
> So the reason it's like that is that I was assuming that gettext.sh
> wouldn't be FUBAR anywhere, but the translations shouldn't kick in
> since we haven't installed them during "make install".
>
> But I wonder if this negatively affects some systems, now we now:
>
>  * Don't use gettext.sh, which means that we're using our fallback
>   shell function instead of the binary gettext(1), which is probably
>   faster.
>
>  * Use our own eval_gettext() instead of using the system one, which
>   uses the GNU binary which is more likely to be in the FS cache
>   already since other programs are probably using it.
>
> Which is why I didn't do something like this to begin with.

Well, if I say NO_GETTEXT, I kind of want none of local gettext,
whether it works, or not.

[PATCH] i18n: disable i18n for shell scripts if NO_GETTEXT defined

[Alex Riesen]

Otherwise the i18n is used in the scripts even with NO_GETTEXT set.
It is very unexpected.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>

---

Junio C Hamano, Tue, Jan 17, 2012 20:08:34 +0100:
> Alex Riesen <raa.lkml@gmail.com> writes:
> > Otherwise the i18n is used in the scripts even with NO_GETTEXT set.
> > It is very unexpected.
> >
...
> 
> But the result of the patch looks almost unreadable. could we restructure
> the script like this instead?
> ...

Done. I simplified the commentary on "poison" a little.

 Makefile       |    1 +
 git-sh-i18n.sh |  102 +++++++++++++++++++++++++++-----------------------------
 2 files changed, 50 insertions(+), 53 deletions(-)

diff --git a/Makefile b/Makefile
index a782409..d82ea6a 100644
--- a/Makefile
+++ b/Makefile
@@ -1887,6 +1887,7 @@ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
     -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
     -e 's|@@LOCALEDIR@@|$(localedir_SQ)|g' \
     -e 's/@@NO_CURL@@/$(NO_CURL)/g' \
+    -e 's/@@NO_GETTEXT@@/$(NO_GETTEXT)/g' \
     -e $(BROKEN_PATH_FIX) \
     $@.sh >$@+
 endef
diff --git a/git-sh-i18n.sh b/git-sh-i18n.sh
index b4575fb..1902fb1 100644
--- a/git-sh-i18n.sh
+++ b/git-sh-i18n.sh
@@ -16,61 +16,44 @@ else
 fi
 export TEXTDOMAINDIR
 
-if test -z "$GIT_GETTEXT_POISON"
+GIT_INTERNAL_GETTEXT_SH_SCHEME=fallthrough
+if test -n "@@NO_GETTEXT@@$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS"
+then
+	: no probing necessary
+elif test -n "$GIT_GETTEXT_POISON"
 then
-	if test -z "$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS" && type gettext.sh >/dev/null 2>&1
-	then
-		# This is GNU libintl's gettext.sh, we don't need to do anything
-		# else than setting up the environment and loading gettext.sh
-		GIT_INTERNAL_GETTEXT_SH_SCHEME=gnu
-		export GIT_INTERNAL_GETTEXT_SH_SCHEME
-
-		# Try to use libintl's gettext.sh, or fall back to English if we
-		# can't.
-		. gettext.sh
-
-	elif test -z "$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS" && test "$(gettext -h 2>&1)" = "-h"
-	then
-		# We don't have gettext.sh, but there's a gettext binary in our
-		# path. This is probably Solaris or something like it which has a
-		# gettext implementation that isn't GNU libintl.
-		GIT_INTERNAL_GETTEXT_SH_SCHEME=solaris
-		export GIT_INTERNAL_GETTEXT_SH_SCHEME
-
-		# Solaris has a gettext(1) but no eval_gettext(1)
-		eval_gettext () {
-			gettext "$1" | (
-				export PATH $(git sh-i18n--envsubst --variables "$1");
-				git sh-i18n--envsubst "$1"
-			)
-		}
-
-	else
-		# Since gettext.sh isn't available we'll have to define our own
-		# dummy pass-through functions.
-
-		# Tell our tests that we don't have the real gettext.sh
-		GIT_INTERNAL_GETTEXT_SH_SCHEME=fallthrough
-		export GIT_INTERNAL_GETTEXT_SH_SCHEME
-
-		gettext () {
-			printf "%s" "$1"
-		}
-
-		eval_gettext () {
-			printf "%s" "$1" | (
-				export PATH $(git sh-i18n--envsubst --variables "$1");
-				git sh-i18n--envsubst "$1"
-			)
-		}
-	fi
-else
-	# Emit garbage under GETTEXT_POISON=YesPlease. Unlike the C tests
-	# this relies on an environment variable
-
 	GIT_INTERNAL_GETTEXT_SH_SCHEME=poison
-	export GIT_INTERNAL_GETTEXT_SH_SCHEME
+elif test -n type gettext.sh >/dev/null 2>&1
+then
+	# This is GNU libintl's gettext.sh, we don't need to do anything
+	# else than setting up the environment and loading gettext.sh
+	GIT_INTERNAL_GETTEXT_SH_SCHEME=gnu
+elif test "$(gettext -h 2>&1)" = "-h"
+then
+	# We don't have gettext.sh, but there's a gettext binary in our
+	# path. This is probably Solaris or something like it which has a
+	# gettext implementation that isn't GNU libintl.
+	GIT_INTERNAL_GETTEXT_SH_SCHEME=solaris
+fi
+export GIT_INTERNAL_GETTEXT_SH_SCHEME
 
+case "$GIT_INTERNAL_GETTEXT_SH_SCHEME" in
+gnu)
+	# Try to use libintl's gettext.sh, or fall back to English if we
+	# can't.
+	. gettext.sh
+	;;
+solaris)
+	# Solaris has a gettext(1) but no eval_gettext(1)
+	eval_gettext () {
+		gettext "$1" | (
+			export PATH $(git sh-i18n--envsubst --variables "$1");
+			git sh-i18n--envsubst "$1"
+		)
+	}
+	;;
+poison)
+	# Used in tests
 	gettext () {
 		printf "%s" "# GETTEXT POISON #"
 	}
@@ -78,7 +61,20 @@ else
 	eval_gettext () {
 		printf "%s" "# GETTEXT POISON #"
 	}
-fi
+	;;
+*)
+	gettext () {
+		printf "%s" "$1"
+	}
+
+	eval_gettext () {
+		printf "%s" "$1" | (
+			export PATH $(git sh-i18n--envsubst --variables "$1");
+			git sh-i18n--envsubst "$1"
+		)
+	}
+	;;
+esac
 
 # Git-specific wrapper functions
 gettextln () {
-- 
1.7.9.rc0.84.g0aa6c

Re: [PATCH] i18n: disable i18n for shell scripts if NO_GETTEXT defined

[Ævar Arnfjörð Bjarmason]

On Wed, Jan 18, 2012 at 19:57, Alex Riesen <raa.lkml@gmail.com> wrote:
> On Wed, Jan 18, 2012 at 16:22, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
>> On Tue, Jan 17, 2012 at 14:42, Alex Riesen <raa.lkml@gmail.com> wrote:
>>> Otherwise the i18n is used in the scripts even with NO_GETTEXT set.
>>> It is very unexpected.
>>
>> So the reason it's like that is that I was assuming that gettext.sh
>> wouldn't be FUBAR anywhere, but the translations shouldn't kick in
>> since we haven't installed them during "make install".
>>
>> But I wonder if this negatively affects some systems, now we now:
>>
>>  * Don't use gettext.sh, which means that we're using our fallback
>>   shell function instead of the binary gettext(1), which is probably
>>   faster.
>>
>>  * Use our own eval_gettext() instead of using the system one, which
>>   uses the GNU binary which is more likely to be in the FS cache
>>   already since other programs are probably using it.
>>
>> Which is why I didn't do something like this to begin with.
>
> Well, if I say NO_GETTEXT, I kind of want none of local gettext,
> whether it works, or not.

That's not what NO_GETTEXT means, and not what it *should* mean. It
means that your output won't be translated, but we might still make
use of a locally installed library to provide the gettext() and
eval_gettext() functions.

This approach has worked everywhere so far (Linux, OSX, *BSD etc.),
and you want to change *everywhere* because you have some completely
broken Cygwin install.

How did you even get that install? Is it a known issue? Some ancient
now-fixed bug? What version of Cygwin / gettext etc.

Now I'm not saying that we shouldn't fix this, I just don't think that
this is the right way to go about it.

Now I haven't done exhaustive tests but this is the sort of slowdown
we might be looking at on Linux for output, both with warm cache:

    $ cat our-eval_gettext.sh
    #!/bin/bash

    eval_gettext () {
            printf "%s" "$1" | (
                    export PATH $(git sh-i18n--envsubst --variables "$1");
                    git sh-i18n--envsubst "$1"
            )
    }
    for i in {1..1000}
    do
        some_variable="for speed"
        eval_gettext "benchmark this \$some_variable"
    done
    $ time bash our-eval_gettext.sh >/dev/null

    real    0m3.336s
    user    0m0.052s
    sys     0m0.128s

Compared to using the system eval_gettext, which for me is much
faster:

    $ cat system-eval_gettext.sh
    #!/bin/bash

    . gettext.sh
    for i in {1..1000}
    do
        some_variable="for speed"
        eval_gettext "benchmark this \$some_variable"
    done
    $ time bash system-eval_gettext.sh >/dev/null

    real    0m1.671s
    user    0m0.048s
    sys     0m0.140s

And then we have the gettext() function itself:

    $ cat our-gettext.sh
    #!/bin/bash

    gettext () {
            printf "%s" "$1"
    }

    for i in {1..1000}
    do
        gettext "benchmark this"
    done
    $ cat system-gettext.sh
    #!/bin/bash

    for i in {1..1000}
    do
        gettext "benchmark this"
    done

Where our fallback is faster, because printf() is a bash built-in:

    $ time bash system-gettext.sh >/dev/null

    real    0m0.534s
    user    0m0.016s
    sys     0m0.084s
    $ time bash our-gettext.sh >/dev/null

    real    0m0.018s
    user    0m0.016s
    sys     0m0.000s

Anyway speed is the least of the issues here, it's not like we're very
constrained by spewing out gettext output.

I just think we should consider portability more carefully than "it
doesn't work on one obscure setup, let's change it everywhere", when
actually it's working just fine in most places.

I think a better fix would be to add probes for whether the system
functions actually work in the autoconf script.

I'd also love to be able to use C macros in the git-*.sh scripts, it
would make the code in git-sh-i18n.sh much nicer since we can
determine what functions we want at compile time.

Another option would be to pipe our shellscripts through some
pre-processor that would completely remove the gettext and
eval_gettext calls. Then we'd be doing the same thing we're doing on
the C-Level, and we wouldn't have the previously cited command-call
overhead or Win32.

But in summary: We shouldn't be *always* using fallback functions
whether they're the C stuff in compat/* or the gettext fallbacks in
git-sh-i18n.sh just because there's some version out there of the
system-supplied functions that's broken.

It makes sense to prefer the system functions by default in both
cases, but when the OS one can be broken or lacking we can just add
probes or Makefile options like we do for fnmatch() with the
NO_FNMATCH_CASEFOLD switch.

Re: [PATCH] i18n: disable i18n for shell scripts if NO_GETTEXT defined

[Jonathan Nieder]

Hi,

Alex Riesen wrote:

> [Subject: i18n: disable i18n for shell scripts if NO_GETTEXT define]
>
> Otherwise the i18n is used in the scripts even with NO_GETTEXT set.
> It is very unexpected.

Sounds like a good idea.  Quick comments:

[...]
> --- a/git-sh-i18n.sh
> +++ b/git-sh-i18n.sh
> @@ -16,61 +16,44 @@ else
>  fi
>  export TEXTDOMAINDIR
>  
> -if test -z "$GIT_GETTEXT_POISON"
> +GIT_INTERNAL_GETTEXT_SH_SCHEME=fallthrough
> +if test -n "@@NO_GETTEXT@@$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS"
> +then
> +	: no probing necessary
> +elif test -n "$GIT_GETTEXT_POISON"
>  then
> -	if test -z "$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS" && type gettext.sh >/dev/null 2>&1
> -	then
> +elif test -n type gettext.sh >/dev/null 2>&1
> +then

I like the unindenting.  Alas, I get

	1: test: type: unexpected operator

I suspect this should just say "elif type gettext.sh >/dev/null 2>&1".

The rest looks good.  Thanks for writing it.

Re: [PATCH] i18n: disable i18n for shell scripts if NO_GETTEXT defined

[Jonathan Nieder]

Ævar Arnfjörð Bjarmason wrote:

> That's not what NO_GETTEXT means, and not what it *should* mean. It
> means that your output won't be translated, but we might still make
> use of a locally installed library to provide the gettext() and
> eval_gettext() functions.
>
> This approach has worked everywhere so far (Linux, OSX, *BSD etc.),
> and you want to change *everywhere* because you have some completely
> broken Cygwin install.

I thought NO_GETTEXT meant either "I'm aware that there is this new
translation feature, and it may or may not be useful to me some day,
but no thanks for now, since I cannot tolerate the possibility of
regressions" (i.e., opting out of a new feature) or "my platform does
not have suitable gettext infrastructure so please do not use it"
(i.e., reducing build-time dependencies by making some optional).

"I don't want localized messages" is spelled as "LC_MESSAGES=C; export
LC_MESSAGES", not as "make NO_GETTEXT=YesPlease".

I guess I am wondering, does the approach in Alex's patch have the
potential to cause actual problems?  If it doesn't, I don't see what
there is to complain about.

Hope that helps,
Jonathan

Re: [PATCH] i18n: disable i18n for shell scripts if NO_GETTEXT defined

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> On Wed, Jan 18, 2012 at 19:57, Alex Riesen <raa.lkml@gmail.com> wrote:
> ...
>> Well, if I say NO_GETTEXT, I kind of want none of local gettext,
>> whether it works, or not.
>
> That's not what NO_GETTEXT means, and not what it *should* mean. It
> means that your output won't be translated, but we might still make
> use of a locally installed library to provide the gettext() and
> eval_gettext() functions.

You are right.

In the current approach we take for shell scripts, we cannot have "No i18n
whatsoever and messages are emit with printf and echo". We always have to
go through gettext/eval_gettext even though they may be an implementation
that does not do i18n at all.

> Now I haven't done exhaustive tests but this is the sort of slowdown
> we might be looking at on Linux for output,...

I think we judged that it is OK not to worry about the performance of
message generation, back when we decided to take the current approach.

> Anyway speed is the least of the issues here, it's not like we're very
> constrained by spewing out gettext output.
>
> I just think we should consider portability more carefully than "it
> doesn't work on one obscure setup, let's change it everywhere", when
> actually it's working just fine in most places.
> ...
> But in summary: We shouldn't be *always* using fallback functions
> whether they're the C stuff in compat/* or the gettext fallbacks in
> git-sh-i18n.sh just because there's some version out there of the
> system-supplied functions that's broken.
> 
> It makes sense to prefer the system functions by default in both
> cases, but when the OS one can be broken or lacking we can just add
> probes or Makefile options like we do for fnmatch() with the
> NO_FNMATCH_CASEFOLD switch.

So we need "MY_GETTEXT_IS_BROKEN" to decline the use of system gettext
in addition to "NO_GETTEXT" to ask Git not to translate the messages. Is
that correct?

If that is the case, should we do something like

	LANG=C LC_ALL=C
        export LANG LC_ALL

in our shell scripts, when building for NO_GETTEXT target?

Re: [PATCH] i18n: disable i18n for shell scripts if NO_GETTEXT defined

[Johannes Sixt]

Am 1/19/2012 1:17, schrieb Junio C Hamano:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> 
>> On Wed, Jan 18, 2012 at 19:57, Alex Riesen <raa.lkml@gmail.com> wrote:
>> ...
>>> Well, if I say NO_GETTEXT, I kind of want none of local gettext,
>>> whether it works, or not.
>>
>> That's not what NO_GETTEXT means, and not what it *should* mean. It
>> means that your output won't be translated, but we might still make
>> use of a locally installed library to provide the gettext() and
>> eval_gettext() functions.
> 
> You are right.

Sorry to disagree: We have, e.g., NO_MMAP, and I can set it to request
that some alternative is used, even if I have a working mmap(). The option
name "NO_GETTEXT" is in exactly the same spirit.

> In the current approach we take for shell scripts, we cannot have "No i18n
> whatsoever and messages are emit with printf and echo". We always have to
> go through gettext/eval_gettext even though they may be an implementation
> that does not do i18n at all.

Just like we go through _() in C code, even though there may be an
implementation that does not do i18n at all, right?

gettext/eval_gettext annotations are the shell equivalent of _()
annotations in C code, aren't they? Neither go away just by defining
NO_GETTEXT. It is just a quality-of-implementation issue that those
annotations have as little overhead as possible if NO_GETTEXT is defined.
In C, it is easy, in shell code it may be more involved.

-- Hannes

Re: [PATCH] i18n: disable i18n for shell scripts if NO_GETTEXT defined

[Alex Riesen]

On Thu, Jan 19, 2012 at 00:18, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> On Wed, Jan 18, 2012 at 19:57, Alex Riesen <raa.lkml@gmail.com> wrote:
>>
>> Well, if I say NO_GETTEXT, I kind of want none of local gettext,
>> whether it works, or not.
>
> That's not what NO_GETTEXT means, and not what it *should* mean. It
> means that your output won't be translated, but we might still make
> use of a locally installed library to provide the gettext() and
> eval_gettext() functions.

I would never guess all that by its name: NO_GETTEXT. I wanted to
say: there is no gettext in this installation, don't even try it.

> This approach has worked everywhere so far (Linux, OSX, *BSD etc.),
> and you want to change *everywhere* because you have some completely
> broken Cygwin install.

Just as I said.

> How did you even get that install? Is it a known issue? Some ancient
> now-fixed bug?

It is very likely. Or probably just a one installation problem: the
problem is not consistently everywhere here. Some installations
work (if slow).

> What version of Cygwin / gettext etc.

No idea. The person or persons who did this to me have no idea either.

> Now I'm not saying that we shouldn't fix this, I just don't think that
> this is the right way to go about it.

And I agree.

> But in summary: We shouldn't be *always* using fallback functions
> whether they're the C stuff in compat/* or the gettext fallbacks in
> git-sh-i18n.sh just because there's some version out there of the
> system-supplied functions that's broken.
>
> It makes sense to prefer the system functions by default in both
> cases, but when the OS one can be broken or lacking we can just add
> probes or Makefile options like we do for fnmatch() with the
> NO_FNMATCH_CASEFOLD switch.

Yes, and I personally shall welcome a chance to insult the local IT
by suggesting BROKEN_SH_GETTEXT. Not that they get the point...

Re: [PATCH] i18n: disable i18n for shell scripts if NO_GETTEXT defined

[Alex Riesen]

On Thu, Jan 19, 2012 at 01:12, Jonathan Nieder <jrnieder@gmail.com> wrote:
> I like the unindenting.  Alas, I get
>
>        1: test: type: unexpected operator
>
> I suspect this should just say "elif type gettext.sh >/dev/null 2>&1".

Indeed. Copy-paste error. I wonder why I didn't notice it...
Yes, ran the tests, both with and without NO_GETTEXT.

> The rest looks good.  Thanks for writing it.

Was Junios idea, I just liked it.

Re: [PATCH] i18n: disable i18n for shell scripts if NO_GETTEXT defined

[Alex Riesen]

On Thu, Jan 19, 2012 at 01:17, Junio C Hamano <gitster@pobox.com> wrote:
> So we need "MY_GETTEXT_IS_BROKEN" to decline the use of system gettext
> in addition to "NO_GETTEXT" to ask Git not to translate the messages. Is
> that correct?

I think yes.

> If that is the case, should we do something like
>
>        LANG=C LC_ALL=C
>        export LANG LC_ALL
>
> in our shell scripts, when building for NO_GETTEXT target?

Just for the record: gettext here stays broken with LANG and LC_ALL set to C.
But the locale-dependent formatting in C functions will change. Wont be a
problem here, though. The named formatting is broken, too: strftime, for
instance, always formats in C locale.

Re: [PATCH] i18n: disable i18n for shell scripts if NO_GETTEXT defined

[Junio C Hamano]

Johannes Sixt <j.sixt@viscovery.net> writes:

> ... We have, e.g., NO_MMAP, and I can set it to request
> that some alternative is used, even if I have a working mmap(). The option
> name "NO_GETTEXT" is in exactly the same spirit.
>
>> In the current approach we take for shell scripts, we cannot have "No i18n
>> whatsoever and messages are emit with printf and echo". We always have to
>> go through gettext/eval_gettext even though they may be an implementation
>> that does not do i18n at all.
>
> Just like we go through _() in C code, even though there may be an
> implementation that does not do i18n at all, right?

Yes, just like that. The small detail that _() can be #define'd out to
empty while gettext/eval_gettext cannot be made to be no-impact like that
does not really matter.

> In C, it is easy, in shell code it may be more involved.

Correct.

[PATCH] add a Makefile switch to avoid gettext translation in shell scripts

[Alex Riesen]

Some systems have gettext.sh (GNU gettext) installed, but it is either broken
or misconfigured in such a way so its output is not usable.
For instance, on this particular system, a Cygwin installations gettext
produces no output whatsoever.

In case the users of these systems are unable or not interested in fixing
them, setting the new Makefile switch should help:

    USE_FALLTHROUGH_GETTEXT_SCHEME=yes

This will replace the translation routines with fallthrough versions, which
currently used only for regression testing.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---

Alex Riesen, Thu, Jan 19, 2012 10:13:20 +0100:
> On Thu, Jan 19, 2012 at 00:18, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> > It makes sense to prefer the system functions by default in both
> > cases, but when the OS one can be broken or lacking we can just add
> > probes or Makefile options like we do for fnmatch() with the
> > NO_FNMATCH_CASEFOLD switch.
> 
> Yes, and I personally shall welcome a chance to insult the local IT
> by suggesting BROKEN_SH_GETTEXT. Not that they get the point...

I believe this patch does just that. It is certainly enough for my purposes.
The copy-paste error noticed by Jonathan is also fixed, thanks!
I didn't add the tracking of the switch in GIT-BUILD-OPTIONS: didn't found
how to do it quickly enough in this time of evening, and gave up, thinking
that no one sane would need to set the option anyway. So at the moment a
"make clean" needed when changing it.

 Makefile       |    4 ++
 git-sh-i18n.sh |  102 +++++++++++++++++++++++++++-----------------------------
 2 files changed, 53 insertions(+), 53 deletions(-)

diff --git a/Makefile b/Makefile
index a782409..806d126 100644
--- a/Makefile
+++ b/Makefile
@@ -47,6 +47,9 @@ all::
 # A translated Git requires GNU libintl or another gettext implementation,
 # plus libintl-perl at runtime.
 #
+# Define USE_FALLTHROUGH_GETTEXT_SCHEME, if you don't want to trust the
+# installed gettext translation of the shell scripts output.
+#
 # Define HAVE_LIBCHARSET_H if you haven't set NO_GETTEXT and you can't
 # trust the langinfo.h's nl_langinfo(CODESET) function to return the
 # current character set. GNU and Solaris have a nl_langinfo(CODESET),
@@ -1887,6 +1890,7 @@ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
     -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
     -e 's|@@LOCALEDIR@@|$(localedir_SQ)|g' \
     -e 's/@@NO_CURL@@/$(NO_CURL)/g' \
+    -e 's/@@USE_FALLTHROUGH_GETTEXT_SCHEME@@/$(USE_FALLTHROUGH_GETTEXT_SCHEME)/g' \
     -e $(BROKEN_PATH_FIX) \
     $@.sh >$@+
 endef
diff --git a/git-sh-i18n.sh b/git-sh-i18n.sh
index b4575fb..da8b214 100644
--- a/git-sh-i18n.sh
+++ b/git-sh-i18n.sh
@@ -16,61 +16,44 @@ else
 fi
 export TEXTDOMAINDIR
 
-if test -z "$GIT_GETTEXT_POISON"
+GIT_INTERNAL_GETTEXT_SH_SCHEME=fallthrough
+if test -n "@@USE_FALLTHROUGH_GETTEXT_SCHEME@@$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS"
+then
+	: no probing necessary
+elif test -n "$GIT_GETTEXT_POISON"
 then
-	if test -z "$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS" && type gettext.sh >/dev/null 2>&1
-	then
-		# This is GNU libintl's gettext.sh, we don't need to do anything
-		# else than setting up the environment and loading gettext.sh
-		GIT_INTERNAL_GETTEXT_SH_SCHEME=gnu
-		export GIT_INTERNAL_GETTEXT_SH_SCHEME
-
-		# Try to use libintl's gettext.sh, or fall back to English if we
-		# can't.
-		. gettext.sh
-
-	elif test -z "$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS" && test "$(gettext -h 2>&1)" = "-h"
-	then
-		# We don't have gettext.sh, but there's a gettext binary in our
-		# path. This is probably Solaris or something like it which has a
-		# gettext implementation that isn't GNU libintl.
-		GIT_INTERNAL_GETTEXT_SH_SCHEME=solaris
-		export GIT_INTERNAL_GETTEXT_SH_SCHEME
-
-		# Solaris has a gettext(1) but no eval_gettext(1)
-		eval_gettext () {
-			gettext "$1" | (
-				export PATH $(git sh-i18n--envsubst --variables "$1");
-				git sh-i18n--envsubst "$1"
-			)
-		}
-
-	else
-		# Since gettext.sh isn't available we'll have to define our own
-		# dummy pass-through functions.
-
-		# Tell our tests that we don't have the real gettext.sh
-		GIT_INTERNAL_GETTEXT_SH_SCHEME=fallthrough
-		export GIT_INTERNAL_GETTEXT_SH_SCHEME
-
-		gettext () {
-			printf "%s" "$1"
-		}
-
-		eval_gettext () {
-			printf "%s" "$1" | (
-				export PATH $(git sh-i18n--envsubst --variables "$1");
-				git sh-i18n--envsubst "$1"
-			)
-		}
-	fi
-else
-	# Emit garbage under GETTEXT_POISON=YesPlease. Unlike the C tests
-	# this relies on an environment variable
-
 	GIT_INTERNAL_GETTEXT_SH_SCHEME=poison
-	export GIT_INTERNAL_GETTEXT_SH_SCHEME
+elif type gettext.sh >/dev/null 2>&1
+then
+	# This is GNU libintl's gettext.sh, we don't need to do anything
+	# else than setting up the environment and loading gettext.sh
+	GIT_INTERNAL_GETTEXT_SH_SCHEME=gnu
+elif test "$(gettext -h 2>&1)" = "-h"
+then
+	# We don't have gettext.sh, but there's a gettext binary in our
+	# path. This is probably Solaris or something like it which has a
+	# gettext implementation that isn't GNU libintl.
+	GIT_INTERNAL_GETTEXT_SH_SCHEME=solaris
+fi
+export GIT_INTERNAL_GETTEXT_SH_SCHEME
 
+case "$GIT_INTERNAL_GETTEXT_SH_SCHEME" in
+gnu)
+	# Try to use libintl's gettext.sh, or fall back to English if we
+	# can't.
+	. gettext.sh
+	;;
+solaris)
+	# Solaris has a gettext(1) but no eval_gettext(1)
+	eval_gettext () {
+		gettext "$1" | (
+			export PATH $(git sh-i18n--envsubst --variables "$1");
+			git sh-i18n--envsubst "$1"
+		)
+	}
+	;;
+poison)
+	# Used in tests
 	gettext () {
 		printf "%s" "# GETTEXT POISON #"
 	}
@@ -78,7 +61,20 @@ else
 	eval_gettext () {
 		printf "%s" "# GETTEXT POISON #"
 	}
-fi
+	;;
+*)
+	gettext () {
+		printf "%s" "$1"
+	}
+
+	eval_gettext () {
+		printf "%s" "$1" | (
+			export PATH $(git sh-i18n--envsubst --variables "$1");
+			git sh-i18n--envsubst "$1"
+		)
+	}
+	;;
+esac
 
 # Git-specific wrapper functions
 gettextln () {
-- 
1.7.9.rc1.92.ga90a1

Re: [PATCH] i18n: disable i18n for shell scripts if NO_GETTEXT defined

[Ævar Arnfjörð Bjarmason]

On Thu, Jan 19, 2012 at 19:30, Junio C Hamano <gitster@pobox.com> wrote:
> Johannes Sixt <j.sixt@viscovery.net> writes:
>
>> ... We have, e.g., NO_MMAP, and I can set it to request
>> that some alternative is used, even if I have a working mmap(). The option
>> name "NO_GETTEXT" is in exactly the same spirit.
>>
>>> In the current approach we take for shell scripts, we cannot have "No i18n
>>> whatsoever and messages are emit with printf and echo". We always have to
>>> go through gettext/eval_gettext even though they may be an implementation
>>> that does not do i18n at all.
>>
>> Just like we go through _() in C code, even though there may be an
>> implementation that does not do i18n at all, right?
>
> Yes, just like that. The small detail that _() can be #define'd out to
> empty while gettext/eval_gettext cannot be made to be no-impact like that
> does not really matter.
>
>> In C, it is easy, in shell code it may be more involved.
>
> Correct.

To elaborate, the C code can:

 * Use the system gettext library to get translations.

 * Use the system gettext library, but effectively be pass-through
   because the user has the C locale.

 * Use our fallback functions which in any modern compiler will be
   optimized out.

However with the shell code we can:

 1. Be using the system gettext & eval_gettext to get translations.

 2. Be using the system gettext & eval_gettext as pass-through, either
    because we don't have translations since we've installed with
    NO_GETTEXT=YesPlease, or because we're in the C locale.

 3. Haven't detected that gettext.sh etc. exists, so we have to provide
    our own fallbacks.

The proposed patch would move all users of NO_GETTEXT=YesPlease to #3,
even though on most platforms we don't need to define our own dummy
fallbacks since the system already provides them.

I don't particularly like it because I'd rather use the OS vendor's
implementation if possible, even for fallback.

However it being broken is also unacceptable, but I think the way
forward is to detect the breakage either at compile time or at
runtime, to that end Alex could you provide us with the output from
the following commands on the offending system where this is broken:

    $ type gettext.sh
    $ gettext.sh --version
    $ gettext -h
    $ gettext "some test text"
    $ . gettext.sh
    eval_gettext
    $ variable=value eval_gettext "some \$variable"

Then how the eval_gettext function is defined:

    $ type eval_gettext
    eval_gettext is a function
    eval_gettext ()
    {
        gettext "$1" | ( export PATH `envsubst --variables "$1"`;
        envsubst "$1" )
    }

And then a --version for whatever programs that function uses,
e.g. here:

    $ envsubst --version

Once we know how it breaks we can e.g. add configure tests for
checking whether we can use the system's gettext library for the
fallbacks.

Could you also run the git test suite as described in t/README? I'd
expect a lot of the i18n tests to fail, but it would be curious to see
which ones exactly.

Re: [PATCH] i18n: disable i18n for shell scripts if NO_GETTEXT defined

[Alex Riesen]

On Fri, Jan 20, 2012 at 10:50, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> However with the shell code we can:
>
>  1. Be using the system gettext & eval_gettext to get translations.
>
>  2. Be using the system gettext & eval_gettext as pass-through, either
>    because we don't have translations since we've installed with
>    NO_GETTEXT=YesPlease, or because we're in the C locale.
>
>  3. Haven't detected that gettext.sh etc. exists, so we have to provide
>    our own fallbacks.
>
> The proposed patch would move all users of NO_GETTEXT=YesPlease to #3,
> even though on most platforms we don't need to define our own dummy
> fallbacks since the system already provides them.
>
> I don't particularly like it because I'd rather use the OS vendor's
> implementation if possible, even for fallback.

Well, I dunno. I wouldn't trust anything to this particular "OS vendoer".

> However it being broken is also unacceptable, but I think the way
> forward is to detect the breakage either at compile time or at
> runtime, ...

Better at runtime, unless the packager explicitly stated they don't want
any of this.

> ... to that end Alex could you provide us with the output from
> the following commands on the offending system where this is broken:
>
>    $ type gettext.sh

gettext.sh is /usr/bin/gettext.sh

>    $ gettext.sh --version

/usr/bin/gettext.sh (GNU gettext-runtime) 0.18.1
Copyright (C) 2003-2007 Free Software Foundation, Inc.
License GPLv2+: GNU GPL version 2 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Written by Bruno Haible

>    $ gettext -h

Nothing. Exit code 127.

>    $ gettext "some test text"

Nothing. Exit code 127.

>    $ . gettext.sh

Nothing. Exit code 0.

>    eval_gettext

Nothing. Exit code 127.

>    $ variable=value eval_gettext "some \$variable"

Nothing. Exit code 127.

> Then how the eval_gettext function is defined:
>
>    $ type eval_gettext

eval_gettext is a function
eval_gettext ()
{
    gettext "$1" | ( export PATH `envsubst --variables "$1"`;
    envsubst "$1" )
}

> And then a --version for whatever programs that function uses,
> e.g. here:
>
>    $ envsubst --version

Nothing. Exit code 127.

> Once we know how it breaks we can e.g. add configure tests for
> checking whether we can use the system's gettext library for the
> fallbacks.

The exit code seems to be a good enough test here, but testing some
output (or even translation) would be safer.

I believe gettext (the binary) just doesn't start at all here. Maybe
some Cygwin library wrong or missing library. Happens all the time
here, as we have different Cygwin installations depending on the
currently used toolchain. QNX Momentics, in particular. Different
versions of them, and it is too cumbersome to keep them apart.

> Could you also run the git test suite as described in t/README? I'd
> expect a lot of the i18n tests to fail, but it would be curious to see
> which ones exactly.

Yes, they do. Can't run them on this problematic system, because they
tend to crash it, if run for an undetermined while. On the other system,
which can run them, gettext works (it is an older Cygwin installation),
so almost all tests pass (some still don't, but for reasons unrelated).
Strangely enough, the problematic system can build. So I don't copy
the git binaries, they are actually built on that system.

[PATCH] git-sh-i18n: detect and avoid broken gettext(1) implementation

[Ævar Arnfjörð Bjarmason]

Even though we can load gettext.sh the gettext(1) and eval_gettext
functions it provides might be completely broken. This reportedly
happens on some Cygwin installations where we can load gettext.sh, but
gettext and eval_gettext both return exit code 127 and no output.

The reason we're trying to load gettext.sh (or the equivalent Solaris
implementation) at all is so we don't have to provide our own fallback
implementation if the OS already has one installed, but because we
didn't test whether it actually worked under GNU gettext we might end
up with broken functions.

Change the detection in git-sh-i18n so that it tests that the output
of "gettext test" produces "test", on Solaris we already test that
"gettext -h" produces "-h", so we were already guarded against the
same sort of failure there.

Reported-by: Alex Riesen <raa.lkml@gmail.com>
---
Here's a minimal patch to git-sh-i18n that should make things work on
Cygwin and any other platforms with broken gettext functions while
also using the OS-provided functions if they work.

I've added a new t0201-gettext-fallbacks-broken-gettext.sh test that
tests this. This required a small change in lib-gettext.sh so I
wouldn't load test-lib.sh twice.

Note that there's already a t0201* test in the repo. Maybe we want to
increment all the gettext test numbers by one to make room for it?

As an aside I'm really not a big fan of having hardcoded numbers in
the test files like this. We don't care about the order of execution
here.

 git-sh-i18n.sh                              |    2 +-
 t/lib-gettext.sh                            |    7 +++++-
 t/t0201-gettext-fallbacks-broken-gettext.sh |   28 +++++++++++++++++++++++++++
 3 files changed, 35 insertions(+), 2 deletions(-)
 create mode 100755 t/t0201-gettext-fallbacks-broken-gettext.sh

diff --git a/git-sh-i18n.sh b/git-sh-i18n.sh
index b4575fb..26a57b0 100644
--- a/git-sh-i18n.sh
+++ b/git-sh-i18n.sh
@@ -18,7 +18,7 @@ export TEXTDOMAINDIR
 
 if test -z "$GIT_GETTEXT_POISON"
 then
-	if test -z "$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS" && type gettext.sh >/dev/null 2>&1
+	if test -z "$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS" && type gettext.sh >/dev/null 2>&1 && test "$(gettext test 2>&1)" = "test"
 	then
 		# This is GNU libintl's gettext.sh, we don't need to do anything
 		# else than setting up the environment and loading gettext.sh
diff --git a/t/lib-gettext.sh b/t/lib-gettext.sh
index 0f76f6c..2c5b758 100644
--- a/t/lib-gettext.sh
+++ b/t/lib-gettext.sh
@@ -3,7 +3,12 @@
 # Copyright (c) 2010 Ævar Arnfjörð Bjarmason
 #
 
-. ./test-lib.sh
+if test -z "$TEST_DIRECTORY"
+then
+	# In case the test loaded test-lib.sh by itself to do some tests
+	# prior to loading us.
+	. ./test-lib.sh
+fi
 
 GIT_TEXTDOMAINDIR="$GIT_BUILD_DIR/po/build/locale"
 GIT_PO_PATH="$GIT_BUILD_DIR/po"
diff --git a/t/t0201-gettext-fallbacks-broken-gettext.sh b/t/t0201-gettext-fallbacks-broken-gettext.sh
new file mode 100755
index 0000000..92b95ae
--- /dev/null
+++ b/t/t0201-gettext-fallbacks-broken-gettext.sh
@@ -0,0 +1,28 @@
+#!/bin/sh
+#
+# Copyright (c) 2012 Ævar Arnfjörð Bjarmason
+#
+
+test_description='Gettext Shell fallbacks with broken gettext'
+
+. ./test-lib.sh
+
+test_expect_success 'set up a fake broken gettext(1)' '
+	cat >gettext <<-\EOF &&
+	#!/bin/sh
+	exit 1
+	EOF
+	chmod +x gettext &&
+    ! ./gettext
+'
+
+PATH=.:$PATH
+. "$TEST_DIRECTORY"/lib-gettext.sh
+
+test_expect_success C_LOCALE_OUTPUT '$GIT_INTERNAL_GETTEXT_SH_SCHEME" is fallthrough with broken gettext(1)' '
+    echo fallthrough >expect &&
+    echo $GIT_INTERNAL_GETTEXT_SH_SCHEME >actual &&
+    test_cmp expect actual
+'
+
+test_done
-- 
1.7.7.3

Re: [PATCH] git-sh-i18n: detect and avoid broken gettext(1) implementation

[Alex Riesen]

On Fri, Jan 20, 2012 at 13:49, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> Even though we can load gettext.sh the gettext(1) and eval_gettext
> functions it provides might be completely broken. This reportedly
> happens on some Cygwin installations where we can load gettext.sh, but
> gettext and eval_gettext both return exit code 127 and no output.
>
> The reason we're trying to load gettext.sh (or the equivalent Solaris
> implementation) at all is so we don't have to provide our own fallback
> implementation if the OS already has one installed, but because we
> didn't test whether it actually worked under GNU gettext we might end
> up with broken functions.
>
> Change the detection in git-sh-i18n so that it tests that the output
> of "gettext test" produces "test", on Solaris we already test that
> "gettext -h" produces "-h", so we were already guarded against the
> same sort of failure there.
>
> Reported-by: Alex Riesen <raa.lkml@gmail.com>
> ---
> Here's a minimal patch to git-sh-i18n that should make things work on
> Cygwin and any other platforms with broken gettext functions while
> also using the OS-provided functions if they work.

FWIW, I confirm it works (which is quite obvious).

Just for giggles, I even risked running the tests and, of course, crashed
that piece of junk with broken Cygwin installation.
Please don't ask me to do that again :) restarting it is PITA as well.

Just for future reference to all poor Cygwin users:

I also left NO_GETTEXT in the config.mak. This, BTW, explains why git
works, while gettext binary does not: one .dll dependency less.

Re: [PATCH] i18n: disable i18n for shell scripts if NO_GETTEXT defined

[Junio C Hamano]

Alex Riesen <raa.lkml@gmail.com> writes:

>> And then a --version for whatever programs that function uses,
>> e.g. here:
>>
>>    $ envsubst --version
>
> Nothing. Exit code 127.

Interesting.

    $ wonbsubst --version; echo $?
    bash: wonbsubst: command not found
    127

Perhaps your distro lacks a necessary package dependencies between gettext
and envsubst?

> I believe gettext (the binary) just doesn't start at all here. Maybe
> some Cygwin library wrong or missing library. Happens all the time
> here,...

Aha.

I guess we either leave it broken for broken installation or add an extra
"MY_GETTEXT_IS_BROKEN" option; in either way, it does not sound like a
serious enough issue that is widespread to be urgently fixed during the
feature freeze.

Re: [PATCH] i18n: disable i18n for shell scripts if NO_GETTEXT defined

[Alex Riesen]

On Fri, Jan 20, 2012 at 20:35, Junio C Hamano <gitster@pobox.com> wrote:
> Alex Riesen <raa.lkml@gmail.com> writes:
>
>>> And then a --version for whatever programs that function uses,
>>> e.g. here:
>>>
>>>    $ envsubst --version
>>
>> Nothing. Exit code 127.
>
> Interesting.
>
>    $ wonbsubst --version; echo $?
>    bash: wonbsubst: command not found
>    127
>
> Perhaps your distro lacks a necessary package dependencies between gettext
> and envsubst?
>
>> I believe gettext (the binary) just doesn't start at all here. Maybe
>> some Cygwin library wrong or missing library. Happens all the time
>> here,...
>
> Aha.
>
> I guess we either leave it broken for broken installation or add an extra
> "MY_GETTEXT_IS_BROKEN" option; in either way, it does not sound like a
> serious enough issue that is widespread to be urgently fixed during the
> feature freeze.

Ævar already posted a very cute little fix for this, which looks
pretty minimal and is very obvious.

P.S. Sorry for html mail before (tried to use the Android Gmail client).

Re: [PATCH] git-sh-i18n: detect and avoid broken gettext(1) implementation

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Here's a minimal patch to git-sh-i18n that should make things work on
> Cygwin and any other platforms with broken gettext functions while
> also using the OS-provided functions if they work.

> I've added a new t0201-gettext-fallbacks-broken-gettext.sh test that
> tests this. This required a small change in lib-gettext.sh so I
> wouldn't load test-lib.sh twice.
>
> Note that there's already a t0201* test in the repo. Maybe we want to
> increment all the gettext test numbers by one to make room for it?
>
> As an aside I'm really not a big fan of having hardcoded numbers in
> the test files like this. We don't care about the order of execution
> here.

We do not care the order but we do care about the uniqueness in parallel
test execution.

It does appear that we need a bit better preprocessing of git-sh-i18n at
the compile time now. How about applying the restructuring shown in the
patch by Alex (without the @@NO_GETTEXT@@ bit) first without changing any
logic, then try making the "First decide what scheme to use" part lighter
weight by replacing the runtime "type gettext.sh" and such checks with
some preprocessing?

IOW, the first step would look like the attached patch, and then we can
replace the entire "First decide" part if/elif/fi chain with just this:

	# The scheme to use
        : ${GIT_INTERNAL_GETTEXT_SH_SCHEME:=@@GETTEXT_SH_SCHEME@@}

so that t/lib-gettext.sh can define and export GIT_INTERNAL_GETTEXT_SH to
always get what it wants to test (fallthrough?). At build time, instead
of, or in addition to, the $(cmd_munge_script), we could replace the
single @@GETTEXT_SH_SCHEME@@ token above with whatever scheme we want to
use to hardcode the decision we make at the compile time.

Hmm?

 git-sh-i18n.sh |  103 +++++++++++++++++++++++++++-----------------------------
 1 files changed, 50 insertions(+), 53 deletions(-)

diff --git a/git-sh-i18n.sh b/git-sh-i18n.sh
index 26a57b0..6648bd3 100644
--- a/git-sh-i18n.sh
+++ b/git-sh-i18n.sh
@@ -16,61 +16,45 @@ else
 fi
 export TEXTDOMAINDIR
 
-if test -z "$GIT_GETTEXT_POISON"
+# First decide what scheme to use...
+GIT_INTERNAL_GETTEXT_SH_SCHEME=fallthrough
+if test -n "$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS"
+then
+	: no probing necessary
+elif test -n "$GIT_GETTEXT_POISON"
 then
-	if test -z "$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS" && type gettext.sh >/dev/null 2>&1 && test "$(gettext test 2>&1)" = "test"
-	then
-		# This is GNU libintl's gettext.sh, we don't need to do anything
-		# else than setting up the environment and loading gettext.sh
-		GIT_INTERNAL_GETTEXT_SH_SCHEME=gnu
-		export GIT_INTERNAL_GETTEXT_SH_SCHEME
-
-		# Try to use libintl's gettext.sh, or fall back to English if we
-		# can't.
-		. gettext.sh
-
-	elif test -z "$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS" && test "$(gettext -h 2>&1)" = "-h"
-	then
-		# We don't have gettext.sh, but there's a gettext binary in our
-		# path. This is probably Solaris or something like it which has a
-		# gettext implementation that isn't GNU libintl.
-		GIT_INTERNAL_GETTEXT_SH_SCHEME=solaris
-		export GIT_INTERNAL_GETTEXT_SH_SCHEME
-
-		# Solaris has a gettext(1) but no eval_gettext(1)
-		eval_gettext () {
-			gettext "$1" | (
-				export PATH $(git sh-i18n--envsubst --variables "$1");
-				git sh-i18n--envsubst "$1"
-			)
-		}
-
-	else
-		# Since gettext.sh isn't available we'll have to define our own
-		# dummy pass-through functions.
-
-		# Tell our tests that we don't have the real gettext.sh
-		GIT_INTERNAL_GETTEXT_SH_SCHEME=fallthrough
-		export GIT_INTERNAL_GETTEXT_SH_SCHEME
-
-		gettext () {
-			printf "%s" "$1"
-		}
-
-		eval_gettext () {
-			printf "%s" "$1" | (
-				export PATH $(git sh-i18n--envsubst --variables "$1");
-				git sh-i18n--envsubst "$1"
-			)
-		}
-	fi
-else
-	# Emit garbage under GETTEXT_POISON=YesPlease. Unlike the C tests
-	# this relies on an environment variable
-
 	GIT_INTERNAL_GETTEXT_SH_SCHEME=poison
-	export GIT_INTERNAL_GETTEXT_SH_SCHEME
+elif type gettext.sh >/dev/null 2>&1
+then
+	# GNU libintl's gettext.sh
+	GIT_INTERNAL_GETTEXT_SH_SCHEME=gnu
+elif test "$(gettext -h 2>&1)" = "-h"
+then
+	# gettext binary exists but no gettext.sh. likely to be a gettext
+	# binary on a Solaris or something that is not GNU libintl and
+	# lack eval_gettext.
+	GIT_INTERNAL_GETTEXT_SH_SCHEME=gettext_without_eval_gettext
+fi
+export GIT_INTERNAL_GETTEXT_SH_SCHEME
 
+# ... and then follow that decision.
+case "$GIT_INTERNAL_GETTEXT_SH_SCHEME" in
+gnu)
+	# Use libintl's gettext.sh, or fall back to English if we can't.
+	. gettext.sh
+	;;
+gettext_without_eval_gettext)
+	# Solaris has a gettext(1) but no eval_gettext(1)
+	eval_gettext () {
+		gettext "$1" | (
+			export PATH $(git sh-i18n--envsubst --variables "$1");
+			git sh-i18n--envsubst "$1"
+		)
+	}
+	;;
+poison)
+	# Emit garbage so that tests that incorrectly rely on translatable
+	# strings will fail.
 	gettext () {
 		printf "%s" "# GETTEXT POISON #"
 	}
@@ -78,7 +62,20 @@ else
 	eval_gettext () {
 		printf "%s" "# GETTEXT POISON #"
 	}
-fi
+	;;
+*)
+	gettext () {
+		printf "%s" "$1"
+	}
+
+	eval_gettext () {
+		printf "%s" "$1" | (
+			export PATH $(git sh-i18n--envsubst --variables "$1");
+			git sh-i18n--envsubst "$1"
+		)
+	}
+	;;
+esac
 
 # Git-specific wrapper functions
 gettextln () {

Re: [PATCH] git-sh-i18n: detect and avoid broken gettext(1) implementation

[Alex Riesen]

On Fri, Jan 20, 2012 at 21:00, Junio C Hamano <gitster@pobox.com> wrote:
> IOW, the first step would look like the attached patch, and then we can
> replace the entire "First decide" part if/elif/fi chain with just this:
>
>        # The scheme to use
>        : ${GIT_INTERNAL_GETTEXT_SH_SCHEME:=@@GETTEXT_SH_SCHEME@@}
>
> so that t/lib-gettext.sh can define and export GIT_INTERNAL_GETTEXT_SH to
> always get what it wants to test (fallthrough?). At build time, instead
> of, or in addition to, the $(cmd_munge_script), we could replace the
> single @@GETTEXT_SH_SCHEME@@ token above with whatever scheme we want to
> use to hardcode the decision we make at the compile time.

I can imagine a Solaris system being upgraded to GNU gettext _after_ Git
installation. Hardcoding the decision might break git scripts then.

Re: [PATCH] git-sh-i18n: detect and avoid broken gettext(1) implementation

[Junio C Hamano]

Alex Riesen <raa.lkml@gmail.com> writes:

>> ... At build time, instead
>> of, or in addition to, the $(cmd_munge_script), we could replace the
>> single @@GETTEXT_SH_SCHEME@@ token above with whatever scheme we want to
>> use to hardcode the decision we make at the compile time.
>
> I can imagine a Solaris system being upgraded to GNU gettext _after_ Git
> installation. Hardcoding the decision might break git scripts then.

Just like you would break http transport by removing libcurl after
installing Git? What else is new?

Re: [PATCH] git-sh-i18n: detect and avoid broken gettext(1) implementation

[Alex Riesen]

On Fri, Jan 20, 2012 at 21:21, Junio C Hamano <gitster@pobox.com> wrote:
> Alex Riesen <raa.lkml@gmail.com> writes:
>
>>> ... At build time, instead
>>> of, or in addition to, the $(cmd_munge_script), we could replace the
>>> single @@GETTEXT_SH_SCHEME@@ token above with whatever scheme we want to
>>> use to hardcode the decision we make at the compile time.
>>
>> I can imagine a Solaris system being upgraded to GNU gettext _after_ Git
>> installation. Hardcoding the decision might break git scripts then.
>
> Just like you would break http transport by removing libcurl after
> installing Git? What else is new?

Removing - yes, upgrading it - very unlikely. Besides, the current version
wont have problems with such an upgrade.

Re: [PATCH] git-sh-i18n: detect and avoid broken gettext(1) implementation

[Junio C Hamano]

Alex Riesen <raa.lkml@gmail.com> writes:

> .... Besides, the current version
> wont have problems with such an upgrade.

Yes, but at what cost?

Re: [PATCH] git-sh-i18n: detect and avoid broken gettext(1) implementation

[Alex Riesen]

On Fri, Jan 20, 2012 at 21:26, Junio C Hamano <gitster@pobox.com> wrote:
> Alex Riesen <raa.lkml@gmail.com> writes:
>
>> .... Besides, the current version
>> wont have problems with such an upgrade.
>
> Yes, but at what cost?

A fixed number of "fork and exec" for every executed ". git-sh-i18n" line?
The overhead might be considered negligible...

Er. Am I missing something very obvious? (not obvious to me, yet)

Re: [PATCH] add a Makefile switch to avoid gettext translation in shell scripts

[Junio C Hamano]

Alex Riesen <raa.lkml@gmail.com> writes:

> Some systems have gettext.sh (GNU gettext) installed, but it is either broken
> or misconfigured in such a way so its output is not usable.
> For instance, on this particular system, a Cygwin installations gettext
> produces no output whatsoever.
>
> In case the users of these systems are unable or not interested in fixing
> them, setting the new Makefile switch should help:
>
>     USE_FALLTHROUGH_GETTEXT_SCHEME=yes
>
> This will replace the translation routines with fallthrough versions, which
> currently used only for regression testing.
>
> Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
> ---

I'll split this into two, the first one only to restructure the code and
the guts of this one that avoids the autodetection.

Also I'd rename this so that:

	$ make USE_GETTEXT_SCHEME=fallthrough
	$ make USE_GETTEXT_SCHEME=gnu

could be used to avoid extra and unnecessary runtime overhead when the
person building git knows what is on the system.

[PATCH 1/2] git-sh-i18n: restructure the logic to compute gettext.sh scheme

[Junio C Hamano]

Instead of having a single long and complex chain of commands to decide
what to do and carry out the decision, split the code so that we first
decide which scheme to use, and in the second section define what exactly
is done by the chosen scheme. It makes the code much easier to follow and
update.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-sh-i18n.sh |  103 +++++++++++++++++++++++++++-----------------------------
 1 files changed, 50 insertions(+), 53 deletions(-)

diff --git a/git-sh-i18n.sh b/git-sh-i18n.sh
index b4575fb..6648bd3 100644
--- a/git-sh-i18n.sh
+++ b/git-sh-i18n.sh
@@ -16,61 +16,45 @@ else
 fi
 export TEXTDOMAINDIR
 
-if test -z "$GIT_GETTEXT_POISON"
+# First decide what scheme to use...
+GIT_INTERNAL_GETTEXT_SH_SCHEME=fallthrough
+if test -n "$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS"
+then
+	: no probing necessary
+elif test -n "$GIT_GETTEXT_POISON"
 then
-	if test -z "$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS" && type gettext.sh >/dev/null 2>&1
-	then
-		# This is GNU libintl's gettext.sh, we don't need to do anything
-		# else than setting up the environment and loading gettext.sh
-		GIT_INTERNAL_GETTEXT_SH_SCHEME=gnu
-		export GIT_INTERNAL_GETTEXT_SH_SCHEME
-
-		# Try to use libintl's gettext.sh, or fall back to English if we
-		# can't.
-		. gettext.sh
-
-	elif test -z "$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS" && test "$(gettext -h 2>&1)" = "-h"
-	then
-		# We don't have gettext.sh, but there's a gettext binary in our
-		# path. This is probably Solaris or something like it which has a
-		# gettext implementation that isn't GNU libintl.
-		GIT_INTERNAL_GETTEXT_SH_SCHEME=solaris
-		export GIT_INTERNAL_GETTEXT_SH_SCHEME
-
-		# Solaris has a gettext(1) but no eval_gettext(1)
-		eval_gettext () {
-			gettext "$1" | (
-				export PATH $(git sh-i18n--envsubst --variables "$1");
-				git sh-i18n--envsubst "$1"
-			)
-		}
-
-	else
-		# Since gettext.sh isn't available we'll have to define our own
-		# dummy pass-through functions.
-
-		# Tell our tests that we don't have the real gettext.sh
-		GIT_INTERNAL_GETTEXT_SH_SCHEME=fallthrough
-		export GIT_INTERNAL_GETTEXT_SH_SCHEME
-
-		gettext () {
-			printf "%s" "$1"
-		}
-
-		eval_gettext () {
-			printf "%s" "$1" | (
-				export PATH $(git sh-i18n--envsubst --variables "$1");
-				git sh-i18n--envsubst "$1"
-			)
-		}
-	fi
-else
-	# Emit garbage under GETTEXT_POISON=YesPlease. Unlike the C tests
-	# this relies on an environment variable
-
 	GIT_INTERNAL_GETTEXT_SH_SCHEME=poison
-	export GIT_INTERNAL_GETTEXT_SH_SCHEME
+elif type gettext.sh >/dev/null 2>&1
+then
+	# GNU libintl's gettext.sh
+	GIT_INTERNAL_GETTEXT_SH_SCHEME=gnu
+elif test "$(gettext -h 2>&1)" = "-h"
+then
+	# gettext binary exists but no gettext.sh. likely to be a gettext
+	# binary on a Solaris or something that is not GNU libintl and
+	# lack eval_gettext.
+	GIT_INTERNAL_GETTEXT_SH_SCHEME=gettext_without_eval_gettext
+fi
+export GIT_INTERNAL_GETTEXT_SH_SCHEME
 
+# ... and then follow that decision.
+case "$GIT_INTERNAL_GETTEXT_SH_SCHEME" in
+gnu)
+	# Use libintl's gettext.sh, or fall back to English if we can't.
+	. gettext.sh
+	;;
+gettext_without_eval_gettext)
+	# Solaris has a gettext(1) but no eval_gettext(1)
+	eval_gettext () {
+		gettext "$1" | (
+			export PATH $(git sh-i18n--envsubst --variables "$1");
+			git sh-i18n--envsubst "$1"
+		)
+	}
+	;;
+poison)
+	# Emit garbage so that tests that incorrectly rely on translatable
+	# strings will fail.
 	gettext () {
 		printf "%s" "# GETTEXT POISON #"
 	}
@@ -78,7 +62,20 @@ else
 	eval_gettext () {
 		printf "%s" "# GETTEXT POISON #"
 	}
-fi
+	;;
+*)
+	gettext () {
+		printf "%s" "$1"
+	}
+
+	eval_gettext () {
+		printf "%s" "$1" | (
+			export PATH $(git sh-i18n--envsubst --variables "$1");
+			git sh-i18n--envsubst "$1"
+		)
+	}
+	;;
+esac
 
 # Git-specific wrapper functions
 gettextln () {
-- 
1.7.9.rc2.48.g92994

[PATCH 2/2] add a Makefile switch to avoid gettext translation in shell scripts

[Junio C Hamano]

From: Alex Riesen <raa.lkml@gmail.com>

Some systems have gettext.sh (GNU gettext) installed, but it is either
broken or misconfigured in such a way so its output is not usable.  In
case the users of these systems are unable or not interested in fixing
them, setting the new Makefile switch should help:

    make USE_GETTEXT_SCHEME=fallthrough

This will replace the translation routines with fallthrough versions,
that does not use gettext from the platform.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Makefile       |    4 ++++
 git-sh-i18n.sh |    5 ++++-
 2 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index 9470a10..4435854 100644
--- a/Makefile
+++ b/Makefile
@@ -47,6 +47,9 @@ all::
 # A translated Git requires GNU libintl or another gettext implementation,
 # plus libintl-perl at runtime.
 #
+# Define USE_GETTEXT_SCHEME and set it to 'fallthrough', if you don't trust
+# the installed gettext translation of the shell scripts output.
+#
 # Define HAVE_LIBCHARSET_H if you haven't set NO_GETTEXT and you can't
 # trust the langinfo.h's nl_langinfo(CODESET) function to return the
 # current character set. GNU and Solaris have a nl_langinfo(CODESET),
@@ -1874,6 +1877,7 @@ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
     -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
     -e 's|@@LOCALEDIR@@|$(localedir_SQ)|g' \
     -e 's/@@NO_CURL@@/$(NO_CURL)/g' \
+    -e 's/@@USE_GETTEXT_SCHEME@@/$(USE_GETTEXT_SCHEME)/g' \
     -e $(BROKEN_PATH_FIX) \
     $@.sh >$@+
 endef
diff --git a/git-sh-i18n.sh b/git-sh-i18n.sh
index 6648bd3..d5fae99 100644
--- a/git-sh-i18n.sh
+++ b/git-sh-i18n.sh
@@ -18,7 +18,10 @@ export TEXTDOMAINDIR
 
 # First decide what scheme to use...
 GIT_INTERNAL_GETTEXT_SH_SCHEME=fallthrough
-if test -n "$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS"
+if test -n "@@USE_GETTEXT_SCHEME@@"
+then
+	GIT_INTERNAL_GETTEXT_SH_SCHEME="@@USE_GETTEXT_SCHEME@@"
+elif test -n "@@USE_FALLTHROUGH_GETTEXT_SCHEME@@$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS"
 then
 	: no probing necessary
 elif test -n "$GIT_GETTEXT_POISON"
-- 
1.7.9.rc2.48.g92994

Re: [PATCH 2/2] add a Makefile switch to avoid gettext translation in shell scripts

[Jonathan Nieder]

Junio C Hamano wrote:

>     make USE_GETTEXT_SCHEME=fallthrough
>
> This will replace the translation routines with fallthrough versions,
> that does not use gettext from the platform.

Nice implementation.  I still don't understand why NO_GETTEXT=YesPlease
should not imply this.  Is it to ensure the GETTEXT_SCHEME=gnu mode
gets more testing?

Here's a patch to consider squashing in that makes the option take
effect if it changes between builds.

diff --git i/Makefile w/Makefile
index 63dfd64d..b2b738bb 100644
--- i/Makefile
+++ w/Makefile
@@ -2268,7 +2268,7 @@ cscope:
 ### Detect prefix changes
 TRACK_CFLAGS = $(CC):$(subst ','\'',$(ALL_CFLAGS)):\
              $(bindir_SQ):$(gitexecdir_SQ):$(template_dir_SQ):$(prefix_SQ):\
-             $(localedir_SQ)
+             $(localedir_SQ):$(USE_GETTEXT_SCHEME)
 
 GIT-CFLAGS: FORCE
 	@FLAGS='$(TRACK_CFLAGS)'; \

Re: [PATCH 2/2] add a Makefile switch to avoid gettext translation in shell scripts

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> Junio C Hamano wrote:
>
>>     make USE_GETTEXT_SCHEME=fallthrough
>>
>> This will replace the translation routines with fallthrough versions,
>> that does not use gettext from the platform.
>
> Nice implementation.  I still don't understand why NO_GETTEXT=YesPlease
> should not imply this.

Should be easy to do so, like this?

diff --git a/Makefile b/Makefile
index a782409..c4c1066 100644
--- a/Makefile
+++ b/Makefile
@@ -1521,6 +1521,7 @@ ifdef GETTEXT_POISON
 endif
 ifdef NO_GETTEXT
 	BASIC_CFLAGS += -DNO_GETTEXT
+	USE_GETTEXT_SCHEME = fallthrough
 endif
 ifdef NO_STRCASESTR
 	COMPAT_CFLAGS += -DNO_STRCASESTR

Re: [PATCH 2/2] add a Makefile switch to avoid gettext translation in shell scripts

[Jonathan Nieder]

Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:

>> Nice implementation.  I still don't understand why NO_GETTEXT=YesPlease
>> should not imply this.
>
> Should be easy to do so, like this?
>
> diff --git a/Makefile b/Makefile
> index a782409..c4c1066 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1521,6 +1521,7 @@ ifdef GETTEXT_POISON
>  endif
>  ifdef NO_GETTEXT
>  	BASIC_CFLAGS += -DNO_GETTEXT
> +	USE_GETTEXT_SCHEME = fallthrough
>  endif

Yep, that would make my worries about intuitive behavior evaporate. :)
(Maybe "USE_GETTEXT_SCHEME ?= fallthrough" to make it easier to
override in config.mak.)

Thanks.  I also would not actually mind the behavior without that
tweak, as long as it's explained somewhere.

Ciao,
Jonathan

[PATCH/RFC 3/2] i18n: do not use gettext.sh by default when NO_GETTEXT is set

[Jonathan Nieder]

From: Junio C Hamano <gitster@pobox.com>

When NO_GETTEXT is set, even if a usable installed gettext.sh is
detected to be present, it can only help to use git's simple fallback
stub implementations of gettext and eval_gettext for shell scripts
instead.  That way:

 1) we avoid the complication of autodetection of gettext.sh support
    at runtime;

 2) in particular, if the operating system provides gettext.sh but
    it is unusable, we will not be tricked into trying to use it.

So this patch makes USE_GETTEXT_SCHEME default to fallthrough when
NO_GETTEXT is set.  This is only a default so the operator can set
USE_GETTEXT_SCHEME=gnu or ...=gettext_without_eval_gettext to try the
other schemes.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Junio C Hamano wrote:

> Should be easy to do so, like this?

Probably too late to be useful, but here's a patch with commit message
implementing the same.

 Makefile |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index b2b738bb..e9b4a2f9 100644
--- a/Makefile
+++ b/Makefile
@@ -1524,6 +1524,9 @@ ifdef GETTEXT_POISON
 endif
 ifdef NO_GETTEXT
 	BASIC_CFLAGS += -DNO_GETTEXT
+	ifndef USE_GETTEXT_SCHEME
+		USE_GETTEXT_SCHEME = fallthrough
+	endif
 endif
 ifdef NO_STRCASESTR
 	COMPAT_CFLAGS += -DNO_STRCASESTR
-- 
1.7.9.rc2

Re: [PATCH 2/2] add a Makefile switch to avoid gettext translation in shell scripts

[Ævar Arnfjörð Bjarmason]

On Mon, Jan 23, 2012 at 23:12, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Junio C Hamano wrote:
>
>>     make USE_GETTEXT_SCHEME=fallthrough
>>
>> This will replace the translation routines with fallthrough versions,
>> that does not use gettext from the platform.
>
> Nice implementation.  I still don't understand why NO_GETTEXT=YesPlease
> should not imply this.  Is it to ensure the GETTEXT_SCHEME=gnu mode
> gets more testing?

I was the only one with an objection to doing that. The main (and I
admit, at least slightly irrational) reason being that I simply don't
like using fallback functions when the system supplies us with
perfectly good functions we can use instead.

It means we're less likely to share code / fixes / eyeballs / cache
with other programs. I.e. by using envsubst(1) instead of
git-sh-i18n--envsubst--variables(1).

Ironically this is all my fault by naming the option for turning off
translations NO_GETTEXT. What it should be called is
DO_NOT_TRANSLATE_OUTPUT, but since we *need* shell functions to output
anything it might have used a system gettext library to do that,
NO_GETTEXT should have been "I don't have any gettext library, please
supply some fallbacks".

Which would have meant that for people who simply don't want
translated output we'd be using the maintained by upstream envsubst(1)
instead of the doomed to bitrot forever hack I ripped out of some old
GPL2 version of GNU gettext.

Anyway in the grand scheme of things none of this really matters,
these patches can all go in as far as I'm concerned. I can submit
patches to improve it once the dust has settled if I still care
enough.

Aside from this I think not having the ability to run a pre-processor
on the shellscripts results in some really ugly workarounds. This
stuff would be much nicer if we could just generate git-sh-i18n.sh at
compile time depending on some autoconf tests or Makefile options.

And by hacking up a pre-processor that just searches/replaces all the
gettext/eval_gettext calls out of the shell code we could sidestep
this whole issue and there wouldn't be any need for fallback
functions, ever. This would also result in a real improvement on
Windows where exec overhead is much larger.

Like this hack, which doesn't even work, but gives you some idea of
what we could do:

    #!/usr/bin/env perl
    BEGIN { $^I = ""; }

    sub unescape {
    	my $str = shift;
    	$str =~ s/\\\$/\$/gs;
    	$str;
    }

    LINE: while (defined($_ = <ARGV>)) {
    	s["\$\(gettext "([^"]+?)"\)"]["$1"]g;
    	s["\$\(eval_gettext "([^"]+?)"\)"]['"' . unescape($1) . '"']eg;
    	s[eval_gettextln "([^"]+?)"]['echo "' . unescape($1) . '"']eg;
    	s[gettext "([^"]+?)"][printf "%s" "$1"]g;
    	s[gettextln "([^"]+?)"][echo "$1"]g;
    #	s[gettextln "([^"]+?)"][echo "$1"]g;
    #	s/foo/bar/;
    	print;

    }


When run:

    for f in $(git grep -l gettext -- *.sh); do perl replace-gettext.pl $f; done

Produces output like:

    @@ -351 +351 @@ split_patches () {
    -                       clean_abort "$(eval_gettext "Patch format
\$patch_format is not supported.")"
    +                       clean_abort "Patch format $patch_format is
not supported."
    @@ -353 +353 @@ split_patches () {
    -                       clean_abort "$(gettext "Patch format
detection failed.")"
    +                       clean_abort "Patch format detection failed."
    @@ -403 +403 @@ do
    -               die "$(gettext "-d option is no longer supported.
Do not use.")"
    +               die "-d option is no longer supported.  Do not use."
    @@ -466 +466 @@ then
    -       die "$(eval_gettext "previous rebase directory \$dotest
still exists but mbox given.")"
    +       die "previous rebase directory $dotest still exists but mbox given."

It would be relatively easy to hack up a basic POSIX shell
pre-processor like this that would work on our *.sh files, thus
eliminating the need for all of this fallback business.

Re: [PATCH 2/2] add a Makefile switch to avoid gettext translation in shell scripts

[Alex Riesen]

On Mon, Jan 23, 2012 at 23:12, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Here's a patch to consider squashing in that makes the option take
> effect if it changes between builds.

Which actually bit me once, when I was playing with the code :)

Re: [PATCH 2/2] add a Makefile switch to avoid gettext translation in shell scripts

[Alex Riesen]

On Mon, Jan 23, 2012 at 23:04, Junio C Hamano <gitster@pobox.com> wrote:
> From: Alex Riesen <raa.lkml@gmail.com>
>
> Some systems have gettext.sh (GNU gettext) installed, but it is either
> broken or misconfigured in such a way so its output is not usable.  In
> case the users of these systems are unable or not interested in fixing
> them, setting the new Makefile switch should help:
>
>    make USE_GETTEXT_SCHEME=fallthrough
>
> This will replace the translation routines with fallthrough versions,
> that does not use gettext from the platform.
>
> Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Amen :)

Re: [PATCH/RFC 3/2] i18n: do not use gettext.sh by default when NO_GETTEXT is set

[Alex Riesen]

On Tue, Jan 24, 2012 at 01:31, Jonathan Nieder <jrnieder@gmail.com> wrote:
> +       ifndef USE_GETTEXT_SCHEME
> +               USE_GETTEXT_SCHEME = fallthrough
> +       endif

We already use GNU make features (+=), so this can be just

USE_GETTEXT_SCHEME ?= fallthrough

I think...

Re: [PATCH 2/2] add a Makefile switch to avoid gettext translation in shell scripts

[Junio C Hamano]

> Amen :)

Thanks.