threads / patch / 44892

patch, 2 partsconfigure.ac: Fix --without-iconv

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

## tl;dr

3 messages between Jan 16, 2017 and Jan 16, 2017. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Bernd Kuhls· Jan 16, 2017, 19:56 UTC · lore

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

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

Show changes to configure.ac +2 −2

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

Bernd Kuhls· Jan 16, 2017, 19:56 UTC · re: Bernd Kuhls · lore

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

Show changes to configure.ac +3 −3

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

Jeff King· Jan 16, 2017, 21:41 UTC · re: Bernd Kuhls · lore

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

On Mon, Jan 16, 2017 at 08:56:37PM +0100, Bernd Kuhls wrote:

Show 9 quoted lines

> 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