[8/8] Require readline 7 or newer

Message ID 87o910ehgf.fsf@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey Aug. 7, 2019, 10:30 p.m. UTC
  >>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> I'd be much better user experience if this were done at by the
Pedro> build system, at configure time, with AC_TRY_COMPILE, IMO.  Something
Pedro> similar to the "GNU regex" check should do it.

Makes sense.  Here's a new patch that addresses this and the NEWS thing.

Tom

commit 332eb34e3d21c1a3de2b4f6c874912321da1b3a4
Author: Tom Tromey <tom@tromey.com>
Date:   Sun Apr 21 13:58:49 2019 -0600

    Require readline 7 or newer
    
    This changes gdb to require readline 7 or newer at build time.
    
    gdb/ChangeLog
    2019-08-07  Tom Tromey  <tom@tromey.com>
    
            * configure: Rebuild.
            * configure.ac: Check for readline 7.
            * NEWS: Mention readline 7 requirement.
            * README: Update.
    
    gdb/doc/ChangeLog
    2019-04-21  Tom Tromey  <tom@tromey.com>
    
            * gdb.texinfo (Configure Options): Document minimum version of
            readline.
  

Comments

Eli Zaretskii Aug. 8, 2019, 2:37 a.m. UTC | #1
> From: Tom Tromey <tom@tromey.com>
> Cc: Tom Tromey <tom@tromey.com>,  gdb-patches@sourceware.org
> Date: Wed, 07 Aug 2019 16:30:56 -0600
> 
> >>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> I'd be much better user experience if this were done at by the
> Pedro> build system, at configure time, with AC_TRY_COMPILE, IMO.  Something
> Pedro> similar to the "GNU regex" check should do it.
> 
> Makes sense.  Here's a new patch that addresses this and the NEWS thing.

OK for the documentation parts.

Thanks.
  
Pedro Alves Aug. 8, 2019, 11:25 a.m. UTC | #2
On 8/7/19 11:30 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> I'd be much better user experience if this were done at by the
> Pedro> build system, at configure time, with AC_TRY_COMPILE, IMO.  Something
> Pedro> similar to the "GNU regex" check should do it.
> 
> Makes sense.  Here's a new patch that addresses this and the NEWS thing.

Looks good, thanks.

Pedro Alves
  
Pedro Alves Aug. 8, 2019, 11:29 a.m. UTC | #3
On 8/7/19 11:30 PM, Tom Tromey wrote:
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -299,6 +299,11 @@ maint show test-options-completion-result
>    Using another implementation of the make program or an earlier version of
>    GNU make to build GDB or GDBserver is not supported.
>  
> +* Building GDB and GDBserver now requires GNU readline >= 7.0.

Actually, the "and GDBserver" part is incorrect, since GDBserver
does not depend on readline.  Better remove that IMO, lest someone
mistakenly thinks that building gdbserver (without building gdb)
now requires readline.

Thanks,
Pedro Alves
  
Tom Tromey Aug. 8, 2019, 8:38 p.m. UTC | #4
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> On 8/7/19 11:30 PM, Tom Tromey wrote:
>> --- a/gdb/NEWS
>> +++ b/gdb/NEWS
>> @@ -299,6 +299,11 @@ maint show test-options-completion-result
>> Using another implementation of the make program or an earlier version of
>> GNU make to build GDB or GDBserver is not supported.
>> 
>> +* Building GDB and GDBserver now requires GNU readline >= 7.0.

Pedro> Actually, the "and GDBserver" part is incorrect, since GDBserver
Pedro> does not depend on readline.  Better remove that IMO, lest someone
Pedro> mistakenly thinks that building gdbserver (without building gdb)
Pedro> now requires readline.

Oops, bad cut-and-paste. I fixed this.

Tom
  

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 2e05431607c..54d35df7a01 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@ 
+2019-08-07  Tom Tromey  <tom@tromey.com>
+
+	* configure: Rebuild.
+	* configure.ac: Check for readline 7.
+	* NEWS: Mention readline 7 requirement.
+	* README: Update.
+
 2019-08-04  Tom Tromey  <tom@tromey.com>
 
 	* mingw-hdep.c (gdb_select): Remove readline hack.
