[RFA/commit] prefer in-tree libiconv over the system libiconv

[RFA/commit] prefer in-tree libiconv over the system libiconv

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 1024 bytes --]

[switching to gdb-patches]

The more I kept thinking about it, the more it seemed to make sense
to me that we should use in-tree libiconv if present. Right now,
we automatically build it, but we cannot use the current configure
switch (--with-libiconv-prefix) to force GDB to use the libiconv
we just built. That means that an extra switch would be necessary,
not just convenient.

This patch implements this change. I've only tested it on GNU/Linux
at the moment, with and without in-tree libiconv. I get the expected
result both times. I'm checked this patch in AdaCore tree, and I should
get build results tomorrow.

2009-04-21  Joel Brobecker  <brobecker@adacore.com>

        * acinclude.m4 (AM_ICONV): Prefer in-tree libiconv if present over
        system iconv.
        * configure: Regenerate.

The diff seems awful, but all I did was move the block for ../libiconv
up before we check for iconv in the libc. And I made us check iconv
in libc only if we haven't found iconv in ../libiconv.

Any objection?

-- 
Joel

[-- Attachment #2: in-tree-iconv.diff --]
[-- Type: text/x-diff, Size: 3098 bytes --]

commit b29fa82cb5acbb4423a4515f694b22beb7622aa2
Author: Joel Brobecker <brobecker@adacore.com>
Date:   Tue Apr 21 13:32:27 2009 -0700

        * acinclude.m4 (AM_ICONV): Prefer in-tree libiconv if present over
        system iconv.
        * configure: Regenerate.

diff --git a/gdb/acinclude.m4 b/gdb/acinclude.m4
index 5e77230..320b7dd 100644
--- a/gdb/acinclude.m4
+++ b/gdb/acinclude.m4
@@ -189,47 +189,51 @@ AC_DEFUN([AM_ICONV],
     am_cv_func_iconv="no, consider installing GNU libiconv"
     am_cv_lib_iconv=no
     am_cv_use_build_libiconv=no
-    # First, try to find iconv in libc.
-    AC_TRY_LINK([#include <stdlib.h>
-#include <iconv.h>],
-      [iconv_t cd = iconv_open("","");
-       iconv(cd,NULL,NULL,NULL,NULL);
-       iconv_close(cd);],
-      am_cv_func_iconv=yes)
 
-    # If iconv was not in libc, try -liconv.  In this case, arrange to
-    # look in the libiconv prefix, if it was specified by the user.
-    if test "$am_cv_func_iconv" != yes; then
-      am_save_CPPFLAGS="$CPPFLAGS"
+    # If libiconv is part of the build tree, then try using it over
+    # any system iconv.
+    if test -d ../libiconv; then
       am_save_LIBS="$LIBS"
-      if test -n "$LIBICONV_INCLUDE"; then
-        CPPFLAGS="$CPPFLAGS $LIBICONV_INCLUDE"
-        LIBS="$LIBS $LIBICONV_LIBDIR"
-      fi
-      LIBS="$LIBS -liconv"
+      am_save_CPPFLAGS="$CPPFLAGS"
+      LIBS="$LIBS $BUILD_LIBICONV_LIBDIR -liconv"
+      CPPFLAGS="$CPPFLAGS $BUILD_LIBICONV_INCLUDE"
       AC_TRY_LINK([#include <stdlib.h>
 #include <iconv.h>],
         [iconv_t cd = iconv_open("","");
          iconv(cd,NULL,NULL,NULL,NULL);
          iconv_close(cd);],
+	am_cv_use_build_libiconv=yes
         am_cv_lib_iconv=yes
         am_cv_func_iconv=yes)
       LIBS="$am_save_LIBS"
       CPPFLAGS="$am_save_CPPFLAGS"
     fi
 
-    # If that didn't work, try to find libiconv in the build tree.
-    if test "$am_cv_func_iconv" != yes && test -d ../libiconv; then
-      am_save_LIBS="$LIBS"
+    # Next, try to find iconv in libc.
+    if test "$am_cv_func_iconv" != yes; then
+      AC_TRY_LINK([#include <stdlib.h>
+#include <iconv.h>],
+        [iconv_t cd = iconv_open("","");
+         iconv(cd,NULL,NULL,NULL,NULL);
+         iconv_close(cd);],
+        am_cv_func_iconv=yes)
+    fi
+
+    # If iconv was not in libc, try -liconv.  In this case, arrange to
+    # look in the libiconv prefix, if it was specified by the user.
+    if test "$am_cv_func_iconv" != yes; then
       am_save_CPPFLAGS="$CPPFLAGS"
-      LIBS="$LIBS $BUILD_LIBICONV_LIBDIR -liconv"
-      CPPFLAGS="$CPPFLAGS $BUILD_LIBICONV_INCLUDE"
+      am_save_LIBS="$LIBS"
+      if test -n "$LIBICONV_INCLUDE"; then
+        CPPFLAGS="$CPPFLAGS $LIBICONV_INCLUDE"
+        LIBS="$LIBS $LIBICONV_LIBDIR"
+      fi
+      LIBS="$LIBS -liconv"
       AC_TRY_LINK([#include <stdlib.h>
 #include <iconv.h>],
         [iconv_t cd = iconv_open("","");
          iconv(cd,NULL,NULL,NULL,NULL);
          iconv_close(cd);],
-	am_cv_use_build_libiconv=yes
         am_cv_lib_iconv=yes
         am_cv_func_iconv=yes)
       LIBS="$am_save_LIBS"

Re: [RFA/commit] prefer in-tree libiconv over the system libiconv

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> The diff seems awful, but all I did was move the block for ../libiconv
Joel> up before we check for iconv in the libc. And I made us check iconv
Joel> in libc only if we haven't found iconv in ../libiconv.

The code change looks fine FWIW.

Tom

Re: [RFA/commit] prefer in-tree libiconv over the system libiconv

[Joel Brobecker]

> Joel> The diff seems awful, but all I did was move the block for ../libiconv
> Joel> up before we check for iconv in the libc. And I made us check iconv
> Joel> in libc only if we haven't found iconv in ../libiconv.
> 
> The code change looks fine FWIW.

Thanks! I'll let this patch sit for a week, and then check it in,
if there aren't any objection.

I'm starting to use this method for my own builds, and I have to say
that it is really convenient. Dropped the libiconv sources in my source
tree once, and never have to remember how to configure GDB, or where
the lib[...] install is. I'd like to do the same for libexpat. Thoughts?

-- 
Joel

Re: [RFA/commit] prefer in-tree libiconv over the system libiconv

[Tom Tromey]

Joel> I'd like to do the same for libexpat. Thoughts?

Sounds reasonable to me.  Do it.

Tom

Re: [RFA/commit] prefer in-tree libiconv over the system libiconv

[Joel Brobecker]

> 2009-04-21  Joel Brobecker  <brobecker@adacore.com>
> 
>         * acinclude.m4 (AM_ICONV): Prefer in-tree libiconv if present over
>         system iconv.
>         * configure: Regenerate.

FYI: Checked in.

-- 
Joel