[Libvir] [PATCH] fix compilation if readline lib is not available

The current CVS code doesn't compile on RHEL5/F6 because readline lib there require ncurses, and we removed that check earlier. Sor we find the readline headers but the lib is not found. Unfortunately virsh.c tests the availablility of the header to use readline, and link time failures follow. The patch below: - exports a READLINE_CFLAGS from configure if used - use the READLINE_CFLAGS when compiling virsh.c - change virsh.c to rely on this instead of the header test - adds a message about readline usage at the end of configure configure: Miscellaneous configure: configure: Debug: no configure: Readline: yes configure: Another patch available soon should fix the readline library detection itself, Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/

Daniel Veillard <veillard@redhat.com> wrote:
The current CVS code doesn't compile on RHEL5/F6 because readline lib there require ncurses, and we removed that check earlier. Sor we find the readline headers but the lib is not found. Unfortunately virsh.c tests the availablility of the header to use readline, and link time failures follow. The patch below: - exports a READLINE_CFLAGS from configure if used - use the READLINE_CFLAGS when compiling virsh.c - change virsh.c to rely on this instead of the header test - adds a message about readline usage at the end of configure
configure: Miscellaneous configure: configure: Debug: no configure: Readline: yes configure:
Another patch available soon should fix the readline library detection itself, ... +USE_READLINE= +READLINE_CFLAGS=
That looks fine, but would you please do s/READLINE_CFLAGS/VIRSH_CPPFLAGS/? Otherwise, we may end up with too many FEATURE_CPPFLAGS variables, rather than a more manageable number of PROGRAM_CPPFLAGS. As soon as you've committed that, I'll adjust my patch (which overlaps) and post the other fix.

Jim Meyering <jim@meyering.net> wrote:
Daniel Veillard <veillard@redhat.com> wrote:
The current CVS code doesn't compile on RHEL5/F6 because readline lib there require ncurses, and we removed that check earlier. Sor we find the readline headers but the lib is not found. Unfortunately virsh.c tests the availablility of the header to use readline, and link time failures follow. The patch below: - exports a READLINE_CFLAGS from configure if used - use the READLINE_CFLAGS when compiling virsh.c - change virsh.c to rely on this instead of the header test - adds a message about readline usage at the end of configure
configure: Miscellaneous configure: configure: Debug: no configure: Readline: yes configure:
Another patch available soon should fix the readline library detection itself, ... +USE_READLINE= +READLINE_CFLAGS=
That looks fine, but would you please do s/READLINE_CFLAGS/VIRSH_CPPFLAGS/?
Oops. Seeing the rest of your patch, you're probably right. Even using _CFLAGS is ok, for consistency with all of the other libvirt-specific _CFLAGS variable names. so +1 for your unmodified patch.

On Thu, Dec 06, 2007 at 11:22:12AM +0100, Jim Meyering wrote:
Jim Meyering <jim@meyering.net> wrote:
Daniel Veillard <veillard@redhat.com> wrote:
The current CVS code doesn't compile on RHEL5/F6 because readline lib there require ncurses, and we removed that check earlier. Sor we find the readline headers but the lib is not found. Unfortunately virsh.c tests the availablility of the header to use readline, and link time failures follow. The patch below: - exports a READLINE_CFLAGS from configure if used - use the READLINE_CFLAGS when compiling virsh.c - change virsh.c to rely on this instead of the header test - adds a message about readline usage at the end of configure
configure: Miscellaneous configure: configure: Debug: no configure: Readline: yes configure:
Another patch available soon should fix the readline library detection itself, ... +USE_READLINE= +READLINE_CFLAGS=
That looks fine, but would you please do s/READLINE_CFLAGS/VIRSH_CPPFLAGS/?
Oops. Seeing the rest of your patch, you're probably right. Even using _CFLAGS is ok, for consistency with all of the other libvirt-specific _CFLAGS variable names.
so +1 for your unmodified patch.
Okay commited now ! thanks, Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/

