public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCHSET] [4/4] Fix various issue in TUI
@ 2014-12-31 17:56 Eli Zaretskii
  2015-01-05 19:55 ` Pedro Alves
  0 siblings, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2014-12-31 17:56 UTC (permalink / raw)
  To: gdb-patches

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.  This is to avoid shadowing of ncurses functions by the
stubs in windows-termcap.c.

OK to commit?

2014-12-31  Eli Zaretskii  <eliz@gnu.org>

	* gdb/configure.ac [mingw32]: Don't add windows-termcap.o to
	CONFIG_OBJS if a curses library is going to be used.

	* gdb/windows-termcap.c: Make the entire file a no-op if any
	kind of curses library i being used.


--- gdb/configure.ac~0	2014-10-29 21:45:50 +0200
+++ gdb/configure.ac	2014-12-30 07:42:27 +0200
@@ -627,9 +627,10 @@
     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 ;;
 esac
 
 # These are the libraries checked by Readline.


--- gdb/windows-termcap.c~0	2014-06-11 18:34:41 +0300
+++ gdb/windows-termcap.c	2014-12-29 15:42:44 +0200
@@ -19,6 +19,11 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+
+#include "config.h"
+
+#if !defined HAVE_CURSES_H && !defined HAVE_NCURSES_H && !defined HAVE_NCURSES_NCURSES_H
+
 #include <stdlib.h>
 
 /* -Wmissing-prototypes */
@@ -71,3 +76,5 @@
 {
   return NULL;
 }
+
+#endif	/* !HAVE_CURSES_H && !HAVE_NCURSES_H && !HAVE_NCURSES_NCURSES_H */

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHSET] [4/4] Fix various issue in TUI
  2014-12-31 17:56 [PATCHSET] [4/4] Fix various issue in TUI Eli Zaretskii
@ 2015-01-05 19:55 ` Pedro Alves
  2015-01-05 20:06   ` Eli Zaretskii
  0 siblings, 1 reply; 15+ messages in thread
From: Pedro Alves @ 2015-01-05 19:55 UTC (permalink / raw)
  To: Eli Zaretskii, gdb-patches

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?

> This is to avoid shadowing of ncurses functions by the
> stubs in windows-termcap.c.
> 
> OK to commit?
> 
> 2014-12-31  Eli Zaretskii  <eliz@gnu.org>
> 
> 	* gdb/configure.ac [mingw32]: Don't add windows-termcap.o to
> 	CONFIG_OBJS if a curses library is going to be used.

No "gdb/" prefix.

> 
> 	* gdb/windows-termcap.c: Make the entire file a no-op if any
> 	kind of curses library i being used.

typo: "is".

> 
> 
> --- gdb/configure.ac~0	2014-10-29 21:45:50 +0200
> +++ gdb/configure.ac	2014-12-30 07:42:27 +0200
> @@ -627,9 +627,10 @@
>      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"

I'm confused on the predicate here.  How can we tell
from $prefer_curses that the curses library doesn't include
termcap?  AFAICS, if the TUI is enabled, it'll always be
"yes".  But isn't that the case where you _don't_
want windows-termcap.o?  And shouldn't this be checking
$curses_found?

> +    fi ;;
>  esac
>  
>  # These are the libraries checked by Readline.
> 
> 
> --- gdb/windows-termcap.c~0	2014-06-11 18:34:41 +0300
> +++ gdb/windows-termcap.c	2014-12-29 15:42:44 +0200
> @@ -19,6 +19,11 @@
>     You should have received a copy of the GNU General Public License
>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>  
> +
> +#include "config.h"

All files in gdb/ should start with "defs.h" instead.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHSET] [4/4] Fix various issue in TUI
  2015-01-05 19:55 ` Pedro Alves
@ 2015-01-05 20:06   ` Eli Zaretskii
  2015-01-17  9:55     ` Eli Zaretskii
  0 siblings, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2015-01-05 20:06 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

> 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.

