* [PATCH] gdb, gdbsupport: configure: factor out yes/no/auto value checking
@ 2022-08-17 17:53 Simon Marchi via Gdb-patches
2022-08-23 19:17 ` Tom Tromey
0 siblings, 1 reply; 4+ messages in thread
From: Simon Marchi via Gdb-patches @ 2022-08-17 17:53 UTC (permalink / raw)
To: gdb-patches; +Cc: Simon Marchi
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] gdb, gdbsupport: configure: factor out yes/no/auto value checking
2022-08-17 17:53 [PATCH] gdb, gdbsupport: configure: factor out yes/no/auto value checking Simon Marchi via Gdb-patches
@ 2022-08-23 19:17 ` Tom Tromey
2022-08-26 14:29 ` Simon Marchi via Gdb-patches
0 siblings, 1 reply; 4+ messages in thread
From: Tom Tromey @ 2022-08-23 19:17 UTC (permalink / raw)
To: Simon Marchi via Gdb-patches; +Cc: Simon Marchi
>>>>> "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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] gdb, gdbsupport: configure: factor out yes/no/auto value checking
2022-08-23 19:17 ` Tom Tromey
@ 2022-08-26 14:29 ` Simon Marchi via Gdb-patches
2022-08-29 16:24 ` Tom Tromey
0 siblings, 1 reply; 4+ messages in thread
From: Simon Marchi via Gdb-patches @ 2022-08-26 14:29 UTC (permalink / raw)
To: Tom Tromey, Simon Marchi via Gdb-patches; +Cc: Simon Marchi
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] gdb, gdbsupport: configure: factor out yes/no/auto value checking
2022-08-26 14:29 ` Simon Marchi via Gdb-patches
@ 2022-08-29 16:24 ` Tom Tromey
0 siblings, 0 replies; 4+ messages in thread
From: Tom Tromey @ 2022-08-29 16:24 UTC (permalink / raw)
To: Simon Marchi; +Cc: Simon Marchi, Tom Tromey, Simon Marchi via Gdb-patches
>> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-08-29 16:26 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-17 17:53 [PATCH] gdb, gdbsupport: configure: factor out yes/no/auto value checking Simon Marchi via Gdb-patches
2022-08-23 19:17 ` Tom Tromey
2022-08-26 14:29 ` Simon Marchi via Gdb-patches
2022-08-29 16:24 ` Tom Tromey
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox