On Thu, 2014-06-26 at 14:58 -0500, Dan Williams wrote:
> On Thu, 2014-06-26 at 12:44 -0500, Robby Workman wrote:
> > On Thu, 26 Jun 2014 12:27:14 -0500
> > Robby Workman <ro...@rlworkman.net> wrote:
> > 
> > > On Thu, 26 Jun 2014 10:16:41 -0500
> > > Dan Williams <d...@redhat.com> wrote:
> > > 
> > > > On Thu, 2014-06-26 at 01:19 -0500, Robby Workman wrote:
> > > > > On Thu, 26 Jun 2014 07:55:58 +0200
> > > > > Vincent Bernat <ber...@luffy.cx> wrote:
> > > > > 
> > > > > >  ❦ 25 juin 2014 21:36 -0500, Robby Workman
> > > > > > <ro...@rlworkman.net> :
> > > > > > 
> > > > > > > Okay, looks like that's ncurses, so let's link ncurses too:
> > > > > > >
> > > > > > >   [rworkman@liberty NetworkManager-0.9.9.98]$ gcc -o testrl
> > > > > > > -lreadline -lncurses testrl.c [rworkman@liberty
> > > > > > > NetworkManager-0.9.9.98]$ strings testrl | grep readline
> > > > > > > libreadline.so.6 readline
> > > > > > >   readline
> > > > > > >
> > > > > > > Now, here's where I'm unclear.  If I add LDFLAGS="-lnurses" to
> > > > > > > the configure environment, the test passes and the complete
> > > > > > > build occurs successfully. What's unclear is *why* that's
> > > > > > > needed -- is this an omission in the NM sources (isn't nmtui a
> > > > > > > curses client and thus ncurses should be linked?) or is
> > > > > > > something different about how we (Slackware) build readline?
> > > > > > 
> > > > > > GNU readline requires linking to ncurses as well to get termcap
> > > > > > symbols. Here is an autoconf recipe for that:
> > > > > >  http://www.gnu.org/software/autoconf-archive/ax_lib_readline.html
> > > > > 
> > > > > If I'm reading that correctly, that snippet above tries to link
> > > > > in curses if it encounters failure, so perhaps NM should use it?
> > > > > Maybe not though - there's at least one other project (nftables)
> > > > > where I've encountered the same problem.
> > > > > 
> > > > > While digging around a bit after sending this, I found that we add
> > > > > "--with-curses" to the readline configure to use it instead of 
> > > > > libtermcap, so it's *supposed* to be linked in already and this 
> > > > > would not be a problem.  However, for whatever reason (it's not
> > > > > clear whether it's intentional or not), the upstream sources don't
> > > > > use TERMCAP_LIBS (set to "-lcurses" per the configure flag) when
> > > > > building the shared library.  It appears that at least one distro
> > > > > solves that by running 'make SHLIB_LIBS="-lcurses"' when building,
> > > > > but I don't see where Fedora did that, so it's not clear at all 
> > > > > how they link curses (or even if they do, when means I wonder how
> > > > > the configure test in OP works there). 
> > > > > 
> > > > > Assuming this is indeed a problem, it's been in Slackware for
> > > > > years without causing any actual issues - the readline library
> > > > > from 13.37 (released in April 2011) doesn't link ncurses either,
> > > > > so I'm not convinced that we're doing anything wrong (but I'm
> > > > > open to argument).
> > > > 
> > > > What does 'ldd' on your libreadline.so return?  Mine has:
> > > > 
> > > >         linux-vdso.so.1 =>  (0x00007fff191fe000)
> > > >         libtinfo.so.5 => /lib64/libtinfo.so.5 (0x0000003473400000)
> > > >         libc.so.6 => /lib64/libc.so.6 (0x000000345ac00000)
> > > >         /lib64/ld-linux-x86-64.so.2 (0x000000345a800000)
> > > > 
> > > > so at least the dynamic linker knows that libtinfo (provided by
> > > > ncurses) is required here, and should load that in for the configure
> > > > test program.
> > > 
> > > 
> > > # ldd /usr/lib64/libreadline.so.6
> > >   linux-vdso.so.1 (0x00007ffff43ff000)
> > >   libc.so.6 => /lib64/libc.so.6 (0x00007ffda6b37000)
> > >   /lib64/ld-linux-x86-64.so.2 (0x00007ffda7179000)
> > > 
> > > I don't have any links to support this, but in talking with a couple
> > > of folks who've "been around a while," it seems that the lack of
> > > direct linking is intentional on the part of upstream readlinke --
> > > this allows the specific apps to use whichever (termcap / ncurses) is
> > > appropriate for them.  The autoconf stuff linked earlier seems to
> > > somewhat support that, but again, I don't have a definitive answer on
> > > it.
> > 
> > 
> > ...and comment #1 seems to support that (but apparently the official
> > RH position changed since then):
> > 
> > https://bugzilla.redhat.com/show_bug.cgi?id=162023
> 
> Fun.  nmcli doesn't care which one you link with, so NM would rather
> just use whatever you have lying around.  Unfortunately it seems we have
> to make the user specify one?