> > --- gdb/configure.ac~0	2014-10-29 21:45:50 +0200
> > +++ gdb/configure.ac	2014-12-30 07:42:27 +0200
> > @@ -627,9 +627,10 @@
> >      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"
> 
> I'm confused on the predicate here.  How can we tell
> from $prefer_curses that the curses library doesn't include
> termcap?  AFAICS, if the TUI is enabled, it'll always be
> "yes".  But isn't that the case where you _don't_
> want windows-termcap.o?  And shouldn't this be checking
> $curses_found?

Sorry, I sent the wrong patch here.  It should be !=, of course.

> > --- gdb/windows-termcap.c~0	2014-06-11 18:34:41 +0300
> > +++ gdb/windows-termcap.c	2014-12-29 15:42:44 +0200
> > @@ -19,6 +19,11 @@
> >     You should have received a copy of the GNU General Public License
> >     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> >  
> > +
> > +#include "config.h"
> 
> All files in gdb/ should start with "defs.h" instead.

OK.

Thanks.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHSET] [4/4] Fix various issue in TUI
  2015-01-05 20:06   ` Eli Zaretskii
@ 2015-01-17  9:55     ` Eli Zaretskii
  2015-01-19 14:50       ` Joel Brobecker
                         ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Eli Zaretskii @ 2015-01-17  9:55 UTC (permalink / raw)
  To: palves; +Cc: gdb-patches

> 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.
> 
> > > --- gdb/configure.ac~0	2014-10-29 21:45:50 +0200
> > > +++ gdb/configure.ac	2014-12-30 07:42:27 +0200
> > > @@ -627,9 +627,10 @@
> > >      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"
> > 
> > I'm confused on the predicate here.  How can we tell
> > from $prefer_curses that the curses library doesn't include
> > termcap?  AFAICS, if the TUI is enabled, it'll always be
> > "yes".  But isn't that the case where you _don't_
> > want windows-termcap.o?  And shouldn't this be checking
> > $curses_found?
> 
> Sorry, I sent the wrong patch here.  It should be !=, of course.
> 
> > > --- gdb/windows-termcap.c~0	2014-06-11 18:34:41 +0300
> > > +++ gdb/windows-termcap.c	2014-12-29 15:42:44 +0200
> > > @@ -19,6 +19,11 @@
> > >     You should have received a copy of the GNU General Public License
> > >     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> > >  
> > > +
> > > +#include "config.h"
> > 
> > All files in gdb/ should start with "defs.h" instead.
> 
> OK.

I just realized that I cannot push this changeset because Autoconf I
have installed isn't version 2.64, and so I cannot regenerate
'configure'.  Could someone please do that for me?  The patch is
below.

Or I could push the patch below, and then someone else could do the
generated files as a separate commit.

Thanks in advance.

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 ;;
 esac
 
 # These are the libraries checked by Readline.
diff --git a/gdb/windows-termcap.c b/gdb/windows-termcap.c
index 026c3d2..b366f65 100644
--- a/gdb/windows-termcap.c
+++ b/gdb/windows-termcap.c
@@ -19,6 +19,11 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+
+#include "defs.h"
+
+#if !defined HAVE_CURSES_H && !defined HAVE_NCURSES_H && !defined HAVE_NCURSES_NCURSES_H
+
 #include <stdlib.h>
 
 /* -Wmissing-prototypes */
@@ -71,3 +76,5 @@
 {
   return NULL;
 }
+
+#endif	/* !HAVE_CURSES_H && !HAVE_NCURSES_H && !HAVE_NCURSES_NCURSES_H */
diff --git a/libiberty/ChangeLog b/libiberty/ChangeLog
index ac23875..832ce0b 100644
--- a/libiberty/ChangeLog
+++ b/libiberty/ChangeLog
@@ -1,3 +1,8 @@
+2015-01-17  Eli Zaretskii  <eliz@gnu.org>
+
+	* strerror.c <sys_nerr, sys_errlist>: Declare only if they aren't
+	macros.
+
 2014-12-24  Uros Bizjak  <ubizjak@gmail.com>
 	    Ben Elliston  <bje@au.ibm.com>
 	    Manuel Lopez-Ibanez  <manu@gcc.gnu.org>
diff --git a/libiberty/strerror.c b/libiberty/strerror.c
index 0efadc3..a8a0bd1 100644
--- a/libiberty/strerror.c
+++ b/libiberty/strerror.c
@@ -469,8 +469,13 @@ struct error_info
 
 #else
 
+
+#ifndef sys_nerr
 extern int sys_nerr;
+#endif
+#ifndef sys_errlist
 extern char *sys_errlist[];
+#endif
 
 #endif
 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHSET] [4/4] Fix various issue in TUI
  2015-01-17  9:55     ` Eli Zaretskii
