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

From: Alex Riesen <redacted>

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 <redacted>
Signed-off-by: Junio C Hamano <redacted>
---
 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

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)'; \

On Mon, Jan 23, 2012 at 23:12, Jonathan Nieder [off-list ref] wrote:

Junio C Hamano wrote:

quoted

    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.

On Mon, Jan 23, 2012 at 23:12, Jonathan Nieder [off-list ref] 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 :)

On Mon, Jan 23, 2012 at 23:04, Junio C Hamano [off-list ref] wrote:

From: Alex Riesen <redacted>

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 <redacted>
Signed-off-by: Junio C Hamano <redacted>

Amen :)