| Message ID | 83y4p1zq7k.fsf@gnu.org |
|---|---|
| State | New, archived |
| Headers |
Received: (qmail 26077 invoked by alias); 17 Jan 2015 09:55:13 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 26065 invoked by uid 89); 17 Jan 2015 09:55:12 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_NONE, SPF_SOFTFAIL autolearn=no version=3.3.2 X-HELO: mtaout20.012.net.il Received: from mtaout20.012.net.il (HELO mtaout20.012.net.il) (80.179.55.166) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 17 Jan 2015 09:55:10 +0000 Received: from conversion-daemon.a-mtaout20.012.net.il by a-mtaout20.012.net.il (HyperSendmail v2007.08) id <0NIB00100F3JJM00@a-mtaout20.012.net.il> for gdb-patches@sourceware.org; Sat, 17 Jan 2015 11:55:07 +0200 (IST) Received: from HOME-C4E4A596F7 ([87.69.4.28]) by a-mtaout20.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0NIB001P7FJVEV20@a-mtaout20.012.net.il>; Sat, 17 Jan 2015 11:55:07 +0200 (IST) Date: Sat, 17 Jan 2015 11:55:27 +0200 From: Eli Zaretskii <eliz@gnu.org> Subject: Re: [PATCHSET] [4/4] Fix various issue in TUI In-reply-to: <83bnmd8028.fsf@gnu.org> To: palves@redhat.com Cc: gdb-patches@sourceware.org Reply-to: Eli Zaretskii <eliz@gnu.org> Message-id: <83y4p1zq7k.fsf@gnu.org> References: <83vbkrbt4d.fsf@gnu.org> <54AAEC02.7020500@redhat.com> <83bnmd8028.fsf@gnu.org> X-IsSubscribed: yes |
Commit Message
Eli Zaretskii
Jan. 17, 2015, 9:55 a.m. UTC
> 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.
Comments
> 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.
> 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.
> 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).
> 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.
> 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.
> > 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.
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
> 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?
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
> 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.
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