@ 2015-01-19 14:50       ` Joel Brobecker
  2015-01-19 16:26         ` Eli Zaretskii
  2015-01-19 15:43       ` Joel Brobecker
  2015-01-22 12:07       ` Pedro Alves
  2 siblings, 1 reply; 15+ messages in thread
From: Joel Brobecker @ 2015-01-19 14:50 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: palves, gdb-patches

> I just realized that I cannot push this changeset because Autoconf I
> have installed isn't version 2.64, and so I cannot regenerate
> 'configure'.  Could someone please do that for me?  The patch is
> below.
> 
> Or I could push the patch below, and then someone else could do the
> generated files as a separate commit.

I will take care of it all.

-- 
Joel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHSET] [4/4] Fix various issue in TUI
  2015-01-17  9:55     ` Eli Zaretskii
  2015-01-19 14:50       ` Joel Brobecker
@ 2015-01-19 15:43       ` Joel Brobecker
  2015-01-19 17:31         ` Eli Zaretskii
  2015-01-22 12:07       ` Pedro Alves
  2 siblings, 1 reply; 15+ messages in thread
From: Joel Brobecker @ 2015-01-19 15:43 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: palves, gdb-patches

> I just realized that I cannot push this changeset because Autoconf I
> have installed isn't version 2.64, and so I cannot regenerate
> 'configure'.  Could someone please do that for me?  The patch is
> below.
> 
> Or I could push the patch below, and then someone else could do the
> generated files as a separate commit.
> 
> Thanks in advance.
> 
> 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.

I pushed this patch after having regenerated the configure script.
I edited the ChangeLog entry to add a line for configure being
regenerated.

> --- a/libiberty/ChangeLog
> +++ b/libiberty/ChangeLog
> @@ -1,3 +1,8 @@
> +2015-01-17  Eli Zaretskii  <eliz@gnu.org>
> +
> +	* strerror.c <sys_nerr, sys_errlist>: Declare only if they aren't
> +	macros.

I pushed this change separately (and first).

To answer your question, such changes should be pushed to both
the GCC repository (SVN), and the binutils-gdb repository (git).

The commits are now in master; I can't remember whether these were
approved for 7.9, but if they were, then you can cherry-pick them
from master.

-- 
Joel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHSET] [4/4] Fix various issue in TUI
  2015-01-19 14:50       ` Joel Brobecker
@ 2015-01-19 16:26         ` Eli Zaretskii
  2015-01-19 16:54           ` Joel Brobecker
  0 siblings, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2015-01-19 16:26 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: palves, gdb-patches

> Date: Mon, 19 Jan 2015 15:50:40 +0100
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: palves@redhat.com, gdb-patches@sourceware.org
> 
> > I just realized that I cannot push this changeset because Autoconf I
> > have installed isn't version 2.64, and so I cannot regenerate
> > 'configure'.  Could someone please do that for me?  The patch is
> > below.
> > 
> > Or I could push the patch below, and then someone else could do the
> > generated files as a separate commit.
> 
> I will take care of it all.

Thank you.

Btw, what is the reason we insist on that particular version of
Autoconf, and not on "2.64 or later"?  It's a pain to have an old
version installed (already there are packages for which 2.65 is too
old).

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHSET] [4/4] Fix various issue in TUI
  2015-01-19 16:26         ` Eli Zaretskii
@ 2015-01-19 16:54           ` Joel Brobecker
  0 siblings, 0 replies; 15+ messages in thread