Robby, can you test the attached patch?  If it works for you we'll try
to get it into git master and 0.9.10.

Thanks!
Dan
>From ee41ed22e292a47fa88ff66c36e5a0ce5b15bef8 Mon Sep 17 00:00:00 2001
From: Dan Williams <d...@redhat.com>
Date: Thu, 26 Jun 2014 12:04:19 -0500
Subject: [PATCH] build: check harder for readline

Not all distros build their readline linked with a termcap library,
since apps are (apparently) supposed to choose one for themselves
and explicitly link to it when using readline.  So add some checks
to figure out whether readline is already linked, and if not, prefer
ncurses since we use that for nmtui already.

ax_lib_readline based off:

http://www.gnu.org/software/autoconf-archive/ax_lib_readline.html
---
 configure.ac          |   7 +---
 m4/ax_lib_readline.m4 | 106 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 107 insertions(+), 6 deletions(-)
 create mode 100644 m4/ax_lib_readline.m4

diff --git a/configure.ac b/configure.ac
index 8634203..98ebe3c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -386,21 +386,16 @@ AC_SUBST(LIBNL_CFLAGS)
 AC_SUBST(LIBNL_LIBS)
 
 # uuid library
 PKG_CHECK_MODULES(UUID, uuid)
 AC_SUBST(UUID_CFLAGS)
 AC_SUBST(UUID_LIBS)
 
-dnl
 dnl Checks for readline library - used by nmcli
-dnl
-#PKG_CHECK_MODULES(READLINE, readline)
-AC_CHECK_LIB(readline, readline, [READLINE_LIBS=-lreadline], [AC_MSG_ERROR(readline library is required)])
-AC_CHECK_HEADERS(readline/readline.h, [], [AC_MSG_ERROR(readline/readline.h - readline devel files required)])
-AC_SUBST(READLINE_LIBS)
+AX_LIB_READLINE
 
 # Intel WiMAX SDK checks
 PKG_CHECK_MODULES(IWMX_SDK, [libiWmxSdk-0 >= 1.5.1], [have_wimax=yes],[have_wimax=no])
 AC_ARG_ENABLE(wimax, AS_HELP_STRING([--enable-wimax], [enable WiMAX support]),
                      [enable_wimax=${enableval}], [enable_wimax=${have_wimax}])
 if (test "${enable_wimax}" = "yes"); then
 	if test x"$have_wimax" = x"no"; then
