Merge branch 'ab/detox-gettext-tests'

gitster · gitster · commit 466f94ec45e6 · 2021-02-10T14:48:33.000-08:00
Get rid of "GETTEXT_POISON" support altogether, which may or may
not be controversial.

* ab/detox-gettext-tests:
  tests: remove uses of GIT_TEST_GETTEXT_POISON=false
  tests: remove support for GIT_TEST_GETTEXT_POISON
  ci: remove GETTEXT_POISON jobs
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
@@ -289,7 +289,7 @@ jobs:
           - jobname: osx-gcc
             cc: gcc
             pool: macos-latest
-          - jobname: GETTEXT_POISON
+          - jobname: linux-gcc-default
             cc: gcc
             pool: ubuntu-latest
     env:
diff --git a/.travis.yml b/.travis.yml
@@ -16,7 +16,7 @@ compiler:
 
 matrix:
   include:
-    - env: jobname=GETTEXT_POISON
+    - env: jobname=linux-gcc-default
       os: linux
       compiler:
       addons:
diff --git a/Documentation/MyFirstContribution.txt b/Documentation/MyFirstContribution.txt
@@ -664,7 +664,7 @@ mention the right animal somewhere:
 ----
 test_expect_success 'runs correctly with no args and good output' '
 	git psuh >actual &&
-	test_i18ngrep Pony actual
+	grep Pony actual
 '
 ----
 
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
@@ -72,7 +72,7 @@ Documentation)
 	test -n "$ALREADY_HAVE_ASCIIDOCTOR" ||
 	sudo gem install --version 1.5.8 asciidoctor
 	;;
-linux-gcc-4.8|GETTEXT_POISON)
+linux-gcc-default|linux-gcc-4.8)
 	sudo apt-get -q update
 	sudo apt-get -q -y install $UBUNTU_COMMON_PKGS
 	;;
diff --git a/ci/lib.sh b/ci/lib.sh
@@ -220,8 +220,7 @@ osx-clang|osx-gcc)
 	# Travis CI OS X
 	export GIT_SKIP_TESTS="t9810 t9816"
 	;;
-GETTEXT_POISON)
-	export GIT_TEST_GETTEXT_POISON=true
+linux-gcc-default)
 	;;
 Linux32)
 	CC=gcc
diff --git a/config.c b/config.c
@@ -1155,15 +1155,6 @@ static void die_bad_number(const char *name, const char *value)
 	if (!value)
 		value = "";
 
-	if (!strcmp(name, "GIT_TEST_GETTEXT_POISON"))
-		/*
-		 * We explicitly *don't* use _() here since it would
-		 * cause an infinite loop with _() needing to call
-		 * use_gettext_poison(). This is why marked up
-		 * translations with N_() above.
-		 */
-		die(bad_numeric, value, name, error_type);
-
 	if (!(cf && cf->name))
 		die(_(bad_numeric), value, name, _(error_type));
 
diff --git a/gettext.c b/gettext.c
@@ -65,14 +65,6 @@ const char *get_preferred_languages(void)
 	return NULL;
 }
 
