bug#62697: gdb-mi.el: Change target-async to mi-async

bug#62697: gdb-mi.el: Change target-async to mi-async

[Gustaf Waldemarson]

[-- Attachment #1.1: Type: text/plain, Size: 15059 bytes --]

Hello,

This may be a bit too early, given how prevalent it is to be using older
versions of GDB but I figured I should send this patch anyways to get it
out of
my mind:

For GDB version >= 11, starting up a debugging session with 'gdb-mi.el'
always
ends with this message:

> (gdb) Warning: 'set target-async', an alias for the command 'set
mi-async', is
> deprecated.  Use 'set mi-async'.

This is a bit of an eyesore and also breaks the initial prompt (although
that is
easily fixed by pressing enter, etc, but that may still be confusing for new
users).

It may be reasonable to either query the GDB version (not sure how though,
unfortunately) and then use the old command (`set target-async`) or execute
the
non-deprecated command instead (`set mi-async`), and then possibly trying
the
old on if the new one did not work.

Apparently, `set mi-async` is a GDB 7.8 feature (Released Jul 2014), so this
change may cause incompatibilities with earlier GDB versions. That said, I
suspect that it will only cause a warning message to appear for using an
unsupported command, but unfortunately I don't have an old enough version
of GDB
handy to try that out.

Any strong opinions either way? The attached patch only replaces the old
command
with the new one, but I'm open for suggestions.

Best regards,
Gustaf

In GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
 3.24.33, cairo version 1.16.0) of 2023-03-26 built on ShadowX
Repository revision: 248aabd9514c9eb21f4aa2e9062f8ce927d9fe54
Repository branch: master
System Description: Ubuntu 22.04.2 LTS

Configured using:
 'configure --prefix=/home/xaldew/.local
 '--program-transform-name=s/^ctags$/ctags.emacs/' --without-makeinfo
 --with-xpm=ifavailable --with-jpeg=ifavailable --with-gif=ifavailable
 --with-tiff=ifavailable'

Configured features:
CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG LCMS2
LIBSELINUX LIBXML2 MODULES NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP SOUND
THREADS TIFF TOOLKIT_SCROLL_BARS WEBP X11 XDBE XIM XINPUT2 XPM GTK3 ZLIB

Important settings:
  value of $LANG: sv_SE.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: Magit

Minor modes in effect:
  pyvenv-mode: t
  dap-tooltip-mode: t
  dap-ui-many-windows-mode: t
  dap-ui-controls-mode: t
  dap-ui-mode: t
  gdb-many-windows: t
  treemacs-filewatch-mode: t
  treemacs-follow-mode: t
  treemacs-git-mode: t
  treemacs-fringe-indicator-mode: t
  dap-auto-configure-mode: t
  dap-mode: t
  global-git-commit-mode: t
  magit-auto-revert-mode: t
  shell-dirtrack-mode: t
  beacon-mode: t
  server-mode: t
  hes-mode: t
  projectile-mode: t
  global-display-fill-column-indicator-mode: t
  global-display-line-numbers-mode: t
  global-so-long-mode: t
  yas-global-mode: t
  yas-minor-mode: t
  global-company-mode: t
  company-mode: t
  global-undo-tree-mode: t
  undo-tree-mode: t
  global-anzu-mode: t
  anzu-mode: t
  global-atomic-chrome-edit-mode: t
  windmove-mode: t
  which-key-mode: t
  anyclip-mode: t
  override-global-mode: t
  electric-pair-mode: t
  save-place-mode: t
  global-subword-mode: t
  subword-mode: t
  winner-mode: t
  global-auto-revert-mode: t
  xterm-mouse-mode: t
  savehist-mode: t
  ido-everywhere: t
  tooltip-mode: t
  global-eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  buffer-read-only: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
/home/xaldew/.config/emacs/elpa/transient-20230315.1520/transient hides
/home/xaldew/.local/share/emacs/30.0.50/lisp/transient

Features:
(shadow sort bbdb-message mail-extr warnings emacsbug cdlatex reftex
reftex-loaddefs reftex-vars org-element org-persist org-id org-refile
avl-tree oc-basic ol-eww eww xdg url-queue mm-url ol-rmail ol-mhe ol-irc
ol-info ol-gnus nnselect gnus-art mm-uu mml2015 mm-view mml-smime smime
dig gnus-sum shr pixel-fill kinsoku url-file svg gnus-group gnus-undo
gnus-start gnus-dbus dbus gnus-cloud nnimap nnmail mail-source utf7 nnoo
gnus-spec gnus-int gnus-range gnus-win gnus nnheader range ol-docview
doc-view jka-compr ol-bibtex bibtex ol-bbdb ol-w3m org-tempo tempo
cus-start ob-latex ob-plantuml ob-org ob-shell ob-gnuplot ob-C ob-python
ob-ditaa ob-dot org ob ob-tangle ob-ref ob-lob ob-table ob-exp org-macro
org-src ob-comint org-pcomplete org-list org-footnote org-faces
org-entities ob-emacs-lisp ob-core ob-eval org-cycle org-table ol
org-fold org-fold-core org-keys oc org-loaddefs cal-menu calendar
cal-loaddefs org-version org-compat org-macs shortdoc help-fns
radix-tree magit-annex magit-patch magit-subtree magit-gitignore
magit-ediff ediff ediff-merg ediff-mult ediff-wind ediff-diff ediff-help
ediff-init ediff-util image-mode exif guess-language pyvenv eshell
esh-cmd esh-ext esh-opt esh-proc esh-io esh-arg esh-module esh-groups
esh-util highlight-indentation anaconda-mode pythonic tramp
tramp-loaddefs trampver tramp-integration files-x tramp-compat
parse-time iso8601 ls-lisp python-mode/.yas-setup.el dap-python python
treesit c++-genostream cmake-mode rst ace-window all-the-icons
all-the-icons-faces data-material data-weathericons data-octicons
data-fileicons data-faicons data-alltheicons vc-hg vc-bzr vc-src vc-sccs
vc-svn vc-cvs vc-rcs log-view vc bug-reference misearch multi-isearch
lsp-diagnostics lsp-headerline lsp-icons lsp-modeline dap-mouse dap-ui
gdb-mi bui bui-list bui-info bui-entry bui-core bui-history bui-button
bui-utils lsp-lens vc-git vc-dispatcher modern-cpp-font-lock
.yas-setup.el cc-mode/.yas-setup.el view lsp-zig lsp-tilt lsp-steep
lsp-svelte lsp-sqls lsp-ruby-syntax-tree lsp-ruby-lsp lsp-yaml lsp-xml
lsp-vimscript lsp-vhdl lsp-volar lsp-vetur lsp-html lsp-verilog lsp-vala
lsp-v lsp-typeprof lsp-ttcn3 lsp-toml lsp-terraform lsp-tex lsp-sorbet
lsp-solargraph lsp-rust lsp-rf lsp-ruff-lsp lsp-remark lsp-racket lsp-r
lsp-purescript lsp-pylsp lsp-pyls lsp-pwsh lsp-php lsp-pls
lsp-perlnavigator lsp-perl lsp-openscad lsp-ocaml lsp-magik lsp-nix
lsp-nim lsp-nginx lsp-mint lsp-marksman lsp-markdown lsp-lua lsp-ltex
lsp-kotlin lsp-json lsp-javascript lsp-idris lsp-haxe lsp-groovy
lsp-hack lsp-graphql lsp-gleam lsp-go lsp-completion lsp-gdscript
lsp-fsharp lsp-fortran lsp-eslint lsp-erlang lsp-emmet lsp-elixir
lsp-elm lsp-dockerfile lsp-dhall lsp-d lsp-css lsp-csharp gnutls
lsp-crystal lsp-cmake lsp-clojure lsp-treemacs lsp-treemacs-generic
lsp-treemacs-themes treemacs-treelib treemacs treemacs-header-line
treemacs-compatibility treemacs-mode treemacs-interface
treemacs-persistence treemacs-filewatch-mode treemacs-follow-mode
treemacs-rendering treemacs-annotations treemacs-async
treemacs-workspaces treemacs-dom treemacs-visuals
treemacs-fringe-indicator treemacs-scope pulse treemacs-faces
treemacs-icons treemacs-themes treemacs-core-utils pfuture hl-line
treemacs-logging treemacs-customization treemacs-macros
lsp-semantic-tokens lsp-clangd lsp-beancount lsp-bash lsp-astro
lsp-ansible lsp-angular lsp-ada lsp-actionscript dap-gdb-lldb dap-utils
dom xml dap-mode dap-tasks dap-launch lsp-docker yaml posframe
dap-overlays magit-extras magit-submodule magit-blame magit-stash
magit-reflog magit-bisect magit-push magit-pull magit-fetch magit-clone
magit-remote magit-commit magit-sequence magit-notes magit-worktree
magit-tag magit-merge magit-branch magit-reset magit-files magit-refs
magit-status magit magit-repos magit-apply magit-wip magit-log
which-func magit-diff smerge-mode diff-mode git-commit log-edit message
sendmail yank-media rfc822 mml mml-sec epa derived gnus-util time-date
mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045 mm-util
ietf-drums mail-prsvr mailabbrev mail-utils gmm-utils mailheader
pcvs-util add-log magit-core magit-autorevert magit-margin
magit-transient magit-process with-editor shell pcomplete magit-mode
transient magit-git magit-base magit-section format-spec crm compat
dired-aux dired dired-loaddefs term/tmux term/xterm xterm pinentry
beacon server form-feed paredit nameless flyspell ispell whitespace
rainbow-delimiters highlight-escape-sequences projectile lisp-mnt grep
ibuf-ext ibuffer ibuffer-loaddefs lsp-ui lsp-ui-flycheck lsp-ui-doc
goto-addr lsp-ui-imenu lsp-ui-peek lsp-ui-sideline flycheck lsp-mode
tree-widget spinner network-stream puny nsm markdown-mode noutline
outline inline imenu f f-shortdoc ewoc epg rfc6068 epg-config compile
text-property-search lsp-ui-util face-remap find-func lsp-protocol s ht
dash display-fill-column-indicator display-line-numbers so-long
py-snippets yasnippet-radical-snippets yasnippet-snippets yasnippet
company-oddmuse company-keywords company-etags etags fileloop generator
xref project company-gtags company-dabbrev-code company-dabbrev
company-files company-clang company-capf company-cmake company-semantic
company-template company-bbdb company undo-tree diff queue anzu
thingatpt atomic-chrome websocket bindat let-alist
color-theme-approximate advice color delim-col hydra-examples windmove
rect hydra lv bbdb bbdb-site timezone cus-edit pp cus-load icons
wid-edit ace-link avy which-key anyclip-mode cl-extra help-mode edmacro
kmacro diminish use-package use-package-ensure use-package-delight
use-package-diminish use-package-bind-key bind-key easy-mmode
use-package-core finder-inf local-autoloads cwarn cc-mode cc-fonts
cc-guess cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars cc-defs
elec-pair saveplace cap-words superword subword winner autorevert
filenotify xt-mouse tango-dark-theme savehist ido gud comint ansi-osc
ansi-color ring keybinds terminals ivy-autoloads
spacemacs-theme-autoloads ace-link-autoloads kotlin-mode-autoloads
company-auctex-autoloads gl-conf-mode-autoloads css-eldoc-autoloads
beacon-autoloads lsp-ltex-autoloads paredit-autoloads
glsl-mode-autoloads highlight-escape-sequences-autoloads
powershell-autoloads gnuplot-mode-autoloads cuda-mode-autoloads
ein-autoloads request-autoloads ssh-config-mode-autoloads
git-modes-autoloads mmm-mode-autoloads ebdb-autoloads rmsbolt-autoloads
debbugs-autoloads dart-mode-autoloads sublime-themes-autoloads
coffee-mode-autoloads graphviz-dot-mode-autoloads magit-annex-autoloads
opencl-mode-autoloads json-mode-autoloads rx company-anaconda-autoloads
pinentry-autoloads rainbow-mode-autoloads htmlize-autoloads
projectile-autoloads flycheck-rust-autoloads bbdb-vcard-autoloads
jira-markup-mode-autoloads browse-kill-ring-autoloads
helm-dash-autoloads sx-autoloads jenkins-autoloads smart-jump-autoloads
bbdb-autoloads which-key-autoloads anzu-autoloads x86-lookup-autoloads
cmake-mode-autoloads flycheck-kotlin-autoloads undo-tree-autoloads
rust-mode-autoloads plantuml-mode-autoloads magit-gerrit-autoloads
lua-mode-autoloads yaml-mode-autoloads evil-autoloads deferred-autoloads
iedit-autoloads unfill-autoloads srefactor-autoloads abc-mode-autoloads
autoinsert solarized-theme-autoloads alert-autoloads gntp-autoloads
emms-autoloads cider-autoloads sesman-autoloads queue-autoloads
parseedn-autoloads parseclj-autoloads clojure-mode-autoloads
flycheck-package-autoloads package-lint-autoloads
atomic-chrome-autoloads websocket-autoloads powerthesaurus-autoloads
jeison-autoloads dash-docs-autoloads rainbow-delimiters-autoloads
form-feed-autoloads yasnippet-radical-snippets-autoloads
pyimport-autoloads shut-up-autoloads json-snatcher-autoloads
helm-lsp-autoloads helm-autoloads clang-format-autoloads
poly-markdown-autoloads polymode-autoloads toml-mode-autoloads
diminish-autoloads cdlatex-autoloads eclim-autoloads anaphora-autoloads
py-snippets-autoloads yasnippet-snippets-autoloads
zenburn-theme-autoloads dap-mode-autoloads lsp-docker-autoloads
lsp-treemacs-autoloads treemacs-autoloads cfrs-autoloads
posframe-autoloads pfuture-autoloads ace-window-autoloads avy-autoloads
bui-autoloads magit-svn-autoloads magit-autoloads pcase
git-commit-autoloads with-editor-autoloads transient-autoloads
go-mode-autoloads goto-chg-autoloads log4e-autoloads
color-theme-approximate-autoloads cargo-autoloads calfw-autoloads
company-quickhelp-autoloads helm-core-autoloads async-autoloads
ecb-autoloads modern-cpp-font-lock-autoloads dts-mode-autoloads
nov-autoloads esxml-autoloads kv-autoloads company-math-autoloads
math-symbol-lists-autoloads elpy-autoloads pyvenv-autoloads
highlight-indentation-autoloads ob-ipython-autoloads
dash-functional-autoloads hydra-autoloads auto-complete-auctex-autoloads
auto-complete-autoloads yasnippet-autoloads auctex-latexmk-autoloads
auctex-autoloads tex-site yaml-autoloads ahk-mode-autoloads
magit-section-autoloads compat-autoloads cask-mode-autoloads
company-c-headers-autoloads company-autoloads csv-mode-autoloads
gnuplot-autoloads anaconda-mode-autoloads pythonic-autoloads
pos-tip-autoloads zerodark-theme-autoloads all-the-icons-autoloads
image+-autoloads expand-region-autoloads gnus-desktop-notify-autoloads
flycheck-autoloads pkg-info-autoloads epl-autoloads ggtags-autoloads
web-mode-autoloads lsp-ui-autoloads lsp-mode-autoloads lv-autoloads
markdown-mode-autoloads spinner-autoloads ht-autoloads f-autoloads
uimage-autoloads guess-language-autoloads dumb-jump-autoloads
popup-autoloads dash-autoloads s-autoloads google-c-style-autoloads
nameless-autoloads info slime-autoloads macrostep-autoloads package
browse-url url url-proxy url-privacy url-expand url-methods url-history
url-cookie generate-lisp-file url-domsuf url-util mailcap url-handlers
url-parse auth-source cl-seq eieio eieio-core cl-macs password-cache
json subr-x map byte-opt gv bytecomp byte-compile url-vars cl-loaddefs
cl-lib rmc iso-transl tooltip cconv eldoc paren electric uniquify
ediff-hook vc-hooks lisp-float-type elisp-mode mwheel term/x-win x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode lisp-mode prog-mode register
page tab-bar menu-bar rfn-eshadow isearch easymenu timer select
scroll-bar mouse jit-lock font-lock syntax font-core term/tty-colors
frame minibuffer nadvice seq simple cl-generic indonesian philippine
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure
cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp
files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget keymap hashtable-print-readable backquote
threads dbusbind inotify lcms2 dynamic-setting system-font-setting
font-render-setting cairo move-toolbar gtk x-toolkit xinput2 x multi-tty
make-network-process emacs)

