# [PATCH 2/2] configure.ac: Fix --without-iconv 3 messages from 2017-01-16 to 2017-01-16. Participants: Bernd Kuhls, Jeff King. Thread: https://gitlist.dev/t/44892 ## Bernd Kuhls, 2017-01-16 19:56 Subject: [PATCH 2/2] configure.ac: Fix --without-iconv Message-ID: <20170116195638.3713-2-bernd.kuhls@writeme.com> URL: https://gitlist.dev/e/20170116195638.3713-2-bernd.kuhls%40writeme.com In-Reply-To: <20170116195638.3713-1-bernd.kuhls@writeme.com> ``` GIT_PARSE_WITH(iconv)) sets NO_ICONV=YesPlease in https://github.com/git/git/blob/maint/configure.ac#L327 But the command GIT_CONF_SUBST([NO_ICONV]) in https://github.com/git/git/blob/maint/configure.ac#L618 is only executed when NO_ICONV is an empty variable https://github.com/git/git/blob/maint/configure.ac#L578 which has the effect that NO_ICONV=YesPlease is not written to config.mak.autogen which breaks compilation in systems without iconv. Signed-off-by: Bernd Kuhls --- configure.ac | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/configure.ac b/configure.ac index 63e71a472..419469315 100644 --- a/configure.ac +++ b/configure.ac @@ -614,15 +614,15 @@ LIBS="$old_LIBS" GIT_UNSTASH_FLAGS($ICONVDIR) -GIT_CONF_SUBST([NEEDS_LIBICONV]) -GIT_CONF_SUBST([NO_ICONV]) - if test -n "$NO_ICONV"; then NEEDS_LIBICONV= fi fi +GIT_CONF_SUBST([NEEDS_LIBICONV]) +GIT_CONF_SUBST([NO_ICONV]) + # # Define NO_DEFLATE_BOUND if deflateBound is missing from zlib. -- 2.11.0 ``` ## Bernd Kuhls, 2017-01-16 19:56 Subject: [PATCH 1/2] configure.ac: fix old iconv check Message-ID: <20170116195638.3713-1-bernd.kuhls@writeme.com> URL: https://gitlist.dev/e/20170116195638.3713-1-bernd.kuhls%40writeme.com ``` According to https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Running-the-Compiler.html the parameter syntax of AC_COMPILE_IFELSE is (input, [action-if-true], [action-if-false]) Displaying "no" when the test was positive and enabling support for old iconv implementations by OLD_ICONV=UnfortunatelyYes when the test fails it obviously wrong. This patch switches the actions to fix the problem. Signed-off-by: Bernd Kuhls --- configure.ac | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 0b15f04b1..63e71a472 100644 --- a/configure.ac +++ b/configure.ac @@ -759,9 +759,9 @@ GIT_STASH_FLAGS($ICONVDIR) AC_MSG_CHECKING([for old iconv()]) AC_COMPILE_IFELSE([OLDICONVTEST_SRC], - [AC_MSG_RESULT([no])], [AC_MSG_RESULT([yes]) - OLD_ICONV=UnfortunatelyYes]) + OLD_ICONV=UnfortunatelyYes], + [AC_MSG_RESULT([no])]) GIT_UNSTASH_FLAGS($ICONVDIR) -- 2.11.0 ``` ## Jeff King, 2017-01-16 21:41 Subject: Re: [PATCH 1/2] configure.ac: fix old iconv check Message-ID: <20170116214111.v2imkedzyofcpt6b@sigill.intra.peff.net> URL: https://gitlist.dev/e/20170116214111.v2imkedzyofcpt6b%40sigill.intra.peff.net In-Reply-To: <20170116195638.3713-1-bernd.kuhls@writeme.com> ``` On Mon, Jan 16, 2017 at 08:56:37PM +0100, Bernd Kuhls wrote: > According to > https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Running-the-Compiler.html > the parameter syntax of AC_COMPILE_IFELSE is > > (input, [action-if-true], [action-if-false]) > > Displaying "no" when the test was positive and enabling support for old > iconv implementations by OLD_ICONV=UnfortunatelyYes when the test fails > it obviously wrong. This patch switches the actions to fix the problem. Hrm. But the test code is: # Define OLD_ICONV if your library has an old iconv(), where the second # (input buffer pointer) parameter is declared with type (const char **). AC_DEFUN([OLDICONVTEST_SRC], [ AC_LANG_PROGRAM([[ #include extern size_t iconv(iconv_t cd, char **inbuf, size_t *inbytesleft, char **outbuf, size_t *outbytesleft); ]], [])]) Which will compile correctly for the _new_ code, but not the old. So when the test is positive, then "no", we do not need to set the OLD_ICONV flag. The logic would probably be clearer if the test were reversed. It would mean that other compilation problems (e.g., you do not have iconv.h at all) would fail to see OLD_ICONV, but that seems reasonable. -Peff ```