8:10 a.m.
This series drops a bunch of compatibility code that's no longer
necessary to support our target platforms and introduces support
for getting readline's CFLAGS and LIBS via pkg-config, which in
turn makes it possible to automatically enable the feature on
FreeBSD.
There are a couple of caveats:
* while most of the existing kludges are dropped, a new one had
to be introduced :( The upsides are that the new kludge is small
and straightforward, and that there's an expiration date on it
already, since a patch making it unnecessary has already been
provided for upstream readline;
* autodetection on macOS still doesn't work because of a
downstream packaging issue, but that's not any worse than the
current situation anyway.
Andrea Bolognani (6):
tools: vsh: Drop obsolete readline compatibility code
m4: readline: Extract code setting -D_FUNCTION_DEF
m4: readline: Stop looking for rl_completion_quote_character()
m4: readline: Drop extra_LIBS machinery
m4: readline: Use pkg-config
m4: readline: Add gross kludge for include path
m4/virt-readline.m4 | 68 ++++++++++++++++++++++-----------------------
tools/vsh.c | 17 ------------
2 files changed, 33 insertions(+), 52 deletions(-)
--
2.20.1
8:10 a.m.
New subject: [libvirt] [PATCH 1/6] tools: vsh: Drop obsolete readline compatibility code
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(a)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
8:10 a.m.
New subject: [libvirt] [PATCH 2/6] m4: readline: Extract code setting -D_FUNCTION_DEF
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(a)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 34b3ff3c4a..ffaeacef75 100644
--- a/m4/virt-readline.m4
+++ b/m4/virt-readline.m4
@@ -58,10 +58,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
8:10 a.m.
New subject: [libvirt] [PATCH 3/6] m4: readline: Stop looking for rl_completion_quote_character()
The function was introduced in readline 5.0, and all our target
platforms ship with at least 6.0 these days.
The check is also used as part of the extra_LIBS machinery, but
we're going to drop that too next anyway.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
m4/virt-readline.m4 | 13 -------------
1 file changed, 13 deletions(-)
diff --git a/m4/virt-readline.m4 b/m4/virt-readline.m4
index ffaeacef75..09d5d820ea 100644
--- a/m4/virt-readline.m4
+++ b/m4/virt-readline.m4
@@ -38,19 +38,6 @@ AC_DEFUN([LIBVIRT_CHECK_READLINE],[
LIBS="$lv_saved_libs $extra_LIBS"
fi
- 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
-
# The normal library check...
LIBVIRT_CHECK_LIB([READLINE], [readline], [readline], [readline/readline.h])
--
2.20.1
8:10 a.m.
New subject: [libvirt] [PATCH 4/6] m4: readline: Drop extra_LIBS machinery
The first implementation of this logic was introduced with
commit 2ec759fc58f 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(a)redhat.com>
---
m4/virt-readline.m4 | 23 -----------------------
1 file changed, 23 deletions(-)
diff --git a/m4/virt-readline.m4 b/m4/virt-readline.m4
index 09d5d820ea..a2788cb4aa 100644
--- a/m4/virt-readline.m4
+++ b/m4/virt-readline.m4
@@ -22,33 +22,10 @@ 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"
- *) # anything else is a -lLIBRARY
- extra_LIBS=$LIBS ;;
- esac
- LIBS="$lv_saved_libs $extra_LIBS"
- fi
# 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
8:10 a.m.
New subject: [libvirt] [PATCH 5/6] m4: readline: Use pkg-config
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.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
m4/virt-readline.m4 | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/m4/virt-readline.m4 b/m4/virt-readline.m4
index a2788cb4aa..6fde47d2c8 100644
--- a/m4/virt-readline.m4
+++ b/m4/virt-readline.m4
@@ -23,8 +23,17 @@ AC_DEFUN([LIBVIRT_ARG_READLINE],[
AC_DEFUN([LIBVIRT_CHECK_READLINE],[
- # The normal library check...
- LIBVIRT_CHECK_LIB([READLINE], [readline], [readline], [readline/readline.h])
+ # 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
+ # 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
8:10 a.m.
New subject: [libvirt] [PATCH 6/6] m4: readline: Add gross kludge for include path
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(a)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 6fde47d2c8..0e83b026e0 100644
--- a/m4/virt-readline.m4
+++ b/m4/virt-readline.m4
@@ -43,6 +43,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
8:29 a.m.
On Tue, 2019-04-09 at 15:10 +0200, Andrea Bolognani wrote:
[...]
* autodetection on macOS still doesn't work because of a
downstream packaging issue, but that's not any worse than the
current situation anyway.
Actually this breaks compilation on macOS, because now when the
pkg-config file is not present we'll pick up libedit from the base
system, which shadows readline, and fail because it doesn't provide
all the features we're expecting :(
I'll post a v2 that introduces no regressions in a bit.
--
Andrea Bolognani / Red Hat / Virtualization
2054
days inactive
2054
days old
7 comments
1 participants
participants (1)
-
Andrea Bolognani