[PATCH 1/2] configure.ac: fix old iconv check

[PATCH 1/2] configure.ac: fix old iconv check

[Bernd Kuhls]

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 <bernd.kuhls@writeme.com>
---
 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

[PATCH 2/2] configure.ac: Fix --without-iconv

[Bernd Kuhls]

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 <bernd.kuhls@writeme.com>
---
 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

Re: [PATCH 1/2] configure.ac: fix old iconv check

[Jeff King]

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 <iconv.h>
  
  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