public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2] gdb: remove --disable-gdbmi configure option
@ 2023-02-14 16:13 Simon Marchi
  2023-02-14 17:36 ` Tom Tromey
  0 siblings, 1 reply; 4+ messages in thread
From: Simon Marchi @ 2023-02-14 16:13 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

New in v2:

  - I had forgotten to include the re-generated configure in the patch,
    it's there now.

I noticed that the --disable-gdbmi option was broken for almost a year
(since 740b42ceb7c "gdb/python/mi: create MI commands using python").

The problem today is the python/py-cmd.c file.  It is included in the
build if Python support is enabled, and it calls into some MI functions
(e.g. insert_mi_cmd_entry).  If MI support is disabled, we get some
undefined symbols like:

    mold: error: undefined symbol: insert_mi_cmd_entry(std::unique_ptr<mi_command, std::default_delete<mi_command> >)
    >>> referenced by py-micmd.c
    >>>               python/py-micmd.o:(micmdpy_install_command(micmdpy_object*))

The python/py-cmd.c file should be included in the build if both Python
and MI support are enabled.  It is not a case we support today, but it
could be done with a bit more configure code.  However, I think we
should just remove the --disable-gdbmi option, and just include MI
support unconditionally.

Tom Tromey proposed a while ago to remove this option, but it ended
staying:

  https://inbox.sourceware.org/gdb-patches/20180628172132.28843-1-tom@tromey.com/

However, there was no strong opposition to remove it.  The argument was
just "bah, it doesn't hurt anybody".

But given today's case, I would rather remove complexity rather than add
some.  I couldn't find anybody caring deeply for that option, and it's
not like MI adds any external dependency.  It's just a bit more code.

Removing the option will not break anybody using --disable-gdbmi (it can
be found in many build scripts [1]), since we don't flag invalid
configure flags.

So, remove the option from configure.ac, and adjust Makefile.in
accordingly to always include the MI objects in the build.

[1] https://github.com/search?q=%22--disable-gdbmi%22&type=code

Change-Id: Ifcaa8c9fc4abc6fa686ed5fd984598644f745240
---
 gdb/Makefile.in  |  8 +++-----
 gdb/configure    | 31 ++-----------------------------
 gdb/configure.ac | 14 --------------
 3 files changed, 5 insertions(+), 48 deletions(-)

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index dac5a66e9ada..b9fa0470fda2 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -293,6 +293,7 @@ SUBDIR_MI_SRCS = \
 	mi/mi-cmd-target.c \
 	mi/mi-cmd-var.c \
 	mi/mi-cmds.c \
+	mi/mi-common.c \
 	mi/mi-console.c \
 	mi/mi-getopt.c \
 	mi/mi-interp.c \
@@ -303,10 +304,6 @@ SUBDIR_MI_SRCS = \
 
 SUBDIR_MI_OBS = $(patsubst %.c,%.o,$(SUBDIR_MI_SRCS))
 
-SUBDIR_MI_DEPS =
-SUBDIR_MI_LDFLAGS =
-SUBDIR_MI_CFLAGS =
-
 #
 # TUI sub directory definitions
 #
@@ -1249,8 +1246,8 @@ SFILES = \
 	stub-termcap.c \
 	symfile-mem.c \
 	ui-file.h \
-	mi/mi-common.c \
 	$(SUBDIR_CLI_SRCS) \
+	$(SUBDIR_MI_SRCS) \
 	$(SUBDIR_TARGET_SRCS) \
 	$(COMMON_SFILES) \
 	$(SUBDIR_GCC_COMPILE_SRCS)
@@ -1860,6 +1857,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
 	xml-builtin.o \
 	$(patsubst %.c,%.o,$(COMMON_SFILES)) \
 	$(SUBDIR_CLI_OBS) \
+	$(SUBDIR_MI_OBS) \
 	$(SUBDIR_TARGET_OBS) \
 	$(SUBDIR_GCC_COMPILE_OBS)
 
diff --git a/gdb/configure b/gdb/configure
index 1114bcdc0af1..4cc359c1a8bf 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -913,7 +913,6 @@ with_auto_load_safe_path
 enable_targets
 enable_64_bit_bfd
 with_amd_dbgapi
-enable_gdbmi
 enable_tui
 enable_gdbtk
 with_debuginfod
@@ -1630,7 +1629,6 @@ Optional Features:
   --enable-targets=TARGETS
                           alternative target configurations
   --enable-64-bit-bfd     64-bit support (on hosts with narrower word sizes)
-  --disable-gdbmi         disable machine-interface (MI)
   --enable-tui            enable full-screen terminal user interface (TUI)
   --enable-gdbtk          enable gdbtk graphical user interface (GUI)
   --enable-profiling      enable profiling of GDB
@@ -11450,7 +11448,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 11453 "configure"
+#line 11451 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -11556,7 +11554,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 11559 "configure"
+#line 11557 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -18304,31 +18302,6 @@ _ACEOF
 
 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
-
-else
-  enable_gdbmi=yes
-fi
-
-if test x"$enable_gdbmi" = xyes; then
-  if test -d "$srcdir/mi"; then
-    CONFIG_OBS="$CONFIG_OBS \$(SUBDIR_MI_OBS)"
-    CONFIG_DEPS="$CONFIG_DEPS \$(SUBDIR_MI_DEPS)"
-    CONFIG_SRCS="$CONFIG_SRCS \$(SUBDIR_MI_SRCS)"
-    ENABLE_CFLAGS="$ENABLE_CFLAGS \$(SUBDIR_MI_CFLAGS)"
-  fi
-fi
-
 # Enable TUI.
 # Check whether --enable-tui was given.
 if test "${enable_tui+set}" = set; then :
diff --git a/gdb/configure.ac b/gdb/configure.ac
index 47e35f467f8e..6874767ea095 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -318,20 +318,6 @@ if test "x$targ_defvec" != x; then
     [Define to BFD's default target vector. ])
 fi
 
-# Enable MI.
-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)"
-    CONFIG_DEPS="$CONFIG_DEPS \$(SUBDIR_MI_DEPS)"
-    CONFIG_SRCS="$CONFIG_SRCS \$(SUBDIR_MI_SRCS)"
-    ENABLE_CFLAGS="$ENABLE_CFLAGS \$(SUBDIR_MI_CFLAGS)"
-  fi
-fi
-
 # Enable TUI.
 AC_ARG_ENABLE(tui,
 AS_HELP_STRING([--enable-tui],
-- 
2.39.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] gdb: remove --disable-gdbmi configure option
  2023-02-14 16:13 [PATCH v2] gdb: remove --disable-gdbmi configure option Simon Marchi
@ 2023-02-14 17:36 ` Tom Tromey
  2023-02-14 18:34   ` Simon Marchi
  0 siblings, 1 reply; 4+ messages in thread
From: Tom Tromey @ 2023-02-14 17:36 UTC (permalink / raw)
  To: Simon Marchi via Gdb-patches; +Cc: Simon Marchi

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

Simon> But given today's case, I would rather remove complexity rather than add
Simon> some.  I couldn't find anybody caring deeply for that option, and it's
Simon> not like MI adds any external dependency.

For me that's the deciding factor.

Approved-By: Tom Tromey <tom@tromey.com>

Tom

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] gdb: remove --disable-gdbmi configure option
  2023-02-14 17:36 ` Tom Tromey
