[PATCH] gdb: add --with-curses to --configuration output

[PATCH] gdb: add --with-curses to --configuration output

[Philippe Blain]

'gdb --configuration' does not mention if GDB was built with curses.
Since b5075fb68d4 (Rename to allow_tui_tests, 2023-01-08) it does show
--enable-tui (or --disable-tui), but one might want to know if GDB was
built with curses independently of the availability of the TUI.

Since configure.ac uses AC_SEARCH_LIBS to check for the curses library,
we do not get an automatically defined HAVE_LIBCURSES symbol in
config.in. We do have symbols defined by AC_CHECK_HEADERS
(HAVE_CURSES_H, etc.) but it would be cumbersome to use those in
print_gdb_configuration because we would have to check for all 6 symbols
corresponding the 6 headers listed. This would also increase the
maintenance burden if support for other variations of curses are added.

Instead, define 'HAVE_LIBCURSES' ourselves by adding an
'action-if-found' argument to AC_SEARCH_LIBS, and use it in
print_gdb_configuration.

---
I was building GDB 12 and noticed that '--configuration' did not show if it was
built with curses. I see that on master it now shows '--enable|disable-tui',
which is what I wanted to know in the first place, but I had already written
this patch so I figured I might as well send it :)

I built tested this on x86_64-pc-linux-gnu, with both --with-curses and
--without-curses, and also ran the test suite before and after the patch. The
patch does not seem to influence the test results.

I'm including the generated configure and config.in since it seems (from
browsing the list) this is what most people do, but I'm noticing that the
"DeveloperTips" page on the wiki [1] mentions to _not_ include these changes
when sending the patch. Maybe the page should be updated ? I did not do it
since I was not sure...

Cheers,
Philippe.

P.S. I do not have push access.
P.P.S. I did sign the FSF copyright assignment :)
P.P.P.S. I'm trying out 'b4 send' [2], I hope the patch will end up OK :)

[1] https://sourceware.org/gdb/wiki/DeveloperTips#Editing_configure.ac
[2] https://b4.docs.kernel.org/en/latest/contributor/overview.html
---
 gdb/config.in    |  3 +++
 gdb/configure    |  3 +++
 gdb/configure.ac |  5 ++++-
 gdb/top.c        | 10 ++++++++++
 4 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/gdb/config.in b/gdb/config.in
index 7da131ebf04..54110bc9043 100644
--- a/gdb/config.in
+++ b/gdb/config.in
@@ -235,6 +235,9 @@
 /* Define if libbacktrace is being used. */
 #undef HAVE_LIBBACKTRACE
 
+/* Define to 1 if curses is enabled. */
+#undef HAVE_LIBCURSES
+
 /* Define to 1 if debuginfod is enabled. */
 #undef HAVE_LIBDEBUGINFOD
 
diff --git a/gdb/configure b/gdb/configure
index 113b7cf8a30..2aafd2097c8 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -20621,6 +20621,9 @@ ac_res=$ac_cv_search_waddstr
 if test "$ac_res" != no; then :
   test "$ac_res" = "none required" || LIBS="$ac_res $LIBS"
 
+$as_echo "#define HAVE_LIBCURSES 1" >>confdefs.h
+
+
 fi
 
 
diff --git a/gdb/configure.ac b/gdb/configure.ac
index 7c7bf88b3fb..eac1b8f1aba 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -563,7 +563,10 @@ if test x"$prefer_curses" = xyes; then
   # search /usr/local/include, if ncurses is installed in /usr/local.  A
   # default installation of ncurses on alpha*-dec-osf* will lead to such
   # a situation.
-  AC_SEARCH_LIBS(waddstr, [ncursesw ncurses cursesX curses])
+  AC_SEARCH_LIBS(waddstr, [ncursesw ncurses cursesX curses],
+                 [AC_DEFINE([HAVE_LIBCURSES], [1],
+                            [Define to 1 if curses is enabled.])
+                 ])
 
   if test "$ac_cv_search_waddstr" != no; then
     curses_found=yes
diff --git a/gdb/top.c b/gdb/top.c
index 205eb360ba3..18990cf6a73 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -1619,6 +1619,16 @@ This GDB was configured as follows:\n\
 "));
 #endif
 
