git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / Atom feed
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
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


  parent reply index

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

git@vger.kernel.org list mirror (unofficial, one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Example config snippet for mirrors

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git