From: Taylor Blau <ttaylorr@github.com>
To: "Đoàn Trần Công Danh" <congdanhqx@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] t: use user-specific utf-8 locale for testing
Date: Wed, 2 Jun 2021 15:56:38 -0400 [thread overview]
Message-ID: <YLfiYXxQqXL7RyHC@nand.local> (raw)
In-Reply-To: <20210602114646.17463-1-congdanhqx@gmail.com>
On Wed, Jun 02, 2021 at 06:46:46PM +0700, Đoàn Trần Công Danh wrote:
> Despite being required by POSIX, locale(1) is unavailable in some
> systems, e.g. Linux with musl libc. Some of those systems support
> utf-8 locale out of the box.
Hmmph. I would have imagined that locale was available everywhere, but
unfortunately not.
> diff --git a/Makefile b/Makefile
> index c3565fc0f8..4b2c24e5ea 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -398,6 +398,9 @@ all::
> # with a different indexfile format version. If it isn't set the index
> # file format used is index-v[23].
> #
> +# Define GIT_TEST_UTF8_LOCALE to prefered utf-8 locale for testing.
> +# If it isn't set, use the first utf-8 locale returned by "locale -a".
s/prefered/preferred
> +#
> # Define HAVE_CLOCK_GETTIME if your platform has clock_gettime.
> #
> # Define HAVE_CLOCK_MONOTONIC if your platform has CLOCK_MONOTONIC.
> @@ -2801,6 +2804,9 @@ ifdef GIT_TEST_CMP
> endif
> ifdef GIT_TEST_CMP_USE_COPIED_CONTEXT
> @echo GIT_TEST_CMP_USE_COPIED_CONTEXT=YesPlease >>$@+
> +endif
> +ifdef GIT_TEST_UTF8_LOCALE
> + @echo GIT_TEST_UTF8_LOCALE=\''$(subst ','\'',$(subst ','\'',$(GIT_TEST_UTF8_LOCALE)))'\' >>$@+
> endif
> @echo NO_GETTEXT=\''$(subst ','\'',$(subst ','\'',$(NO_GETTEXT)))'\' >>$@+
> ifdef GIT_PERF_REPEAT_COUNT
> diff --git a/t/lib-git-svn.sh b/t/lib-git-svn.sh
> index 547eb3c31a..df319593f7 100644
> --- a/t/lib-git-svn.sh
> +++ b/t/lib-git-svn.sh
> @@ -121,12 +121,15 @@ start_svnserve () {
> --listen-host 127.0.0.1 &
> }
>
> -prepare_a_utf8_locale () {
> - a_utf8_locale=$(locale -a | sed -n '/\.[uU][tT][fF]-*8$/{
> - p
> - q
> -}')
> - if test -n "$a_utf8_locale"
> +prepare_utf8_locale () {
> + if test -z "$GIT_TEST_UTF8_LOCALE"
> + then
> + GIT_TEST_UTF8_LOCALE=$(locale -a | sed -n '/\.[uU][tT][fF]-*8$/{
> + p
> + q
> + }')
> + fi
OK, so we bind GIT_TEST_UTF8_LOCALE to the value of $a_utf8_locale in
the pre-image, unless the user said otherwise.
> + if test -n "$GIT_TEST_UTF8_LOCALE"
...Then we go on to handle things like before, except we read from
"$GIT_TEST_UTF8_LOCALE" instead of "$a_utf8_locale". Makes sense to me.
> then
> test_set_prereq UTF8
> else
> diff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh
> index 1d3fdcc997..d5563ec35f 100755
> --- a/t/t9100-git-svn-basic.sh
> +++ b/t/t9100-git-svn-basic.sh
> @@ -4,21 +4,13 @@
> #
>
> test_description='git svn basic tests'
> -GIT_SVN_LC_ALL=${LC_ALL:-$LANG}
>
> GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
> export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>
> . ./lib-git-svn.sh
>
> -case "$GIT_SVN_LC_ALL" in
> -*.UTF-8)
> - test_set_prereq UTF8
> - ;;
> -*)
> - say "# UTF-8 locale not set, some tests skipped ($GIT_SVN_LC_ALL)"
> - ;;
> -esac
> +prepare_utf8_locale
This change (and the omitted ones below in later hunks) look like it
isn't changing any behavior (and just running the same code behind the
prepare_utf8_locale function instead of inlining it).
They all look right to me, but it may be helpful to either point it out
in the commit message and/or prepare the separately. I'd probably err on
the side of the former.
That said, this patch looks good to me with minor touch-ups (my only
nits are the above and the spelling mistake in the Makefile).
Thanks,
Taylor
next prev parent reply other threads:[~2021-06-02 19:57 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-02 11:46 [PATCH] t: use user-specific utf-8 locale for testing Đoàn Trần Công Danh
2021-06-02 19:56 ` Taylor Blau [this message]
2021-06-08 10:49 ` Ævar Arnfjörð Bjarmason
2021-06-03 19:27 ` Jeff King
2021-06-04 3:32 ` Bagas Sanjaya
2021-06-04 5:20 ` Đoàn Trần Công Danh
2021-06-06 16:33 ` [PATCH v2] " Đoàn Trần Công Danh
2021-06-06 20:06 ` Torsten Bögershausen
2021-06-07 0:20 ` Junio C Hamano
2021-06-07 0:48 ` [PATCH v3] t: use pre-defined utf-8 locale for testing svn Đoàn Trần Công Danh
2021-06-07 1:01 ` Junio C Hamano
2021-06-07 14:38 ` Torsten Bögershausen
2021-06-07 15:42 ` Đoàn Trần Công Danh
2021-06-08 6:35 ` Jeff King
2021-06-08 6:45 ` Đoàn Trần Công Danh
2021-06-07 1:08 ` [PATCH v4] t: use user-specified " Đoàn Trần Công Danh
2021-06-08 6:38 ` Jeff King
2021-06-08 6:56 ` [PATCH v5] " Đoàn Trần Công Danh
2021-06-08 7:26 ` 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=YLfiYXxQqXL7RyHC@nand.local \
--to=ttaylorr@github.com \
--cc=congdanhqx@gmail.com \
--cc=git@vger.kernel.org \
/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).