Commit Message
I took a look at what it would take to upgrade to the latest readline 7
patch release. I have a few questions, mostly about Windows-related
preprocessor defines.
First I diff'd readline 6.2 patch 1 against gdb's readline.
From the history it appears that this is the correct baseline to use.
Then I diff'd readline 6.2 patch 1 against readline 7.0 patch 5.
This is the latest stable release of readline.
Next, for each hunk in the readline-to-gdb diff, I searched for an
equivalent hunk in the readline-to-readline diff. If the hunk was
found, I removed it.
This left a patch consisting of changes that were made to the gdb
readline but not upstreamed. They fall into four categories:
* Changes that seem necessary or at least desirable for gdb. In the
patch below this includes the configure.in, hsuser.texi, and
Makefile.in changes.
* Changes that should be retested. This includes the, emacs_keymap.c,
and complete.c changes.
* Changes that were simply not upstream. In general I don't know why.
This covers the signals.c and xfree.c changes, though perhaps
signals.c falls into the "re-test" category. I think for xfree.c I
can maybe do a bit of patch archaeology to find the reason.
* Changes that are upstream but in a different form. display.c and
terminal.c have different #if conditions and input.c has different
code:
display.c
readline upstream:
+#ifdef __DJGPP__
gdb:
+#ifdef __MSDOS__
or
+#if defined (__GO32__)
terminal.c
readline
+#ifdef __MSDOS__
gdb:
+#ifdef __DJGPP__
input.c, readline:
+int
+win32_isatty (int fd)
+{
+ return (_isatty (fd) ? ((((long) (HANDLE) _get_osfhandle (fd)) & 3) == 3) : 0);
+}
(for gdb see the patch, the gdb version is longer and calls GetConsoleMode)
For most of the differences, I know what to do. But, I don't know the
difference between __DJGPP__, __MSDOS__, and __GO32__ -- and the sources
also check __MINGW32__, _WIN32, and __CYGWIN__ at places. So, I am not
sure how to go about merging those parts, and whether changes need to be
sent upstream.
My other question is how best to go about merging. In the CVS days I
would have done a cvs import on a vendor branch. I could simulate this
pretty well in an external git repository I suppose. It's tempting to
switch entirely to git submodules for external dependencies, but maybe
just having a gdb "import area" git repository would be alright. The
idea being, do the merge work there, but in the end just check in a
giant patch. Then the next person to do this could re-use the
repository.
I'm attaching my edited readline->gdb patch so you can see the details
if you want. The readline 6->7 diff is very big but you can see it by
cloning:
git://git.savannah.gnu.org/readline.git
and diffing like
git diff d0a9f21f36b2bf3ac28e1b11c1962bbb2ca0be2c..origin/master
That revision is, I believe, readline 6.2 patch 1.
Maybe this can also be seen online somewhere, I don't know.
Tom
Comments
> From: Tom Tromey <tom@tromey.com>
> Date: Sat, 29 Sep 2018 05:52:37 -0600
>
> input.c, readline:
> +int
> +win32_isatty (int fd)
> +{
> + return (_isatty (fd) ? ((((long) (HANDLE) _get_osfhandle (fd)) & 3) == 3) : 0);
> +}
The Readline version reportedly doesn't work with newer versions of
Windows, so I think we should ask upstream Readline to use the newer
code we have in GDB.
> I don't know the difference between __DJGPP__, __MSDOS__, and
> __GO32__
They are all equivalent, so I suggest to change them all to use
__DJGPP__, unless there's some reason not to.
> the sources also check __MINGW32__, _WIN32, and __CYGWIN__ at
> places.
These are unrelated to DOS/DJGPP/GO32, they are for MinGW builds and
Cygwin builds.
Thanks for doing this.
On 09/29/2018 12:52 PM, Tom Tromey wrote:
> I took a look at what it would take to upgrade to the latest readline 7
> patch release. I have a few questions, mostly about Windows-related
> preprocessor defines.
Thanks for doing this. Note that Patrick Palka attempted this
a couple of years back. The update went into master, but
was subsequently reverted because of problems on non-x86 ports.
See:
https://sourceware.org/ml/gdb-patches/2015-07/msg00422.html
https://sourceware.org/ml/gdb-patches/2015-07/msg00759.html
And:
https://sourceware.org/ml/gdb-patches/2016-02/msg00484.html
Thanks,
Pedro Alves
>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:
Eli> The Readline version reportedly doesn't work with newer versions of
Eli> Windows, so I think we should ask upstream Readline to use the newer
Eli> code we have in GDB.
I sent a note to bug-readline with the patch.
>> I don't know the difference between __DJGPP__, __MSDOS__, and
>> __GO32__
Eli> They are all equivalent, so I suggest to change them all to use
Eli> __DJGPP__, unless there's some reason not to.
>> the sources also check __MINGW32__, _WIN32, and __CYGWIN__ at
>> places.
Eli> These are unrelated to DOS/DJGPP/GO32, they are for MinGW builds and
Eli> Cygwin builds.
When I did the merge, these changes disappeared, so maybe I was
misreading the diffs somehow. Or maybe I did the merge wrong, hard to
say.
What I did is make a new git repository and made a branch based on
readline 6.2 patch 1 -- the gdb base revision. Then I copied the
readline sources from the gdb repository on top of that and committed
it. Finally, I did a git merge from readline 7.0 patch 5 and fixed up
the conflicts.
You can see the resulting repository here:
https://github.com/tromey/gdb-readline/tree/gdb-readline
Future merges can be done this way pretty easily, either by just
recreating this process, or by cloning the repository and merging from
the latest readline.
readline 8 is in testing, so perhaps it would be good to try that to see
if there are any new readline bugs affecting gdb.
Tom
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> On 09/29/2018 12:52 PM, Tom Tromey wrote:
>> I took a look at what it would take to upgrade to the latest readline 7
>> patch release. I have a few questions, mostly about Windows-related
>> preprocessor defines.
Pedro> Thanks for doing this. Note that Patrick Palka attempted this
Pedro> a couple of years back. The update went into master, but
Pedro> was subsequently reverted because of problems on non-x86 ports.
Pedro> See:
Pedro> https://sourceware.org/ml/gdb-patches/2015-07/msg00422.html
Pedro> https://sourceware.org/ml/gdb-patches/2015-07/msg00759.html
Pedro> And:
Pedro> https://sourceware.org/ml/gdb-patches/2016-02/msg00484.html
Yeah, I saw those - he was trying with 7.0 alpha, and I was hoping maybe
doing it with a later (non-alpha) version would help.
I have a patch. And, I went through the local changes and I think none
of them are needed, so I've removed them one-by-one in the resulting
series.
However, it's too big to push through "buildbot try".
So, it may be quite some time until I'm able to test it suitably.
If someone wants to help with this, it's in my github repository.
Tom
@@ -481,6 +482,10 @@
{
int c;
+/* Disabled for GDB due to the gdb.base/readline-ask.exp regression.
+ [patch] testsuite: Test readline-6.2 "ask" regression
+ http://sourceware.org/ml/gdb-patches/2011-05/msg00002.html */
+#if 0
/* For now, disable pager in callback mode, until we later convert to state
driven functions. Have to wait until next major version to add new
state definition, since it will change value of RL_STATE_DONE. */
@@ -488,6 +493,7 @@
if (RL_ISSTATE (RL_STATE_CALLBACK))
return 1;
#endif
+#endif
for (;;)
{
@@ -22,19 +22,24 @@
AC_REVISION([for Readline 6.2, version 2.67])
+m4_include([../config/override.m4])
+
AC_INIT(readline, 6.2, bug-readline@gnu.org)
dnl make sure we are using a recent autoconf version
AC_PREREQ(2.50)
AC_CONFIG_SRCDIR(readline.h)
-AC_CONFIG_AUX_DIR(./support)
+dnl GDB LOCAL
+dnl AC_CONFIG_AUX_DIR(./support)
+AC_CONFIG_AUX_DIR(`cd $srcdir;pwd`/..)
AC_CONFIG_HEADERS(config.h)
dnl update the value of RL_READLINE_VERSION in readline.h when this changes
LIBVERSION=6.2
AC_CANONICAL_HOST
+AC_CANONICAL_BUILD
dnl configure defaults
opt_curses=no
@@ -57,10 +62,10 @@
dnl option parsing for optional features
opt_multibyte=yes
opt_static_libs=yes
-opt_shared_libs=yes
+opt_shared_libs=no
AC_ARG_ENABLE(multibyte, AC_HELP_STRING([--enable-multibyte], [enable multibyte characters if OS supports them]), opt_multibyte=$enableval)
-AC_ARG_ENABLE(shared, AC_HELP_STRING([--enable-shared], [build shared libraries [[default=YES]]]), opt_shared_libs=$enableval)
+dnl AC_ARG_ENABLE(shared, AC_HELP_STRING([--enable-shared], [build shared libraries [[default=YES]]]), opt_shared_libs=$enableval)
AC_ARG_ENABLE(static, AC_HELP_STRING([--enable-static], [build static libraries [[default=YES]]]), opt_static_libs=$enableval)
if test $opt_multibyte = no; then
@@ -193,6 +198,9 @@
TERMCAP_LIB=-ltermcap #default
fi
fi
+if test "$TERMCAP_LIB" = "-lncurses"; then
+ AC_CHECK_HEADERS(ncurses/termcap.h)
+fi
BASH_CHECK_MULTIBYTE
@@ -264,7 +272,7 @@
AC_SUBST(STATIC_INSTALL_TARGET)
AC_SUBST(SHARED_INSTALL_TARGET)
-case "$host_os" in
+case "$build_os" in
msdosdjgpp*) BUILD_DIR=`pwd.exe` ;; # to prevent //d/path/file
*) BUILD_DIR=`pwd` ;;
esac
@@ -2056,9 +2060,18 @@
}
else
{ /* delta < 0 */
+#ifdef __MSDOS__
+ int row, col;
+
+ fflush (rl_outstream); /* make sure the cursor pos is current! */
+ ScreenGetCursor (&row, &col);
+ ScreenSetCursor (row + delta, col);
+ i = -delta; /* in case someone wants to use it after the loop */
+#else /* !__MSDOS__ */
if (_rl_term_up && *_rl_term_up)
for (i = 0; i < -delta; i++)
tputs (_rl_term_up, 1, _rl_output_character_function);
+#endif /* !__MSDOS__ */
}
_rl_last_v_pos = to; /* Now TO is here */
@@ -2341,10 +2357,15 @@
void
_rl_clear_screen ()
{
+#if defined (__GO32__)
+ ScreenClear (); /* FIXME: only works in text modes */
+ ScreenSetCursor (0, 0); /* term_clrpag is "cl" which homes the cursor */
+#else
if (_rl_term_clrpag)
tputs (_rl_term_clrpag, 1, _rl_output_character_function);
else
rl_crlf ();
+#endif
}
/* Insert COUNT characters from STRING to the output stream at column COL. */
@@ -2353,7 +2374,7 @@
char *string;
int count, col;
{
-#if defined (__MSDOS__) || defined (__MINGW32__)
+#if defined (__MSDOS__) || (defined (__MINGW32__) && !defined (NCURSES_VERSION))
_rl_output_some_chars (string, count);
#else
/* DEBUGGING */
@@ -2405,7 +2426,7 @@
if (count > _rl_screenwidth) /* XXX */
return;
-#if !defined (__MSDOS__) && !defined (__MINGW32__)
+#if !defined (__MSDOS__) && !(defined (__MINGW32__) && !defined (NCURSES_VERSION))
if (_rl_term_DC && *_rl_term_DC)
{
char *buffer;
@@ -26,9 +26,10 @@
@node Using History Interactively
@chapter Using History Interactively
-@ifclear BashFeatures
-@defcodeindex bt
-@end ifclear
+@c GDB bundling modification:
+@c @ifclear BashFeatures
+@c @defcodeindex bt
+@c @end ifclear
@ifset BashFeatures
This chapter describes how to use the @sc{gnu} History Library
@@ -41,7 +42,8 @@
This chapter describes how to use the @sc{gnu} History Library interactively,
from a user's standpoint. It should be considered a user's guide. For
information on using the @sc{gnu} History Library in your own programs,
-@pxref{Programming with GNU History}.
+@c GDB bundling modification:
+@pxref{Programming with GNU History, , , history, GNU History Library}.
@end ifclear
@ifset BashFeatures
@@ -277,7 +277,13 @@
{ ISFUNC, rl_insert }, /* Latin capital letter Y with acute */
{ ISFUNC, rl_insert }, /* Latin capital letter thorn (Icelandic) */
{ ISFUNC, rl_insert }, /* Latin small letter sharp s (German) */
+#ifndef __MINGW32__
{ ISFUNC, rl_insert }, /* Latin small letter a with grave */
+#else
+ /* Temporary - this is a bug in readline 5.1 that should be fixed in
+ readline 5.2. */
+ { ISFUNC, 0 }, /* Must leave this unbound for the arrow keys to work. */
+#endif
{ ISFUNC, rl_insert }, /* Latin small letter a with acute */
{ ISFUNC, rl_insert }, /* Latin small letter a with circumflex */
{ ISFUNC, rl_insert }, /* Latin small letter a with tilde */
@@ -86,6 +86,37 @@
static int rl_get_char PARAMS((int *));
static int rl_gather_tyi PARAMS((void));
+#if defined (_WIN32) && !defined (__CYGWIN__)
+
+/* 'isatty' in the Windows runtime returns non-zero for every
+ character device, including the null device. Repair that. */
+#include <io.h>
+#include <conio.h>
+#define WIN32_LEAN_AND_MEAN 1
+#include <windows.h>
+
+int w32_isatty (int fd)
+{
+ if (_isatty(fd))
+ {
+ HANDLE h = (HANDLE) _get_osfhandle (fd);
+ DWORD ignored;
+
+ if (h == INVALID_HANDLE_VALUE)
+ {
+ errno = EBADF;
+ return 0;
+ }
+ if (GetConsoleMode (h, &ignored) != 0)
+ return 1;
+ }
+ errno = ENOTTY;
+ return 0;
+}
+
+#define isatty(x) w32_isatty(x)
+#endif
+
/* **************************************************************** */
/* */
/* Character Input Buffering */
@@ -211,7 +211,17 @@
force:
-install: $(INSTALL_TARGETS)
+## GDB LOCAL
+## Don't mess with people's installed readline's.
+## This tries to install this version of readline over whatever
+## version is already installed on the system (which could be a
+## newer version). There is no real reason for us to install
+## readline along with GDB. GDB links statically against readline,
+## so it doesn't depend on us installing it on the system.
+
+install:
+
+#install: $(INSTALL_TARGETS)
install-headers: installdirs ${INSTALLED_HEADERS}
for f in ${INSTALLED_HEADERS}; do \
@@ -412,7 +422,7 @@
vi_mode.o: readline.h keymaps.h rltypedefs.h chardefs.h tilde.h
vi_mode.o: history.h ansi_stdlib.h rlstdc.h
xfree.o: ${BUILD_DIR}/config.h
-xfree.o: ansi_stdlib.h
+xfree.o: ansi_stdlib.h readline.h
xmalloc.o: ${BUILD_DIR}/config.h
xmalloc.o: ansi_stdlib.h
@@ -580,6 +580,7 @@
sigint_blocked = 0;
}
+#ifdef SIGWINCH
/* Cause SIGWINCH to not be delivered until the corresponding call to
release_sigwinch(). */
void
@@ -627,6 +628,7 @@
sigwinch_blocked = 0;
}
+#endif /* SIGWINCH */
/* **************************************************************** */
/* */
@@ -661,12 +694,17 @@
default:
break;
case VISIBLE_BELL:
+#ifdef __MSDOS__
+ ScreenVisualBell ();
+ break;
+#else
if (_rl_visible_bell)
{
tputs (_rl_visible_bell, 1, _rl_output_character_function);
break;
}
/* FALLTHROUGH */
+#endif
case AUDIBLE_BELL:
fprintf (stderr, "\007");
fflush (stderr);
@@ -31,7 +31,10 @@
# include "ansi_stdlib.h"
#endif /* HAVE_STDLIB_H */
+#include <stdio.h>
+
#include "xmalloc.h"
+#include "readline.h"
/* **************************************************************** */
/* */
@@ -45,6 +48,10 @@
xfree (string)
PTR_T string;
{
+ /* Leak a bit. */
+ if (RL_ISSTATE(RL_STATE_SIGHANDLER))
+ return;
+
if (string)
free (string);
}
@@ -38,6 +38,9 @@
#endif /* !PTR_T */
+/* xmalloc and xrealloc should be also protected from RL_STATE_SIGHANDLER. */
+#define xfree xfree_readline
+
extern PTR_T xmalloc PARAMS((size_t));
extern PTR_T xrealloc PARAMS((void *, size_t));
extern void xfree PARAMS((void *));