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

[Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>]

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