Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

M-x realgud:gdb errors with void realgud:run-process function #269

Closed
Apteryks opened this issue Jun 6, 2020 · 9 comments · Fixed by #270
Closed

M-x realgud:gdb errors with void realgud:run-process function #269

Apteryks opened this issue Jun 6, 2020 · 9 comments · Fixed by #270

Comments

@Apteryks
Copy link
Contributor

Apteryks commented Jun 6, 2020

Hello!

I'm using the GNU Guix package emacs-realgud, currently at version 1.5.1. It is byte compiled, and the realgud:gdb entry point is automatically found because of autoloads.

When doing M-x realgud:gdb, typing a command and press RET, I am greeted with: Symbol’s function definition is void: realgud:run-process. If I manually hunt the function and eval it, I can workaround the problem.

I thought it might be a mistake in the declare-function directives at the start of the gdb.el file, but so far I haven't been able to fix the problem (I don't understand what are the symbols passed to declare-function; the doc says it should be a file name). Failed attempt at a solution here: Apteryks@bbc78b8.

Thanks for this package!

@rocky
Copy link
Collaborator

rocky commented Jun 6, 2020

Thanks for the bug report, your investigation, and effort to fix.

As far as I know, the declare-function() serve only to squelch compiler warning about a function being used, but not defined. It has no operational effect .

The function that causes other internal files (that is files within the package as opposed to files outside of the package) is the require-relative-list() .

And indeed in constrast to all of the other debuggers, this one does indeed seem to be missing mention to pull in (relative to itself) the file ../../common/run.

So try applying this patch:

index eae0020..8507520 100644
--- a/realgud/debugger/gdb/gdb.el
+++ b/realgud/debugger/gdb/gdb.el
@@ -1,4 +1,4 @@
-;; Copyright (C) 2015-2016, 2019 Free Software Foundation, Inc
+;; Copyright (C) 2015-2016, 2019-2020 Free Software Foundation, Inc
 
 ;; Author: Rocky Bernstein <rocky@gnu.org>
 
@@ -19,6 +19,7 @@
 (require 'load-relative)
 (require-relative-list '("../../common/cmds"
 			 "../../common/helper"
+			 "../../common/run"
 			 "../../common/utils")
 		       "realgud-")
 

If this works, I will refresh the ELPA savannah sources and put out a new release with what's currently in this repository.

As best as I can tell this is the only debugger missing this.


A word about require-relative-list. That is my doing. Like the name require it was suggest pulling in a file or library. But it does so using a relative path. Sort of the difference in C between

#include "stdio.h"

vs.

#include <stdio.h>

I gave a talk about this a while ago and the slides for that are https://rocky.github.io/NYC-Lisp-Elisp-talk/#/6

@rocky
Copy link
Collaborator

rocky commented Jun 6, 2020

P.S. I do make a lot of mistakes — that's one reason I work on debuggers.

So please double check everything I write.

@Apteryks
Copy link
Contributor Author

Apteryks commented Jun 7, 2020

Hello :-)

Thanks for the quick feedback. I was able to to make it work by adding the missing require-relative-list calls in gdb.el (and gub.el). One gotcha was that the optional prefix that is concatenated in front of the bare file name must match the symbol provided by that file.

E.g., trying the suggested change above would throw an error, because the run.el file provides 'realgud:run rather than 'realgud-run.

I'm surprised byte compiling Realgud doesn't catch this kind of error -- it usually does for other packages. Perhaps it has to do with the relative requires code? Could it be made to fail at byte compilation time so that we'd find those errors earlier?

Thanks for the pointers and link!

I'll send a PR in a few moments.

Apteryks added a commit to Apteryks/realgud that referenced this issue Jun 7, 2020
Fixes <realgud#269>.

The 'realgud:run-process' function was not defined in the context of
the gdb and gub debuggers, causing an error when running 'M-x
realgud:gdb', for example.

These are the important bits that fix the above problem:

* realgud/debugger/gdb/gdb.el: Require "../../common/run".
* realgud/debugger/gub/gub.el: Likewise.

The following changes are cosmetic, non-functional:

* realgud/debugger/gdb/gdb.el (realgud:run-process): Fix "file"
reference.
* realgud/debugger/gub/gub.el (realgud:run-process): Likewise.
* realgud/debugger/pdb/pdb.el (realgud:run-process): Likewise.
* realgud/debugger/trepan2/trepan2.el (realgud:run-process): Likewise.
* realgud/debugger/trepan3k/trepan3k.el (realgud:run-process): Likewise.
@rocky
Copy link
Collaborator

rocky commented Jun 7, 2020

Could it be made to fail at byte compilation time so that we'd find those errors earlier?

I don't know. Feel free to investigate and make better.

@rocky rocky closed this as completed in #270 Jun 7, 2020
@fabrice-baray
Copy link

fabrice-baray commented Jun 8, 2020

Hi,

I got a new version of realgud installed this morning through emacs/elpa. It created that folder so I believe this is a pretty recent one: realgud-20200607.416

I had to change two things in the realgud el code for it to work, I believe this could be related to the change made by this fix. Basically I removed two ".../../common/run" in gdb.el and gud.el because they were in require-relative-list of "realgud-" and not "realgud:", like:

diff -r ./realgud/debugger/gdb/gdb.el new/realgud/debugger/gdb/gdb.el
22d21
< "../../common/run"

diff -r ./realgud/debugger/gub/gub.el new/realgud/debugger/gub/gub.el
22d21
< (require-relative-list '("../../common/run") "realgud-")

In case you feel that correct and need to be patched, I thought to mention it.

Regards,
Fabrice

@rocky
Copy link
Collaborator

rocky commented Jun 8, 2020

@fabrice-baray Would you put in a PR for this so that @Apteryks who was having the problem could see if this works for him as well?

Thanks.

@rocky rocky reopened this Jun 8, 2020
@Apteryks
Copy link
Contributor Author

Apteryks commented Jun 9, 2020

@fabrice-baray My changes didn't take into accounts the latest commits from rocky.; sorry about that. I've sent a PR with the proposed change here: #272. Thank you!

@jgarte
Copy link

jgarte commented May 9, 2023

Hi, what's the latest status on this? Should this ticket be closed?

@Apteryks @rocky

@Apteryks
Copy link
Contributor Author

I think that's long been fixed :-) Closing, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants