Re: [PATCH v4] t: use user-specified utf-8 locale for testing svn

From: Jeff King <peff@peff.net>
Date: Jun 8, 2021, 06:38 UTC
Message-ID: <YL8Qe31z1Q5/DkmF@coredump.intra.peff.net>
In-Reply-To: <20210607010844.24587-1-congdanhqx@gmail.com>

On Mon, Jun 07, 2021 at 08:08:44AM +0700, Đoàn Trần Công Danh wrote:

Show 17 quoted lines

> In some test-cases, UTF-8 locale is required. To find such locale,
> we're using the first available UTF-8 locale that returned by
> "locale -a".
> 
> However, the locale(1) utility is unavailable on some systems,
> e.g. Linux with musl libc.
> 
> However, without "locale -a", we can't guess provided UTF-8 locale.
> 
> Add a Makefile knob GIT_TEST_UTF8_LOCALE and activate it for
> linux-musl in our CI system.
> 
> Rename t/lib-git-svn.sh:prepare_a_utf8_locale to prepare_utf8_locale,
> since we no longer prepare the variable named "a_utf8_locale",
> but set up a fallback value for GIT_TEST_UTF8_LOCALE instead.
> The fallback will be LC_ALL, LANG environment variable,
> or the first UTF-8 locale from output of "locale -a", in that order.

This patch looks good to me. I have one tiny nit below, but I'm OK if it's left as-is:

> +prepare_utf8_locale () {
> +	if test -n "$GIT_TEST_UTF8_LOCALE"
> +	then
> +		: test_set_prereq UTF8

This ":" is really just a comment, but since it contains code we would like to run (but which actually gets run later), it confused me. Something like:

  : already set by the caller, nothing to do

would have made it easier to understand.

And then the rest of the function is pretty self-explanatory.

-Peff