+#if HAVE_LIBCURSES
+  gdb_printf (stream, _("\
+	     --with-curses\n\
+"));
+#else
+  gdb_printf (stream, _("\
+	     --without-curses\n\
+"));
+#endif
+
 #if HAVE_GUILE
   gdb_printf (stream, _("\
 	     --with-guile\n\

---
base-commit: ec78da9ce6540bdcc2aeb3e01ffdbbac957cbe07
change-id: 20230211-configuration-show-curses-8e06d0df1806
--
b4 

Re: [PATCH] gdb: add --with-curses to --configuration output

[Simon Marchi]

> diff --git a/gdb/configure.ac b/gdb/configure.ac
> index 7c7bf88b3fb..eac1b8f1aba 100644
> --- a/gdb/configure.ac
> +++ b/gdb/configure.ac
> @@ -563,7 +563,10 @@ if test x"$prefer_curses" = xyes; then
>    # search /usr/local/include, if ncurses is installed in /usr/local.  A
>    # default installation of ncurses on alpha*-dec-osf* will lead to such
>    # a situation.
> -  AC_SEARCH_LIBS(waddstr, [ncursesw ncurses cursesX curses])
> +  AC_SEARCH_LIBS(waddstr, [ncursesw ncurses cursesX curses],
> +                 [AC_DEFINE([HAVE_LIBCURSES], [1],
> +                            [Define to 1 if curses is enabled.])
> +                 ])

So we now have:

  AC_SEARCH_LIBS(waddstr, [ncursesw ncurses cursesX curses],
                 [AC_DEFINE([HAVE_LIBCURSES], [1],
                            [Define to 1 if curses is enabled.])
                 ])

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

I think the `if test ...` serves the same purpose as the action-if-found
you used.  Perhaps consolidate both actions to use the same mechanism?
Using the action-if-found parameter seems a bit nicer to me.

Simon

Re: [PATCH] gdb: add --with-curses to --configuration output

[Philippe Blain]

Hi Simon,

Le 2023-02-15 à 13:58, Simon Marchi a écrit :
>> diff --git a/gdb/configure.ac b/gdb/configure.ac
>> index 7c7bf88b3fb..eac1b8f1aba 100644
>> --- a/gdb/configure.ac
>> +++ b/gdb/configure.ac
>> @@ -563,7 +563,10 @@ if test x"$prefer_curses" = xyes; then
>>    # search /usr/local/include, if ncurses is installed in /usr/local.  A
>>    # default installation of ncurses on alpha*-dec-osf* will lead to such
>>    # a situation.
>> -  AC_SEARCH_LIBS(waddstr, [ncursesw ncurses cursesX curses])
>> +  AC_SEARCH_LIBS(waddstr, [ncursesw ncurses cursesX curses],
>> +                 [AC_DEFINE([HAVE_LIBCURSES], [1],
>> +                            [Define to 1 if curses is enabled.])
>> +                 ])
> 
> So we now have:
> 
>   AC_SEARCH_LIBS(waddstr, [ncursesw ncurses cursesX curses],
>                  [AC_DEFINE([HAVE_LIBCURSES], [1],
>                             [Define to 1 if curses is enabled.])
>                  ])
> 
>   if test "$ac_cv_search_waddstr" != no; then
>     curses_found=yes
>   fi
> 
> I think the `if test ...` serves the same purpose as the action-if-found
> you used.  Perhaps consolidate both actions to use the same mechanism?
> Using the action-if-found parameter seems a bit nicer to me.

Yes, this makes sense. I'll do that for v2.

Philippe.

[PATCH v2] gdb: add --with-curses to --configuration output

[Philippe Blain]

'gdb --configuration' does not mention if GDB was built with curses.
Since b5075fb68d4 (Rename to allow_tui_tests, 2023-01-08) it does show
--enable-tui (or --disable-tui), but one might want to know if GDB was
built with curses independently of the availability of the TUI.

Since configure.ac uses AC_SEARCH_LIBS to check for the curses library,
we do not get an automatically defined HAVE_LIBCURSES symbol in
config.in. We do have symbols defined by AC_CHECK_HEADERS
(HAVE_CURSES_H, etc.) but it would be cumbersome to use those in
print_gdb_configuration because we would have to check for all 6 symbols
corresponding the 6 headers listed. This would also increase the
maintenance burden if support for other variations of curses are added.

Instead, define 'HAVE_LIBCURSES' ourselves by adding an
'action-if-found' argument to AC_SEARCH_LIBS, and use it in
print_gdb_configuration.

While at it, remove the condition on 'ac_cv_search_waddstr' and set
'curses_found' directly in 'action-if-found'.

---
Changes in v2:
- set 'curses_found' directly in action-if-found, as suggested by Simon

v1:
I was building GDB 12 and noticed that '--configuration' did not show if it was
built with curses. I see that on master it now shows '--enable|disable-tui',
which is what I wanted to know in the first place, but I had already written
this patch so I figured I might as well send it :)

I built tested this on x86_64-pc-linux-gnu, with both --with-curses and
--without-curses, and also ran the test suite before and after the patch. The
patch does not seem to influence the test results.

I'm including the generated configure and config.in since it seems (from
browsing the list) this is what most people do, but I'm noticing that the
"DeveloperTips" page on the wiki [1] mentions to _not_ include these changes
when sending the patch. Maybe the page should be updated ? I did not do it
since I was not sure...

Cheers,
Philippe.

P.S. I do not have push access.
P.P.S. I did sign the FSF copyright assignment :)
P.P.P.S. I'm trying out 'b4 send' [2], I hope the patch will end up OK :)

[1] https://sourceware.org/gdb/wiki/DeveloperTips#Editing_configure.ac
[2] https://b4.docs.kernel.org/en/latest/contributor/overview.html
---
 gdb/config.in    |  3 +++
 gdb/configure    |  8 ++++----
 gdb/configure.ac | 10 +++++-----
 gdb/top.c        | 10 ++++++++++
 4 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/gdb/config.in b/gdb/config.in
index 7da131ebf04..54110bc9043 100644
--- a/gdb/config.in
+++ b/gdb/config.in
@@ -235,6 +235,9 @@
 /* Define if libbacktrace is being used. */
 #undef HAVE_LIBBACKTRACE
 
+/* Define to 1 if curses is enabled. */
+#undef HAVE_LIBCURSES
+
 /* Define to 1 if debuginfod is enabled. */
 #undef HAVE_LIBDEBUGINFOD
 
diff --git a/gdb/configure b/gdb/configure
index 113b7cf8a30..9be220e28fa 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -20620,13 +20620,13 @@ $as_echo "$ac_cv_search_waddstr" >&6; }
 ac_res=$ac_cv_search_waddstr
 if test "$ac_res" != no; then :
   test "$ac_res" = "none required" || LIBS="$ac_res $LIBS"
+  curses_found=yes &&
 
-fi
+$as_echo "#define HAVE_LIBCURSES 1" >>confdefs.h
 
 
-  if test "$ac_cv_search_waddstr" != no; then
-    curses_found=yes
-  fi
+fi
+
 fi
 
 # Check whether we should enable the TUI, but only do so if we really
diff --git a/gdb/configure.ac b/gdb/configure.ac
index 7c7bf88b3fb..6ad0f9d8815 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -563,11 +563,11 @@ if test x"$prefer_curses" = xyes; then
   # search /usr/local/include, if ncurses is installed in /usr/local.  A
   # default installation of ncurses on alpha*-dec-osf* will lead to such
   # a situation.
-  AC_SEARCH_LIBS(waddstr, [ncursesw ncurses cursesX curses])
-
-  if test "$ac_cv_search_waddstr" != no; then
-    curses_found=yes
-  fi
+  AC_SEARCH_LIBS(waddstr, [ncursesw ncurses cursesX curses],
+                 [curses_found=yes &&
+                  AC_DEFINE([HAVE_LIBCURSES], [1],
+                            [Define to 1 if curses is enabled.])
+                 ])
 fi
 
 # Check whether we should enable the TUI, but only do so if we really
diff --git a/gdb/top.c b/gdb/top.c
index 205eb360ba3..18990cf6a73 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -1619,6 +1619,16 @@ This GDB was configured as follows:\n\
 "));
 #endif
 
