Thread (10 messages) flat view 10 messages, 4 authors, 2018-11-08
STALE2862d

[PATCH v2] i18n: make GETTEXT_POISON a runtime option

From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2018-10-24 11:47:41
Subsystem: kernel build + files below scripts/ (unless maintained elsewhere), the rest · Maintainers: Nathan Chancellor, Nicolas Schier, Linus Torvalds 

Change the GETTEXT_POISON compile-time + runtime GIT_GETTEXT_POISON
test parameter to only be a GIT_TEST_GETTEXT_POISON=<boolean> runtime
parameter, to be consistent with other parameters documented in
"Running tests with special setups" in t/README.

When I added GETTEXT_POISON in bb946bba76 ("i18n: add GETTEXT_POISON
to simulate unfriendly translator", 2011-02-22) I was concerned with
ensuring that the _() function would get constant folded if NO_GETTEXT
was defined or if it wasn't and GETTEXT_POISON wasn't defined.

But as the benchmark in my [1] shows doing a one-off runtime
getenv("GIT_TEST_[...]") is trivial, and since GETTEXT_POISON was
originally added this has become the common idiom in the codebase for
turning on special test setups.

So change GETTEXT_POISON to work the same way. Now the
GETTEXT_POISON=YesPlease compile-time option is gone, and running the
tests with GIT_TEST_GETTEXT_POISON=[true|false] can be toggled on/off
without recompiling.

This allows for conditionally amending tests to test with/without
poison, similar to what 859fdc0c3c ("commit-graph: define
GIT_TEST_COMMIT_GRAPH", 2018-08-29) did for GIT_TEST_COMMIT_GRAPH. Do
some of that, now we e.g. always run the t0205-gettext-poison.sh test.

I did enough there to remove the GETTEXT_POISON prerequisite, but its
inverse C_LOCALE_OUTPUT is still around, and surely some tests using
it can be converted to e.g. always set GIT_TEST_GETTEXT_POISON=false.

Notes on the implementation:

 * The only reason we need a new "git-sh-i18n--helper" is to expose
   git_env_bool() to shellscripts, since git-sh-i18n and friends need
   to inspect the $GIT_TEST_GETTEXT_POISON variable.

   We only call it if $GIT_TEST_GETTEXT_POISON is set, or in the test
   suite. This code can go away for non-test code once the last
   i18n-using shellscript is rewritten in C, or if "git config" learns
   to combine --bool and a value fed into it, see [2].

 * We still compile a dedicated GETTEXT_POISON build in Travis CI,
   this is probably the wrong thing to do and should be followed-up
   with something similar to ae59a4e44f ("travis: run tests with
   GIT_TEST_SPLIT_INDEX", 2018-01-07) to re-use an existing test setup
   for running in the GIT_TEST_GETTEXT_POISON mode.

 * The reason for not doing:

       test_lazy_prereq C_LOCALE_OUTPUT [...]

   in test-lib.sh is because there's some interpolation problem with
   that syntax which makes t6040-tracking-info.sh fail. Hence using
   the simpler test_set_prereq. It'll fail with:

       + git branch -vv
       + sed -n -e
       mkdir -p "$TRASH_DIRECTORY/prereq-test-dir" &&
       (
               cd "$TRASH_DIRECTORY/prereq-test-dir" &&! git sh-i18n--helper --git-test-gettext-poison
       )
       sed: -e expression #1, char 2: unknown command: `m'
       error: last command exited with $?=1
       not ok 3 - branch -vv

   This is some interpolation problem with how test_lazy_prereq works
   that doesn't affect test_set_prereq.

 * We now skip a test in t0000-basic.sh under
   GIT_TEST_GETTEXT_POISON=true that wasn't skipped before. This test
   relies on C locale output, but due to an edge case in how the
   previous implementation of GETTEXT_POISON worked (reading it from
   GIT-BUILD-OPTIONS) wasn't enabling poison correctly. Now it does,
   and needs to be skipped.

See also
https://public-inbox.org/git/878t2pd6yu.fsf@evledraar.gmail.com/ for
more discussion.

1. https://public-inbox.org/git/871s8gd32p.fsf@evledraar.gmail.com/

2. https://public-inbox.org/git/20181024074400.GA31239@sigill.intra.peff.net/

Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---

On Wed, Oct 24 2018, Junio C Hamano wrote:

Ævar Arnfjörð Bjarmason  [off-list ref] writes:
quoted
Notes on the implementation:

 * The only reason we need a new "git-sh-i18n--helper" and the
   corresponding "test-tool gettext-poison" is to expose
   git_env_bool() to shellscripts, since git-sh-i18n and friends need
   to inspect the $GIT_TEST_GETTEXT_POISON variable.

   We only call these if $GIT_TEST_GETTEXT_POISON is set, or in the
   test suite, and this code can go away for non-test code once the
   last i18n-using shellscript is rewritten in C.
Makes me wonder if we want to teach "git config" a new "--env-bool"
option that can be used by third-party scripts.  Or would it be just
the matter of writing

	git config --default=false --type=bool "$GIT_WHATEVER_ENV"

in these third-party scripts and we do not need to add such a thing?
As Jeff notes in a follow-up reply git-config can't quite do this
yet. So in lieu of implementing that I have a more minimal helper,
which could be migrated to something like the --get-env mode Jeff is
talking about if we add that.
quoted
 * The reason for not doing:

       test_lazy_prereq GETTEXT_POISON 'test-tool gettext-poison'
       test_lazy_prereq C_LOCALE_OUTPUT '! test-tool gettext-poison'

   In test-lib.sh is because there's some interpolation problem with
   that syntax which makes t6040-tracking-info.sh fail. Hence using
   the simpler test_set_prereq.
s/In/in/, as you haven't finished the sentence yet.  But more
importantly, what is this "some interpolation problem"?  Are you
saying that test_lazy_prereq implementation is somehow broken and
cannot take certain strings?  If so, perhaps we want to fix that,
and people other than you can help to do so, once you let them know
what the problem is.
I haven't fixed (don't know what the fix is) this issue, but the new
version elaborates on what the error is.
quoted
See also
https://public-inbox.org/git/871s8gd32p.fsf@evledraar.gmail.com/ for
more discussion.
"See also [*1*] for more discussion" as you've already have
reference below.
Thanks, mispasted E-Mail ID. Fixed.
quoted
1. https://public-inbox.org/git/871s8gd32p.fsf@evledraar.gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---

Here's a polished version of that. I think it makes sense to queue
this up before any other refactoring of GETTEXT_POISON, and some patch
to unconditionally preserve format specifiers as I suggested upthread
could go on top of this.
...
+int cmd_sh_i18n__helper(int argc, const char **argv, const char *prefix)
+{
+	int poison = -1;
+	struct option options[] = {
+		OPT_BOOL(0, "git-test-gettext-poison", &poison,
+			 N_("is GIT_TEST_GETTEXT_POISON in effect?")),
+		OPT_END()
+	};
+
+	argc = parse_options(argc, argv, NULL, options,
+			     builtin_sh_i18n_helper_usage, PARSE_OPT_KEEP_ARGV0);
+
+	if (poison != -1)
+		return !git_env_bool("GIT_TEST_GETTEXT_POISON", 0);
Hmm?  If "--[no-]git-test-gettext-poison" is given, poison is either
0 or 1, and we return the value we read from the environment?  What
convoluted way to implement the option is that, or is there anything
subtle that I am not getting?

If the "default" parameter to git_env_bool() were poison, and then
the option was renamed to "--get-git-text-gettext-poison", then I
sort of understand the code, though (it's like "git config" with
"--default" option).
Yeah this didn't make much sense. The test helper is now gone, and the
main helper doesn't use git_env_bool() but the utility function in
gettext.h.

But if there is nothing subtle, it may make sense to implement this
as an opt-cmdmode instead.
You mean something that works like "helper foo" instead of an option
via "helper --foo"? Seemed easier just to use the opt parse mechanism.
quoted
diff --git a/po/README b/po/README
index fef4c0f0b5..dba46c4a40 100644
--- a/po/README
+++ b/po/README
@@ -289,16 +289,11 @@ something in the test suite might still depend on the US English
 version of the strings, e.g. to grep some error message or other
 output.
 
-To smoke out issues like these Git can be compiled with gettext poison
-support, at the top-level:
+To smoke out issues like these Git tested with a translation mode that
+emits gibberish on every call to gettext. To use it run the test suite
+with it, e.g.:
s/these Git tested/these, Git can be tested/; even though the comma
is not a new issue you introduced.
Fixed. Range-diff of the whole thing below:

1:  c93bf2f23f ! 1:  7bcb95a82d i18n: make GETTEXT_POISON a runtime option
    @@ -24,19 +24,23 @@
     
         This allows for conditionally amending tests to test with/without
         poison, similar to what 859fdc0c3c ("commit-graph: define
    -    GIT_TEST_COMMIT_GRAPH", 2018-08-29) did for GIT_TEST_COMMIT_GRAPH, but
    -    this patch doesn't change any of the existing tests to work like that.
    +    GIT_TEST_COMMIT_GRAPH", 2018-08-29) did for GIT_TEST_COMMIT_GRAPH. Do
    +    some of that, now we e.g. always run the t0205-gettext-poison.sh test.
    +
    +    I did enough there to remove the GETTEXT_POISON prerequisite, but its
    +    inverse C_LOCALE_OUTPUT is still around, and surely some tests using
    +    it can be converted to e.g. always set GIT_TEST_GETTEXT_POISON=false.
     
         Notes on the implementation:
     
    -     * The only reason we need a new "git-sh-i18n--helper" and the
    -       corresponding "test-tool gettext-poison" is to expose
    +     * The only reason we need a new "git-sh-i18n--helper" is to expose
            git_env_bool() to shellscripts, since git-sh-i18n and friends need
            to inspect the $GIT_TEST_GETTEXT_POISON variable.
     
    -       We only call these if $GIT_TEST_GETTEXT_POISON is set, or in the
    -       test suite, and this code can go away for non-test code once the
    -       last i18n-using shellscript is rewritten in C.
    +       We only call it if $GIT_TEST_GETTEXT_POISON is set, or in the test
    +       suite. This code can go away for non-test code once the last
    +       i18n-using shellscript is rewritten in C, or if "git config" learns
    +       to combine --bool and a value fed into it, see [2].
     
          * We still compile a dedicated GETTEXT_POISON build in Travis CI,
            this is probably the wrong thing to do and should be followed-up
    @@ -46,19 +50,40 @@
     
          * The reason for not doing:
     
    -           test_lazy_prereq GETTEXT_POISON 'test-tool gettext-poison'
    -           test_lazy_prereq C_LOCALE_OUTPUT '! test-tool gettext-poison'
    +           test_lazy_prereq C_LOCALE_OUTPUT [...]
     
    -       In test-lib.sh is because there's some interpolation problem with
    +       in test-lib.sh is because there's some interpolation problem with
            that syntax which makes t6040-tracking-info.sh fail. Hence using
    -       the simpler test_set_prereq.
    +       the simpler test_set_prereq. It'll fail with:
    +
    +           + git branch -vv
    +           + sed -n -e
    +           mkdir -p "$TRASH_DIRECTORY/prereq-test-dir" &&
    +           (
    +                   cd "$TRASH_DIRECTORY/prereq-test-dir" &&! git sh-i18n--helper --git-test-gettext-poison
    +           )
    +           sed: -e expression #1, char 2: unknown command: `m'
    +           error: last command exited with $?=1
    +           not ok 3 - branch -vv
    +
    +       This is some interpolation problem with how test_lazy_prereq works
    +       that doesn't affect test_set_prereq.
    +
    +     * We now skip a test in t0000-basic.sh under
    +       GIT_TEST_GETTEXT_POISON=true that wasn't skipped before. This test
    +       relies on C locale output, but due to an edge case in how the
    +       previous implementation of GETTEXT_POISON worked (reading it from
    +       GIT-BUILD-OPTIONS) wasn't enabling poison correctly. Now it does,
    +       and needs to be skipped.
     
         See also
    -    https://public-inbox.org/git/871s8gd32p.fsf@evledraar.gmail.com/ for
    +    https://public-inbox.org/git/878t2pd6yu.fsf@evledraar.gmail.com/ for
         more discussion.
     
         1. https://public-inbox.org/git/871s8gd32p.fsf@evledraar.gmail.com/
     
    +    2. https://public-inbox.org/git/20181024074400.GA31239@sigill.intra.peff.net/
    +
         Signed-off-by: Ævar Arnfjörð Bjarmason [off-list ref]
     
      diff --git a/.gitignore b/.gitignore
    @@ -101,14 +126,6 @@
      # Define JSMIN to point to JavaScript minifier that functions as
      # a filter to have gitweb.js minified.
      #
    -@@
    - TEST_BUILTINS_OBJS += test-dump-untracked-cache.o
    - TEST_BUILTINS_OBJS += test-example-decorate.o
    - TEST_BUILTINS_OBJS += test-genrandom.o
    -+TEST_BUILTINS_OBJS += test-gettext-poison.o
    - TEST_BUILTINS_OBJS += test-hashmap.o
    - TEST_BUILTINS_OBJS += test-index-version.o
    - TEST_BUILTINS_OBJS += test-json-writer.o
     @@
      BUILTIN_OBJS += builtin/rm.o
      BUILTIN_OBJS += builtin/send-pack.o
    @@ -167,8 +184,9 @@
     +{
     +	int poison = -1;
     +	struct option options[] = {
    -+		OPT_BOOL(0, "git-test-gettext-poison", &poison,
    -+			 N_("is GIT_TEST_GETTEXT_POISON in effect?")),
    ++		OPT_BOOL_F(0, "git-test-gettext-poison", &poison,
    ++			   N_("is GIT_TEST_GETTEXT_POISON in effect?"),
    ++			   PARSE_OPT_NONEG),
     +		OPT_END()
     +	};
     +
    @@ -176,7 +194,7 @@
     +			     builtin_sh_i18n_helper_usage, PARSE_OPT_KEEP_ARGV0);
     +
     +	if (poison != -1)
    -+		return !git_env_bool("GIT_TEST_GETTEXT_POISON", 0);
    ++		return !use_gettext_poison();
     +
     +	usage_with_options(builtin_sh_i18n_helper_usage, options);
     +}
    @@ -275,7 +293,7 @@
      
     -To smoke out issues like these Git can be compiled with gettext poison
     -support, at the top-level:
    -+To smoke out issues like these Git tested with a translation mode that
    ++To smoke out issues like these, Git tested with a translation mode that
     +emits gibberish on every call to gettext. To use it run the test suite
     +with it, e.g.:
      
    @@ -308,44 +326,18 @@
      test suite. Accept any boolean values that are accepted by git-config.
      
     
    - diff --git a/t/helper/test-gettext-poison.c b/t/helper/test-gettext-poison.c
    - new file mode 100644
    - --- /dev/null
    - +++ b/t/helper/test-gettext-poison.c
    + diff --git a/t/lib-gettext.sh b/t/lib-gettext.sh
    + --- a/t/lib-gettext.sh
    + +++ b/t/lib-gettext.sh
     @@
    -+#include "test-tool.h"
    -+#include "git-compat-util.h"
    -+#include "thread-utils.h"
    -+#include "gettext.h"
    -+
    -+int cmd__gettext_poison(int argc, const char **argv)
    -+{
    -+	return use_gettext_poison() ? 0 : 1;
    -+}
    -
    - diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
    - --- a/t/helper/test-tool.c
    - +++ b/t/helper/test-tool.c
    -@@
    - 	{ "dump-untracked-cache", cmd__dump_untracked_cache },
    - 	{ "example-decorate", cmd__example_decorate },
    - 	{ "genrandom", cmd__genrandom },
    -+	{ "gettext-poison", cmd__gettext_poison },
    - 	{ "hashmap", cmd__hashmap },
    - 	{ "index-version", cmd__index_version },
    - 	{ "json-writer", cmd__json_writer },
    -
    - diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
    - --- a/t/helper/test-tool.h
    - +++ b/t/helper/test-tool.h
    -@@
    - int cmd__dump_untracked_cache(int argc, const char **argv);
    - int cmd__example_decorate(int argc, const char **argv);
    - int cmd__genrandom(int argc, const char **argv);
    -+int cmd__gettext_poison(int argc, const char **argv);
    - int cmd__hashmap(int argc, const char **argv);
    - int cmd__index_version(int argc, const char **argv);
    - int cmd__json_writer(int argc, const char **argv);
    + 
    + . "$GIT_BUILD_DIR"/git-sh-i18n
    + 
    +-if test_have_prereq GETTEXT && ! test_have_prereq GETTEXT_POISON
    ++if test_have_prereq GETTEXT && test_have_prereq C_LOCALE_OUTPUT
    + then
    + 	# is_IS.UTF-8 on Solaris and FreeBSD, is_IS.utf8 on Debian
    + 	is_IS_locale=$(locale -a 2>/dev/null |
     
      diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
      --- a/t/t0000-basic.sh
    @@ -360,6 +352,37 @@
      		test-verbose "test verbose" --verbose <<-\EOF &&
      	test_expect_success "passing test" true
     
    + diff --git a/t/t0205-gettext-poison.sh b/t/t0205-gettext-poison.sh
    + --- a/t/t0205-gettext-poison.sh
    + +++ b/t/t0205-gettext-poison.sh
    +@@
    + 
    + test_description='Gettext Shell poison'
    + 
    ++GIT_TEST_GETTEXT_POISON=true
    ++export GIT_TEST_GETTEXT_POISON
    + . ./lib-gettext.sh
    + 
    +-test_expect_success GETTEXT_POISON 'sanity: $GIT_INTERNAL_GETTEXT_SH_SCHEME" is poison' '
    ++test_expect_success 'sanity: $GIT_INTERNAL_GETTEXT_SH_SCHEME" is poison' '
    +     test "$GIT_INTERNAL_GETTEXT_SH_SCHEME" = "poison"
    + '
    + 
    +-test_expect_success GETTEXT_POISON 'gettext: our gettext() fallback has poison semantics' '
    ++test_expect_success 'gettext: our gettext() fallback has poison semantics' '
    +     printf "# GETTEXT POISON #" >expect &&
    +     gettext "test" >actual &&
    +     test_cmp expect actual &&
    +@@
    +     test_cmp expect actual
    + '
    + 
    +-test_expect_success GETTEXT_POISON 'eval_gettext: our eval_gettext() fallback has poison semantics' '
    ++test_expect_success 'eval_gettext: our eval_gettext() fallback has poison semantics' '
    +     printf "# GETTEXT POISON #" >expect &&
    +     eval_gettext "test" >actual &&
    +     test_cmp expect actual &&
    +
      diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
      --- a/t/t3406-rebase-message.sh
      +++ b/t/t3406-rebase-message.sh
    @@ -377,18 +400,33 @@
      --- a/t/t7201-co.sh
      +++ b/t/t7201-co.sh
     @@
    + test_expect_success 'checkout to detach HEAD' '
    + 	git config advice.detachedHead true &&
      	git checkout -f renamer && git clean -f &&
    - 	git checkout renamer^ 2>messages &&
    - 	test_i18ngrep "HEAD is now at 7329388" messages &&
    +-	git checkout renamer^ 2>messages &&
    +-	test_i18ngrep "HEAD is now at 7329388" messages &&
     -	(test_line_count -gt 1 messages || test -n "$GETTEXT_POISON") &&
    -+	(
    -+		test_line_count -gt 1 messages ||
    -+		test_have_prereq GETTEXT_POISON
    -+	) &&
    ++	GIT_TEST_GETTEXT_POISON=false git checkout renamer^ 2>messages &&
    ++	grep "HEAD is now at 7329388" messages &&
    ++	test_line_count -gt 1 messages &&
      	H=$(git rev-parse --verify HEAD) &&
      	M=$(git show-ref -s --verify refs/heads/master) &&
      	test "z$H" = "z$M" &&
     
    + diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
    + --- a/t/t9902-completion.sh
    + +++ b/t/t9902-completion.sh
    +@@
    + 	verbose test -z "$__git_all_commands"
    + '
    + 
    +-test_expect_success !GETTEXT_POISON 'sourcing the completion script clears cached merge strategies' '
    ++test_expect_success 'sourcing the completion script clears cached merge strategies' '
    ++	GIT_TEST_GETTEXT_POISON=false &&
    + 	__git_compute_merge_strategies &&
    + 	verbose test -n "$__git_merge_strategies" &&
    + 	. "$GIT_BUILD_DIR/contrib/completion/git-completion.bash" &&
    +
      diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
      --- a/t/test-lib-functions.sh
      +++ b/t/test-lib-functions.sh
    @@ -417,7 +455,7 @@
      	fi
      
     -	if test -n "$GETTEXT_POISON"
    -+	if ! test_have_prereq C_LOCALE_OUTPUT
    ++	if test_have_prereq !C_LOCALE_OUTPUT
      	then
      		# pretend success
      		return 0
    @@ -438,8 +476,9 @@
     -else
     -	test_set_prereq C_LOCALE_OUTPUT
     -fi
    -+test-tool gettext-poison && test_set_prereq GETTEXT_POISON
    -+test-tool gettext-poison || test_set_prereq C_LOCALE_OUTPUT
    ++## Fails in the 'branch -vv' test in t6040-tracking-info.sh
    ++#test_lazy_prereq C_LOCALE_OUTPUT '! git sh-i18n--helper --git-test-gettext-poison'
    ++git sh-i18n--helper --git-test-gettext-poison || test_set_prereq C_LOCALE_OUTPUT
      
      if test -z "$GIT_TEST_CHECK_CACHE_TREE"
      then

 .gitignore                |  1 +
 .travis.yml               |  2 +-
 Makefile                  | 10 +---------
 builtin.h                 |  1 +
 builtin/sh-i18n--helper.c | 28 ++++++++++++++++++++++++++++
 ci/lib-travisci.sh        |  4 ++--
 gettext.c                 |  5 ++---
 gettext.h                 |  4 ----
 git-sh-i18n.sh            |  3 ++-
 git.c                     |  1 +
 po/README                 | 13 ++++---------
 t/README                  |  6 ++++++
 t/lib-gettext.sh          |  2 +-
 t/t0000-basic.sh          |  2 +-
 t/t0205-gettext-poison.sh |  8 +++++---
 t/t3406-rebase-message.sh |  2 +-
 t/t7201-co.sh             |  6 +++---
 t/t9902-completion.sh     |  3 ++-
 t/test-lib-functions.sh   |  8 ++++----
 t/test-lib.sh             | 12 +++---------
 20 files changed, 69 insertions(+), 52 deletions(-)
 create mode 100644 builtin/sh-i18n--helper.c

diff --git a/.gitignore b/.gitignore
index 9d1363a1eb..f7b7977910 100644
--- a/.gitignore
+++ b/.gitignore
@@ -148,6 +148,7 @@
 /git-serve
 /git-sh-i18n
 /git-sh-i18n--envsubst
+/git-sh-i18n--helper
 /git-sh-setup
 /git-sh-i18n
 /git-shell
diff --git a/.travis.yml b/.travis.yml
index 4d4e26c9df..4523a2e5ec 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -26,7 +26,7 @@ addons:
 
 matrix:
   include:
-    - env: jobname=GETTEXT_POISON
+    - env: jobname=GIT_TEST_GETTEXT_POISON
       os: linux
       compiler:
       addons:
diff --git a/Makefile b/Makefile
index d18ab0fe78..086600b099 100644
--- a/Makefile
+++ b/Makefile
@@ -362,11 +362,6 @@ all::
 # Define HAVE_DEV_TTY if your system can open /dev/tty to interact with the
 # user.
 #
-# Define GETTEXT_POISON if you are debugging the choice of strings marked
-# for translation.  In a GETTEXT_POISON build, you can turn all strings marked
-# for translation into gibberish by setting the GIT_GETTEXT_POISON variable
-# (to any value) in your environment.
-#
 # Define JSMIN to point to JavaScript minifier that functions as
 # a filter to have gitweb.js minified.
 #
@@ -1099,6 +1094,7 @@ BUILTIN_OBJS += builtin/revert.o
 BUILTIN_OBJS += builtin/rm.o
 BUILTIN_OBJS += builtin/send-pack.o
 BUILTIN_OBJS += builtin/serve.o
+BUILTIN_OBJS += builtin/sh-i18n--helper.o
 BUILTIN_OBJS += builtin/shortlog.o
 BUILTIN_OBJS += builtin/show-branch.o
 BUILTIN_OBJS += builtin/show-index.o
@@ -1439,9 +1435,6 @@ endif
 ifdef NO_SYMLINK_HEAD
 	BASIC_CFLAGS += -DNO_SYMLINK_HEAD
 endif
-ifdef GETTEXT_POISON
-	BASIC_CFLAGS += -DGETTEXT_POISON
-endif
 ifdef NO_GETTEXT
 	BASIC_CFLAGS += -DNO_GETTEXT
 	USE_GETTEXT_SCHEME ?= fallthrough
@@ -2591,7 +2584,6 @@ ifdef GIT_TEST_CMP_USE_COPIED_CONTEXT
 	@echo GIT_TEST_CMP_USE_COPIED_CONTEXT=YesPlease >>$@+
 endif
 	@echo NO_GETTEXT=\''$(subst ','\'',$(subst ','\'',$(NO_GETTEXT)))'\' >>$@+
-	@echo GETTEXT_POISON=\''$(subst ','\'',$(subst ','\'',$(GETTEXT_POISON)))'\' >>$@+
 ifdef GIT_PERF_REPEAT_COUNT
 	@echo GIT_PERF_REPEAT_COUNT=\''$(subst ','\'',$(subst ','\'',$(GIT_PERF_REPEAT_COUNT)))'\' >>$@+
 endif
diff --git a/builtin.h b/builtin.h
index 962f0489ab..a40c56e7a2 100644
--- a/builtin.h
+++ b/builtin.h
@@ -219,6 +219,7 @@ extern int cmd_revert(int argc, const char **argv, const char *prefix);
 extern int cmd_rm(int argc, const char **argv, const char *prefix);
 extern int cmd_send_pack(int argc, const char **argv, const char *prefix);
 extern int cmd_serve(int argc, const char **argv, const char *prefix);
+extern int cmd_sh_i18n__helper(int argc, const char **argv, const char *prefix);
 extern int cmd_shortlog(int argc, const char **argv, const char *prefix);
 extern int cmd_show(int argc, const char **argv, const char *prefix);
 extern int cmd_show_branch(int argc, const char **argv, const char *prefix);
diff --git a/builtin/sh-i18n--helper.c b/builtin/sh-i18n--helper.c
new file mode 100644
index 0000000000..aa62787514
--- /dev/null
+++ b/builtin/sh-i18n--helper.c
@@ -0,0 +1,28 @@
+#include "builtin.h"
+#include "cache.h"
+#include "config.h"
+#include "parse-options.h"
+
+static const char * const builtin_sh_i18n_helper_usage[] = {
+	N_("git sh-i18n--helper [<options>]"),
+	NULL
+};
+
+int cmd_sh_i18n__helper(int argc, const char **argv, const char *prefix)
+{
+	int poison = -1;
+	struct option options[] = {
+		OPT_BOOL_F(0, "git-test-gettext-poison", &poison,
+			   N_("is GIT_TEST_GETTEXT_POISON in effect?"),
+			   PARSE_OPT_NONEG),
+		OPT_END()
+	};
+
+	argc = parse_options(argc, argv, NULL, options,
+			     builtin_sh_i18n_helper_usage, PARSE_OPT_KEEP_ARGV0);
+
+	if (poison != -1)
+		return !use_gettext_poison();
+
+	usage_with_options(builtin_sh_i18n_helper_usage, options);
+}
diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh
index 06970f7213..6a89d0d7d8 100755
--- a/ci/lib-travisci.sh
+++ b/ci/lib-travisci.sh
@@ -123,7 +123,7 @@ osx-clang|osx-gcc)
 	# Travis CI OS X
 	export GIT_SKIP_TESTS="t9810 t9816"
 	;;
-GETTEXT_POISON)
-	export GETTEXT_POISON=YesPlease
+GIT_TEST_GETTEXT_POISON)
+	export GIT_TEST_GETTEXT_POISON=true
 	;;
 esac
diff --git a/gettext.c b/gettext.c
index 7272771c8e..722a2f726c 100644
--- a/gettext.c
+++ b/gettext.c
@@ -7,6 +7,7 @@
 #include "gettext.h"
 #include "strbuf.h"
 #include "utf8.h"
+#include "config.h"
 
 #ifndef NO_GETTEXT
 #	include <locale.h>
@@ -46,15 +47,13 @@ const char *get_preferred_languages(void)
 	return NULL;
 }
 