Daniel Veillard <veillard@redhat.com> wrote: ...
Okay commited now !
With the patch below, libvirt builds again on RHEL 5.1. It passes "make check", too. I tweaked a few shell variable names to have a lv_ prefix. Just in case... ---------------------- Build also on systems where -lreadline requires e.g., -lncurses * configure.in: If the test for -lreadline fails, search for a library with termcap support. If one is found (often -lncurses), rerun the test for -lreadline, linking also with the new library. Signed-off-by: Jim Meyering <meyering@redhat.com> --- configure.in | 48 ++++++++++++++++++++++++++++++++++++------------ 1 files changed, 36 insertions(+), 12 deletions(-) diff --git a/configure.in b/configure.in index 74028d8..35db64d 100644 --- a/configure.in +++ b/configure.in @@ -448,19 +448,43 @@ AC_SUBST(AVAHI_CFLAGS) AC_SUBST(AVAHI_LIBS) dnl virsh libraries -USE_READLINE= -READLINE_CFLAGS= AC_CHECK_HEADERS([readline/readline.h]) -AC_CHECK_LIB(readline, main, - [USE_READLINE=yes], - [USE_READLINE=no], - [$VIRSH_LIBS]) -if test "$USE_READLINE" = "yes" ; then - VIRSH_LIBS="$VIRSH_LIBS -lreadline" - AC_DEFINE_UNQUOTED(USE_READLINE, 1, [whether virsh use readline]) - READLINE_CFLAGS="-DUSE_READLINE" + +# Check for readline. +AC_CHECK_LIB(readline, readline, + [lv_use_readline=yes; VIRSH_LIBS="$VIRSH_LIBS -lreadline"], + [lv_use_readline=no]) + +# If the above test failed, it may simply be that -lreadline requires +# some termcap-related code, e.g., from one of the following libraries. +# See if adding one of them to LIBS helps. +if test $lv_use_readline = no; then + lv_saved_libs=$LIBS + LIBS= + AC_SEARCH_LIBS(tgetent, ncurses curses termcap termlib) + case $LIBS in + no*) ;; # handle "no" and "none required" + *) # anything else is a -lLIBRARY + # Now, check for -lreadline again, also using $LIBS. + # Note: this time we use a different function, so that + # we don't get a cached "no" result. + AC_CHECK_LIB(readline, rl_initialize, + [lv_use_readline=yes + VIRSH_LIBS="$VIRSH_LIBS -lreadline $LIBS"],, + [$LIBS]) + ;; + esac + test $lv_use_readline = no && + AC_MSG_WARN([readline library not found]) + LIBS=$lv_saved_libs +fi + +if test $lv_use_readline = yes; then + AC_DEFINE_UNQUOTED([USE_READLINE], 1, + [whether virsh can use readline]) + READLINE_CFLAGS=-DUSE_READLINE else - AC_MSG_WARN([readline library not found]) + READLINE_CFLAGS= fi AC_SUBST(READLINE_CFLAGS) AC_SUBST(VIRSH_LIBS) @@ -697,5 +721,5 @@ AC_MSG_NOTICE([]) AC_MSG_NOTICE([Miscellaneous]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([ Debug: $enable_debug]) -AC_MSG_NOTICE([ Readline: $USE_READLINE]) +AC_MSG_NOTICE([ Readline: $lv_use_readline]) AC_MSG_NOTICE([]) -- 1.5.3.7.1006.g8c6a6

+1. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903

On Thu, Dec 06, 2007 at 05:05:33AM -0500, Daniel Veillard wrote:
The current CVS code doesn't compile on RHEL5/F6 because readline lib there require ncurses, and we removed that check earlier. Sor we find the readline headers but the lib is not found. Unfortunately virsh.c tests the availablility of the header to use readline, and link time failures follow. The patch below: - exports a READLINE_CFLAGS from configure if used - use the READLINE_CFLAGS when compiling virsh.c - change virsh.c to rely on this instead of the header test - adds a message about readline usage at the end of configure
configure: Miscellaneous configure: configure: Debug: no configure: Readline: yes configure:
Oops actual good patch enclosed, Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/

Daniel Veillard wrote:
The current CVS code doesn't compile on RHEL5/F6 because readline lib there require ncurses, and we removed that check earlier. Sor we find the readline headers but the lib is not found. Unfortunately virsh.c tests the availablility of the header to use readline, and link time failures follow. The patch below: - exports a READLINE_CFLAGS from configure if used - use the READLINE_CFLAGS when compiling virsh.c - change virsh.c to rely on this instead of the header test - adds a message about readline usage at the end of configure
configure: Miscellaneous configure: configure: Debug: no configure: Readline: yes configure:
Another patch available soon should fix the readline library detection itself,
+1 Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
participants (3)
-
Daniel Veillard
-
Jim Meyering
-
Richard W.M. Jones