[PATCH] gdb, gdbsupport: configure: factor out yes/no/auto value checking

[PATCH] gdb, gdbsupport: configure: factor out yes/no/auto value checking

[Simon Marchi via Gdb-patches]

From: Simon Marchi <simon.marchi@efficios.com>

Factor out the code that checks that a value is yes/no or yes/no/auto.
Add two macros to gdbsupport/common.m4 and use them in gdb/configure.ac

I inspected the changes to configure.  Other than whitespace changes, we
have some benign changes to the error messages (one of them had an error
actually).  There are changes to the --enable-source-highlight and
--enable-libbacktrace handling, but setting enable_source_highlight /
enable_libbacktrace was not really useful anyway, they already had the
right value.

Change-Id: I92587aec36874309e1605e2d60244649f09a757a
---
 gdb/configure        | 96 ++++++++++++++++++++++++++------------------
 gdb/configure.ac     | 84 +++++++++++++-------------------------
 gdbsupport/common.m4 | 28 +++++++++++++
 3 files changed, 115 insertions(+), 93 deletions(-)

diff --git a/gdb/configure b/gdb/configure
index 4b5e031bff90..75af07bc6a08 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -6817,12 +6817,15 @@ fi
 # Enable MI.
 # Check whether --enable-gdbmi was given.
 if test "${enable_gdbmi+set}" = set; then :
-  enableval=$enable_gdbmi; case $enableval in
-    yes | no)
-      ;;
-    *)
-      as_fn_error $? "bad value $enableval for --enable-gdbmi" "$LINENO" 5 ;;
-  esac
+  enableval=$enable_gdbmi;
+	   case $enableval in
+	     yes | no)
+	       ;;
+	     *)
+	       as_fn_error $? "bad value $enableval for --enable-gdbmi" "$LINENO" 5
+	       ;;
+	   esac
+
 else
   enable_gdbmi=yes
 fi
@@ -6839,12 +6842,15 @@ fi
 # Enable TUI.
 # Check whether --enable-tui was given.
 if test "${enable_tui+set}" = set; then :
-  enableval=$enable_tui; case $enableval in
-    yes | no | auto)
-      ;;
-    *)
-      as_fn_error $? "bad value $enableval for --enable-tui" "$LINENO" 5 ;;
-  esac
+  enableval=$enable_tui;
+	   case $enableval in
+	     yes | no | auto)
+	       ;;
+	     *)
+	       as_fn_error $? "bad value $enableval for --enable-tui" "$LINENO" 5
+	       ;;
+	   esac
+
 else
   enable_tui=auto
 fi
@@ -6853,18 +6859,21 @@ fi
 # Enable gdbtk.
 # Check whether --enable-gdbtk was given.
 if test "${enable_gdbtk+set}" = set; then :
-  enableval=$enable_gdbtk; case $enableval in
-    yes | no)
-      ;;
-    *)
-      as_fn_error $? "bad value $enableval for --enable-gdbtk" "$LINENO" 5 ;;
-  esac
+  enableval=$enable_gdbtk;
+	   case $enableval in
+	     yes | no)
+	       ;;
+	     *)
+	       as_fn_error $? "bad value $enableval for --enable-gdbtk" "$LINENO" 5
+	       ;;
+	   esac
+
 else
   if test -d "$srcdir/gdbtk"; then
-    enable_gdbtk=yes
-  else
-    enable_gdbtk=no
-  fi
+		 enable_gdbtk=yes
+	       else
+		 enable_gdbtk=no
+	       fi
 fi
 
 # We unconditionally disable gdbtk tests on selected platforms.
@@ -7205,12 +7214,15 @@ fi
 # Profiling support.
 # Check whether --enable-profiling was given.
 if test "${enable_profiling+set}" = set; then :
-  enableval=$enable_profiling; case $enableval in
-    yes | no)
-      ;;
-    *)
-      as_fn_error $? "bad value $enableval for --enable-profile" "$LINENO" 5 ;;
-  esac
+  enableval=$enable_profiling;
+	   case $enableval in
+	     yes | no)
+	       ;;
+	     *)
+	       as_fn_error $? "bad value $enableval for --enable-profiling" "$LINENO" 5
+	       ;;
+	   esac
+
 else
   enable_profiling=no
 fi
@@ -12039,11 +12051,15 @@ SRCHIGH_CFLAGS=
 
 # Check whether --enable-source-highlight was given.
 if test "${enable_source_highlight+set}" = set; then :
