From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
"Jeff King" <peff@peff.net>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH v2 0/8] Change <non-empty?> GIT_TEST_* variables to <boolean>
Date: Thu, 20 Jun 2019 23:09:07 +0200 [thread overview]
Message-ID: <20190620210915.11297-1-avarab@gmail.com> (raw)
In-Reply-To: <20190619233046.27503-1-avarab@gmail.com>
This v2 fixes tricky bugs I noticed after sending v1 in calling
gettext so early in the setup, and the t0016 name clash with pu Junio
pointed out.
I didn't change the "env--helper" interface as suggested because I
already had this ready and figured I'd send a v2 for review of the
stuff I have now.
The interface suggested in
<xmqqa7ecnjot.fsf@gitster-ct.c.googlers.com> is indeed prettier. FWIW
I did things like --mode-bool instead of --type=bool because it's
easier to do just with the getopt framework, likewise --variable=X
instead of "X" being last on the argv since you can do it all with the
flag parsing doing the work for you, and since it's an internal-only
command I figured it didn't matter much.
Ævar Arnfjörð Bjarmason (8):
config tests: simplify include cycle test
env--helper: new undocumented builtin wrapping git_env_*()
config.c: refactor die_bad_number() to not call gettext() early
t6040 test: stop using global "script" variable
tests: make GIT_TEST_GETTEXT_POISON a boolean
tests README: re-flow a previously changed paragraph
tests: replace test_tristate with "git env--helper"
tests: make GIT_TEST_FAIL_PREREQS a boolean
.gitignore | 1 +
Makefile | 1 +
builtin.h | 1 +
builtin/env--helper.c | 74 +++++++++++++++++++++++++++++++++
ci/lib.sh | 2 +-
config.c | 28 +++++++++----
gettext.c | 6 +--
git-sh-i18n.sh | 4 +-
git.c | 1 +
po/README | 2 +-
t/README | 12 +++---
t/lib-git-daemon.sh | 7 ++--
t/lib-git-svn.sh | 11 ++---
t/lib-httpd.sh | 15 ++++---
t/t0000-basic.sh | 10 ++---
t/t0017-env-helper.sh | 86 +++++++++++++++++++++++++++++++++++++++
t/t0205-gettext-poison.sh | 7 +++-
t/t1305-config-include.sh | 21 ++++------
t/t5512-ls-remote.sh | 3 +-
t/t6040-tracking-info.sh | 6 +--
t/t7201-co.sh | 2 +-
t/t9902-completion.sh | 2 +-
t/test-lib-functions.sh | 58 ++++----------------------
t/test-lib.sh | 33 +++++++++++----
24 files changed, 266 insertions(+), 127 deletions(-)
create mode 100644 builtin/env--helper.c
create mode 100755 t/t0017-env-helper.sh
Range-diff:
-: ---------- > 1: c3483c37a1 config tests: simplify include cycle test
1: 8da3cd4240 ! 2: e689759f7c env--helper: new undocumented builtin wrapping git_env_*()
@@ -154,10 +154,10 @@
{ "fetch", cmd_fetch, RUN_SETUP },
{ "fetch-pack", cmd_fetch_pack, RUN_SETUP | NO_PARSEOPT },
- diff --git a/t/t0016-env-helper.sh b/t/t0016-env-helper.sh
+ diff --git a/t/t0017-env-helper.sh b/t/t0017-env-helper.sh
new file mode 100755
--- /dev/null
- +++ b/t/t0016-env-helper.sh
+ +++ b/t/t0017-env-helper.sh
@@
+#!/bin/sh
+
-: ---------- > 3: f759d5e91e config.c: refactor die_bad_number() to not call gettext() early
2: 8315c4ecdc = 4: 1ac798e8ce t6040 test: stop using global "script" variable
3: f1ee208d70 ! 5: d7d6e6c874 tests: make GIT_TEST_GETTEXT_POISON a boolean
@@ -7,7 +7,30 @@
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.
+ 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 t0016-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.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
@@ -24,6 +47,26 @@
esac
+ diff --git a/config.c b/config.c
+ --- a/config.c
+ +++ b/config.c
+@@
+ if (!value)
+ value = "";
+
++ if (!strcmp(name, "GIT_TEST_GETTEXT_POISON"))
++ /*
++ * We explicitly *don't* use _() here since it would
++ * cause an infinite loop with _() needing to call
++ * use_gettext_poison(). This is why marked up
++ * translations with N_() above.
++ */
++ die(bad_numeric, value, name, error_type);
++
+ if (!(cf && cf->name))
+ die(_(bad_numeric), value, name, _(error_type));
+
+
diff --git a/gettext.c b/gettext.c
--- a/gettext.c
+++ b/gettext.c
@@ -84,6 +127,31 @@
prerequisite when adding more strings for translation. See "Testing
marked strings" in po/README for details.
+ diff --git a/t/t0017-env-helper.sh b/t/t0017-env-helper.sh
+ --- a/t/t0017-env-helper.sh
+ +++ b/t/t0017-env-helper.sh
+@@
+ test_cmp expected actual
+ '
+
++test_expect_success 'env--helper reads config thanks to trace2' '
++ mkdir home &&
++ git config -f home/.gitconfig include.path cycle &&
++ git config -f home/cycle include.path .gitconfig &&
++
++ test_must_fail \
++ env HOME="$(pwd)/home" GIT_TEST_GETTEXT_POISON=false \
++ git config -l 2>err &&
++ grep "exceeded maximum include depth" err &&
++
++ test_must_fail \
++ env HOME="$(pwd)/home" GIT_TEST_GETTEXT_POISON=true \
++ git -C cycle env--helper --mode-bool --variable=GIT_TEST_GETTEXT_POISON --default=0 --exit-code --quiet 2>err &&
++ grep "# GETTEXT POISON #" err
++'
++
+ test_done
+
diff --git a/t/t0205-gettext-poison.sh b/t/t0205-gettext-poison.sh
--- a/t/t0205-gettext-poison.sh
+++ b/t/t0205-gettext-poison.sh
@@ -96,6 +164,29 @@
export GIT_TEST_GETTEXT_POISON
. ./lib-gettext.sh
+@@
+ test_cmp expect actual
+ '
+
++test_expect_success "gettext: invalid GIT_TEST_GETTEXT_POISON value doesn't infinitely loop" "
++ test_must_fail env GIT_TEST_GETTEXT_POISON=xyz git version 2>error &&
++ grep \"fatal: bad numeric config value 'xyz' for 'GIT_TEST_GETTEXT_POISON': invalid unit\" error
++"
++
+ test_done
+
+ diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh
+ --- a/t/t1305-config-include.sh
+ +++ b/t/t1305-config-include.sh
+@@
+ git -C cycle config include.path cycle &&
+ git config -f cycle/cycle include.path config &&
+ test_must_fail \
+- env GIT_TEST_GETTEXT_POISON= \
++ env GIT_TEST_GETTEXT_POISON=false \
+ git -C cycle config --get-all test.value 2>stderr &&
+ grep "exceeded maximum include depth" stderr
+ '
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
--- a/t/t7201-co.sh
@@ -130,10 +221,14 @@
unset GIT_TEST_GETTEXT_POISON_ORIG
fi
+-# Can we rely on git's output in the C locale?
+-if test -z "$GIT_TEST_GETTEXT_POISON"
+-then
+- test_set_prereq C_LOCALE_OUTPUT
+-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"
+
+ if test -z "$GIT_TEST_CHECK_CACHE_TREE"
then
4: f1e28dff36 = 6: 954428b3cd tests README: re-flow a previously changed paragraph
5: f046cf21fb = 7: 79b41cf01b tests: replace test_tristate with "git env--helper"
6: e2c68e0239 = 8: a9aa166b66 tests: make GIT_TEST_FAIL_PREREQS a boolean
--
2.22.0.455.g172b71a6c5
next prev parent reply other threads:[~2019-06-20 21:09 UTC|newest]
Thread overview: 61+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-19 9:46 [PATCH] fetch: only run 'gc' once when fetching multiple remotes Nguyễn Thái Ngọc Duy
2019-06-19 10:26 ` [RFC/PATCH] gc: run more pre-detach operations under lock Ævar Arnfjörð Bjarmason
2019-06-19 12:51 ` Duy Nguyen
2019-06-19 18:01 ` Ævar Arnfjörð Bjarmason
2019-06-19 19:10 ` Jeff King
2019-06-19 22:49 ` Ævar Arnfjörð Bjarmason
2019-06-19 23:30 ` [PATCH 0/6] Change <non-empty?> GIT_TEST_* variables to <boolean> Ævar Arnfjörð Bjarmason
2019-06-20 18:13 ` Junio C Hamano
2019-06-20 21:00 ` Ævar Arnfjörð Bjarmason
2019-06-20 20:03 ` Junio C Hamano
2019-06-20 21:09 ` Ævar Arnfjörð Bjarmason [this message]
2019-06-21 10:18 ` [PATCH v3 0/8] " Ævar Arnfjörð Bjarmason
2019-06-21 10:18 ` [PATCH v3 1/8] config tests: simplify include cycle test Ævar Arnfjörð Bjarmason
2019-06-21 10:18 ` [PATCH v3 2/8] env--helper: new undocumented builtin wrapping git_env_*() Ævar Arnfjörð Bjarmason
2019-06-21 17:07 ` Junio C Hamano
2019-06-21 10:18 ` [PATCH v3 3/8] config.c: refactor die_bad_number() to not call gettext() early Ævar Arnfjörð Bjarmason
2019-06-21 10:18 ` [PATCH v3 4/8] t6040 test: stop using global "script" variable Ævar Arnfjörð Bjarmason
2019-06-21 10:18 ` [PATCH v3 5/8] tests: make GIT_TEST_GETTEXT_POISON a boolean Ævar Arnfjörð Bjarmason
2019-06-24 16:47 ` Junio C Hamano
2019-06-21 10:18 ` [PATCH v3 6/8] tests README: re-flow a previously changed paragraph Ævar Arnfjörð Bjarmason
2019-06-21 10:18 ` [PATCH v3 7/8] tests: replace test_tristate with "git env--helper" Ævar Arnfjörð Bjarmason
2019-09-06 12:13 ` [PATCH 1/2] t/lib-git-svn.sh: check GIT_TEST_SVN_HTTPD when running SVN HTTP tests SZEDER Gábor
2019-09-06 12:13 ` [PATCH 2/2] ci: restore running httpd tests SZEDER Gábor
2019-09-06 17:03 ` Junio C Hamano
2019-09-06 19:13 ` Jeff King
2019-09-07 10:16 ` SZEDER Gábor
2019-11-22 13:14 ` [PATCH 0/2] tests: catch non-bool GIT_TEST_* values SZEDER Gábor
2019-11-22 13:14 ` [PATCH 1/2] tests: add 'test_bool_env' to " SZEDER Gábor
2019-11-25 13:50 ` Jeff King
2019-11-22 13:14 ` [PATCH 2/2] t5608-clone-2gb.sh: turn GIT_TEST_CLONE_2GB into a bool SZEDER Gábor
2019-11-25 13:53 ` Jeff King
2019-06-21 10:18 ` [PATCH v3 8/8] tests: make GIT_TEST_FAIL_PREREQS a boolean Ævar Arnfjörð Bjarmason
2019-06-20 21:09 ` [PATCH v2 1/8] config tests: simplify include cycle test Ævar Arnfjörð Bjarmason
2019-06-20 21:09 ` [PATCH v2 2/8] env--helper: new undocumented builtin wrapping git_env_*() Ævar Arnfjörð Bjarmason
2019-06-20 22:11 ` Junio C Hamano
2019-06-20 22:21 ` Junio C Hamano
2019-06-21 8:11 ` Ævar Arnfjörð Bjarmason
2019-06-21 15:04 ` Junio C Hamano
2019-06-20 21:09 ` [PATCH v2 3/8] config.c: refactor die_bad_number() to not call gettext() early Ævar Arnfjörð Bjarmason
2019-06-20 21:09 ` [PATCH v2 4/8] t6040 test: stop using global "script" variable Ævar Arnfjörð Bjarmason
2019-06-20 21:09 ` [PATCH v2 5/8] tests: make GIT_TEST_GETTEXT_POISON a boolean Ævar Arnfjörð Bjarmason
2019-06-20 21:09 ` [PATCH v2 6/8] tests README: re-flow a previously changed paragraph Ævar Arnfjörð Bjarmason
2019-06-20 21:09 ` [PATCH v2 7/8] tests: replace test_tristate with "git env--helper" Ævar Arnfjörð Bjarmason
2019-06-20 21:09 ` [PATCH v2 8/8] tests: make GIT_TEST_FAIL_PREREQS a boolean Ævar Arnfjörð Bjarmason
2019-06-19 23:30 ` [PATCH 1/6] env--helper: new undocumented builtin wrapping git_env_*() Ævar Arnfjörð Bjarmason
2019-06-20 19:25 ` Junio C Hamano
2019-06-19 23:30 ` [PATCH 2/6] t6040 test: stop using global "script" variable Ævar Arnfjörð Bjarmason
2019-06-20 19:54 ` Junio C Hamano
2019-06-19 23:30 ` [PATCH 3/6] tests: make GIT_TEST_GETTEXT_POISON a boolean Ævar Arnfjörð Bjarmason
2019-06-20 20:00 ` Junio C Hamano
2019-06-19 23:30 ` [PATCH 4/6] tests README: re-flow a previously changed paragraph Ævar Arnfjörð Bjarmason
2019-06-19 23:30 ` [PATCH 5/6] tests: replace test_tristate with "git env--helper" Ævar Arnfjörð Bjarmason
2019-06-19 23:30 ` [PATCH 6/6] tests: make GIT_TEST_FAIL_PREREQS a boolean Ævar Arnfjörð Bjarmason
2019-06-20 10:26 ` [RFC/PATCH] gc: run more pre-detach operations under lock Duy Nguyen
2019-06-20 21:49 ` Ævar Arnfjörð Bjarmason
2019-06-20 10:43 ` Jeff King
2019-06-20 18:55 ` Junio C Hamano
2019-06-19 19:08 ` Jeff King
2019-06-19 18:59 ` [PATCH] fetch: only run 'gc' once when fetching multiple remotes Jeff King
2019-06-20 10:11 ` Duy Nguyen
2019-06-20 10:21 ` Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190620210915.11297-1-avarab@gmail.com \
--to=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=pclouds@gmail.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.