Re: [PATCHSET] [4/4] Fix various issue in TUI

[Pedro Alves <palves at redhat dot com>]

On 01/17/2015 09:55 AM, Eli Zaretskii wrote:
>> > Date: Mon, 05 Jan 2015 22:06:07 +0200
>> > From: Eli Zaretskii <eliz@gnu.org>
>> > Cc: gdb-patches@sourceware.org
>> > 
>>> > > Date: Mon, 05 Jan 2015 19:54:42 +0000
>>> > > From: Pedro Alves <palves@redhat.com>
>>> > > 
>>> > > On 12/31/2014 05:56 PM, Eli Zaretskii wrote:
>>>> > > > Well, one patch is Windows-specific after all.  This patch makes sure
>>>> > > > windows-termcap is not compiled when GDB is linked against ncurses,
>>> > > 
>>> > > ...
>>> > > 
>>>> > > > and also makes the file a no-op should it compile in that
>>>> > > > configuration.  
>>> > > 
>>> > > With the configure.ac change, how can that happen?
>> > 
>> > It shouldn't.

I'd have preferred to drop that hunk then.  It just seems to
be either pointless or hiding some problem with the
configure.ac check.

> Don't use windows-termcap.c when linking against a curses library
> 
> gdb/
> 2015-01-17  Eli Zaretskii  <eliz@gnu.org>
> 
> 	* configure.ac [*mingw32*]: Only add windows-termcap.o to
> 	CONFIG_OBS if not building with a curses library.
> 
> 	* windows-termcap.c: Include defs.h.  Make the whole body empty if
> 	either one of HAVE_CURSES_H or HAVE_NCURSES_H or
> 	HAVE_NCURSES_NCURSES_H is defined.
> 
> diff --git a/gdb/configure.ac b/gdb/configure.ac
> index 8dd7f8f..b852b93 100644
> --- a/gdb/configure.ac
> +++ b/gdb/configure.ac
> @@ -611,9 +611,10 @@ case $host_os in
>      ac_cv_search_tgetent="none required"
>      ;;
>    *mingw32*)
> -    ac_cv_search_tgetent="none required"
> -    CONFIG_OBS="$CONFIG_OBS windows-termcap.o"
> -    ;;
> +    if test x"$prefer_curses" != xyes; then
> +      ac_cv_search_tgetent="none required"
> +      CONFIG_OBS="$CONFIG_OBS windows-termcap.o"
> +    fi ;;

I'm still confused and not convinced this is the right
predicate.  You said:

 "This patch makes sure windows-termcap is not compiled
  when GDB is linked against ncurses".

But that doesn't look to be what the patch does.

I asked about $curses_found before because AFAICS, it's quite
possible for $prefer_curses to be set, but $curses_found
to be "no".  By default, we try configuring the TUI:

 # Enable TUI.
 AC_ARG_ENABLE(tui,
 AS_HELP_STRING([--enable-tui], [enable full-screen terminal user interface (TUI)]),
   [case $enableval in
     yes | no | auto)
       ;;
     *)
       AC_MSG_ERROR([bad value $enableval for --enable-tui]) ;;
   esac],enable_tui=auto)

And that always results in $prefer_curses set to yes:

 # For the TUI, we need enhanced curses functionality.
 if test x"$enable_tui" != xno; then
   prefer_curses=yes
 fi


But then we check if curses is really found:

 curses_found=no
 if test x"$prefer_curses" = xyes; then
 ...
   AC_SEARCH_LIBS(waddstr, [ncurses cursesX curses])

   if test "$ac_cv_search_waddstr" != no; then
     curses_found=yes
   fi
 fi


And then it's $curses_found that we check:

# Check whether we should enable the TUI, but only do so if we really
# can.
if test x"$enable_tui" != xno; then
  if test -d $srcdir/tui; then
    if test "$curses_found" != no; then
...
    else
      if test x"$enable_tui" = xyes; then
	AC_MSG_ERROR([no enhanced curses library found; disable TUI])
      else
	AC_MSG_WARN([no enhanced curses library found; disabling TUI])
      fi
    fi
  fi
fi


So shouldn't the right check be this:

    if test x"$curses_found" != xyes; then
      ac_cv_search_tgetent="none required"
      CONFIG_OBS="$CONFIG_OBS windows-termcap.o"
    fi ;;

?

Thanks,
Pedro Alves