diff mbox series

[3/6] tests: make GIT_TEST_GETTEXT_POISON a boolean

Message ID 20190619233046.27503-4-avarab@gmail.com (mailing list archive)
State New, archived
Headers show
Series Change <non-empty?> GIT_TEST_* variables to <boolean> | expand

Commit Message

Ævar Arnfjörð Bjarmason June 19, 2019, 11:30 p.m. UTC
Change the GIT_TEST_GETTEXT_POISON variable from being "non-empty?" to
being a more standard boolean variable.

Since it needed to be checked in both C code and shellscript (via test
-n) it was one of the remaining shellscript-like variables. Now that
we have "git env--helper" we can change that.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 ci/lib.sh                 | 2 +-
 gettext.c                 | 6 ++----
 git-sh-i18n.sh            | 4 +++-
 po/README                 | 2 +-
 t/README                  | 4 ++--
 t/t0205-gettext-poison.sh | 2 +-
 t/t7201-co.sh             | 2 +-
 t/t9902-completion.sh     | 2 +-
 t/test-lib.sh             | 4 ++++
 9 files changed, 16 insertions(+), 12 deletions(-)

Comments

Junio C Hamano June 20, 2019, 8 p.m. UTC | #1
Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Change the GIT_TEST_GETTEXT_POISON variable from being "non-empty?" to
> being a more standard boolean variable.
>
> Since it needed to be checked in both C code and shellscript (via test
> -n) it was one of the remaining shellscript-like variables. Now that
> we have "git env--helper" we can change that.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---

Hmph.  Even though I earlier said "we do not terribly mind breaking
developers and that is why we allow these patches", I have second
thoughts.  Turning "If it is empty, it is false" to "if you want to
say false, say it in one of those approved ways" is one thing.
Forcing SWITCH=YesPlease to be rewritten to SWITCH=yes is quite
different---we are breaking everybody who would have to read and
follow po/README and t/README.

We _might_ have to grandfarther YesPlease as a special value that is
understood by "git env" (but not "git config") to ease the
transition, as that token has been used as a sample true value in
many places.

But let's read on.  Assuming that breaking those who hardcoded
YesPlease in their scripts is OK, this patch looked sensible.

Thanks.
diff mbox series

Patch

diff --git a/ci/lib.sh b/ci/lib.sh
index 288a5b3884..fd799ae663 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -184,7 +184,7 @@  osx-clang|osx-gcc)
 	export GIT_SKIP_TESTS="t9810 t9816"
 	;;
 GIT_TEST_GETTEXT_POISON)
-	export GIT_TEST_GETTEXT_POISON=YesPlease
+	export GIT_TEST_GETTEXT_POISON=true
 	;;
 esac
 
diff --git a/gettext.c b/gettext.c
index d4021d690c..5c71f4c8b9 100644
--- a/gettext.c
+++ b/gettext.c
@@ -50,10 +50,8 @@  const char *get_preferred_languages(void)
 int use_gettext_poison(void)
 {
 	static int poison_requested = -1;
-	if (poison_requested == -1) {
-		const char *v = getenv("GIT_TEST_GETTEXT_POISON");
-		poison_requested = v && strlen(v) ? 1 : 0;
-	}
+	if (poison_requested == -1)
+		poison_requested = git_env_bool("GIT_TEST_GETTEXT_POISON", 0);
 	return poison_requested;
 }
 
diff --git a/git-sh-i18n.sh b/git-sh-i18n.sh
index e1d917fd27..de8ae67d7b 100644
--- a/git-sh-i18n.sh
+++ b/git-sh-i18n.sh
@@ -17,7 +17,9 @@  export TEXTDOMAINDIR
 
 # First decide what scheme to use...
 GIT_INTERNAL_GETTEXT_SH_SCHEME=fallthrough
-if test -n "$GIT_TEST_GETTEXT_POISON"
+if test -n "$GIT_TEST_GETTEXT_POISON" &&
+	    git env--helper --mode-bool --variable=GIT_TEST_GETTEXT_POISON \
+		--default=0 --exit-code --quiet
 then
 	GIT_INTERNAL_GETTEXT_SH_SCHEME=poison
 elif test -n "@@USE_GETTEXT_SCHEME@@"
diff --git a/po/README b/po/README
index aa704ffcb7..07595d369b 100644
--- a/po/README
+++ b/po/README
@@ -293,7 +293,7 @@  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=YesPlease 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 9747971d58..9a131f472e 100644
--- a/t/README
+++ b/t/README
@@ -343,8 +343,8 @@  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=<non-empty?> turns all strings marked for
-translation into gibberish if non-empty (think "test -n"). Used for
+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.
diff --git a/t/t0205-gettext-poison.sh b/t/t0205-gettext-poison.sh
index a06269f38a..1675d3e171 100755
--- a/t/t0205-gettext-poison.sh
+++ b/t/t0205-gettext-poison.sh
@@ -5,7 +5,7 @@ 
 
 test_description='Gettext Shell poison'
 
-GIT_TEST_GETTEXT_POISON=YesPlease
+GIT_TEST_GETTEXT_POISON=true
 export GIT_TEST_GETTEXT_POISON
 . ./lib-gettext.sh
 
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 5990299fc9..b696bae5f5 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -249,7 +249,7 @@  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_TEST_GETTEXT_POISON= git checkout renamer^ 2>messages &&
+	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) &&
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 43cf313a1c..75512c3403 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1706,7 +1706,7 @@  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= &&
+	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.sh b/t/test-lib.sh
index 4b346467df..bdd5017d24 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1443,6 +1443,10 @@  then
 	unset GIT_TEST_GETTEXT_POISON_ORIG
 fi
 
+test_lazy_prereq C_LOCALE_OUTPUT '
+	! git env--helper --mode-bool --variable=GIT_TEST_GETTEXT_POISON --default=0 --exit-code --quiet
+'
+
 # Can we rely on git's output in the C locale?
 if test -z "$GIT_TEST_GETTEXT_POISON"
 then