git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>, "Jeff King" <peff@peff.net>,
	"SZEDER Gábor" <szeder.dev@gmail.com>,
	"Vasco Almeida" <vascomalmeida@sapo.pt>,
	"Jiang Xin" <worldhello.net@gmail.com>,
	"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	"Eric Sunshine" <sunshine@sunshineco.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH v4 0/2] i18n: make GETTEXT_POISON a runtime option
Date: Thu,  8 Nov 2018 21:15:28 +0000	[thread overview]
Message-ID: <20181108211530.29017-1-avarab@gmail.com> (raw)
In-Reply-To: <20181101193115.32681-1-avarab@gmail.com>

Addresses all the feedback against v3. Includes a patch by Junio
sitting in "pu" (and I fixed the grammar error Eric pointed out in its
commit message).

Range-diff:

1:  cc24ba8de8 ! 1:  2210ee8bd9 i18n: make GETTEXT_POISON a runtime option
    @@ -34,11 +34,11 @@
     
         Notes on the implementation:
     
    -     * We still compile a dedicated GETTEXT_POISON build in Travis CI.
    -       This is probably the wrong thing to do and should be followed-up
    -       with something similar to ae59a4e44f ("travis: run tests with
    -       GIT_TEST_SPLIT_INDEX", 2018-01-07) to re-use an existing test setup
    -       for running in the GIT_TEST_GETTEXT_POISON mode.
    +     * We still compile a dedicated GETTEXT_POISON build in Travis
    +       CI. Perhaps this should be revisited and integrated into the
    +       "linux-gcc" build, see ae59a4e44f ("travis: run tests with
    +       GIT_TEST_SPLIT_INDEX", 2018-01-07) for prior art in that area. Then
    +       again maybe not, see [2].
     
          * We now skip a test in t0000-basic.sh under
            GIT_TEST_GETTEXT_POISON=YesPlease that wasn't skipped before. This
    @@ -56,12 +56,22 @@
            so we populate the "poison_requested" variable in a codepath that's
            won't suffer from that race condition.
     
    -    See also [3] for more on the motivation behind this patch, and the
    +     * We error out in the Makefile if you're still saying
    +       GETTEXT_POISON=YesPlease to prompt users to change their
    +       invocation.
    +
    +     * We should not print out poisoned messages during the test
    +       initialization itself to keep it more readable, so the test library
    +       hides the variable if set in $GIT_TEST_GETTEXT_POISON_ORIG during
    +       setup. See [3].
    +
    +    See also [4] for more on the motivation behind this patch, and the
         history of the GETTEXT_POISON facility.
     
         1. https://public-inbox.org/git/871s8gd32p.fsf@evledraar.gmail.com/
    -    2. https://public-inbox.org/git/87woq7b58k.fsf@evledraar.gmail.com/
    -    3. https://public-inbox.org/git/878t2pd6yu.fsf@evledraar.gmail.com/
    +    2. https://public-inbox.org/git/20181102163725.GY30222@szeder.dev/
    +    3. https://public-inbox.org/git/20181022202241.18629-2-szeder.dev@gmail.com/
    +    4. https://public-inbox.org/git/878t2pd6yu.fsf@evledraar.gmail.com/
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
    @@ -181,7 +191,7 @@
      #else
      static inline void git_setup_gettext(void)
      {
    -+	use_gettext_poison();; /* getenv() reentrancy paranoia */
    ++	use_gettext_poison(); /* getenv() reentrancy paranoia */
      }
      static inline int gettext_width(const char *s)
      {
    @@ -392,8 +402,32 @@
      --- a/t/test-lib.sh
      +++ b/t/test-lib.sh
     @@
    + TZ=UTC
    + export LANG LC_ALL PAGER TZ
    + EDITOR=:
    ++
    ++# GIT_TEST_GETTEXT_POISON should not influence git commands executed
    ++# during initialization of test-lib and the test repo. Back it up,
    ++# unset and then restore after initialization is finished.
    ++if test -n "$GIT_TEST_GETTEXT_POISON"
    ++then
    ++	GIT_TEST_GETTEXT_POISON_ORIG=$GIT_TEST_GETTEXT_POISON
    ++	unset GIT_TEST_GETTEXT_POISON
    ++fi
    ++
    + # A call to "unset" with no arguments causes at least Solaris 10
    + # /usr/xpg4/bin/sh and /bin/ksh to bail out.  So keep the unsets
    + # deriving from the command substitution clustered with the other
    +@@
    + test -n "$USE_LIBPCRE2" && test_set_prereq LIBPCRE2
      test -z "$NO_GETTEXT" && test_set_prereq GETTEXT
      
    ++if test -n "$GIT_TEST_GETTEXT_POISON_ORIG"
    ++then
    ++	GIT_TEST_GETTEXT_POISON=$GIT_TEST_GETTEXT_POISON_ORIG
    ++	unset GIT_TEST_GETTEXT_POISON_ORIG
    ++fi
    ++
      # Can we rely on git's output in the C locale?
     -if test -n "$GETTEXT_POISON"
     +if test -z "$GIT_TEST_GETTEXT_POISON"
-:  ---------- > 2:  a6948d936a Makefile: ease dynamic-gettext-poison transition


Junio C Hamano (1):
  Makefile: ease dynamic-gettext-poison transition

Ævar Arnfjörð Bjarmason (1):
  i18n: make GETTEXT_POISON a runtime option

 .travis.yml               |  2 +-
 Makefile                  |  8 +-------
 ci/lib-travisci.sh        |  4 ++--
 gettext.c                 | 11 +++++++----
 gettext.h                 |  9 +++------
 git-sh-i18n.sh            |  2 +-
 po/README                 | 13 ++++---------
 t/README                  |  6 ++++++
 t/lib-gettext.sh          |  2 +-
 t/t0000-basic.sh          |  2 +-
 t/t0205-gettext-poison.sh |  8 +++++---
 t/t3406-rebase-message.sh |  2 +-
 t/t7201-co.sh             |  6 +++---
 t/t9902-completion.sh     |  3 ++-
 t/test-lib-functions.sh   |  8 ++++----
 t/test-lib.sh             | 22 +++++++++++++++++-----
 16 files changed, 59 insertions(+), 49 deletions(-)

-- 
2.19.1.930.g4563a0d9d0


  parent reply	other threads:[~2018-11-08 21:15 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-22 15:36 [PATCH] Poison gettext with the Ook language Nguyễn Thái Ngọc Duy
2018-10-22 20:22 ` SZEDER Gábor
2018-10-22 20:22   ` [PATCH 1/8] test-lib.sh: preserve GIT_GETTEXT_POISON from the environment SZEDER Gábor
2018-10-22 20:22   ` [PATCH 2/8] gettext: don't poison if GIT_GETTEXT_POISON is set but empty SZEDER Gábor
2018-10-22 20:38     ` Ævar Arnfjörð Bjarmason
2018-10-22 20:22   ` [PATCH 3/8] lib-rebase: loosen GETTEXT_POISON check in fake editor SZEDER Gábor
2018-10-22 20:22   ` [PATCH 4/8] gettext: #ifdef away GETTEXT POISON-related code from _() and Q_() SZEDER Gábor
2018-10-22 20:22   ` [PATCH 5/8] gettext: put "# GETTEXT POISON #" string literal into a macro SZEDER Gábor
2018-10-22 20:22   ` [PATCH 6/8] gettext: use an enum for the mode of GETTEXT POISONing SZEDER Gábor
2018-10-22 20:22   ` [PATCH 7/8] gettext: introduce GIT_GETTEXT_POISON=scrambled SZEDER Gábor
2018-10-23 14:44     ` Duy Nguyen
2018-10-22 20:22   ` [PATCH 8/8] travis-ci: run GETTEXT POISON build job in scrambled mode, too SZEDER Gábor
2018-10-23 14:37   ` [PATCH] Poison gettext with the Ook language Duy Nguyen
2018-10-27 10:11   ` Jakub Narebski
2018-10-22 20:52 ` Johannes Schindelin
2018-10-22 21:09 ` Ævar Arnfjörð Bjarmason
2018-10-22 21:46   ` Ævar Arnfjörð Bjarmason
2018-10-22 23:04   ` Junio C Hamano
2018-10-23 21:01     ` [PATCH] i18n: make GETTEXT_POISON a runtime option Ævar Arnfjörð Bjarmason
2018-10-24  5:45       ` Junio C Hamano
2018-10-24  7:44         ` Jeff King
2018-10-25  1:00           ` Junio C Hamano
2018-10-25  1:09             ` Jeff King
2018-10-25  1:24               ` Ramsay Jones
2018-10-25 21:23                 ` Jeff King
2018-10-26 19:20                   ` Ævar Arnfjörð Bjarmason
2018-10-27  6:59                     ` Jeff King
2018-10-27 10:42                       ` Ævar Arnfjörð Bjarmason
2018-10-24 11:47         ` [PATCH v2] " Ævar Arnfjörð Bjarmason
2018-11-01 19:31           ` [PATCH v3] " Ævar Arnfjörð Bjarmason
2018-11-02  3:11             ` Junio C Hamano
2018-11-02 16:37             ` SZEDER Gábor
2018-11-08 20:26               ` Ævar Arnfjörð Bjarmason
2018-11-08 20:51                 ` SZEDER Gábor
2018-11-08  3:24             ` Junio C Hamano
2018-11-08 19:12               ` Eric Sunshine
2018-11-08 21:15             ` Ævar Arnfjörð Bjarmason [this message]
2018-11-08 21:15             ` [PATCH v4 1/2] " Ævar Arnfjörð Bjarmason
2018-11-08 21:15             ` [PATCH v4 2/2] Makefile: ease dynamic-gettext-poison transition Ævar Arnfjörð Bjarmason
2018-10-23  9:30   ` [PATCH] Poison gettext with the Ook language Johannes Schindelin
2018-10-23 10:17     ` Ævar Arnfjörð Bjarmason
2018-10-23 11:07       ` Johannes Schindelin
2018-10-23 15:00       ` Duy Nguyen
2018-10-23 16:45         ` Ævar Arnfjörð Bjarmason
2018-10-24 14:41           ` Duy Nguyen
2018-10-24 17:54             ` Ævar Arnfjörð Bjarmason
2018-10-25  3:52             ` Junio C Hamano
2018-10-25  6:20               ` Jeff King
2018-10-27  6:55                 ` Junio C Hamano

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=20181108211530.29017-1-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    --cc=sunshine@sunshineco.com \
    --cc=szeder.dev@gmail.com \
    --cc=vascomalmeida@sapo.pt \
    --cc=worldhello.net@gmail.com \
    /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).