@matheusbsbs / matheus-bagliano / commits / 1ff750b128

tests: make GIT_TEST_GETTEXT_POISON a boolean

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 "env--helper" we can change that. There's a couple of tricky edge cases that arise because we're using git_env_bool() early, and the config-reading "env--helper". If GIT_TEST_GETTEXT_POISON is set to an invalid value die_bad_number() will die, but to do so it would usually call gettext(). Let's detect the special case of GIT_TEST_GETTEXT_POISON and always emit that message in the C locale, lest we infinitely loop. As seen in the updated tests in t0017-env-helper.sh there's also a caveat related to "env--helper" needing to read the config for trace2 purposes. Since the C_LOCALE_OUTPUT prerequisite is lazy and relies on "env--helper" we could get invalid results if we failed to read the config (e.g. because we'd loop on includes) when combined with e.g. "test_i18ngrep" wanting to check with "env--helper" if GIT_TEST_GETTEXT_POISON was true or not. I'm crossing my fingers and hoping that a test similar to the one I removed in the earlier "config tests: simplify include cycle test" change in this series won't happen again, and testing for this explicitly in "env--helper"'s own tests. This change breaks existing uses of e.g. GIT_TEST_GETTEXT_POISON=YesPlease, which we've documented in po/README and other places. As noted in [1] we might want to consider also accepting "YesPlease" in "env--helper" as a special-case. But as the lack of uproar over 6cdccfce1e ("i18n: make GETTEXT_POISON a runtime option", 2018-11-08) demonstrates the audience for this option is a really narrow set of git developers, who shouldn't have much trouble modifying their test scripts, so I think it's better to deal with that minor headache now and make all the relevant GIT_TEST_* variables boolean in the same way than carry the "YesPlease" special-case forward. 1. https://public-inbox.org/git/xmqqtvckm3h8.fsf@gitster-ct.c.googlers.com/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ævar Arnfjörð Bjarmason committed Jun 21, 2019 at 12:18 UTC 1ff750b128e041b3477ef9ee2768990e606d1c33

12 files changed +46 -18

ci/lib.sh

+1 -1

@@ -184,7 +184,7 @@ osx-clang|osx-gcc)

184 export GIT_SKIP_TESTS="t9810 t9816"

185 ;;

186 GIT_TEST_GETTEXT_POISON)

187 - export GIT_TEST_GETTEXT_POISON=YesPlease

187 + export GIT_TEST_GETTEXT_POISON=true

188 ;;

189 esac

190

config.c

@@ -956,6 +956,15 @@ static void die_bad_number(const char *name, const char *value)

956 if (!value)

957 value = "";

958

959 + if (!strcmp(name, "GIT_TEST_GETTEXT_POISON"))

960 + /*

961 + * We explicitly *don't* use _() here since it would

962 + * cause an infinite loop with _() needing to call

963 + * use_gettext_poison(). This is why marked up

964 + * translations with N_() above.

965 + */

966 + die(bad_numeric, value, name, error_type);

967 +

968 if (!(cf && cf->name))

969 die(_(bad_numeric), value, name, _(error_type));

970

gettext.c

+2 -4

@@ -50,10 +50,8 @@ const char *get_preferred_languages(void)

50 int use_gettext_poison(void)

51 {

52 static int poison_requested = -1;

53 - if (poison_requested == -1) {

54 - const char *v = getenv("GIT_TEST_GETTEXT_POISON");

55 - poison_requested = v && strlen(v) ? 1 : 0;

56 - }

53 + if (poison_requested == -1)

54 + poison_requested = git_env_bool("GIT_TEST_GETTEXT_POISON", 0);

55 return poison_requested;

56 }

git-sh-i18n.sh

+3 -1

@@ -17,7 +17,9 @@ export TEXTDOMAINDIR

18 # First decide what scheme to use...

19 GIT_INTERNAL_GETTEXT_SH_SCHEME=fallthrough

20 -if test -n "$GIT_TEST_GETTEXT_POISON"

20 +if test -n "$GIT_TEST_GETTEXT_POISON" &&

21 + git env--helper --type=bool --default=0 --exit-code \

22 + GIT_TEST_GETTEXT_POISON

23 then

24 GIT_INTERNAL_GETTEXT_SH_SCHEME=poison

25 elif test -n "@@USE_GETTEXT_SCHEME@@"

po/README

+1 -1

@@ -293,7 +293,7 @@ To smoke out issues like these, Git tested with a translation mode that

293 emits gibberish on every call to gettext. To use it run the test suite

294 with it, e.g.:

295

296 - cd t && GIT_TEST_GETTEXT_POISON=YesPlease prove -j 9 ./t[0-9]*.sh

296 + cd t && GIT_TEST_GETTEXT_POISON=true prove -j 9 ./t[0-9]*.sh

297

298 If tests break with it you should inspect them manually and see if

299 what you're translating is sane, i.e. that you're not translating

t/README

+2 -2

@@ -343,8 +343,8 @@ whether this mode is active, and e.g. skip some tests that are hard to