diff --git a/m4/ax_lib_readline.m4 b/m4/ax_lib_readline.m4
new file mode 100644
index 0000000..f64eb75
--- /dev/null
+++ b/m4/ax_lib_readline.m4
@@ -0,0 +1,106 @@
+# ===========================================================================
+#      http://www.gnu.org/software/autoconf-archive/ax_lib_readline.html
+# ===========================================================================
+#
+# SYNOPSIS
+#
+#   AX_LIB_READLINE
+#
+# DESCRIPTION
+#
+#   Searches for a readline compatible library. If found, defines
+#   `HAVE_LIBREADLINE'. If the found library has the `add_history' function,
+#   sets also `HAVE_READLINE_HISTORY'. Also checks for the locations of the
+#   necessary include files and sets `HAVE_READLINE_H' or
+#   `HAVE_READLINE_READLINE_H' and `HAVE_READLINE_HISTORY_H' or
+#   'HAVE_HISTORY_H' if the corresponding include files exists.
+#
+#   The libraries that may be readline compatible are `libedit',
+#   `libeditline' and `libreadline'. Sometimes we need to link a termcap
+#   library for readline to work, this macro tests these cases too by trying
+#   to link with `libtermcap', `libcurses' or `libncurses' before giving up.
+#
+#   Here is an example of how to use the information provided by this macro
+#   to perform the necessary includes or declarations in a C file:
+#
+#     #ifdef HAVE_LIBREADLINE
+#     #  if defined(HAVE_READLINE_READLINE_H)
+#     #    include <readline/readline.h>
+#     #  elif defined(HAVE_READLINE_H)
+#     #    include <readline.h>
+#     #  else /* !defined(HAVE_READLINE_H) */
+#     extern char *readline ();
+#     #  endif /* !defined(HAVE_READLINE_H) */
+#     char *cmdline = NULL;
+#     #else /* !defined(HAVE_READLINE_READLINE_H) */
+#       /* no readline */
+#     #endif /* HAVE_LIBREADLINE */
+#
+#     #ifdef HAVE_READLINE_HISTORY
+#     #  if defined(HAVE_READLINE_HISTORY_H)
+#     #    include <readline/history.h>
+#     #  elif defined(HAVE_HISTORY_H)
+#     #    include <history.h>
+#     #  else /* !defined(HAVE_HISTORY_H) */
+#     extern void add_history ();
+#     extern int write_history ();
+#     extern int read_history ();
+#     #  endif /* defined(HAVE_READLINE_HISTORY_H) */
+#       /* no history */
+#     #endif /* HAVE_READLINE_HISTORY */
+#
+# LICENSE
+#
+#   Copyright (c) 2008 Ville Laurikari <v...@iki.fi>
+#
+#   Copying and distribution of this file, with or without modification, are
+#   permitted in any medium without royalty provided the copyright notice
+#   and this notice are preserved. This file is offered as-is, without any
+#   warranty.
+
+#serial 6
+
+AU_ALIAS([VL_LIB_READLINE], [AX_LIB_READLINE])
+AC_DEFUN([AX_LIB_READLINE], [
+  AC_CACHE_CHECK([for a readline compatible library],
+                 ax_cv_lib_readline, [
+    ORIG_LIBS="$LIBS"
+    for readline_lib in readline edit editline; do
+      # prefer ncurses since we use it for nmtui too
+      for termcap_lib in "" ncurses curses termcap; do
+        if test -z "$termcap_lib"; then
+          TRY_LIB="-l$readline_lib"
+        else
+          TRY_LIB="-l$readline_lib -l$termcap_lib"
+        fi
+        LIBS="$ORIG_LIBS $TRY_LIB"
+        AC_TRY_LINK_FUNC(readline, ax_cv_lib_readline="$TRY_LIB")
+        if test -n "$ax_cv_lib_readline"; then
+          break
+        fi
+      done
+      if test -n "$ax_cv_lib_readline"; then
+        break
+      fi
+    done
+    LIBS="$ORIG_LIBS"
+  ])
+
+  if test -z "$ax_cv_lib_readline"; then
+    AC_MSG_ERROR(readline library is required)
+  fi
+
+  LIBS="$LIBS $ax_cv_lib_readline"
+  AC_CHECK_HEADERS(readline.h readline/readline.h)
+  AC_CACHE_CHECK([whether readline supports history],
+                 ax_cv_lib_readline_history, [
+    ax_cv_lib_readline_history="no"
+    AC_TRY_LINK_FUNC(add_history, ax_cv_lib_readline_history="yes")
+  ])
+  if test "$ax_cv_lib_readline_history" != "yes"; then
+    AC_MSG_ERROR(readline history support is required)
+  fi
+  AC_CHECK_HEADERS(history.h readline/history.h)
+  READLINE_LIBS="$ax_cv_lib_readline"
+  AC_SUBST(READLINE_LIBS)
+])dnl
-- 
1.9.3

_______________________________________________
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list

Reply via email to