From: Joel Brobecker @ 2015-01-19 16:54 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: palves, gdb-patches

> Btw, what is the reason we insist on that particular version of
> Autoconf, and not on "2.64 or later"?  It's a pain to have an old
> version installed (already there are packages for which 2.65 is too
> old).

I am not absolutely certain. I have a feeling from past experience
that upgrading to new versions is not necessarily a big deal, but
something that needs to be done with care, as it might not work
with new versions of configure. I know that I accidently used
a newer version of configuring in the past, and got errors, so
it doesn't always work, at least for me.

But other than that, one of my reasons is that we know it works, and
using the exact version also allows you to generate changes that are
directly related to your changes and only your changes - which makes
reviewing the patch for the generated file a little easier.

-- 
Joel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHSET] [4/4] Fix various issue in TUI
  2015-01-19 15:43       ` Joel Brobecker
@ 2015-01-19 17:31         ` Eli Zaretskii
  2015-01-19 17:54           ` Joel Brobecker
  0 siblings, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2015-01-19 17:31 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: palves, gdb-patches

> Date: Mon, 19 Jan 2015 16:43:01 +0100
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: palves@redhat.com, gdb-patches@sourceware.org
> 
> > 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.
> 
> I pushed this patch after having regenerated the configure script.
> I edited the ChangeLog entry to add a line for configure being
> regenerated.

Thanks!

> > --- a/libiberty/ChangeLog
> > +++ b/libiberty/ChangeLog
> > @@ -1,3 +1,8 @@
> > +2015-01-17  Eli Zaretskii  <eliz@gnu.org>
> > +
> > +	* strerror.c <sys_nerr, sys_errlist>: Declare only if they aren't
> > +	macros.
> 
> I pushed this change separately (and first).

Thanks again.

> To answer your question, such changes should be pushed to both
> the GCC repository (SVN), and the binutils-gdb repository (git).

OK, so I presume DJ will commit to the GCC repository?  I don't think
I have write access there.  Or did you do that already?

> The commits are now in master; I can't remember whether these were
> approved for 7.9, but if they were, then you can cherry-pick them
> from master.

Done.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHSET] [4/4] Fix various issue in TUI
  2015-01-19 17:31         ` Eli Zaretskii
@ 2015-01-19 17:54           ` Joel Brobecker
  0 siblings, 0 replies; 15+ messages in thread
From: Joel Brobecker @ 2015-01-19 17:54 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: palves, gdb-patches

> > To answer your question, such changes should be pushed to both
> > the GCC repository (SVN), and the binutils-gdb repository (git).
> 
> OK, so I presume DJ will commit to the GCC repository?  I don't think
> I have write access there.  Or did you do that already?

Sorry - I wasn't clear. I did push it to gcc.

-- 
Joel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHSET] [4/4] Fix various issue in TUI
  2015-01-17  9:55     ` Eli Zaretskii
  2015-01-19 14:50       ` Joel Brobecker
  2015-01-19 15:43       ` Joel Brobecker
@ 2015-01-22 12:07       ` Pedro Alves
  2015-01-22 16:26         ` Eli Zaretskii
  2 siblings, 1 reply; 15+ messages in thread
From: Pedro Alves @ 2015-01-22 12:07 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHSET] [4/4] Fix various issue in TUI
  2015-01-22 12:07       ` Pedro Alves
@ 2015-01-22 16:26         ` Eli Zaretskii
  2015-01-22 17:15           ` Pedro Alves
  0 siblings, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2015-01-22 16:26 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

> Date: Thu, 22 Jan 2015 12:07:26 +0000
> From: Pedro Alves <palves@redhat.com>
> CC: gdb-patches@sourceware.org
> 
> >>> > > 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.

At the very least how about an #error in those conditions?  Otherwise,
any bit-rot (something that happens now and then with the MinGW build)
will silently do the wrong thing.

>  "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 ;;
> 
> ?

I'm not sure.  GDB can be configured --without-tui --with-curses.
Does your logic work then?

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHSET] [4/4] Fix various issue in TUI
  2015-01-22 16:26         ` Eli Zaretskii
