* [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-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-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 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).