diff --git a/gdb/NEWS b/gdb/NEWS
index fa01adf6e89..9f37e7c1079 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -299,6 +299,11 @@  maint show test-options-completion-result
   Using another implementation of the make program or an earlier version of
   GNU make to build GDB or GDBserver is not supported.
 
+* Building GDB and GDBserver now requires GNU readline >= 7.0.
+
+  GDB now bundles GNU readline 8.0, but if you choose to use
+  --with-system-readline, only readline >= 7.0 can be used.
+
 *** Changes in GDB 8.3
 
 * GDB and GDBserver now support access to additional registers on
diff --git a/gdb/README b/gdb/README
index 8a91aab2a4c..8883a8a09e3 100644
--- a/gdb/README
+++ b/gdb/README
@@ -439,7 +439,8 @@  more obscure GDB `configure' options are not listed here.
 
 `--with-system-readline'
      Use the readline library installed on the host, rather than the
-     library supplied as part of GDB.
+     library supplied as part of GDB.  Readline 7 or newer is required;
+     this is enforced by the build system.
 
 `--with-system-zlib
      Use the zlib library installed on the host, rather than the
diff --git a/gdb/configure b/gdb/configure
index 9206f0e7f88..2832c836177 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -8952,6 +8952,38 @@  fi
 
 
 if test "$with_system_readline" = yes; then
+   { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether system readline is new enough" >&5
+$as_echo_n "checking whether system readline is new enough... " >&6; }
+if ${gdb_cv_readline_ok+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+#include <stdio.h>
+#include <readline/readline.h>
+int
+main ()
+{
+#if RL_VERSION_MAJOR < 7
+# error "readline version 7 required"
+#endif
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+  gdb_cv_readline_ok=yes
+else
+  gdb_cv_readline_ok=no
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $gdb_cv_readline_ok" >&5
+$as_echo "$gdb_cv_readline_ok" >&6; }
+  if test "$gdb_cv_readline_ok" != yes; then
+    as_fn_error $? "system readline is not new enough" "$LINENO" 5
+  fi
+
   READLINE=-lreadline
   READLINE_DEPS=
   READLINE_CFLAGS=
diff --git a/gdb/configure.ac b/gdb/configure.ac
index 05b722b7f11..0979109d976 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -581,6 +581,20 @@  AC_ARG_WITH([system-readline],
                   [use installed readline library])])
 
 if test "$with_system_readline" = yes; then
+   AC_CACHE_CHECK([whether system readline is new enough],
+     [gdb_cv_readline_ok],
+     [AC_TRY_COMPILE(
+       [#include <stdio.h>
+#include <readline/readline.h>],
+       [#if RL_VERSION_MAJOR < 7
+# error "readline version 7 required"
+#endif],
+    gdb_cv_readline_ok=yes,
+    gdb_cv_readline_ok=no)])
+  if test "$gdb_cv_readline_ok" != yes; then
+    AC_MSG_ERROR([system readline is not new enough])
+  fi
+
   READLINE=-lreadline
   READLINE_DEPS=
   READLINE_CFLAGS=
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 702bb7c7a02..f25b468468b 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,8 @@ 
+2019-04-21  Tom Tromey  <tom@tromey.com>
+
+	* gdb.texinfo (Configure Options): Document minimum version of
+	readline.
+
 2019-08-07  Alan Hayward  <alan.hayward@arm.com>
 
 	* gdb.texinfo (AArch64 Pointer Authentication): New subsection.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 7f8c0aff1cd..e36b2d59745 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -36905,7 +36905,8 @@  details.
 
 @item --with-system-readline
 Use the readline library installed on the host, rather than the
-library supplied as part of @value{GDBN}.
+library supplied as part of @value{GDBN}.  Readline 7 or newer is
+required; this is enforced by the build system.
 
 @item --with-system-zlib
 Use the zlib library installed on the host, rather than the library