@ 2015-01-22 17:15           ` Pedro Alves
  2015-01-22 17:30             ` Eli Zaretskii
  0 siblings, 1 reply; 15+ messages in thread
From: Pedro Alves @ 2015-01-22 17:15 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

On 01/22/2015 04:26 PM, Eli Zaretskii wrote:
>> Date: Thu, 22 Jan 2015 12:07:26 +0000
>> From: Pedro Alves <palves@redhat.com>
>> CC: gdb-patches@sourceware.org
>>
>>>>>>> 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.
> 
> At the very least how about an #error in those conditions?  Otherwise,
> any bit-rot (something that happens now and then with the MinGW build)
> will silently do the wrong thing.

Well, I fear that the #ifdefery can bit rot just as well.  So having
two places that can bitrot (configure and #ifdef) seems worse than
one.

>>
>> 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 ;;
>>
>> ?
> 
> I'm not sure.  GDB can be configured --without-tui --with-curses.
> Does your logic work then?

It's the same thing.  That sets $prefer_curses:

 opt_curses=no
 AC_ARG_WITH(curses, AS_HELP_STRING([--with-curses], [use the curses library instead of the termcap library]), opt_curses=$withval)

 prefer_curses=no
 if test "$opt_curses" = "yes"; then
   prefer_curses=yes
 fi


But that doesn't mean that curses will be used.  We'll again fall here:

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 if waddstr is not found, meaning curses is not really
available, even though it'd be preferred, $prefer_curses is
yes, but $curses_found is no.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHSET] [4/4] Fix various issue in TUI
  2015-01-22 17:15           ` Pedro Alves
@ 2015-01-22 17:30             ` Eli Zaretskii
  2015-01-22 18:36               ` Pedro Alves
  0 siblings, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2015-01-22 17:30 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

> Date: Thu, 22 Jan 2015 17:15:00 +0000
> From: Pedro Alves <palves@redhat.com>
> CC: gdb-patches@sourceware.org
> 
> >> 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 ;;
> >>
> >> ?
> > 
> > I'm not sure.  GDB can be configured --without-tui --with-curses.
> > Does your logic work then?
> 
> It's the same thing.

Then please install your variant (on the branch as well).  I trust you
know this stuff much better than I do.

Thanks.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHSET] [4/4] Fix various issue in TUI
  2015-01-22 17:30             ` Eli Zaretskii