-int use_gettext_poison(void)
-{
-	static int poison_requested = -1;
-	if (poison_requested == -1)
-		poison_requested = git_env_bool("GIT_TEST_GETTEXT_POISON", 0);
-	return poison_requested;
-}
-
 #ifndef NO_GETTEXT
 static int test_vsnprintf(const char *fmt, ...)
 {
@@ -117,8 +109,6 @@ void git_setup_gettext(void)
 	if (!podir)
 		podir = p = system_path(GIT_LOCALE_PATH);
 
-	use_gettext_poison(); /* getenv() reentrancy paranoia */
-
 	if (!is_directory(podir)) {
 		free(p);
 		return;
diff --git a/gettext.h b/gettext.h
@@ -28,15 +28,12 @@
 
 #define FORMAT_PRESERVING(n) __attribute__((format_arg(n)))
 
-int use_gettext_poison(void);
-
 #ifndef NO_GETTEXT
 void git_setup_gettext(void);
 int gettext_width(const char *s);
 #else
 static inline void git_setup_gettext(void)
 {
-	use_gettext_poison(); /* getenv() reentrancy paranoia */
 }
 static inline int gettext_width(const char *s)
 {
@@ -48,14 +45,12 @@ static inline FORMAT_PRESERVING(1) const char *_(const char *msgid)
 {
 	if (!*msgid)
 		return "";
-	return use_gettext_poison() ? "# GETTEXT POISON #" : gettext(msgid);
+	return gettext(msgid);
 }
 
 static inline FORMAT_PRESERVING(1) FORMAT_PRESERVING(2)
 const char *Q_(const char *msgid, const char *plu, unsigned long n)
 {
-	if (use_gettext_poison())
-		return "# GETTEXT POISON #";
 	return ngettext(msgid, plu, n);
 }
 
diff --git a/git-sh-i18n.sh b/git-sh-i18n.sh
@@ -17,12 +17,7 @@ export TEXTDOMAINDIR
 
 # First decide what scheme to use...
 GIT_INTERNAL_GETTEXT_SH_SCHEME=fallthrough
-if test -n "$GIT_TEST_GETTEXT_POISON" &&
-	    git env--helper --type=bool --default=0 --exit-code \
-		GIT_TEST_GETTEXT_POISON
-then
-	GIT_INTERNAL_GETTEXT_SH_SCHEME=poison
-elif test -n "@@USE_GETTEXT_SCHEME@@"
+if test -n "@@USE_GETTEXT_SCHEME@@"
 then
 	GIT_INTERNAL_GETTEXT_SH_SCHEME="@@USE_GETTEXT_SCHEME@@"
 elif test -n "$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS"
@@ -63,21 +58,6 @@ gettext_without_eval_gettext)
 		)
 	}
 	;;