Memory information:
((conses 16 817895 212638)
 (symbols 48 62999 0)
 (strings 32 249530 19612)
 (string-bytes 1 8101995)
 (vectors 16 160320)
 (vector-slots 8 3855694 143961)
 (floats 8 1381 10031)
 (intervals 56 21064 2868)
 (buffers 976 49))

[-- Attachment #1.2: Type: text/html, Size: 16160 bytes --]

[-- Attachment #2: 0001-Convert-gdb-command-target-async-to-mi-async.patch --]
[-- Type: text/x-patch, Size: 1160 bytes --]

From 6682e472e4efee5447c41b6e0e48a1d0070548d6 Mon Sep 17 00:00:00 2001
From: Gustaf Waldemarson <gustaf.waldemarson@gmail.com>
Date: Wed, 18 Jan 2023 16:14:40 +0100
Subject: [PATCH] Convert gdb command `target-async' to `mi-async'.

* lisp/progmodes/gdb-mi.el: Change deprecated gdb command
`target-async' to `my-async'. Functions using the old name has also
updated.
---
 lisp/progmodes/gdb-mi.el | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lisp/progmodes/gdb-mi.el b/lisp/progmodes/gdb-mi.el
index 060957eac29..35af9f60ffc 100644
--- a/lisp/progmodes/gdb-mi.el
+++ b/lisp/progmodes/gdb-mi.el
@@ -1143,10 +1143,10 @@ gdb-non-stop-handler
 	(setq gdb-non-stop nil)
 	(setq gdb-supports-non-stop nil))
     (setq gdb-supports-non-stop t)
-    (gdb-input "-gdb-set target-async 1" 'ignore)
-    (gdb-input "-list-target-features" 'gdb-check-target-async)))
+    (gdb-input "-gdb-set mi-async on" 'ignore)
+    (gdb-input "-list-target-features" 'gdb-check-mi-async)))
 
-(defun gdb-check-target-async ()
+(defun gdb-check-mi-async ()
   (goto-char (point-min))
   (unless (re-search-forward "async" nil t)
     (message
-- 
2.34.1

bug#62697: gdb-mi.el: Change target-async to mi-async

[Eli Zaretskii]

> From: Gustaf Waldemarson <gustaf.waldemarson@gmail.com>
> Date: Thu, 6 Apr 2023 14:48:20 +0200
> 
> This may be a bit too early, given how prevalent it is to be using older
> versions of GDB but I figured I should send this patch anyways to get it out of
> my mind:
> 
> For GDB version >= 11, starting up a debugging session with 'gdb-mi.el' always
> ends with this message:
> 
> > (gdb) Warning: 'set target-async', an alias for the command 'set mi-async', is
> > deprecated.  Use 'set mi-async'.
> 
> This is a bit of an eyesore and also breaks the initial prompt (although that is
> easily fixed by pressing enter, etc, but that may still be confusing for new
> users).
> 
> It may be reasonable to either query the GDB version (not sure how though,
> unfortunately) and then use the old command (`set target-async`) or execute the
> non-deprecated command instead (`set mi-async`), and then possibly trying the
> old on if the new one did not work.
> 
> Apparently, `set mi-async` is a GDB 7.8 feature (Released Jul 2014), so this
> change may cause incompatibilities with earlier GDB versions. That said, I
> suspect that it will only cause a warning message to appear for using an
> unsupported command, but unfortunately I don't have an old enough version of GDB
> handy to try that out.
> 
> Any strong opinions either way? The attached patch only replaces the old command
> with the new one, but I'm open for suggestions.

We cannot simply replace the command, as GDB 7.8 is not old enough to
assume it isn't used anymore.

We could either (1) ask GDB about its version, or (2) filter out the
annoying message so it isn't shown to the users; we'd then have to
revisit this if/when GDB actually drops the command, if it ever does.

Alternative (1) is AFAIR problematic because the initialization of a
GDB session under Emacs is entirely asynchronous, so sending a command
and waiting for its response before sending the rest is not easy.  So
I tend to the second alternative.

Thanks.

bug#62697: gdb-mi.el: Change target-async to mi-async

[Jim Porter]

On 4/6/2023 6:32 AM, Eli Zaretskii wrote:
> We could either (1) ask GDB about its version, or (2) filter out the
> annoying message so it isn't shown to the users; we'd then have to
> revisit this if/when GDB actually drops the command, if it ever does.
> 
> Alternative (1) is AFAIR problematic because the initialization of a
> GDB session under Emacs is entirely asynchronous, so sending a command
> and waiting for its response before sending the rest is not easy.  So
> I tend to the second alternative.

I don't know much about gdb-mi.el's internals, but taking a quick look 
at the code, 'gdb-input' takes a callback, so something like the 
following pseudocode would probably do the trick?

   (gdb-input "-gdb-version"
    (lambda ()
      (if (gdb-should-use-mi-async)  ; Check the version output.
          (gdb-input "-gdb-set mi-async on" #'ignore)
        (gdb-input "-gdb-set target-async on" #'ignore))
      (gdb-input "-list-target-features" #'gdb-check-target-async)))

The existing 'gdb-check-target-async' already chains GDB-MI commands 
like this, so I imagine the above will Just Work.

bug#62697: gdb-mi.el: Change target-async to mi-async

[Eli Zaretskii]

> Date: Thu, 6 Apr 2023 18:26:48 -0700
> Cc: 62697@debbugs.gnu.org
> From: Jim Porter <jporterbugs@gmail.com>
> 
> On 4/6/2023 6:32 AM, Eli Zaretskii wrote:
> > We could either (1) ask GDB about its version, or (2) filter out the
> > annoying message so it isn't shown to the users; we'd then have to
> > revisit this if/when GDB actually drops the command, if it ever does.
> > 
> > Alternative (1) is AFAIR problematic because the initialization of a
> > GDB session under Emacs is entirely asynchronous, so sending a command
> > and waiting for its response before sending the rest is not easy.  So
> > I tend to the second alternative.
> 
> I don't know much about gdb-mi.el's internals, but taking a quick look 
> at the code, 'gdb-input' takes a callback, so something like the 
> following pseudocode would probably do the trick?

The problem is not with the callback, the problem is with _when_ the
callback is called.  gdb-mi doesn't wait for each gdb-input call to
complete and its callback called before calling the next gdb-input.
In practice, we send a dozen gdb-input commands before the response
for the first one is received and its callback called.  You can
clearly see that if you enable gdb-enable-debug and look at the log it
collects.

So sending a command via gdb-input, then conditioning another command
on it is not trivial, since the callback could be called much later.

>    (gdb-input "-gdb-version"
>     (lambda ()
>       (if (gdb-should-use-mi-async)  ; Check the version output.
>           (gdb-input "-gdb-set mi-async on" #'ignore)
>         (gdb-input "-gdb-set target-async on" #'ignore))
>       (gdb-input "-list-target-features" #'gdb-check-target-async)))
> 
> The existing 'gdb-check-target-async' already chains GDB-MI commands 
> like this, so I imagine the above will Just Work.

It "will work", but what if the other commands sent via the other
gdb-input calls during initialization depend, or change the GDB
behavior depending, on whether mi-async was or wasn't already sent?
Or are you saying that mi-async can be sent anywhere during the
initialization sequence, including after it finishes?  The GDB manual
says:

  The frontend may specify a preference for asynchronous execution
  using the '-gdb-set mi-async 1' command, which should be emitted
  before either running the executable or attaching to the target.

If GDB is invoked with, e.g., "gdb -p PID", then we need to send this
command up front, before GDB attaches.

And there could be other issues with that proposal.  Which is why I
said that alternative was "problematic".  Filtering out the annoying
message is much safer, IMO.

bug#62697: gdb-mi.el: Change target-async to mi-async

[Jim Porter]

On 4/6/2023 11:26 PM, Eli Zaretskii wrote:
> It "will work", but what if the other commands sent via the other
> gdb-input calls during initialization depend, or change the GDB
> behavior depending, on whether mi-async was or wasn't already sent?
> Or are you saying that mi-async can be sent anywhere during the
> initialization sequence, including after it finishes?  The GDB manual
> says:
> 
>    The frontend may specify a preference for asynchronous execution
>    using the '-gdb-set mi-async 1' command, which should be emitted
>    before either running the executable or attaching to the target.
> 
> If GDB is invoked with, e.g., "gdb -p PID", then we need to send this
> command up front, before GDB attaches.

Maybe I'm just misunderstanding something, but it looks like "-gdb-set 
target-async 1" is already sent from a callback: specifically, the one 
for "-gdb-set non-stop 1". Is this code already violating GDB's rules? 
Would adding another layer of callback make it any worse? Maybe so, but 
if we're worried about the latter, then couldn't we do something like:

------------------------------

(defun gdb-init-1 ()
   ;; ...

   ;; Add this call:
   (gdb-input "-gdb-version" #'gdb-version-handler)

   ;; ...
   ;; This is existing code:
   (when gdb-non-stop
     (gdb-input "-gdb-set non-stop 1" 'gdb-non-stop-handler))
   ;; ...
   )

(defun gdb-version-handler ()
   (setq gdb-version (some-logic-here)))

;; This is also existing code:
(defun gdb-non-stop-handler ()
   ;; By here, 'gdb-version-handler' should have been called, right?
   ;; I'm assuming handlers are called in the same order as the input was
   ;; sent to gdb.  If so, just check 'gdb-version' and do the right
   ;; thing.
   )

------------------------------

Assuming handlers are called in the same order as their inputs are sent, 
this should work, and then everything will happen in the same order as 
gdb-mi.el currently does.

My main worry with just suppressing the warning is that presumably at 
some point in the future, GDB will remove the old form entirely, and 
then gdb-mi.el breaks. If we could get this code to use the new form 
when available, then there's no need to worry about having to publish a 
hotfix for gdb-mi.el on short notice.

bug#62697: gdb-mi.el: Change target-async to mi-async

[Eli Zaretskii]

> Date: Fri, 7 Apr 2023 00:25:03 -0700
> Cc: 62697@debbugs.gnu.org, gustaf.waldemarson@gmail.com
> From: Jim Porter <jporterbugs@gmail.com>
> 
> On 4/6/2023 11:26 PM, Eli Zaretskii wrote:
> > 
> >    The frontend may specify a preference for asynchronous execution
> >    using the '-gdb-set mi-async 1' command, which should be emitted
> >    before either running the executable or attaching to the target.
> > 
> > If GDB is invoked with, e.g., "gdb -p PID", then we need to send this
> > command up front, before GDB attaches.
> 
> Maybe I'm just misunderstanding something, but it looks like "-gdb-set 
> target-async 1" is already sent from a callback: specifically, the one 
> for "-gdb-set non-stop 1". Is this code already violating GDB's rules? 

I don't know.

> Would adding another layer of callback make it any worse?

I don't know.

> Maybe so, but if we're worried about the latter, then couldn't we do
> something like:

I don't know!  It will take someone who knows a lot more than I do
about both GDB/MI and gdb-mi.el to tell, and the changes need to be
tested with many different versions of GDB.  Feel free to do that and
see if those changes are benign, and then we can install them.
Failing that, given that we don't have an active enough maintainer of
gdb-mi.el on board, I feel that suppressing the annoying message is a
much more easy way out.

> Assuming handlers are called in the same order as their inputs are sent, 

They are called in the order in which responses for inputs are
received, not in the order these inputs are sent.  I don't think these
are the same orders, at least they aren't guaranteed to be.  You can
see this by yourself if you enable the gdb-mi debug mode (each
response is tagged with the number of the corresponding input command,
and those numbers are given to commands by gdb-mi.el in the order it
sends the commands).

> My main worry with just suppressing the warning is that presumably at 
> some point in the future, GDB will remove the old form entirely, and 
> then gdb-mi.el breaks.

I'm not even sure this will be removed at all.  It certainly won't be
removed soon, as they just recently started issuing a warning.  For
example, the annotations feature of GDB, use by "M-x gud-gdb", is
still being supported, although it became deprecated as soon as GDB/MI
was added to GDB.

> If we could get this code to use the new form when available, then
> there's no need to worry about having to publish a hotfix for
> gdb-mi.el on short notice.

I agree.  But if adding that means dealing with tricky
timing-dependent and version-dependent bugs, I prefer to use a less
radical solution, even if it's less clean and less future-proof.

Btw, as an easy alternative, we could add a defcustom whose value
would be the minimum supported value of GDB.  Then users could set it
to, say, 7.8.0, to force gdb-mi.el to use mi-async, and by that avoid
the annoyance.  This could even go into Emacs 29.

bug#62697: gdb-mi.el: Change target-async to mi-async

[Gustaf Waldemarson]

[-- Attachment #1: Type: text/plain, Size: 5030 bytes --]

Filtering the message is probably easy, but as Jim pointed out, it will
probably
have unintended consequences later on. Besides, extracting the version could
enable a lot of different things down the line.

I had actually started on a "check-gdb-version-string", but was never able
to
get it to work. In fact, revisiting that code now makes we wonder if any of
these
"handlers" are actually working as intended.

From what I can see, the handlers scan the current buffer (which one is
that anyways?)
to determine whether to do some operations. However, adding some prints to
these handlers reveal that they seem to always be empty:

(defun gdb-non-stop-handler ()
  (goto-char (point-min))
  (print (buffer-substring-no-properties (point-min) (point-max)))
  (if (re-search-forward "No symbol" nil t)
      (progn
(message
         "This version of GDB doesn't support non-stop mode.  Turning it
off.")
(setq gdb-non-stop nil)
(setq gdb-supports-non-stop nil))
    (setq gdb-supports-non-stop t)
    (gdb-input "-gdb-set mi-async on" 'ignore)
    (gdb-input "-list-target-features" 'gdb-check-mi-async)))

(defun gdb-version-handler ()
  "Set the version of gdb currently being used."
  (message "Searching for GDB version...")
  (goto-char (point-min))
  (print (buffer-substring-no-properties (point-min) (point-max)))
  (with-current-buffer (gdb-get-buffer 'gdb-partial-output-buffer)
    (goto-char (point-min))
    (print (buffer-substring-no-properties (point-min) (point-max)))
    (when (re-search-forward "GNU gdb (GDB) \\(.+\\)" nil t)
      (message
       "Attempting to retrieve GDB version: %s" (match-string 1))
      (setq gdb-version (match-string 1)))))

I also tried to change the buffer in the `gdb-version-handler`, but that
didn't
actually change anything. Am I making some wrong assumption here? To me
at least, it does not make sense that these are empty.

Best regards,
Gustaf

Den fre 7 apr. 2023 kl 12:29 skrev Eli Zaretskii <eliz@gnu.org>:

> > Date: Fri, 7 Apr 2023 00:25:03 -0700
> > Cc: 62697@debbugs.gnu.org, gustaf.waldemarson@gmail.com
> > From: Jim Porter <jporterbugs@gmail.com>
> >
> > On 4/6/2023 11:26 PM, Eli Zaretskii wrote:
> > >
> > >    The frontend may specify a preference for asynchronous execution
> > >    using the '-gdb-set mi-async 1' command, which should be emitted
> > >    before either running the executable or attaching to the target.
> > >
> > > If GDB is invoked with, e.g., "gdb -p PID", then we need to send this
> > > command up front, before GDB attaches.
> >
> > Maybe I'm just misunderstanding something, but it looks like "-gdb-set
> > target-async 1" is already sent from a callback: specifically, the one
> > for "-gdb-set non-stop 1". Is this code already violating GDB's rules?
>
> I don't know.
>
> > Would adding another layer of callback make it any worse?
>
> I don't know.
>
> > Maybe so, but if we're worried about the latter, then couldn't we do
> > something like:
>
> I don't know!  It will take someone who knows a lot more than I do
> about both GDB/MI and gdb-mi.el to tell, and the changes need to be
> tested with many different versions of GDB.  Feel free to do that and
> see if those changes are benign, and then we can install them.
> Failing that, given that we don't have an active enough maintainer of
> gdb-mi.el on board, I feel that suppressing the annoying message is a
> much more easy way out.
>
> > Assuming handlers are called in the same order as their inputs are sent,
>
> They are called in the order in which responses for inputs are
> received, not in the order these inputs are sent.  I don't think these
> are the same orders, at least they aren't guaranteed to be.  You can
> see this by yourself if you enable the gdb-mi debug mode (each
> response is tagged with the number of the corresponding input command,
> and those numbers are given to commands by gdb-mi.el in the order it
> sends the commands).
>
> > My main worry with just suppressing the warning is that presumably at
> > some point in the future, GDB will remove the old form entirely, and
> > then gdb-mi.el breaks.
>
> I'm not even sure this will be removed at all.  It certainly won't be
> removed soon, as they just recently started issuing a warning.  For
> example, the annotations feature of GDB, use by "M-x gud-gdb", is
> still being supported, although it became deprecated as soon as GDB/MI
> was added to GDB.
>
> > If we could get this code to use the new form when available, then
> > there's no need to worry about having to publish a hotfix for
> > gdb-mi.el on short notice.
>
> I agree.  But if adding that means dealing with tricky
> timing-dependent and version-dependent bugs, I prefer to use a less
> radical solution, even if it's less clean and less future-proof.
>
> Btw, as an easy alternative, we could add a defcustom whose value
> would be the minimum supported value of GDB.  Then users could set it
> to, say, 7.8.0, to force gdb-mi.el to use mi-async, and by that avoid
> the annoyance.  This could even go into Emacs 29.
>

[-- Attachment #2: Type: text/html, Size: 6414 bytes --]

bug#62697: gdb-mi.el: Change target-async to mi-async

[Eli Zaretskii]

> From: Gustaf Waldemarson <gustaf.waldemarson@gmail.com>
> Date: Fri, 7 Apr 2023 12:36:31 +0200
> Cc: Jim Porter <jporterbugs@gmail.com>, 62697@debbugs.gnu.org
> 
> Filtering the message is probably easy, but as Jim pointed out, it will probably 
> have unintended consequences later on.

Leave that problem to me (or to the maintainer that will come after
me).  The problem you raised was with the annoying message; let's
solve that first and foremost.

> Besides, extracting the version could
> enable a lot of different things down the line.

It could, but it is "tricky", as you have discovered (and I discovered
before you).

> I had actually started on a "check-gdb-version-string", but was never able to
> get it to work. In fact, revisiting that code now makes we wonder if any of these
> "handlers" are actually working as intended.

They do work.  It's just that the way this is implemented is "tricky",
as I said from the get-go.

> From what I can see, the handlers scan the current buffer (which one is that anyways?)
> to determine whether to do some operations. However, adding some prints to
> these handlers reveal that they seem to always be empty:
> 
> (defun gdb-non-stop-handler ()
>   (goto-char (point-min))
>   (print (buffer-substring-no-properties (point-min) (point-max)))
>   (if (re-search-forward "No symbol" nil t)
>       (progn
> (message
>          "This version of GDB doesn't support non-stop mode.  Turning it off.")
> (setq gdb-non-stop nil)
> (setq gdb-supports-non-stop nil))
>     (setq gdb-supports-non-stop t)
>     (gdb-input "-gdb-set mi-async on" 'ignore)
>     (gdb-input "-list-target-features" 'gdb-check-mi-async)))

The "No symbol" text means that we have invoked a command that is not
supported by the GDB you have installed.  Unless you test this with a
very old version of GDB, you will never see that text.

The way these commands work is like this:

  . gdb-input sends a command which has a serial number (the numbers
    are allocated by gdb-mi.el in increasing order, starting from 1)
  . gdb-input also registers a callback for the response
  . when the response is received, it is recognized by the serial
    number (GDB responds with the same serial number as the command to
    which it responded), and the corresponding callback is then
    called, and removed from the list of callbacks waiting for
    response
  . each callback is programmed to look for possible known GDB
    reactions (per the GDB/MI protocol), which could be one or more
    of:
     - nothing (a normal reaction in many cases)
     - some output, like if the command requests the list of threads
     - an error message
     - free text message
  . these texts are (AFAIR) looked for in the process buffer, where
    the filter puts them