{"thread":{"id":"47768","subject":"[PATCH 0/2] minor GETTEXT_POISON fixes","startedAt":"2018-02-06T08:42:33Z","lastAt":"2018-02-06T11:23:04Z","messageCount":5,"participants":["Jeff King","Lars Schneider"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"338487","messageId":"20180206084226.GA26237@sigill.intra.peff.net","threadId":"47768","inReplyTo":null,"subject":"[PATCH 0/2] minor GETTEXT_POISON fixes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-02-06T08:42:27Z","receivedAt":"2018-02-06T08:42:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"I set NO_GETTEXT=1 in my config.mak, and happened to notice that running\nthe tests with GETTEXT_POISON fails. I think this has been broken for\nyears, but I don't generally play with GETTEXT_POISON. ;)\n\n  [1/2]: t0205: drop redundant test\n  [2/2]: git-sh-i18n: check GETTEXT_POISON before USE_GETTEXT_SCHEME\n\n git-sh-i18n.sh            | 8 ++++----\n t/t0205-gettext-poison.sh | 4 ----\n 2 files changed, 4 insertions(+), 8 deletions(-)\n\n-Peff\n"},{"id":"338488","messageId":"20180206084318.GA26329@sigill.intra.peff.net","threadId":"47768","inReplyTo":"20180206084226.GA26237@sigill.intra.peff.net","subject":"[PATCH 1/2] t0205: drop redundant test","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-02-06T08:43:19Z","receivedAt":"2018-02-06T08:43:24Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We check that a shell variable is non-empty, and then we\ncheck that it's equal to a particular value. Just checking\nthe latter covers both cases.\n\nI suspect the original was trying to give better output when\nthe test fails, but using \"-x\" covers that these days.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t0205-gettext-poison.sh | 4 ----\n 1 file changed, 4 deletions(-)\n\ndiff --git a/t/t0205-gettext-poison.sh b/t/t0205-gettext-poison.sh\nindex 2361590d54..438e778d6a 100755\n--- a/t/t0205-gettext-poison.sh\n+++ b/t/t0205-gettext-poison.sh\n@@ -7,10 +7,6 @@ test_description='Gettext Shell poison'\n \n . ./lib-gettext.sh\n \n-test_expect_success GETTEXT_POISON \"sanity: \\$GIT_INTERNAL_GETTEXT_SH_SCHEME is set (to $GIT_INTERNAL_GETTEXT_SH_SCHEME)\" '\n-    test -n \"$GIT_INTERNAL_GETTEXT_SH_SCHEME\"\n-'\n-\n test_expect_success GETTEXT_POISON 'sanity: $GIT_INTERNAL_GETTEXT_SH_SCHEME\" is poison' '\n     test \"$GIT_INTERNAL_GETTEXT_SH_SCHEME\" = \"poison\"\n '\n-- \n2.16.1.273.gc07cfcd8c9\n\n"},{"id":"338489","messageId":"20180206084456.GB26329@sigill.intra.peff.net","threadId":"47768","inReplyTo":"20180206084226.GA26237@sigill.intra.peff.net","subject":"[PATCH 2/2] git-sh-i18n: check GETTEXT_POISON before USE_GETTEXT_SCHEME","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-02-06T08:44:56Z","receivedAt":"2018-02-06T08:45:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Running \"make NO_GETTEXT=1 GETTEXT_POISON=1\" currently fails\nt0205.\n\nWhile it might seem nonsensical at first glance to both\npoison and disable gettext, it's useful to be able to do a\npoison test-run on a system that doesn't have gettext at\nall. And it works fine for C programs; the problem is only\nwith the shell code.\n\nThe issue is that we check the baked-in USE_GETTEXT_SCHEME\nvalue before GETTEXT_POISON. And when NO_GETTEXT is set, the\nMakefile sets USE_GETTEXT_SCHEME to \"fallthrough\".\n\nSo one fix would be to have the Makefile just set\nUSE_GETTEXT_SCHEME to \"poison\" if GETTEXT_POISON is set.\nBut there are two problems with that:\n\n  1. USE_GETTEXT_SCHEME is actually a user-facing knob, so\n     conceivably somebody could override it with:\n\n       make USE_GETTEXT_SCHEME=gnu GETTEXT_POISON=1\n\n     which would do the wrong thing (though that's much less\n     likely than them having the variable set in their\n     config.mak and just overriding GETTEXT_POISON on the\n     command-line for a one-off test).\n\n  2. We don't actually bake GETTEXT_POISON in to the shell\n     library like we do for the C code. It checks\n     $GIT_GETTEXT_POISON at runtime, which is set up by the\n     test suite. So it makes sense to put the fix in the\n     runtime code, too, which would cover something like:\n\n       GIT_GETTEXT_POISON=foo git foo\n\n     It's not likely that people use the poison code outside\n     of running the test suite, but it's easy enough to make\n     this case work.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n git-sh-i18n.sh | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/git-sh-i18n.sh b/git-sh-i18n.sh\nindex 1ef1889dbd..9d065fb4bf 100644\n--- a/git-sh-i18n.sh\n+++ b/git-sh-i18n.sh\n@@ -17,15 +17,15 @@ export TEXTDOMAINDIR\n \n # First decide what scheme to use...\n GIT_INTERNAL_GETTEXT_SH_SCHEME=fallthrough\n-if test -n \"@@USE_GETTEXT_SCHEME@@\"\n+if test -n \"$GIT_GETTEXT_POISON\"\n+then\n+\tGIT_INTERNAL_GETTEXT_SH_SCHEME=poison\n+elif test -n \"@@USE_GETTEXT_SCHEME@@\"\n then\n \tGIT_INTERNAL_GETTEXT_SH_SCHEME=\"@@USE_GETTEXT_SCHEME@@\"\n elif test -n \"$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS\"\n then\n \t: no probing necessary\n-elif test -n \"$GIT_GETTEXT_POISON\"\n-then\n-\tGIT_INTERNAL_GETTEXT_SH_SCHEME=poison\n elif type gettext.sh >/dev/null 2>&1\n then\n \t# GNU libintl's gettext.sh\n-- \n2.16.1.273.gc07cfcd8c9\n"},{"id":"338490","messageId":"FA353B2B-A5D1-4E2C-A016-38066F611BDB@gmail.com","threadId":"47768","inReplyTo":"20180206084226.GA26237@sigill.intra.peff.net","subject":"Re: [PATCH 0/2] minor GETTEXT_POISON fixes","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2018-02-06T09:06:53Z","receivedAt":"2018-02-06T09:07:13Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 06 Feb 2018, at 09:42, Jeff King <peff@peff.net> wrote:\n> \n> I set NO_GETTEXT=1 in my config.mak, and happened to notice that running\n> the tests with GETTEXT_POISON fails. I think this has been broken for\n> years, but I don't generally play with GETTEXT_POISON. ;)\n\nOn Travis we run GETTEXT_POISON with gettext installed.\nWhat do you think about an additional job with GETTEXT_POISON and NO_GETTEXT=1?\n\n- Lars"},{"id":"338499","messageId":"20180206112255.GB32319@sigill.intra.peff.net","threadId":"47768","inReplyTo":"FA353B2B-A5D1-4E2C-A016-38066F611BDB@gmail.com","subject":"Re: [PATCH 0/2] minor GETTEXT_POISON fixes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-02-06T11:22:55Z","receivedAt":"2018-02-06T11:23:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 06, 2018 at 10:06:53AM +0100, Lars Schneider wrote:\n\n> \n> > On 06 Feb 2018, at 09:42, Jeff King <peff@peff.net> wrote:\n> > \n> > I set NO_GETTEXT=1 in my config.mak, and happened to notice that running\n> > the tests with GETTEXT_POISON fails. I think this has been broken for\n> > years, but I don't generally play with GETTEXT_POISON. ;)\n> \n> On Travis we run GETTEXT_POISON with gettext installed.\n> What do you think about an additional job with GETTEXT_POISON and NO_GETTEXT=1?\n\nI think it's probably a waste of CPU. ;)\n\nIt would be running the whole test suite to fix this one particular bug,\nwhich I don't think is pretty likely to regress (at least not more than\nany other bug). Whereas running with GETTEXT_POISON is meant to shake\nout strings that maybe shouldn't be translated, which is a very likely\nbug to get introduced.\n\n-Peff\n"}]}