* [PATCH 0/2] minor GETTEXT_POISON fixes
@ 2018-02-06 8:42 Jeff King
2018-02-06 8:43 ` [PATCH 1/2] t0205: drop redundant test Jeff King
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Jeff King @ 2018-02-06 8:42 UTC (permalink / raw)
To: git; +Cc: Ævar Arnfjörð Bjarmason
I set NO_GETTEXT=1 in my config.mak, and happened to notice that running
the tests with GETTEXT_POISON fails. I think this has been broken for
years, but I don't generally play with GETTEXT_POISON. ;)
[1/2]: t0205: drop redundant test
[2/2]: git-sh-i18n: check GETTEXT_POISON before USE_GETTEXT_SCHEME
git-sh-i18n.sh | 8 ++++----
t/t0205-gettext-poison.sh | 4 ----
2 files changed, 4 insertions(+), 8 deletions(-)
-Peff
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] t0205: drop redundant test
2018-02-06 8:42 [PATCH 0/2] minor GETTEXT_POISON fixes Jeff King
@ 2018-02-06 8:43 ` Jeff King
2018-02-06 8:44 ` [PATCH 2/2] git-sh-i18n: check GETTEXT_POISON before USE_GETTEXT_SCHEME Jeff King
2018-02-06 9:06 ` [PATCH 0/2] minor GETTEXT_POISON fixes Lars Schneider
2 siblings, 0 replies; 5+ messages in thread
From: Jeff King @ 2018-02-06 8:43 UTC (permalink / raw)
To: git; +Cc: Ævar Arnfjörð Bjarmason
We check that a shell variable is non-empty, and then we
check that it's equal to a particular value. Just checking
the latter covers both cases.
I suspect the original was trying to give better output when
the test fails, but using "-x" covers that these days.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t0205-gettext-poison.sh | 4 ----
1 file changed, 4 deletions(-)
diff --git a/t/t0205-gettext-poison.sh b/t/t0205-gettext-poison.sh
index 2361590d54..438e778d6a 100755
--- a/t/t0205-gettext-poison.sh
+++ b/t/t0205-gettext-poison.sh
@@ -7,10 +7,6 @@ test_description='Gettext Shell poison'
. ./lib-gettext.sh
-test_expect_success GETTEXT_POISON "sanity: \$GIT_INTERNAL_GETTEXT_SH_SCHEME is set (to $GIT_INTERNAL_GETTEXT_SH_SCHEME)" '
- test -n "$GIT_INTERNAL_GETTEXT_SH_SCHEME"
-'
-
test_expect_success GETTEXT_POISON 'sanity: $GIT_INTERNAL_GETTEXT_SH_SCHEME" is poison' '
test "$GIT_INTERNAL_GETTEXT_SH_SCHEME" = "poison"
'
--
2.16.1.273.gc07cfcd8c9
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] git-sh-i18n: check GETTEXT_POISON before USE_GETTEXT_SCHEME
2018-02-06 8:42 [PATCH 0/2] minor GETTEXT_POISON fixes Jeff King
2018-02-06 8:43 ` [PATCH 1/2] t0205: drop redundant test Jeff King
@ 2018-02-06 8:44 ` Jeff King
2018-02-06 9:06 ` [PATCH 0/2] minor GETTEXT_POISON fixes Lars Schneider
2 siblings, 0 replies; 5+ messages in thread
From: Jeff King @ 2018-02-06 8:44 UTC (permalink / raw)
To: git; +Cc: Ævar Arnfjörð Bjarmason
Running "make NO_GETTEXT=1 GETTEXT_POISON=1" currently fails
t0205.
While it might seem nonsensical at first glance to both
poison and disable gettext, it's useful to be able to do a
poison test-run on a system that doesn't have gettext at
all. And it works fine for C programs; the problem is only
with the shell code.
The issue is that we check the baked-in USE_GETTEXT_SCHEME
value before GETTEXT_POISON. And when NO_GETTEXT is set, the
Makefile sets USE_GETTEXT_SCHEME to "fallthrough".
So one fix would be to have the Makefile just set
USE_GETTEXT_SCHEME to "poison" if GETTEXT_POISON is set.
But there are two problems with that:
1. USE_GETTEXT_SCHEME is actually a user-facing knob, so
conceivably somebody could override it with:
make USE_GETTEXT_SCHEME=gnu GETTEXT_POISON=1
which would do the wrong thing (though that's much less
likely than them having the variable set in their
config.mak and just overriding GETTEXT_POISON on the
command-line for a one-off test).
2. We don't actually bake GETTEXT_POISON in to the shell
library like we do for the C code. It checks
$GIT_GETTEXT_POISON at runtime, which is set up by the
test suite. So it makes sense to put the fix in the
runtime code, too, which would cover something like:
GIT_GETTEXT_POISON=foo git foo
It's not likely that people use the poison code outside
of running the test suite, but it's easy enough to make
this case work.
Signed-off-by: Jeff King <peff@peff.net>
---
git-sh-i18n.sh | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/git-sh-i18n.sh b/git-sh-i18n.sh
index 1ef1889dbd..9d065fb4bf 100644
--- a/git-sh-i18n.sh
+++ b/git-sh-i18n.sh
@@ -17,15 +17,15 @@ export TEXTDOMAINDIR
# First decide what scheme to use...
GIT_INTERNAL_GETTEXT_SH_SCHEME=fallthrough
-if test -n "@@USE_GETTEXT_SCHEME@@"
+if test -n "$GIT_GETTEXT_POISON"
+then
+ GIT_INTERNAL_GETTEXT_SH_SCHEME=poison
+elif test -n "@@USE_GETTEXT_SCHEME@@"
then
GIT_INTERNAL_GETTEXT_SH_SCHEME="@@USE_GETTEXT_SCHEME@@"
elif test -n "$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS"
then
: no probing necessary
-elif test -n "$GIT_GETTEXT_POISON"
-then
- GIT_INTERNAL_GETTEXT_SH_SCHEME=poison
elif type gettext.sh >/dev/null 2>&1
then
# GNU libintl's gettext.sh
--
2.16.1.273.gc07cfcd8c9
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 0/2] minor GETTEXT_POISON fixes
2018-02-06 8:42 [PATCH 0/2] minor GETTEXT_POISON fixes Jeff King
2018-02-06 8:43 ` [PATCH 1/2] t0205: drop redundant test Jeff King
2018-02-06 8:44 ` [PATCH 2/2] git-sh-i18n: check GETTEXT_POISON before USE_GETTEXT_SCHEME Jeff King
@ 2018-02-06 9:06 ` Lars Schneider
2018-02-06 11:22 ` Jeff King
2 siblings, 1 reply; 5+ messages in thread
From: Lars Schneider @ 2018-02-06 9:06 UTC (permalink / raw)
To: Jeff King
Cc: Git List, Ævar Arnfjörð Bjarmason, SZEDER Gábor
> On 06 Feb 2018, at 09:42, Jeff King <peff@peff.net> wrote:
>
> I set NO_GETTEXT=1 in my config.mak, and happened to notice that running
> the tests with GETTEXT_POISON fails. I think this has been broken for
> years, but I don't generally play with GETTEXT_POISON. ;)
On Travis we run GETTEXT_POISON with gettext installed.
What do you think about an additional job with GETTEXT_POISON and NO_GETTEXT=1?
- Lars
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 0/2] minor GETTEXT_POISON fixes
2018-02-06 9:06 ` [PATCH 0/2] minor GETTEXT_POISON fixes Lars Schneider
@ 2018-02-06 11:22 ` Jeff King
0 siblings, 0 replies; 5+ messages in thread
From: Jeff King @ 2018-02-06 11:22 UTC (permalink / raw)
To: Lars Schneider
Cc: Git List, Ævar Arnfjörð Bjarmason, SZEDER Gábor
On Tue, Feb 06, 2018 at 10:06:53AM +0100, Lars Schneider wrote:
>
> > On 06 Feb 2018, at 09:42, Jeff King <peff@peff.net> wrote:
> >
> > I set NO_GETTEXT=1 in my config.mak, and happened to notice that running
> > the tests with GETTEXT_POISON fails. I think this has been broken for
> > years, but I don't generally play with GETTEXT_POISON. ;)
>
> On Travis we run GETTEXT_POISON with gettext installed.
> What do you think about an additional job with GETTEXT_POISON and NO_GETTEXT=1?
I think it's probably a waste of CPU. ;)
It would be running the whole test suite to fix this one particular bug,
which I don't think is pretty likely to regress (at least not more than
any other bug). Whereas running with GETTEXT_POISON is meant to shake
out strings that maybe shouldn't be translated, which is a very likely
bug to get introduced.
-Peff
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-02-06 11:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-06 8:42 [PATCH 0/2] minor GETTEXT_POISON fixes Jeff King
2018-02-06 8:43 ` [PATCH 1/2] t0205: drop redundant test Jeff King
2018-02-06 8:44 ` [PATCH 2/2] git-sh-i18n: check GETTEXT_POISON before USE_GETTEXT_SCHEME Jeff King
2018-02-06 9:06 ` [PATCH 0/2] minor GETTEXT_POISON fixes Lars Schneider
2018-02-06 11:22 ` Jeff King
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).