This code is needed to use readline older than 4.1, but all our target platforms ship with at least 6.0 these days so we can safely get rid of it. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- tools/vsh.c | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/tools/vsh.c b/tools/vsh.c index 65b96f87d5..f2486498c9 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -2945,32 +2945,15 @@ vshReadlineInit(vshControl *ctl) /* Opaque data for autocomplete callbacks. */ autoCompleteOpaque = ctl; - /* Allow conditional parsing of the ~/.inputrc file. - * Work around ancient readline 4.1 (hello Mac OS X), - * which declared it as 'char *' instead of 'const char *'. - */ -# if defined(RL_READLINE_VERSION) && RL_READLINE_VERSION > 0x0402 rl_readline_name = ctl->name; -# else - rl_readline_name = (char *) ctl->name; -# endif /* Tell the completer that we want a crack first. */ rl_attempted_completion_function = vshReadlineCompletion; -# if defined(RL_READLINE_VERSION) && RL_READLINE_VERSION > 0x0402 rl_basic_word_break_characters = break_characters; -# else - rl_basic_word_break_characters = (char *) break_characters; -# endif -# if defined(RL_READLINE_VERSION) && RL_READLINE_VERSION > 0x0402 rl_completer_quote_characters = quote_characters; rl_char_is_quoted_p = vshReadlineCharIsQuoted; -# else - rl_completer_quote_characters = (char *) quote_characters; - rl_char_is_quoted_p = (Function *) vshReadlineCharIsQuoted; -# endif if (virAsprintf(&histsize_env, "%s_HISTSIZE", ctl->env_prefix) < 0) goto cleanup; -- 2.20.1

Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- m4/virt-readline.m4 | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/m4/virt-readline.m4 b/m4/virt-readline.m4 index 34b3ff3c4a..4823a27788 100644 --- a/m4/virt-readline.m4 +++ b/m4/virt-readline.m4 @@ -32,8 +32,7 @@ AC_DEFUN([LIBVIRT_CHECK_READLINE],[ AC_SEARCH_LIBS([tgetent], [ncurses curses termcap termlib]) case $LIBS in no*) ;; # handle "no" and "none required" - *) # anything else is a -lLIBRARY - extra_LIBS=$LIBS ;; + *) extra_LIBS=$LIBS ;; # anything else is a -lLIBRARY esac LIBS="$lv_saved_libs $extra_LIBS" fi @@ -41,7 +40,7 @@ AC_DEFUN([LIBVIRT_CHECK_READLINE],[ AC_CHECK_DECLS([rl_completion_quote_character], [], [], [[#include <stdio.h> - #include <readline/readline.h>]]) + #include <readline/readline.h>]]) if test "$ac_cv_have_decl_rl_completion_quote_character" = "no" ; then if test "$with_readline" = "yes" ; then @@ -58,9 +57,9 @@ AC_DEFUN([LIBVIRT_CHECK_READLINE],[ # function, to ensure we aren't being confused by caching. LIBS=$lv_saved_libs AC_CHECK_LIB([readline], [rl_initialize], - [READLINE_CFLAGS="-D_FUNCTION_DEF $READLINE_CFLAGS" - AC_SUBST(READLINE_CFLAGS)], - [READLINE_LIBS="$READLINE_LIBS $extra_LIBS"]) + [READLINE_CFLAGS="-D_FUNCTION_DEF $READLINE_CFLAGS" + AC_SUBST(READLINE_CFLAGS)], + [READLINE_LIBS="$READLINE_LIBS $extra_LIBS"]) LIBS=$lv_saved_libs ]) -- 2.20.1

The check was added in 74416b1d4849 without offering any explanation outside of the commit message. Introduce a comment to make digging through the git history unnecessary. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- m4/virt-readline.m4 | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/m4/virt-readline.m4 b/m4/virt-readline.m4 index 4823a27788..649a52edfa 100644 --- a/m4/virt-readline.m4 +++ b/m4/virt-readline.m4 @@ -37,6 +37,12 @@ AC_DEFUN([LIBVIRT_CHECK_READLINE],[ LIBS="$lv_saved_libs $extra_LIBS" fi + # This function is present in all reasonable (5.0+) readline versions; + # however, the macOS base system contains a library called libedit which + # takes over the readline name despite lacking many of its features. We + # want to make sure we only enable readline support when linking against + # the actual readline library, and the availability of this specific + # functions is as good a witness for that fact as any. AC_CHECK_DECLS([rl_completion_quote_character], [], [], [[#include <stdio.h> -- 2.20.1

The current code is a bit awkward, and we're going to need to share it later anyway. We can drop the call to AC_SUBST() while we're at it, since LIBVIRT_CHECK_LIB() already marks READLINE_CFLAGS for substitution. The new code goes to some extra length to avoid setting -D_FUNCTION_DEF twice: this is mostly for cosmetic reasons, and it's necessary because LIBVIRT_CHECK_READLINE() is called twice: once on its own, and then once more as part of LIBVIRT_CHECK_BASH_COMPLETION(). Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- m4/virt-readline.m4 | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/m4/virt-readline.m4 b/m4/virt-readline.m4 index 649a52edfa..998b891d53 100644 --- a/m4/virt-readline.m4 +++ b/m4/virt-readline.m4 @@ -63,10 +63,18 @@ AC_DEFUN([LIBVIRT_CHECK_READLINE],[ # function, to ensure we aren't being confused by caching. LIBS=$lv_saved_libs AC_CHECK_LIB([readline], [rl_initialize], - [READLINE_CFLAGS="-D_FUNCTION_DEF $READLINE_CFLAGS" - AC_SUBST(READLINE_CFLAGS)], + [], [READLINE_LIBS="$READLINE_LIBS $extra_LIBS"]) LIBS=$lv_saved_libs + + # We need this to avoid compilation issues with modern compilers. + # See 9ea3424a178 for a more detailed explanation + if test "$with_readline" = "yes" ; then + case "$READLINE_CFLAGS" in + *-D_FUNCTION_DEF*) ;; + *) READLINE_CFLAGS="-D_FUNCTION_DEF $READLINE_CFLAGS" ;; + esac + fi ]) AC_DEFUN([LIBVIRT_RESULT_READLINE],[ -- 2.20.1

The first implementation of this logic was introduced with commit 2ec759fc58fe all the way back in 2007; looking at the build logs from our CI environment, however, it's apparent that none of the platforms we currently target are actually using it, so we can assume whatever issue it was working around has been fixed at some point in the last 12 years. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- m4/virt-readline.m4 | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/m4/virt-readline.m4 b/m4/virt-readline.m4 index 998b891d53..6f0090056a 100644 --- a/m4/virt-readline.m4 +++ b/m4/virt-readline.m4 @@ -22,20 +22,6 @@ AC_DEFUN([LIBVIRT_ARG_READLINE],[ ]) AC_DEFUN([LIBVIRT_CHECK_READLINE],[ - extra_LIBS= - lv_saved_libs=$LIBS - if test "x$with_readline" != xno; then - # Linking with -lreadline may require some termcap-related code, e.g., - # from one of the following libraries. Add it to LIBS before using - # canned library checks; then verify later if it was needed. - LIBS= - AC_SEARCH_LIBS([tgetent], [ncurses curses termcap termlib]) - case $LIBS in - no*) ;; # handle "no" and "none required" - *) extra_LIBS=$LIBS ;; # anything else is a -lLIBRARY - esac - LIBS="$lv_saved_libs $extra_LIBS" - fi # This function is present in all reasonable (5.0+) readline versions; # however, the macOS base system contains a library called libedit which @@ -59,14 +45,6 @@ AC_DEFUN([LIBVIRT_CHECK_READLINE],[ # The normal library check... LIBVIRT_CHECK_LIB([READLINE], [readline], [readline], [readline/readline.h]) - # Touch things up to avoid $extra_LIBS, if possible. Test a second - # function, to ensure we aren't being confused by caching. - LIBS=$lv_saved_libs - AC_CHECK_LIB([readline], [rl_initialize], - [], - [READLINE_LIBS="$READLINE_LIBS $extra_LIBS"]) - LIBS=$lv_saved_libs - # We need this to avoid compilation issues with modern compilers. # See 9ea3424a178 for a more detailed explanation if test "$with_readline" = "yes" ; then -- 2.20.1

With the 7.0 release, readline has finally started shipping pkg-config support in the form of a readline.pc file. Unfortunately, most downstreams have yet to catch up with this change: among Linux distributions in particular, Fedora Rawhide seems to be the only one installing it at the moment. Non-Linux operating systems have been faring much better in this regard: both FreeBSD (through ports) and macOS (through homebrew) include pkg-config support in their readline package. This is great news for us, since those are the platforms where pkg-config is more useful on account of them installing headers and libraries outside of the respective default search paths. Our implementation checks whether readline is registered as a pkg-config package, and if so obtains CFLAGS and LIBS using the tool; if not, we just keep using the existing logic. This commit is best viewed with 'git show -w'. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- m4/virt-readline.m4 | 47 +++++++++++++++++++++++++++------------------ 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/m4/virt-readline.m4 b/m4/virt-readline.m4 index 6f0090056a..1bec5deb22 100644 --- a/m4/virt-readline.m4 +++ b/m4/virt-readline.m4 @@ -23,27 +23,36 @@ AC_DEFUN([LIBVIRT_ARG_READLINE],[ AC_DEFUN([LIBVIRT_CHECK_READLINE],[ - # This function is present in all reasonable (5.0+) readline versions; - # however, the macOS base system contains a library called libedit which - # takes over the readline name despite lacking many of its features. We - # want to make sure we only enable readline support when linking against - # the actual readline library, and the availability of this specific - # functions is as good a witness for that fact as any. - AC_CHECK_DECLS([rl_completion_quote_character], - [], [], - [[#include <stdio.h> - #include <readline/readline.h>]]) - - if test "$ac_cv_have_decl_rl_completion_quote_character" = "no" ; then - if test "$with_readline" = "yes" ; then - AC_MSG_ERROR([readline is missing rl_completion_quote_character]) - else - with_readline=no; + # We have to check for readline.pc's presence beforehand because for + # the longest time the library didn't ship a .pc file at all + PKG_CHECK_EXISTS([readline], [use_pkgconfig=1], [use_pkgconfig=0]) + + if test $use_pkgconfig = 1; then + # readline 7.0 is the first version which includes pkg-config support + LIBVIRT_CHECK_PKG([READLINE], [readline], [7.0]) + else + # This function is present in all reasonable (5.0+) readline versions; + # however, the macOS base system contains a library called libedit which + # takes over the readline name despite lacking many of its features. We + # want to make sure we only enable readline support when linking against + # the actual readline library, and the availability of this specific + # functions is as good a witness for that fact as any. + AC_CHECK_DECLS([rl_completion_quote_character], + [], [], + [[#include <stdio.h> + #include <readline/readline.h>]]) + + if test "$ac_cv_have_decl_rl_completion_quote_character" = "no" ; then + if test "$with_readline" = "yes" ; then + AC_MSG_ERROR([readline is missing rl_completion_quote_character]) + else + with_readline=no; + fi fi - fi - # The normal library check... - LIBVIRT_CHECK_LIB([READLINE], [readline], [readline], [readline/readline.h]) + # The normal library check... + LIBVIRT_CHECK_LIB([READLINE], [readline], [readline], [readline/readline.h]) + fi # We need this to avoid compilation issues with modern compilers. # See 9ea3424a178 for a more detailed explanation -- 2.20.1

Unfortunately the data reported by pkg-config is not completely accurate, so until the issue has been fixed in readline we need to work around it in libvirt. The good news is that we only need the fix to land in FreeBSD ports and macOS homebrew before we can drop the kludge, so we're talking months rather than years. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- m4/virt-readline.m4 | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/m4/virt-readline.m4 b/m4/virt-readline.m4 index 1bec5deb22..cd12110c4e 100644 --- a/m4/virt-readline.m4 +++ b/m4/virt-readline.m4 @@ -62,6 +62,23 @@ AC_DEFUN([LIBVIRT_CHECK_READLINE],[ *) READLINE_CFLAGS="-D_FUNCTION_DEF $READLINE_CFLAGS" ;; esac fi + + # Gross kludge for readline include path obtained through pkg-config. + # + # As of 8.0, upstream readline.pc has -I${includedir}/readline among + # its Cflags, which is clearly wrong. This does not affect Linux + # because ${includedir} is already part of the default include path, + # but on other platforms that's not the case and the result is that + # <readline/readline.h> can't be located, causing the build to fail. + # A patch solving this issue has already been posted upstream, so once + # the fix has landed in FreeBSD ports and macOS homebrew we can safely + # drop the kludge and rely on pkg-config alone on those platforms. + # + # [1] http://lists.gnu.org/archive/html/bug-readline/2019-04/msg00007.html + case "$READLINE_CFLAGS" in + *include/readline*) READLINE_CFLAGS=$(echo $READLINE_CFLAGS | sed s,include/readline,include,g) ;; + *) ;; + esac ]) AC_DEFUN([LIBVIRT_RESULT_READLINE],[ -- 2.20.1