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

On 2019-10-31 at 18:11:16, Jeff King wrote:

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

quoted

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).

Yeah, I think we need to do that, and not only for musl.  There are
folks for whom the iconv in libc is missing or inadequate and they use
an additional iconv(3) implementation which may not be the same as
iconv(1) (or vice versa).

Granted, as far as we know this option is only needed for musl, but that
doesn't mean there aren't other environments where this is a problem,
only that we don't yet know about them.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204