Re: [PATCH 2/3] configure.ac: define ICONV_OMITS_BOM if necessary

From: Jeff King <peff@peff.net>
Date: Oct 31, 2019, 18:11 UTC
Message-ID: <20191031181116.GC2133@sigill.intra.peff.net>
In-Reply-To: <20191031092618.29073-3-congdanhqx@gmail.com>

On Thu, Oct 31, 2019 at 04:26:17PM +0700, Doan Tran Cong Danh wrote:

Show 9 quoted lines

> From commit 79444c9294, ("utf8: handle systems that don't write BOM for
> UTF-16", 2019-02-12), we're supporting those systems with iconv that
> omits BOM with:
> 
>     make ICONV_OMITS_BOM=Yes
> 
> However, typing the flag all the time is cumbersome and error-prone.
> 
> Add a checking into configure script to detect this flag automatically.

I think it's worth adding this to the configure script, but note that you can also do:

  echo ICONV_OMITS_BOM=Yes >config.mak

and then "make" by itself will do what you want (it's still annoying and error-prone to realize you have to specify the flag in the first place).

Show 18 quoted lines

> diff --git a/configure.ac b/configure.ac
> index a43b476402..790b53bbdc 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -690,6 +690,28 @@ fi
>  
>  fi
>  
> +#
> +# Define ICONV_OMITS_BOM if you are on a system which
> +# iconv omits bom for utf-{16,32}
> +if test -z "$NO_ICONV"; then
> +AC_CACHE_CHECK([whether iconv omits bom for utf-16 and utf-32],
> + [ac_cv_iconv_omits_bom],
> +[
> +if test "x$cross_compiling" = xyes; then
> +	AC_MSG_FAILURE([please provide ac_cv_iconv_omits_bom])
> +elif test `printf a | iconv -f utf-8 -t utf-16 | wc -c` = 2; then

The ICONV_OMITS_BOM flag is about the libc iconv that Git will be linked against. But this is checking the iconv tool. For a system that is using musl across the board, that would work. But it might not always be the case (in particular, I don't know if people statically link some binaries against musl; certainly I've seen people do it with dietlibc).

I think we should be test-compiling a small program, similar to the way OLD_ICONV works (though I guess we may even need to run the result to see what happens).

-Peff