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
List information: http://vger.kernel.org/majordomo-info.html
* 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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).