@ 2023-02-14 18:34   ` Simon Marchi
  2023-02-23 21:40     ` Simon Marchi
  0 siblings, 1 reply; 4+ messages in thread
From: Simon Marchi @ 2023-02-14 18:34 UTC (permalink / raw)
  To: Tom Tromey, Simon Marchi via Gdb-patches; +Cc: Simon Marchi

On 2/14/23 12:36, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> But given today's case, I would rather remove complexity rather than add
> Simon> some.  I couldn't find anybody caring deeply for that option, and it's
> Simon> not like MI adds any external dependency.
> 
> For me that's the deciding factor.
> 
> Approved-By: Tom Tromey <tom@tromey.com>
> 
> Tom

Thanks.  I will wait a few days just in case there is some feedback from people
who would really miss that option.

Simon

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] gdb: remove --disable-gdbmi configure option
  2023-02-14 18:34   ` Simon Marchi
@ 2023-02-23 21:40     ` Simon Marchi
  0 siblings, 0 replies; 4+ messages in thread
From: Simon Marchi @ 2023-02-23 21:40 UTC (permalink / raw)
  To: Tom Tromey, Simon Marchi via Gdb-patches; +Cc: Simon Marchi

On 2/14/23 13:34, Simon Marchi via Gdb-patches wrote:
> On 2/14/23 12:36, Tom Tromey wrote:
>>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
>>
>> Simon> But given today's case, I would rather remove complexity rather than add
>> Simon> some.  I couldn't find anybody caring deeply for that option, and it's
>> Simon> not like MI adds any external dependency.
>>
>> For me that's the deciding factor.
>>
>> Approved-By: Tom Tromey <tom@tromey.com>
>>
>> Tom
> 
> Thanks.  I will wait a few days just in case there is some feedback from people
> who would really miss that option.
> 
> Simon

I will push this now.

Simon

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-02-23 21:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-14 16:13 [PATCH v2] gdb: remove --disable-gdbmi configure option Simon Marchi
2023-02-14 17:36 ` Tom Tromey
2023-02-14 18:34   ` Simon Marchi
2023-02-23 21:40     ` Simon Marchi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).