Remove sage-gdb-commands from src/bin
Closed this issue · 34 comments
(from #33625)
Critical because this file is not compatible with editable installs (setup.py develop), see #31049
CC: @tornaria @jhpalmieri @kiwifb @dimpase
Component: scripts
Author: Matthias Koeppe
Branch: d24b928
Reviewer: John Palmieri
Issue created by migration from https://trac.sagemath.org/ticket/33627
Description changed:
---
+++
@@ -2,3 +2,4 @@
This is just package_data and should be moved into `src/sage/doctest/`.
+Critical because this file is not compatible with editable installs (`setup.py develop`), see #31049Author: Matthias Koeppe
Would sage-gdb-commands not be in a better location in ext_data? It is not a function that is specific to doctests. That's my only reaction so far to that change, but I wouldn't push for the move too strongly if you tell me that it is too hard.
Replying to @kiwifb:
Would
sage-gdb-commandsnot be in a better location inext_data? It is not a function that is specific to doctests.
The purpose of this file is not documented, and it is only used there
I just looked at the content of the file for the first time in some years.
r
may be we can just get rid of it if it is the only place it is used. And that's what the gdb manual has to say about what it does https://sourceware.org/gdb/current/onlinedocs/gdb/Starting.html#Starting
In any case, ext_data is an outdated design and I'd rather not add to it.
François asks a good question about the purpose of this file. Could we instead just have a variable SAGE_GDB_COMMANDS defined as 'r' in doctest/control.py? Having a file which contains a single character seems a little ridiculous.
The name of the file is passed to gdb, it's a list of commands to execute upon starting. Of course we could also generate this file as a temporary file, but I don't think that's a cleaner design
Yes, it is all coming back to me now. I guess it is meant to be clever. In any case it is the design we have to deal with and if it is only used from the dosctest folder, then yes it should live there.
Actually the file is also used in src/bin/sage. I missed it because it's written as "${SELF}-gdb-commands"
Branch pushed to git repo; I updated commit sha1. New commits:
6cc5205 | src/sage/doctest/control.py: When invoking gdb, use sys.executable |
With recent gdb versions we can just use this option instead and get rid of the command file altogether
--eval-command=COMMAND, -ex
Execute a single GDB command.
Branch pushed to git repo; I updated commit sha1. New commits:
9d2c48f | src/bin/sage, src/sage/doctest/control.py: Eliminate use of sage-gdb-commands |
Description changed:
---
+++
@@ -1,5 +1,4 @@
(from #33625)
-This is just package_data and should be moved into `src/sage/doctest/`.
+Critical because this file is not compatible with editable installs (`setup.py develop`), see #31049
-Critical because this file is not compatible with editable installs (`setup.py develop`), see #31049The code handling ./sage --gdb in src/bin/sage has clearly not been tried in a long time. The SAGE_DEBUG logic is broken because this environment variable is only set in the build environment, not in sage -sh
Branch pushed to git repo; I updated commit sha1. New commits:
d24b928 | src/bin/sage (--gdb): Do not try to run cygdb unless SAGE_DEBUG=yes |
What's the advantage to explicitly including python (a.k.a. sys.executable) in
exec gdb --eval-command="run" --args ...python... sage-runtests --serial --timeout=0 hello_world.py
Without this change it wasn't working for me at all (on macOS): I got
"/Users/mkoeppe/s/sage/sage-rebasing/src/bin/sage-runtests": not in executable format: file format not recognized
I can't get it to work on OS X either way. The SPKG doesn't build for me ("C compiler cannot create executables" because "ld: library not found for -ltinfo"). If I run brew install gdb, then I get complaints about things not being codesigned. Anyway, with this branch things fail a bit better than with develop, and the changes make sense to me. Comments from anyone else?
Yes, the gdb SPKG is too old (see #30158). I have also not done the codesigning dance with the homebrew gdb this time to try out whether it really works on macOS
isn't gdb pretty much useless with macOS clang, and one should use lldb instead?
Let's merge this.
Reviewer: John Palmieri
Thanks!
Changed branch from u/mkoeppe/move_sage_gdb_commands_out_of_src_bin to d24b928
By the way, we missed an example in developer/doctesting.rst that mentions sage-gdb-commands. See #34295 for a follow-up.
Changed commit from d24b928 to none