343 refactor to deal with it. The "SYMLINKS" prerequisite is currently

344 excluded as so much relies on it, but this might change in the future.

345

346 -GIT_TEST_GETTEXT_POISON=<non-empty?> turns all strings marked for

347 -translation into gibberish if non-empty (think "test -n"). Used for

346 +GIT_TEST_GETTEXT_POISON=<boolean> turns all strings marked for

347 +translation into gibberish if true. Used for

348 spotting those tests that need to be marked with a C_LOCALE_OUTPUT

349 prerequisite when adding more strings for translation. See "Testing

350 marked strings" in po/README for details.

t/t0017-env-helper.sh

+16

@@ -80,4 +80,20 @@ test_expect_success 'env--helper --type=ulong' '

80 test_must_be_empty actual.err

81 '

83 +test_expect_success 'env--helper reads config thanks to trace2' '

84 + mkdir home &&

85 + git config -f home/.gitconfig include.path cycle &&

86 + git config -f home/cycle include.path .gitconfig &&

87 +

88 + test_must_fail \

89 + env HOME="$(pwd)/home" GIT_TEST_GETTEXT_POISON=false \

90 + git config -l 2>err &&

91 + grep "exceeded maximum include depth" err &&

92 +

93 + test_must_fail \

94 + env HOME="$(pwd)/home" GIT_TEST_GETTEXT_POISON=true \

95 + git -C cycle env--helper --type=bool --default=0 --exit-code GIT_TEST_GETTEXT_POISON 2>err &&

96 + grep "# GETTEXT POISON #" err

97 +'

98 +

99 test_done

t/t0205-gettext-poison.sh

+6 -1

@@ -5,7 +5,7 @@

6 test_description='Gettext Shell poison'

8 -GIT_TEST_GETTEXT_POISON=YesPlease

8 +GIT_TEST_GETTEXT_POISON=true

9 export GIT_TEST_GETTEXT_POISON

10 . ./lib-gettext.sh

@@ -31,4 +31,9 @@ test_expect_success 'eval_gettext: our eval_gettext() fallback has poison semant

31 test_cmp expect actual

32 '

34 +test_expect_success "gettext: invalid GIT_TEST_GETTEXT_POISON value doesn't infinitely loop" "

35 + test_must_fail env GIT_TEST_GETTEXT_POISON=xyz git version 2>error &&

36 + grep \"fatal: bad numeric config value 'xyz' for 'GIT_TEST_GETTEXT_POISON': invalid unit\" error

37 +"

38 +

39 test_done

t/t1305-config-include.sh

+1 -1

@@ -314,7 +314,7 @@ test_expect_success 'include cycles are detected' '

314 git -C cycle config include.path cycle &&

315 git config -f cycle/cycle include.path config &&

316 test_must_fail \

317 - env GIT_TEST_GETTEXT_POISON= \

317 + env GIT_TEST_GETTEXT_POISON=false \

318 git -C cycle config --get-all test.value 2>stderr &&

319 grep "exceeded maximum include depth" stderr

320 '

t/t7201-co.sh

+1 -1

@@ -249,7 +249,7 @@ test_expect_success 'checkout to detach HEAD (with advice declined)' '

249 test_expect_success 'checkout to detach HEAD' '

250 git config advice.detachedHead true &&

251 git checkout -f renamer && git clean -f &&

252 - GIT_TEST_GETTEXT_POISON= git checkout renamer^ 2>messages &&

252 + GIT_TEST_GETTEXT_POISON=false git checkout renamer^ 2>messages &&

253 grep "HEAD is now at 7329388" messages &&

254 test_line_count -gt 1 messages &&

255 H=$(git rev-parse --verify HEAD) &&

t/t9902-completion.sh

+1 -1

@@ -1706,7 +1706,7 @@ test_expect_success 'sourcing the completion script clears cached commands' '

1706 '

1707

1708 test_expect_success 'sourcing the completion script clears cached merge strategies' '

1709 - GIT_TEST_GETTEXT_POISON= &&

1709 + GIT_TEST_GETTEXT_POISON=false &&

1710 __git_compute_merge_strategies &&

1711 verbose test -n "$__git_merge_strategies" &&

1712 . "$GIT_BUILD_DIR/contrib/completion/git-completion.bash" &&

t/test-lib.sh

+3 -5

@@ -1443,11 +1443,9 @@ then

1443 unset GIT_TEST_GETTEXT_POISON_ORIG

1444 fi

1445

1446 -# Can we rely on git's output in the C locale?

1447 -if test -z "$GIT_TEST_GETTEXT_POISON"

1448 -then

1449 - test_set_prereq C_LOCALE_OUTPUT

1450 -fi

1446 +test_lazy_prereq C_LOCALE_OUTPUT '

1447 + ! git env--helper --type=bool --default=0 --exit-code GIT_TEST_GETTEXT_POISON

1448 +'

1449

1450 if test -z "$GIT_TEST_CHECK_CACHE_TREE"

1451 then