-poison)
-	# Emit garbage so that tests that incorrectly rely on translatable
-	# strings will fail.
-	gettext () {
-		printf "%s" "# GETTEXT POISON #"
-	}
-
-	eval_gettext () {
-		printf "%s" "# GETTEXT POISON #"
-	}
-
-	eval_ngettext () {
-		printf "%s" "# GETTEXT POISON #"
-	}
-	;;
 *)
 	gettext () {
 		printf "%s" "$1"
diff --git a/po/README b/po/README
@@ -284,23 +284,5 @@ Perl:
 Testing marked strings
 ----------------------
 
-Even if you've correctly marked porcelain strings for translation
-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 tested with a translation mode that
-emits gibberish on every call to gettext. To use it run the test suite
-with it, e.g.:
-
-    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
-plumbing output.
-
-If not you should replace calls to grep with test_i18ngrep, or
-test_cmp calls with test_i18ncmp. If that's not enough you can skip
-the whole test by making it depend on the C_LOCALE_OUTPUT
-prerequisite. See existing test files with this prerequisite for
-examples.
+Git's tests are run under LANG=C LC_ALL=C. So the tests do not need be
+changed to account for translations as they're added.
diff --git a/t/README b/t/README
@@ -358,12 +358,6 @@ whether this mode is active, and e.g. skip some tests that are hard to
 refactor to deal with it. The "SYMLINKS" prerequisite is currently
 excluded as so much relies on it, but this might change in the future.
 
-GIT_TEST_GETTEXT_POISON=<boolean> turns all strings marked for
-translation into gibberish if true. 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
@@ -17,7 +17,7 @@ else
 	. "$GIT_BUILD_DIR"/git-sh-i18n
 fi
 
-if test_have_prereq GETTEXT && test_have_prereq C_LOCALE_OUTPUT
+if test_have_prereq GETTEXT
 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/lib-rebase.sh b/t/lib-rebase.sh
@@ -29,7 +29,6 @@ set_fake_editor () {
 	*/COMMIT_EDITMSG)
 		test -z "$EXPECT_HEADER_COUNT" ||
 			test "$EXPECT_HEADER_COUNT" = "$(sed -n '1s/^# This is a combination of \(.*\) commits\./\1/p' < "$1")" ||
-			test "# # GETTEXT POISON #" = "$(sed -n '1p' < "$1")" ||
 			exit
 		test -z "$FAKE_COMMIT_MESSAGE" || echo "$FAKE_COMMIT_MESSAGE" > "$1"
 		test -z "$FAKE_COMMIT_AMEND" || echo "$FAKE_COMMIT_AMEND" >> "$1"
diff --git a/t/t0017-env-helper.sh b/t/t0017-env-helper.sh
@@ -86,14 +86,14 @@ test_expect_success 'env--helper reads config thanks to trace2' '
 	git config -f home/cycle include.path .gitconfig &&
 
 	test_must_fail \
-		env HOME="$(pwd)/home" GIT_TEST_GETTEXT_POISON=false \
+		env HOME="$(pwd)/home" \
 		git config -l 2>err &&
 	grep "exceeded maximum include depth" err &&
 
 	test_must_fail \
-		env HOME="$(pwd)/home" GIT_TEST_GETTEXT_POISON=true \
-		git -C cycle env--helper --type=bool --default=0 --exit-code GIT_TEST_GETTEXT_POISON 2>err &&
-	grep "# GETTEXT POISON #" err
+		env HOME="$(pwd)/home" GIT_TEST_ENV_HELPER=true \
+		git -C cycle env--helper --type=bool --default=0 --exit-code GIT_TEST_ENV_HELPER 2>err &&
+	grep "exceeded maximum include depth" err
 '
 
 test_done
diff --git a/t/t0205-gettext-poison.sh b/t/t0205-gettext-poison.sh
diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh
@@ -352,9 +352,7 @@ test_expect_success 'include cycles are detected' '
 	git init --bare cycle &&
 	git -C cycle config include.path cycle &&
 	git config -f cycle/cycle include.path config &&
-	test_must_fail \
-		env GIT_TEST_GETTEXT_POISON=false \
-		git -C cycle config --get-all test.value 2>stderr &&
+	test_must_fail git -C cycle config --get-all test.value 2>stderr &&
 	grep "exceeded maximum include depth" stderr
 '
 
diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
@@ -67,13 +67,6 @@ test_expect_success 'rebase -n overrides config rebase.stat config' '
 	! grep "^ fileX |  *1 +$" diffstat.txt
 '
 
-# Output to stderr:
-#
-#     "Does not point to a valid commit: invalid-ref"
-#
-# NEEDSWORK: This "grep" is fine in real non-C locales, but
-# 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 &&
 	test_i18ngrep "invalid-ref" err
diff --git a/t/t5411/common-functions.sh b/t/t5411/common-functions.sh
@@ -36,9 +36,8 @@ create_commits_in () {
 # without having to worry about future changes of the commit ID and spaces
 # of the output.  Single quotes are replaced with double quotes, because
 # it is boring to prepare unquoted single quotes in expect text.  We also
-# remove some locale error messages, which break test if we turn on
-# `GIT_TEST_GETTEXT_POISON=true` in order to test unintentional translations
-# on plumbing commands.
+# remove some locale error messages. The emitted human-readable errors are
+# redundant to the more machine-readable output the tests already assert.
 make_user_friendly_and_stable_output () {
 	sed \
 		-e "s/  *\$//" \
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
@@ -248,7 +248,7 @@ test_expect_success 'checkout to detach HEAD' '
 	rev=$(git rev-parse --short renamer^) &&
 	git checkout -f renamer &&
 	git clean -f &&
-	GIT_TEST_GETTEXT_POISON=false git checkout renamer^ 2>messages &&
+	git checkout renamer^ 2>messages &&
 	grep "HEAD is now at $rev" messages &&
 	test_line_count -gt 1 messages &&
 	H=$(git rev-parse --verify HEAD) &&
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
@@ -2366,7 +2366,6 @@ test_expect_success 'sourcing the completion script clears cached commands' '
 '
 
 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
@@ -1016,19 +1016,16 @@ test_cmp_bin () {
 	cmp "$@"
 }
 
-# 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 GIT_TEST_GETTEXT_POISON this pretends that the command produced expected
-# results.
+# Wrapper for test_cmp which used to be used for
+# GIT_TEST_GETTEXT_POISON=false. Only here as a shim for other
+# in-flight changes. Should not be used and will be removed soon.
 test_i18ncmp () {
-	! test_have_prereq C_LOCALE_OUTPUT || test_cmp "$@"
+	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 GIT_TEST_GETTEXT_POISON this pretends that the command produced expected
-# results.
+# Wrapper for grep which used to be used for
+# GIT_TEST_GETTEXT_POISON=false. Only here as a shim for other
+# in-flight changes. Should not be used and will be removed soon.
 test_i18ngrep () {
 	eval "last_arg=\${$#}"
 
@@ -1041,12 +1038,6 @@ test_i18ngrep () {
 		BUG "too few parameters to test_i18ngrep"
 	fi
 
-	if test_have_prereq !C_LOCALE_OUTPUT
-	then
-		# pretend success
-		return 0
-	fi
-
 	if test "x!" = "x$1"
 	then
 		shift
diff --git a/t/test-lib.sh b/t/test-lib.sh
@@ -404,15 +404,6 @@ TZ=UTC
 export LANG LC_ALL PAGER TZ
 EDITOR=:
 
-# GIT_TEST_GETTEXT_POISON should not influence git commands executed
-# during initialization of test-lib and the test repo. Back it up,
-# unset and then restore after initialization is finished.
-if test -n "$GIT_TEST_GETTEXT_POISON"
-then
-	GIT_TEST_GETTEXT_POISON_ORIG=$GIT_TEST_GETTEXT_POISON
-	unset GIT_TEST_GETTEXT_POISON
-fi
-
 # A call to "unset" with no arguments causes at least Solaris 10
 # /usr/xpg4/bin/sh and /bin/ksh to bail out.  So keep the unsets
 # deriving from the command substitution clustered with the other
@@ -1528,16 +1519,10 @@ test -n "$USE_LIBPCRE2" && test_set_prereq PCRE
 test -n "$USE_LIBPCRE2" && test_set_prereq LIBPCRE2
 test -z "$NO_GETTEXT" && test_set_prereq GETTEXT
 
-if test -n "$GIT_TEST_GETTEXT_POISON_ORIG"
-then
-	GIT_TEST_GETTEXT_POISON=$GIT_TEST_GETTEXT_POISON_ORIG
-	export GIT_TEST_GETTEXT_POISON
-	unset GIT_TEST_GETTEXT_POISON_ORIG
-fi
-
-test_lazy_prereq C_LOCALE_OUTPUT '
-	! test_bool_env GIT_TEST_GETTEXT_POISON false
-'
+# Used to be used for GIT_TEST_GETTEXT_POISON=false. Only here as a
+# shim for other in-flight changes. Should not be used and will be
+# removed soon.
+test_set_prereq C_LOCALE_OUTPUT
 
 if test -z "$GIT_TEST_CHECK_CACHE_TREE"
 then

Original file line number	Diff line number	Diff line change
`@@ -664,7 +664,7 @@ mention the right animal somewhere:`
`664`	`664`	`----`
`665`	`665`	`test_expect_success 'runs correctly with no args and good output' '`
`666`	`666`	`git psuh >actual &&`
`667`		`- test_i18ngrep Pony actual`
	`667`	`+ grep Pony actual`
`668`	`668`	`'`
`669`	`669`	`----`
`670`	`670`
Original file line number	Diff line number	Diff line change
`@@ -28,15 +28,12 @@`
`28`	`28`
`29`	`29`	`#define FORMAT_PRESERVING(n) __attribute__((format_arg(n)))`
`30`	`30`
`31`		`-int use_gettext_poison(void);`
`32`		`-`
`33`	`31`	`#ifndef NO_GETTEXT`
`34`	`32`	`void git_setup_gettext(void);`
`35`	`33`	`int gettext_width(const char *s);`
`36`	`34`	`#else`
`37`	`35`	`static inline void git_setup_gettext(void)`
`38`	`36`	`{`
`39`		`- use_gettext_poison(); /* getenv() reentrancy paranoia */`
`40`	`37`	`}`
`41`	`38`	`static inline int gettext_width(const char *s)`
`42`	`39`	`{`
`@@ -48,14 +45,12 @@ static inline FORMAT_PRESERVING(1) const char _(const char msgid)`
`48`	`45`	`{`
`49`	`46`	`if (!*msgid)`
`50`	`47`	`return "";`
`51`		`- return use_gettext_poison() ? "# GETTEXT POISON #" : gettext(msgid);`
	`48`	`+ return gettext(msgid);`
`52`	`49`	`}`
`53`	`50`
`54`	`51`	`static inline FORMAT_PRESERVING(1) FORMAT_PRESERVING(2)`
`55`	`52`	`const char Q_(const char msgid, const char *plu, unsigned long n)`
`56`	`53`	`{`
`57`		`- if (use_gettext_poison())`
`58`		`- return "# GETTEXT POISON #";`
`59`	`54`	`return ngettext(msgid, plu, n);`
`60`	`55`	`}`
`61`	`56`