Addresses all the feedback against v3. Includes a patch by Junio
sitting in "pu" (and I fixed the grammar error Eric pointed out in its
commit message).
Range-diff:
1: cc24ba8de8 ! 1: 2210ee8bd9 i18n: make GETTEXT_POISON a runtime option
@@ -34,11 +34,11 @@
Notes on the implementation:
- * 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.
+ * We still compile a dedicated GETTEXT_POISON build in Travis
+ CI. Perhaps this should be revisited and integrated into the
+ "linux-gcc" build, see ae59a4e44f ("travis: run tests with
+ GIT_TEST_SPLIT_INDEX", 2018-01-07) for prior art in that area. Then
+ again maybe not, see [2].
* We now skip a test in t0000-basic.sh under
GIT_TEST_GETTEXT_POISON=YesPlease that wasn't skipped before. This
@@ -56,12 +56,22 @@
so we populate the "poison_requested" variable in a codepath that's
won't suffer from that race condition.
- See also [3] for more on the motivation behind this patch, and the
+ * We error out in the Makefile if you're still saying
+ GETTEXT_POISON=YesPlease to prompt users to change their
+ invocation.
+
+ * We should not print out poisoned messages during the test
+ initialization itself to keep it more readable, so the test library
+ hides the variable if set in $GIT_TEST_GETTEXT_POISON_ORIG during
+ setup. See [3].
+
+ See also [4] for more on the motivation behind this patch, and the
history of the GETTEXT_POISON facility.
1. https://public-inbox.org/git/[email protected]/
- 2. https://public-inbox.org/git/[email protected]/
- 3. https://public-inbox.org/git/[email protected]/
+ 2. https://public-inbox.org/git/[email protected]/
+ 3.
https://public-inbox.org/git/[email protected]/
+ 4. https://public-inbox.org/git/[email protected]/
Signed-off-by: Ævar Arnfjörð Bjarmason <[email protected]>
@@ -181,7 +191,7 @@
#else
static inline void git_setup_gettext(void)
{
-+ use_gettext_poison();; /* getenv() reentrancy paranoia */
++ use_gettext_poison(); /* getenv() reentrancy paranoia */
}
static inline int gettext_width(const char *s)
{
@@ -392,8 +402,32 @@
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@
+ 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
+@@
+ 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
++ unset GIT_TEST_GETTEXT_POISON_ORIG
++fi
++
# Can we rely on git's output in the C locale?
-if test -n "$GETTEXT_POISON"
+if test -z "$GIT_TEST_GETTEXT_POISON"
-: ---------- > 2: a6948d936a Makefile: ease dynamic-gettext-poison transition
Junio C Hamano (1):
Makefile: ease dynamic-gettext-poison transition
Ævar Arnfjörð Bjarmason (1):
i18n: make GETTEXT_POISON a runtime option
.travis.yml | 2 +-
Makefile | 8 +-------
ci/lib-travisci.sh | 4 ++--
gettext.c | 11 +++++++----
gettext.h | 9 +++------
git-sh-i18n.sh | 2 +-
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 | 22 +++++++++++++++++-----
16 files changed, 59 insertions(+), 49 deletions(-)
--
2.19.1.930.g4563a0d9d0