+#if HAVE_LIBCURSES
+  gdb_printf (stream, _("\
+	     --with-curses\n\
+"));
+#else
+  gdb_printf (stream, _("\
+	     --without-curses\n\
+"));
+#endif
+
 #if HAVE_GUILE
   gdb_printf (stream, _("\
 	     --with-guile\n\

---
base-commit: ec78da9ce6540bdcc2aeb3e01ffdbbac957cbe07
change-id: 20230211-configuration-show-curses-8e06d0df1806
--
b4 

Re: [PATCH v2] gdb: add --with-curses to --configuration output

[Simon Marchi]

> diff --git a/gdb/configure.ac b/gdb/configure.ac
> index 7c7bf88b3fb..6ad0f9d8815 100644
> --- a/gdb/configure.ac
> +++ b/gdb/configure.ac
> @@ -563,11 +563,11 @@ if test x"$prefer_curses" = xyes; then
>    # search /usr/local/include, if ncurses is installed in /usr/local.  A
>    # default installation of ncurses on alpha*-dec-osf* will lead to such
>    # a situation.
> -  AC_SEARCH_LIBS(waddstr, [ncursesw ncurses cursesX curses])
> -
> -  if test "$ac_cv_search_waddstr" != no; then
> -    curses_found=yes
> -  fi
> +  AC_SEARCH_LIBS(waddstr, [ncursesw ncurses cursesX curses],
> +                 [curses_found=yes &&
> +                  AC_DEFINE([HAVE_LIBCURSES], [1],
> +                            [Define to 1 if curses is enabled.])
> +                 ])

I don't think the `&&` is needed, you can just put each action on its
own line.  It becomes regular shell script in the end.  I tried removing
it on my side, it seems to work fine.  If that work for you too, I can
push the patch with that little change.

Simon

Re: [PATCH v2] gdb: add --with-curses to --configuration output

[Philippe Blain]

Hi Simon,

Le 2023-02-20 à 12:08, Simon Marchi a écrit :
>> diff --git a/gdb/configure.ac b/gdb/configure.ac
>> index 7c7bf88b3fb..6ad0f9d8815 100644
>> --- a/gdb/configure.ac
>> +++ b/gdb/configure.ac
>> @@ -563,11 +563,11 @@ if test x"$prefer_curses" = xyes; then
>>    # search /usr/local/include, if ncurses is installed in /usr/local.  A
>>    # default installation of ncurses on alpha*-dec-osf* will lead to such
>>    # a situation.
>> -  AC_SEARCH_LIBS(waddstr, [ncursesw ncurses cursesX curses])
>> -
>> -  if test "$ac_cv_search_waddstr" != no; then
>> -    curses_found=yes
>> -  fi
>> +  AC_SEARCH_LIBS(waddstr, [ncursesw ncurses cursesX curses],
>> +                 [curses_found=yes &&
>> +                  AC_DEFINE([HAVE_LIBCURSES], [1],
>> +                            [Define to 1 if curses is enabled.])
>> +                 ])
> 
> I don't think the `&&` is needed, you can just put each action on its
> own line.  It becomes regular shell script in the end.  I tried removing
> it on my side, it seems to work fine.  If that work for you too, I can
> push the patch with that little change.

Yes, that sounds good to me. Thanks!

Philippe.

Re: [PATCH v2] gdb: add --with-curses to --configuration output

[Simon Marchi]

On 2/21/23 07:52, Philippe Blain wrote:
> Hi Simon,
> 
> Le 2023-02-20 à 12:08, Simon Marchi a écrit :
>>> diff --git a/gdb/configure.ac b/gdb/configure.ac
>>> index 7c7bf88b3fb..6ad0f9d8815 100644
>>> --- a/gdb/configure.ac
>>> +++ b/gdb/configure.ac
>>> @@ -563,11 +563,11 @@ if test x"$prefer_curses" = xyes; then
>>>    # search /usr/local/include, if ncurses is installed in /usr/local.  A
>>>    # default installation of ncurses on alpha*-dec-osf* will lead to such
>>>    # a situation.
>>> -  AC_SEARCH_LIBS(waddstr, [ncursesw ncurses cursesX curses])
>>> -
>>> -  if test "$ac_cv_search_waddstr" != no; then
>>> -    curses_found=yes
>>> -  fi
>>> +  AC_SEARCH_LIBS(waddstr, [ncursesw ncurses cursesX curses],
>>> +                 [curses_found=yes &&
>>> +                  AC_DEFINE([HAVE_LIBCURSES], [1],
>>> +                            [Define to 1 if curses is enabled.])
>>> +                 ])
>>
>> I don't think the `&&` is needed, you can just put each action on its
>> own line.  It becomes regular shell script in the end.  I tried removing
>> it on my side, it seems to work fine.  If that work for you too, I can
>> push the patch with that little change.
> 
> Yes, that sounds good to me. Thanks!
> 
> Philippe.

Pushed, thanks!

Simon