-  enableval=$enable_source_highlight; case "${enableval}" in
-  yes)  enable_source_highlight=yes ;;
-  no)   enable_source_highlight=no  ;;
-  *)    as_fn_error $? "bad value ${enableval} for source-highlight option" "$LINENO" 5 ;;
-esac
+  enableval=$enable_source_highlight;
+	   case $enableval in
+	     yes | no)
+	       ;;
+	     *)
+	       as_fn_error $? "bad value $enableval for --enable-source-hightlight" "$LINENO" 5
+	       ;;
+	   esac
+
 else
   enable_source_highlight=auto
 fi
@@ -18756,11 +18772,15 @@ fi
 # Setup possible use of libbacktrace.
 # Check whether --enable-libbacktrace was given.
 if test "${enable_libbacktrace+set}" = set; then :
-  enableval=$enable_libbacktrace; case "${enableval}" in
-  yes)  enable_libbacktrace=yes ;;
-  no)   enable_libbacktrace=no  ;;
-  *)    as_fn_error $? "bad value ${enableval} for --enable-libbacktrace option" "$LINENO" 5 ;;
-esac
+  enableval=$enable_libbacktrace;
+	   case $enableval in
+	     yes | no)
+	       ;;
+	     *)
+	       as_fn_error $? "bad value $enableval for --enable-libbacktrace" "$LINENO" 5
+	       ;;
+	   esac
+
 else
   enable_libbacktrace=yes
 fi
diff --git a/gdb/configure.ac b/gdb/configure.ac
index b681988d7a4f..60d1dddfb671 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -259,15 +259,10 @@ if test "x$targ_defvec" != x; then
 fi
 
 # Enable MI.
-AC_ARG_ENABLE(gdbmi,
-AS_HELP_STRING([--disable-gdbmi], [disable machine-interface (MI)]),
-  [case $enableval in
-    yes | no)
-      ;;
-    *)
-      AC_MSG_ERROR([bad value $enableval for --enable-gdbmi]) ;;
-  esac],
-  [enable_gdbmi=yes])
+AC_ARG_ENABLE([gdbmi],
+	      [AS_HELP_STRING([--disable-gdbmi], [disable machine-interface (MI)])],
+	      [GDB_CHECK_YES_NO_VAL([$enableval], [--enable-gdbmi])],
+	      [enable_gdbmi=yes])
 if test x"$enable_gdbmi" = xyes; then
   if test -d "$srcdir/mi"; then
     CONFIG_OBS="$CONFIG_OBS \$(SUBDIR_MI_OBS)"
@@ -279,28 +274,20 @@ fi
 
 # 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)
+AS_HELP_STRING([--enable-tui],
+	       [enable full-screen terminal user interface (TUI)]),
+	       [GDB_CHECK_YES_NO_AUTO_VAL([$enableval], [--enable-tui])],
+	       [enable_tui=auto])
 
 # Enable gdbtk.
-AC_ARG_ENABLE(gdbtk,
-AS_HELP_STRING([--enable-gdbtk], [enable gdbtk graphical user interface (GUI)]),
-  [case $enableval in
-    yes | no)
-      ;;
-    *)
-      AC_MSG_ERROR([bad value $enableval for --enable-gdbtk]) ;;
-  esac],
-  [if test -d "$srcdir/gdbtk"; then
-    enable_gdbtk=yes
-  else
-    enable_gdbtk=no
-  fi])
+AC_ARG_ENABLE([gdbtk],
+	      [AS_HELP_STRING([--enable-gdbtk], [enable gdbtk graphical user interface (GUI)])],
+	      [GDB_CHECK_YES_NO_VAL([$enableval], [--enable-gdbtk])],
+	      [if test -d "$srcdir/gdbtk"; then
+		 enable_gdbtk=yes
+	       else
+		 enable_gdbtk=no
+	       fi])
 # We unconditionally disable gdbtk tests on selected platforms.
 case $host_os in
   go32* | windows*)
@@ -359,15 +346,10 @@ if test "$opt_curses" = "yes"; then
 fi
 
 # Profiling support.
