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

On Wed, Jan 18, 2012 at 19:57, Alex Riesen <raa.lkml@gmail.com> 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.

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.