@ 2015-01-22 18:36               ` Pedro Alves
  0 siblings, 0 replies; 15+ messages in thread
From: Pedro Alves @ 2015-01-22 18:36 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

On 01/22/2015 05:29 PM, Eli Zaretskii wrote:
>> Date: Thu, 22 Jan 2015 17:15:00 +0000
>> From: Pedro Alves <palves@redhat.com>
>> CC: gdb-patches@sourceware.org
>>
>>>> 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 ;;
>>>>
>>>> ?
>>>
>>> I'm not sure.  GDB can be configured --without-tui --with-curses.
>>> Does your logic work then?
>>
>> It's the same thing.
> 
> Then please install your variant (on the branch as well).  I trust you
> know this stuff much better than I do.

And indeed, I tried cross compiling current mainline mingw gdb,
on my fedora 20 box, and it's broken:

...
object-run.o compile-loc2c.o compile-c-support.o inflow.o    init.o \
           ../readline/libreadline.a ../opcodes/libopcodes.a ../bfd/libbfd.a ./../intl/libintl.a ../libiberty/libiberty.a ../libdecnumber/libdecnumber.a    -lm /home/pedro/src/expat/install-win64/lib/libexpat.a   ../libiberty/libiberty.a -lws2_32 build-gnulib/import/libgnu.a 
utils.o: In function `init_page_info':
/home/pedro/gdb/mygit/build-mingw/gdb/../../src/gdb/utils.c:1683: undefined reference to `tgetnum'
../readline/libreadline.a(display.o): In function `rl_redisplay':
/home/pedro/gdb/mygit/build-mingw/readline/../../src/readline/display.c:1087: undefined reference to `tputs'
../readline/libreadline.a(display.o): In function `update_line':
/home/pedro/gdb/mygit/build-mingw/readline/../../src/readline/display.c:1528: undefined reference to `tputs'
../readline/libreadline.a(display.o): In function `_rl_move_cursor_relative':
/home/pedro/gdb/mygit/build-mingw/readline/../../src/readline/display.c:1985: undefined reference to `tputs'
../readline/libreadline.a(display.o): In function `_rl_move_vert':
/home/pedro/gdb/mygit/build-mingw/readline/../../src/readline/display.c:2057: undefined reference to `tputs'
/home/pedro/gdb/mygit/build-mingw/readline/../../src/readline/display.c:2073: undefined reference to `tputs'
../readline/libreadline.a(display.o):/home/pedro/gdb/mygit/build-mingw/readline/../../src/readline/display.c:2336: more undefined references to `tputs' follow
../readline/libreadline.a(terminal.o): In function `_rl_get_screen_size':
/home/pedro/gdb/mygit/build-mingw/readline/../../src/readline/terminal.c:278: undefined reference to `tgetnum'
/home/pedro/gdb/mygit/build-mingw/readline/../../src/readline/terminal.c:297: undefined reference to `tgetnum'
../readline/libreadline.a(terminal.o): In function `get_term_capabilities':
/home/pedro/gdb/mygit/build-mingw/readline/../../src/readline/terminal.c:427: undefined reference to `tgetstr'
../readline/libreadline.a(terminal.o): In function `_rl_init_terminal_io':
/home/pedro/gdb/mygit/build-mingw/readline/../../src/readline/terminal.c:481: undefined reference to `tgetent'
/home/pedro/gdb/mygit/build-mingw/readline/../../src/readline/terminal.c:545: undefined reference to `tgetflag'
/home/pedro/gdb/mygit/build-mingw/readline/../../src/readline/terminal.c:545: undefined reference to `tgetflag'
/home/pedro/gdb/mygit/build-mingw/readline/../../src/readline/terminal.c:560: undefined reference to `tgetflag'
../readline/libreadline.a(terminal.o): In function `_rl_backspace':
/home/pedro/gdb/mygit/build-mingw/readline/../../src/readline/terminal.c:665: undefined reference to `tputs'
../readline/libreadline.a(terminal.o): In function `rl_ding':
/home/pedro/gdb/mygit/build-mingw/readline/../../src/readline/terminal.c:703: undefined reference to `tputs'
../readline/libreadline.a(terminal.o): In function `_rl_enable_meta_key':
/home/pedro/gdb/mygit/build-mingw/readline/../../src/readline/terminal.c:729: undefined reference to `tputs'
../readline/libreadline.a(terminal.o): In function `_rl_control_keypad':
/home/pedro/gdb/mygit/build-mingw/readline/../../src/readline/terminal.c:739: undefined reference to `tputs'
/home/pedro/gdb/mygit/build-mingw/readline/../../src/readline/terminal.c:741: undefined reference to `tputs'
../readline/libreadline.a(terminal.o):/home/pedro/gdb/mygit/build-mingw/readline/../../src/readline/terminal.c:765: more undefined references to `tputs' follow
collect2: error: ld returned 1 exit status
make: *** [gdb.exe] Error 1

That's a plain configure:

 $ ../src/configure --disable-ld --disable-gas --disable-binutils --host=x86_64-w64-mingw32 --target=x86_64-w64-mingw32

and so I end up with $prefer_curses set, because that configure
defaults to build the TUI, but I don't have ncurses in my sysroot.

I'm pushing this.

---
From 03b7960334677d33ee7410f2c819f78820c32024 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Thu, 22 Jan 2015 18:30:01 +0000
Subject: [PATCH] mingw32: fix windows-termcap/curses check

When GDB is configured with "--without-tui --with-curses" or "--with-tui",
$prefer_curses is set to yes.  But, that still doesn't mean that curses
will be used.  configure will still search for the curses library, and
continue building without it.  That's done here:

 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

So if waddstr is not found, meaning curses is not really
available, even though it'd be preferred, $prefer_curses is
'yes', but $curses_found is 'no'.

So the right fix to tell whether we're linking with curses is
$curses_found=yes.

gdb/ChangeLog:
2015-01-22  Pedro Alves  <palves@redhat.com>

	* configure.ac [*mingw32*]: Check $curses_found instead of
	$prefer_curses.
	* configure: Regenerate.
	* windows-termcap.c: Remove HAVE_CURSES_H, HAVE_NCURSES_H and
	HAVE_NCURSES_NCURSES_H checks.
---
 gdb/ChangeLog         | 8 ++++++++
 gdb/configure         | 2 +-
 gdb/configure.ac      | 2 +-
 gdb/windows-termcap.c | 4 ----
 4 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a6ba992..6e3e258 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2015-01-22  Pedro Alves  <palves@redhat.com>
+
+	* configure.ac [*mingw32*]: Check $curses_found instead of
+	$prefer_curses.
+	* configure: Regenerate.
+	* windows-termcap.c: Remove HAVE_CURSES_H, HAVE_NCURSES_H and
+	HAVE_NCURSES_NCURSES_H checks.
+
 2015-01-22  Eli Zaretskii  <eliz@gnu.org>
 
 	* gdb/tui/tui.c (tui_enable) [__MINGW32__]: If the call to 'newterm'
diff --git a/gdb/configure b/gdb/configure
index fb2480c..9632f9a 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -7188,7 +7188,7 @@ case $host_os in
     ac_cv_search_tgetent="none required"
     ;;
   *mingw32*)
-    if test x"$prefer_curses" != xyes; then
+    if test x"$curses_found" != xyes; then
       ac_cv_search_tgetent="none required"
       CONFIG_OBS="$CONFIG_OBS windows-termcap.o"
     fi ;;
diff --git a/gdb/configure.ac b/gdb/configure.ac
index 36a74d2..dfc6947 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -611,7 +611,7 @@ case $host_os in
     ac_cv_search_tgetent="none required"
     ;;
   *mingw32*)
-    if test x"$prefer_curses" != xyes; then
+    if test x"$curses_found" != xyes; then
       ac_cv_search_tgetent="none required"
       CONFIG_OBS="$CONFIG_OBS windows-termcap.o"
     fi ;;
diff --git a/gdb/windows-termcap.c b/gdb/windows-termcap.c
index 0154083..caafc47 100644
--- a/gdb/windows-termcap.c
+++ b/gdb/windows-termcap.c
@@ -22,8 +22,6 @@
 
 #include "defs.h"
 
-#if !defined HAVE_CURSES_H && !defined HAVE_NCURSES_H && !defined HAVE_NCURSES_NCURSES_H
-
 #include <stdlib.h>
 
 /* -Wmissing-prototypes */
@@ -76,5 +74,3 @@ tgoto (const char *cap, int col, int row)
 {
   return NULL;
 }
-
-#endif	/* !HAVE_CURSES_H && !HAVE_NCURSES_H && !HAVE_NCURSES_NCURSES_H */
-- 
1.9.3


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2015-01-22 18:36 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-31 17:56 [PATCHSET] [4/4] Fix various issue in TUI Eli Zaretskii
2015-01-05 19:55 ` Pedro Alves
2015-01-05 20:06   ` Eli Zaretskii
2015-01-17  9:55     ` Eli Zaretskii
2015-01-19 14:50       ` Joel Brobecker
2015-01-19 16:26         ` Eli Zaretskii
2015-01-19 16:54           ` Joel Brobecker
2015-01-19 15:43       ` Joel Brobecker
2015-01-19 17:31         ` Eli Zaretskii
2015-01-19 17:54           ` Joel Brobecker
2015-01-22 12:07       ` Pedro Alves
2015-01-22 16:26         ` Eli Zaretskii
2015-01-22 17:15           ` Pedro Alves
2015-01-22 17:30             ` Eli Zaretskii
2015-01-22 18:36               ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).