-AC_ARG_ENABLE(profiling,
-AS_HELP_STRING([--enable-profiling], [enable profiling of GDB]),
-  [case $enableval in
-    yes | no)
-      ;;
-    *)
-      AC_MSG_ERROR([bad value $enableval for --enable-profile]) ;;
-  esac],
- [enable_profiling=no])
+AC_ARG_ENABLE([profiling],
+	      [AS_HELP_STRING([--enable-profiling], [enable profiling of GDB])],
+	      [GDB_CHECK_YES_NO_VAL([$enableval], [--enable-profiling])],
+	      [enable_profiling=no])
 
 AC_CHECK_FUNCS(monstartup _mcleanup)
 AC_CACHE_CHECK(
@@ -1197,15 +1179,11 @@ AM_CONDITIONAL(HAVE_GUILE, test "${have_libguile}" != no)
 SRCHIGH_LIBS=
 SRCHIGH_CFLAGS=
 
-AC_ARG_ENABLE(source-highlight,
-  AS_HELP_STRING([--enable-source-highlight],
-    [enable source-highlight for source listings]),
-  [case "${enableval}" in
-  yes)  enable_source_highlight=yes ;;
-  no)   enable_source_highlight=no  ;;
-  *)    AC_MSG_ERROR(bad value ${enableval} for source-highlight option) ;;
-esac],
-[enable_source_highlight=auto])
+AC_ARG_ENABLE([source-highlight],
+	      [AS_HELP_STRING([--enable-source-highlight],
+			      [enable source-highlight for source listings])],
+	      [GDB_CHECK_YES_NO_VAL([$enableval], [--enable-source-highlight])],
+	      [enable_source_highlight=auto])
 
 if test "${enable_source_highlight}" != "no"; then
   AC_MSG_CHECKING([for the source-highlight library])
@@ -2105,14 +2083,10 @@ fi
 
 # Setup possible use of libbacktrace.
 AC_ARG_ENABLE([libbacktrace],
-[AS_HELP_STRING([--enable-libbacktrace],
-                [use libbacktrace to write a backtrace after a fatal signal.])],
-[case "${enableval}" in
-  yes)  enable_libbacktrace=yes ;;
-  no)   enable_libbacktrace=no  ;;
-  *)    AC_MSG_ERROR(bad value ${enableval} for --enable-libbacktrace option) ;;
-esac],
-enable_libbacktrace=yes)
+	      [AS_HELP_STRING([--enable-libbacktrace],
+			      [use libbacktrace to write a backtrace after a fatal signal.])],
+	      [GDB_CHECK_YES_NO_VAL([$enableval], [--enable-libbacktrace])],
+	      [enable_libbacktrace=yes])
 
 if test "${enable_libbacktrace}" = "yes"; then
   LIBBACKTRACE_INC="-I$srcdir/../libbacktrace/ -I../libbacktrace/"
diff --git a/gdbsupport/common.m4 b/gdbsupport/common.m4
index d3e9ecbe8238..0fed186ae1d9 100644
--- a/gdbsupport/common.m4
+++ b/gdbsupport/common.m4
@@ -216,3 +216,31 @@ AC_DEFUN([GDB_AC_COMMON], [
     BFD_HAVE_SYS_PROCFS_TYPE(elf_fpregset_t)
   fi
 ])
+
+dnl Check that the provided value ($1) is either "yes" or "no".  If not,
+dnl emit an error message mentionning the configure option $2, and abort
+dnl the script.
+AC_DEFUN([GDB_CHECK_YES_NO_VAL],
+	 [
+	   case $1 in
+	     yes | no)
+	       ;;
+	     *)
+	       AC_MSG_ERROR([bad value $1 for $2])
+	       ;;
+	   esac
+	  ])
+
+dnl Check that the provided value ($1) is either "yes", "no" or "auto".  If not,
+dnl emit an error message mentionning the configure option $2, and abort
+dnl the script.
+AC_DEFUN([GDB_CHECK_YES_NO_AUTO_VAL],
+	 [
+	   case $1 in
+	     yes | no | auto)
+	       ;;
+	     *)
+	       AC_MSG_ERROR([bad value $1 for $2])
+	       ;;
+	   esac
+	  ])

base-commit: 02d04eac24f497e34ac9869bac5bfb1e044a6b62
prerequisite-patch-id: cbaa81d72f895c0a42b3995b663c1e2d941b6feb
-- 
2.37.1

Re: [PATCH] gdb, gdbsupport: configure: factor out yes/no/auto value checking

[Tom Tromey]

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> From: Simon Marchi <simon.marchi@efficios.com>
Simon> Factor out the code that checks that a value is yes/no or yes/no/auto.
Simon> Add two macros to gdbsupport/common.m4 and use them in gdb/configure.ac

Simon> I inspected the changes to configure.  Other than whitespace changes, we
Simon> have some benign changes to the error messages (one of them had an error
Simon> actually).  There are changes to the --enable-source-highlight and
Simon> --enable-libbacktrace handling, but setting enable_source_highlight /
Simon> enable_libbacktrace was not really useful anyway, they already had the
Simon> right value.

This looks reasonable to me.