-#ifdef GETTEXT_POISON
 int use_gettext_poison(void)
 {
 	static int poison_requested = -1;
 	if (poison_requested == -1)
-		poison_requested = getenv("GIT_GETTEXT_POISON") ? 1 : 0;
+		poison_requested = git_env_bool("GIT_TEST_GETTEXT_POISON", 0);
 	return poison_requested;
 }
-#endif
 
 #ifndef NO_GETTEXT
 static int test_vsnprintf(const char *fmt, ...)
diff --git a/gettext.h b/gettext.h
index 7eee64a34f..4c492d9f57 100644
--- a/gettext.h
+++ b/gettext.h
@@ -41,11 +41,7 @@ static inline int gettext_width(const char *s)
 }
 #endif
 
-#ifdef GETTEXT_POISON
 extern int use_gettext_poison(void);
-#else
-#define use_gettext_poison() 0
-#endif
 
 static inline FORMAT_PRESERVING(1) const char *_(const char *msgid)
 {
diff --git a/git-sh-i18n.sh b/git-sh-i18n.sh
index 9d065fb4bf..c0713b1ee9 100644
--- a/git-sh-i18n.sh
+++ b/git-sh-i18n.sh
@@ -17,7 +17,8 @@ export TEXTDOMAINDIR
 
 # First decide what scheme to use...
 GIT_INTERNAL_GETTEXT_SH_SCHEME=fallthrough
-if test -n "$GIT_GETTEXT_POISON"
+if test -n "$GIT_TEST_GETTEXT_POISON" &&
+	    git sh-i18n--helper --git-test-gettext-poison
 then
 	GIT_INTERNAL_GETTEXT_SH_SCHEME=poison
 elif test -n "@@USE_GETTEXT_SCHEME@@"
diff --git a/git.c b/git.c
index 5920f8019b..125c523720 100644
--- a/git.c
+++ b/git.c
@@ -539,6 +539,7 @@ static struct cmd_struct commands[] = {
 	{ "rm", cmd_rm, RUN_SETUP },
 	{ "send-pack", cmd_send_pack, RUN_SETUP },
 	{ "serve", cmd_serve, RUN_SETUP },
+	{ "sh-i18n--helper", cmd_sh_i18n__helper, 0 },
 	{ "shortlog", cmd_shortlog, RUN_SETUP_GENTLY | USE_PAGER },
 	{ "show", cmd_show, RUN_SETUP },
 	{ "show-branch", cmd_show_branch, RUN_SETUP },
diff --git a/po/README b/po/README
index fef4c0f0b5..07595d369b 100644
--- a/po/README
+++ b/po/README
@@ -289,16 +289,11 @@ something in the test suite might still depend on the US English
 version of the strings, e.g. to grep some error message or other
 output.
 
-To smoke out issues like these Git can be compiled with gettext poison
-support, at the top-level:
+To smoke out issues like these, Git tested with a translation mode that
+emits gibberish on every call to gettext. To use it run the test suite
+with it, e.g.:
 
-    make GETTEXT_POISON=YesPlease
-
-That'll give you a git which emits gibberish on every call to
-gettext. It's obviously not meant to be installed, but you should run
-the test suite with it:
-
-    cd t && prove -j 9 ./t[0-9]*.sh
+    cd t && GIT_TEST_GETTEXT_POISON=true prove -j 9 ./t[0-9]*.sh
 
 If tests break with it you should inspect them manually and see if
 what you're translating is sane, i.e. that you're not translating
diff --git a/t/README b/t/README
index 8847489640..53c3dee7a9 100644
--- a/t/README
+++ b/t/README
@@ -301,6 +301,12 @@ that cannot be easily covered by a few specific test cases. These
 could be enabled by running the test suite with correct GIT_TEST_
 environment set.
 
+GIT_TEST_GETTEXT_POISON=<boolean> turns all strings marked for
+translation into gibberish. Used for spotting those tests that need to
+be marked with a C_LOCALE_OUTPUT prerequisite when adding more strings
+for translation. See "Testing marked strings" in po/README for
+details.
+
 GIT_TEST_SPLIT_INDEX=<boolean> forces split-index mode on the whole
 test suite. Accept any boolean values that are accepted by git-config.
 
diff --git a/t/lib-gettext.sh b/t/lib-gettext.sh
index eec757f104..755f421431 100644
--- a/t/lib-gettext.sh
+++ b/t/lib-gettext.sh
@@ -12,7 +12,7 @@ export GIT_TEXTDOMAINDIR GIT_PO_PATH
 
 . "$GIT_BUILD_DIR"/git-sh-i18n
 
-if test_have_prereq GETTEXT && ! test_have_prereq GETTEXT_POISON
+if test_have_prereq GETTEXT && test_have_prereq C_LOCALE_OUTPUT
 then
 	# is_IS.UTF-8 on Solaris and FreeBSD, is_IS.utf8 on Debian
 	is_IS_locale=$(locale -a 2>/dev/null |
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 4d23373526..b6566003dd 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -274,7 +274,7 @@ test_expect_success 'pretend we have a mix of all possible results' "
 	EOF
 "
 
-test_expect_success 'test --verbose' '
+test_expect_success C_LOCALE_OUTPUT 'test --verbose' '
 	test_must_fail run_sub_test_lib_test \
 		test-verbose "test verbose" --verbose <<-\EOF &&
 	test_expect_success "passing test" true
diff --git a/t/t0205-gettext-poison.sh b/t/t0205-gettext-poison.sh
index 438e778d6a..1675d3e171 100755
--- a/t/t0205-gettext-poison.sh
+++ b/t/t0205-gettext-poison.sh
@@ -5,13 +5,15 @@
 
 test_description='Gettext Shell poison'
 
+GIT_TEST_GETTEXT_POISON=true
+export GIT_TEST_GETTEXT_POISON
 . ./lib-gettext.sh
 
-test_expect_success GETTEXT_POISON 'sanity: $GIT_INTERNAL_GETTEXT_SH_SCHEME" is poison' '
+test_expect_success 'sanity: $GIT_INTERNAL_GETTEXT_SH_SCHEME" is poison' '
     test "$GIT_INTERNAL_GETTEXT_SH_SCHEME" = "poison"
 '
 
-test_expect_success GETTEXT_POISON 'gettext: our gettext() fallback has poison semantics' '
+test_expect_success 'gettext: our gettext() fallback has poison semantics' '
     printf "# GETTEXT POISON #" >expect &&
     gettext "test" >actual &&
     test_cmp expect actual &&
@@ -20,7 +22,7 @@ test_expect_success GETTEXT_POISON 'gettext: our gettext() fallback has poison s
     test_cmp expect actual
 '
 
-test_expect_success GETTEXT_POISON 'eval_gettext: our eval_gettext() fallback has poison semantics' '
+test_expect_success 'eval_gettext: our eval_gettext() fallback has poison semantics' '
     printf "# GETTEXT POISON #" >expect &&
     eval_gettext "test" >actual &&
     test_cmp expect actual &&
diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
index 0392e36d23..2bdcf83808 100755
--- a/t/t3406-rebase-message.sh
+++ b/t/t3406-rebase-message.sh
@@ -77,7 +77,7 @@ test_expect_success 'rebase -n overrides config rebase.stat config' '
 #     "Does not point to a valid commit: invalid-ref"
 #
 # NEEDSWORK: This "grep" is fine in real non-C locales, but
-# GETTEXT_POISON poisons the refname along with the enclosing
+# GIT_TEST_GETTEXT_POISON poisons the refname along with the enclosing
 # error message.
 test_expect_success 'rebase --onto outputs the invalid ref' '
 	test_must_fail git rebase --onto invalid-ref HEAD HEAD 2>err &&
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 826987ca80..71e8e0167f 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -254,9 +254,9 @@ test_expect_success 'checkout to detach HEAD (with advice declined)' '
 test_expect_success 'checkout to detach HEAD' '
 	git config advice.detachedHead true &&
 	git checkout -f renamer && git clean -f &&
-	git checkout renamer^ 2>messages &&
-	test_i18ngrep "HEAD is now at 7329388" messages &&
-	(test_line_count -gt 1 messages || test -n "$GETTEXT_POISON") &&
+	GIT_TEST_GETTEXT_POISON=false git checkout renamer^ 2>messages &&
+	grep "HEAD is now at 7329388" messages &&
+	test_line_count -gt 1 messages &&
 	H=$(git rev-parse --verify HEAD) &&
 	M=$(git show-ref -s --verify refs/heads/master) &&
 	test "z$H" = "z$M" &&
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 175f83d704..537f5fdada 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1697,7 +1697,8 @@ test_expect_success 'sourcing the completion script clears cached commands' '
 	verbose test -z "$__git_all_commands"
 '
 
-test_expect_success !GETTEXT_POISON 'sourcing the completion script clears cached merge strategies' '
+test_expect_success 'sourcing the completion script clears cached merge strategies' '
+	GIT_TEST_GETTEXT_POISON=false &&
 	__git_compute_merge_strategies &&
 	verbose test -n "$__git_merge_strategies" &&
 	. "$GIT_BUILD_DIR/contrib/completion/git-completion.bash" &&
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 78d8c3783b..2f42b3653c 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -755,16 +755,16 @@ test_cmp_bin() {
 
 # Use this instead of test_cmp to compare files that contain expected and
 # actual output from git commands that can be translated.  When running
-# under GETTEXT_POISON this pretends that the command produced expected
+# under GIT_TEST_GETTEXT_POISON this pretends that the command produced expected
 # results.
 test_i18ncmp () {
-	test -n "$GETTEXT_POISON" || test_cmp "$@"
+	! test_have_prereq C_LOCALE_OUTPUT || test_cmp "$@"
 }
 
 # Use this instead of "grep expected-string actual" to see if the
 # output from a git command that can be translated either contains an
 # expected string, or does not contain an unwanted one.  When running
-# under GETTEXT_POISON this pretends that the command produced expected
+# under GIT_TEST_GETTEXT_POISON this pretends that the command produced expected
 # results.
 test_i18ngrep () {
 	eval "last_arg=\${$#}"
@@ -779,7 +779,7 @@ test_i18ngrep () {
 		error "bug in the test script: too few parameters to test_i18ngrep"
 	fi
 
-	if test -n "$GETTEXT_POISON"
+	if test_have_prereq !C_LOCALE_OUTPUT
 	then
 		# pretend success
 		return 0
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 897e6fcc94..892591ac84 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1104,15 +1104,9 @@ test -n "$USE_LIBPCRE1" && test_set_prereq LIBPCRE1
 test -n "$USE_LIBPCRE2" && test_set_prereq LIBPCRE2
 test -z "$NO_GETTEXT" && test_set_prereq GETTEXT
 
-# Can we rely on git's output in the C locale?
-if test -n "$GETTEXT_POISON"
-then
-	GIT_GETTEXT_POISON=YesPlease
-	export GIT_GETTEXT_POISON
-	test_set_prereq GETTEXT_POISON
-else
-	test_set_prereq C_LOCALE_OUTPUT
-fi
+## Fails in the 'branch -vv' test in t6040-tracking-info.sh
+#test_lazy_prereq C_LOCALE_OUTPUT '! git sh-i18n--helper --git-test-gettext-poison'
+git sh-i18n--helper --git-test-gettext-poison || test_set_prereq C_LOCALE_OUTPUT
 
 if test -z "$GIT_TEST_CHECK_CACHE_TREE"
 then
-- 
2.19.1.568.g152ad8e336
Other recent patches touching these files
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help