Enabled TUI mode on MSYS2

  2016-09-13  Giah de Barag  <gdb@crelg.com>


Summary
-------

Here is a patch and a build recipe for GDB on MSYS2 to enable TUI mode.

The current gdb (7.11.1) in MSYS2 does not support TUI mode. However, GDB
TUI mode works fine on MSYS2. It depends upon python, curses, and three
environment variables: PYTHONHOME (/mingw32 or /mingw64) and TERMINFO
(/usr/share/terminfo) and TERM.


Impact
------

This patch only affects builds where (#if defined __MINGW32__) is true.


Explanation of Changes
----------------------

GDB works fine in emacs, in bash, and in mintty. In the Windows Console (start
cmd.exe) TUI mode works also. TUI mode depends upon ncurses and python. Have
them ready before running configure.

Eli Zaretski (https://www.sourceware.org/ml/gdb/2014-12/msg00039.html)
comments that he had to “hide libncurses from the configure script in order to
build a working debugger.” This comment might be dated.

GDB with TUI mode works fine on MSYS2 with curses, with this small patch. I saw
that getch() (which becomes the ncurses getch() when you link in ncurses) in
input.c returns EOF as soon as you hit any character. Fortunately, Windows
conio has deprecated getch() and has defined _getch() in its place. So I
changed it to _getch(), to access the conio version.

There is a tiny issue now that with this conio _getch(), gdb does not properly
process backspace characters.

Not having backspace characters in plain non-TUI mode in the Windows Console
is hardly bothersome, because you can type “tui enable” and have a better
environment. If you have to be in non-TUI mode for some reason, a workaround
is [Ctrl]-[L] to refresh the line. Therefore I am using conio _getch() not
ncurses getch().


Note to GDB Maintainers
-----------------------

Someone with understanding of event handling by gdb, Windows conio, and UNIX
curses: Can you please look where there is “getch()” in rl_getc() in input.c
and see why it returns EOF no matter what (when linking with ncurses). Then do
as I did in this patch and change it to _getch() to access the conio version
and see why backspace characters are not handled properly. They are processed,
but the display is not refreshed, and you have to hit [Ctrl]-[L] to refresh.

Also, this is the first time I am doing something like this, so if I am
neglecting any rule of communication of this list, please inform me, and I
will correct it.


MSYS2 GDB Build Recipe
----------------------

# Steps to produce a (debuggable) gdb with TUI mode support on MSYS2:

# These steps are tested with Server 2008 R2 which is supposed to be
# similar to Windows 7.

# Install MSYS2

# see also http://msys2.github.io

wget http://repo.msys2.org/distrib/msys2-x86-64-latest.exe

# update msys2

pacman --noconfirm -Sy pacman # update pacman (quit mintty and restart)
pacman --noconfirm -Syu       # update package db (quit mintty and restart)
pacman --noconfirm -Su        # update packages (repeat until nothing changes)

# get a bunch of stuff

arch=i686
# OR
arch=x86-64

# These are all the packages I currently use. You probably don’t need all
# these packages, but you definitely need python and curses.

pacman --needed --noconfirm -S base-devel git svn tar zip unzip asciidoc
pacman --needed --noconfirm -S mingw-w64-${arch}-emacs
pacman --needed --noconfirm -S mingw-w64-${arch}-toolchain
pacman --needed --noconfirm -S mingw-w64-${arch}-libxml2
pacman --needed --noconfirm -S mingw-w64-${arch}-gnutls
pacman --needed --noconfirm -S mingw-w64-${arch}-libxslt
pacman --needed --noconfirm -S mingw-w64-${arch}-libjpeg-turbo
pacman --needed --noconfirm -S mingw-w64-${arch}-libtiff
pacman --needed --noconfirm -S mingw-w64-${arch}-giflib
pacman --needed --noconfirm -S mingw-w64-${arch}-icu
pacman --needed --noconfirm -S mingw-w64-${arch}-libsndfile
pacman --needed --noconfirm -S mingw-w64-${arch}-aspell
pacman --needed --noconfirm -S mingw-w64-${arch}-lcms
pacman --needed --noconfirm -S mingw-w64-${arch}-lcms2
pacman --needed --noconfirm -S mingw-w64-${arch}-sqlite3
pacman --needed --noconfirm -S mingw-w64-${arch}-windows-default-manifest 

# Get and build GDB (same version as in MSYS2 pacman)

# see https://www.gnu.org/software/gdb/current/

git clone -b gdb-7.11-branch git://sourceware.org/git/binutils-gdb.git
./configure
make

# Debug GDB with itself. Use existing gdb in emacs to debug the newly-built
# gdb, in tui mode, in a Windows Console. To improve debugging experience,
# modify the CFLAGS that you just used to build GDB.

cd binutils-gdb/gdb
runemacs
M-x gdb
gdb --annotate=0 -i=mi ./gdb
set new-console on
set args -tui
run


Note to MSYS2 Maintainers
-------------------------

Is this enough info for you to be able to build an improved GDB that supports
TUI mode? Or do you need me to refine down the list of packages that has to be
installed before building GDB?


Patches
-------

Attached is patch relative to gdb-7.11-branch. All it does is change getch()
to _getch() in input.c (and remove compiler warnings about implicitly declared
Windows conio functions _kbhit() and _getch()).


----- CUT HERE -----
  

> From: Giah de Barag <gdb@crelg.com>
> Date: Tue, 13 Sep 2016 17:20:52 -0400
> 
> Here is a patch and a build recipe for GDB on MSYS2 to enable TUI mode.

I don't think there's a need for any patches, I regularly build GDB
for Windows with TUI support.  See the binaries on the ezwinports
site.  All that's needed is to specify TUI support at configure time.

> Eli Zaretski (https://www.sourceware.org/ml/gdb/2014-12/msg00039.html)
> comments that he had to “hide libncurses from the configure script in order to
> build a working debugger.” This comment might be dated.

Yes, it is.  The issues that caused me do that were fixed long ago.

> GDB with TUI mode works fine on MSYS2 with curses, with this small patch. I saw
> that getch() (which becomes the ncurses getch() when you link in ncurses) in
> input.c returns EOF as soon as you hit any character. Fortunately, Windows
> conio has deprecated getch() and has defined _getch() in its place. So I
> changed it to _getch(), to access the conio version.

I submitted such a patch to the Readline maintainer many moons ago,
and the patch is already in the Readline sources.  The problem is that
GDB uses its own copy of an old Readline, and all my requests to
import a new version of Readline failed.  Until GDB does importa a new
version of Readline, this (and other) patches I submitted for Readline
will not be in GDB.

> There is a tiny issue now that with this conio _getch(), gdb does not properly
> process backspace characters.

I see no such problems in my binaries.
> Someone with understanding of event handling by gdb, Windows conio, and UNIX
> curses: Can you please look where there is “getch()” in rl_getc() in input.c
> and see why it returns EOF no matter what (when linking with ncurses).

That's what ncurses' getch does.  It's not a bug.  GDB simply should
not use that function in the MinGW build.

Thanks.
  

> From: Eli Zaretskii <eliz@gnu.org>
> Date: Sep 13, 2016, at 22:42
> 
>> From: Giah de Barag <gdb@crelg.com>
>> Date: Tue, 13 Sep 2016 17:20:52 -0400
>> 
>> Here is a patch and a build recipe for GDB on MSYS2 to enable TUI mode.
> 
> I don't think there's a need for any patches, I regularly build GDB
> for Windows with TUI support.  See the binaries on the ezwinports
> site.  All that's needed is to specify TUI support at configure time.

Question below refers to this statement.

>> Eli Zaretski (https://www.sourceware.org/ml/gdb/2014-12/msg00039.html)
>> comments that he had to “hide libncurses from the configure script in order to
>> build a working debugger.” This comment might be dated.
> 
> Yes, it is.  The issues that caused me do that were fixed long ago.
> 
>> GDB with TUI mode works fine on MSYS2 with curses, with this small patch. I saw
>> that getch() (which becomes the ncurses getch() when you link in ncurses) in
>> input.c returns EOF as soon as you hit any character. Fortunately, Windows
>> conio has deprecated getch() and has defined _getch() in its place. So I
>> changed it to _getch(), to access the conio version.
> 
> I submitted such a patch to the Readline maintainer many moons ago,
> and the patch is already in the Readline sources.  The problem is that
> GDB uses its own copy of an old Readline, and all my requests to
> import a new version of Readline failed.  Until GDB does importa a new
> version of Readline, this (and other) patches I submitted for Readline
> will not be in GDB.
> 
>> There is a tiny issue now that with this conio _getch(), gdb does not properly
>> process backspace characters.
> 
> I see no such problems in my binaries.
>> Someone with understanding of event handling by gdb, Windows conio, and UNIX
>> curses: Can you please look where there is “getch()” in rl_getc() in input.c
>> and see why it returns EOF no matter what (when linking with ncurses).
> 
> That's what ncurses' getch does.  It's not a bug.  GDB simply should
> not use that function in the MinGW build.

I do not understand the apparent conflict between this statement, that “GDB simply should not use that function in the MinGW build,” and the statement above, “I don't think there's a need for any patches.” How should one not use that function without affecting the source? Has this patch (changing getch to _getch) already been applied somewhere other than 7.11 (where I happen to be focused)?

> Thanks.

Thank you.
  

> From: Giah de Barag <gdb@crelg.com>
> Date: Wed, 14 Sep 2016 02:32:09 -0400
> Cc: gdb-patches@sourceware.org
> 
> I do not understand the apparent conflict between this statement, that “GDB simply should not use that function in the MinGW build,” and the statement above, “I don't think there's a need for any patches.” How should one not use that function without affecting the source? Has this patch (changing getch to _getch) already been applied somewhere other than 7.11 (where I happen to be focused)?

Sorry for not explaining this clearly enough.  Let me try again.

The function getch from ncurses can only work when GDB is invoked with
the -tui command line option, i.e. when you activate the TUI user
interface.  Otherwise, it will always return EOF because no text-mode
window was created by GDB, and ncurses requires a window before it can
take any input.

When GDB is linked with ncurses, the getch function call by Readline
is resolved to the ncurses version of that function, and not to getch
in the MS-Windows runtime library.  (The call to getch in Readline is
MinGW-specific, so this problem doesn't show on other platforms.)
Readline only calls getch when TUI is _not_ in use, so the solution is
to replace the call to getch with a call to _getch.

The Readline library is maintained separately from GDB; GDB imports
Readline into its repository, and has its own copy there.  I submitted
the patch to use _getch (and several other MinGW-related patches) to
the Readline maintainer about 2 years ago, and those patches are
already in the latest versions of Readline.  Unfortunately, GDB did
not yet import those latest versions of Readline, so you don't see the
patches in the GDB sources.  (When I produce the binaries I use and
make available from the ezwinports site, I patch Readline to fix all
thos problems.)

I hope this explains the issue.  The bottom line is that GDB should
import a newer version of Readline, and then these problems will be
fixed without any need for further patches.
  

On 09/14/2016 06:21 PM, Eli Zaretskii wrote:

> I hope this explains the issue.  The bottom line is that GDB should
> import a newer version of Readline, and then these problems will be
> fixed without any need for further patches.

If the readline patch in question is upstream already,
I wouldn't object backporting it to our readline copy.

The problem with rebasing our readline copy is simply that it
requires someone motivated to spend the effort to do it.
Patrick Palka tried to import readline 7 a while ago, and the import
made it to master even, but then a few problems were discovered
and the patch was reverted:

 https://sourceware.org/ml/gdb-patches/2015-07/msg00759.html

There was no movement after that.

Thanks,
Pedro Alves
  

> On Sep 14, 2016, at 13:33, Pedro Alves <palves@redhat.comwrote:
> 
>> On 09/14/2016 06:21 PM, Eli Zaretskii wrote:
>> 
>> I hope this explains the issue.  The bottom line is that GDB should
>> import a newer version of Readline, and then these problems will be
>> fixed without any need for further patches.

Eli: Yes, I see now. Thank you for that update. That also explains why
I’m seeing a problem with the backspace handling. I should import the
entire newer readline library not just change getch to _getch.

> If the readline patch in question is upstream already,
> I wouldn't object backporting it to our readline copy.
> 
> The problem with rebasing our readline copy is simply that it
> requires someone motivated to spend the effort to do it.
> Patrick Palka tried to import readline 7 a while ago, and the import
> made it to master even, but then a few problems were discovered
> and the patch was reverted:
> 
> https://sourceware.org/ml/gdb-patches/2015-07/msg00759.html
> 
> There was no movement after that.
> 
> Thanks,
> Pedro Alves

Pedro: Seeing that Eli is able to build and run GDB with the new
readline, we should be able to move forward. I am willing to help.
What do you need from us to move this forward?
  

On 09/14/2016 07:32 PM, Giah de Barag wrote:

> Pedro: Seeing that Eli is able to build and run GDB with the new
> readline, we should be able to move forward. I am willing to help.
> What do you need from us to move this forward?

See the url I pointed at and the discussion surrounding it.
Someone needs to investigate the issues Patrick originally
stumbled on, and come up with some way to fix them.

Thanks,
Pedro Alves
  

> Cc: GDB Patches <gdb-patches@sourceware.org>
> From: Pedro Alves <palves@redhat.com>
> Date: Wed, 14 Sep 2016 19:36:23 +0100
> 
> On 09/14/2016 07:32 PM, Giah de Barag wrote:
> 
> > Pedro: Seeing that Eli is able to build and run GDB with the new
> > readline, we should be able to move forward. I am willing to help.
> > What do you need from us to move this forward?
> 
> See the url I pointed at and the discussion surrounding it.
> Someone needs to investigate the issues Patrick originally
> stumbled on, and come up with some way to fix them.

Yes.  But if importing a newer Readline is not going to happen soon, I
think we should apply my patches to our copy of Readline, to avoid
these issues.
  

On 09/14/2016 07:58 PM, Eli Zaretskii wrote:
>> Cc: GDB Patches <gdb-patches@sourceware.org>
>> From: Pedro Alves <palves@redhat.com>
>> Date: Wed, 14 Sep 2016 19:36:23 +0100
>>
>> On 09/14/2016 07:32 PM, Giah de Barag wrote:
>>
>>> Pedro: Seeing that Eli is able to build and run GDB with the new
>>> readline, we should be able to move forward. I am willing to help.
>>> What do you need from us to move this forward?
>>
>> See the url I pointed at and the discussion surrounding it.
>> Someone needs to investigate the issues Patrick originally
>> stumbled on, and come up with some way to fix them.
> 
> Yes.  But if importing a newer Readline is not going to happen soon, I
> think we should apply my patches to our copy of Readline, to avoid
> these issues.

FAOD, as mentioned upthread, that'd be fine with me.

Thanks,
Pedro Alves
  

> Cc: gdb@crelg.com, gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Wed, 14 Sep 2016 20:04:09 +0100
> 
> > Yes.  But if importing a newer Readline is not going to happen soon, I
> > think we should apply my patches to our copy of Readline, to avoid
> > these issues.
> 
> FAOD, as mentioned upthread, that'd be fine with me.

Done.
  

> Date: Sat, 17 Sep 2016 11:53:29 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> CC: gdb@crelg.com, gdb-patches@sourceware.org
> 
> > Cc: gdb@crelg.com, gdb-patches@sourceware.org
> > From: Pedro Alves <palves@redhat.com>
> > Date: Wed, 14 Sep 2016 20:04:09 +0100
> > 
> > > Yes.  But if importing a newer Readline is not going to happen soon, I
> > > think we should apply my patches to our copy of Readline, to avoid
> > > these issues.
> > 
> > FAOD, as mentioned upthread, that'd be fine with me.
> 
> Done.

I get messages about my commit broke the build.  But I don't
understand the nature of the breakage: all of the errors complain
about min and max in the 'gdb' directory, whereas I didn't change
anything in that directory, and didn't introduce any calls or
definitions of min or max.

Help will be appreciated.

Sorry for the mess.
  

On 09/17/2016 10:39 AM, Eli Zaretskii wrote:

> I get messages about my commit broke the build.  But I don't
> understand the nature of the breakage: all of the errors complain
> about min and max in the 'gdb' directory, whereas I didn't change
> anything in that directory, and didn't introduce any calls or
> definitions of min or max.

Sounds like you're building for 32-bit.  My std::min/std::max
broke those, sorry about that.  I don't know why the buildbot
is putting the blame on your patch.  Anyway, should be fixed now.
Let me know if you still see build problems.

> 
> Help will be appreciated.
> 
> Sorry for the mess.
> 

Thanks,
Pedro Alves
  

On Saturday, September 17 2016, Eli Zaretskii wrote:

>> Date: Sat, 17 Sep 2016 11:53:29 +0300
>> From: Eli Zaretskii <eliz@gnu.org>
>> CC: gdb@crelg.com, gdb-patches@sourceware.org
>> 
>> > Cc: gdb@crelg.com, gdb-patches@sourceware.org
>> > From: Pedro Alves <palves@redhat.com>
>> > Date: Wed, 14 Sep 2016 20:04:09 +0100
>> > 
>> > > Yes.  But if importing a newer Readline is not going to happen soon, I
>> > > think we should apply my patches to our copy of Readline, to avoid
>> > > these issues.
>> > 
>> > FAOD, as mentioned upthread, that'd be fine with me.
>> 
>> Done.
>
> I get messages about my commit broke the build.  But I don't
> understand the nature of the breakage: all of the errors complain
> about min and max in the 'gdb' directory, whereas I didn't change
> anything in that directory, and didn't introduce any calls or
> definitions of min or max.
>
> Help will be appreciated.

BTW, I've contacted Eli off-list and will investigate this issue.

Thanks,