Simon> +AC_ARG_ENABLE([gdbmi],

I was very surprised to learn today that this really can be disabled.

Simon> -AC_ARG_ENABLE(source-highlight,
Simon> -  AS_HELP_STRING([--enable-source-highlight],
Simon> -    [enable source-highlight for source listings]),
Simon> -  [case "${enableval}" in
Simon> -  yes)  enable_source_highlight=yes ;;
Simon> -  no)   enable_source_highlight=no  ;;
Simon> -  *)    AC_MSG_ERROR(bad value ${enableval} for source-highlight option) ;;
Simon> -esac],
Simon> -[enable_source_highlight=auto])
Simon> +AC_ARG_ENABLE([source-highlight],
Simon> +	      [AS_HELP_STRING([--enable-source-highlight],
Simon> +			      [enable source-highlight for source listings])],
Simon> +	      [GDB_CHECK_YES_NO_VAL([$enableval], [--enable-source-highlight])],
Simon> +	      [enable_source_highlight=auto])

This defaults to 'auto' but, weirdly, doesn't let one explicitly specify
'auto'.  I wonder if switching it to GDB_CHECK_YES_NO_AUTO_VAL would be
an improvement.

Tom

Re: [PATCH] gdb, gdbsupport: configure: factor out yes/no/auto value checking

[Simon Marchi via Gdb-patches]

On 2022-08-23 15:17, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> From: Simon Marchi <simon.marchi@efficios.com>
> Simon> Factor out the code that checks that a value is yes/no or yes/no/auto.
> Simon> Add two macros to gdbsupport/common.m4 and use them in gdb/configure.ac
> 
> Simon> I inspected the changes to configure.  Other than whitespace changes, we
> Simon> have some benign changes to the error messages (one of them had an error
> Simon> actually).  There are changes to the --enable-source-highlight and
> Simon> --enable-libbacktrace handling, but setting enable_source_highlight /
> Simon> enable_libbacktrace was not really useful anyway, they already had the
> Simon> right value.
> 
> This looks reasonable to me.

Ok.

> Simon> +AC_ARG_ENABLE([gdbmi],
> 
> I was very surprised to learn today that this really can be disabled.

I don't really know why that is useful.  There is no dependency on any
external library, so there's no disadvantage (other than a bit of space
saving) of having it enabled.  The only real-world usage I could find is
in an ancient Builroot version, but it was removed here in 2007:

  https://github.com/buildroot/buildroot/commit/8759a416c41f8eec02fdb5594275961534ae0c8e#diff-1ba8fa6a02f11bff1ad45de1341ee93aee241a6281298da9ad9721d38246fc91

I just tried a local build with --disable-gdbmi (which has Python
implicitly enabled), and I get:

  /usr/bin/ld: python/py-micmd.o: in function `mi_command_py::invoke(mi_parse*) const':
  /home/simark/src/binutils-gdb/gdb/python/py-micmd.c:343: undefined reference to `mi_parse_argv(char const*, mi_parse*)'

So this new "MI commands in Python feature" should be dependent on
whether MI is enabled.  But I would also support removing the
--disable-gdbmi switch, that would be simpler.

> Simon> -AC_ARG_ENABLE(source-highlight,
> Simon> -  AS_HELP_STRING([--enable-source-highlight],
> Simon> -    [enable source-highlight for source listings]),
> Simon> -  [case "${enableval}" in
> Simon> -  yes)  enable_source_highlight=yes ;;
> Simon> -  no)   enable_source_highlight=no  ;;
> Simon> -  *)    AC_MSG_ERROR(bad value ${enableval} for source-highlight option) ;;
> Simon> -esac],
> Simon> -[enable_source_highlight=auto])
> Simon> +AC_ARG_ENABLE([source-highlight],
> Simon> +	      [AS_HELP_STRING([--enable-source-highlight],
> Simon> +			      [enable source-highlight for source listings])],
> Simon> +	      [GDB_CHECK_YES_NO_VAL([$enableval], [--enable-source-highlight])],
> Simon> +	      [enable_source_highlight=auto])
> 
> This defaults to 'auto' but, weirdly, doesn't let one explicitly specify
> 'auto'.  I wonder if switching it to GDB_CHECK_YES_NO_AUTO_VAL would be
> an improvement.

I think that makes sense, I did the change.  I will push the patch with
that fixed.

Thanks,

Simon

Re: [PATCH] gdb, gdbsupport: configure: factor out yes/no/auto value checking

[Tom Tromey]

>> I was very surprised to learn today that this really can be disabled.

Simon> I don't really know why that is useful.  There is no dependency on any
Simon> external library, so there's no disadvantage (other than a bit of space
Simon> saving) of having it enabled.

Yeah.  I assume it's for some kind of minimalist thing, but honestly
that doesn't seem super worthwhile to me.

Simon> I just tried a local build with --disable-gdbmi (which has Python
Simon> implicitly enabled), and I get:

It worked ok for me where I also disabled Python.

Simon> But I would also support removing the
Simon> --disable-gdbmi switch, that would be simpler.

Me too.

Tom