| Message ID | 20191031092618.29073-3-congdanhqx@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Linux with musl libc improvement | expand |
On Thu, Oct 31, 2019 at 04:26:17PM +0700, Doan Tran Cong Danh wrote: > 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). > 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
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: > > 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.
On 2019-10-31 14:11:16 -0400, Jeff King wrote: > 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). Originally, I wrote a C program for AC_RUN_IFELSE, but I found out the complicated of OLD_ICONV and NEEDS_LIBICONV, then I switch to this approach. But, your reasoning makes sense. I think people could build a static-linked git binary for a clean-room build system. I'll pursue the C program in the re-roll.
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 + ac_cv_iconv_omits_bom=yes +else + ac_cv_iconv_omits_bom=no +fi +]) +if test "x$ac_cv_iconv_omits_bom" = xyes; then + ICONV_OMITS_BOM=Yes +else + ICONV_OMITS_BOM= +fi +GIT_CONF_SUBST([ICONV_OMITS_BOM]) +fi # # Define NO_DEFLATE_BOUND if deflateBound is missing from zlib.
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. Signed-off-by: Doan Tran Cong Danh <congdanhqx@gmail.com> --- Notes: We deliberately fail for ac_cv_iconv_omits_bom on cross-compiling, in order to ask builder provide the value for the target. We're relied on this technik for ac_cv_fread_reads_directories and ac_cv_snprintf_returns_bogus. Adding one more failure for configuring on cross-compiling is not going to be a burden for distro. configure.ac | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)