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

BUG: spin gdb: launch Python directly so that breakpoint is caught #24123

Merged
merged 6 commits into from
Jul 8, 2023

Conversation

stefanv
Copy link
Contributor

@stefanv stefanv commented Jul 5, 2023

Closes #24121

@ngoldbaum
Copy link
Member

ngoldbaum commented Jul 5, 2023

It seems gdb has an option to follow fork to child processes. The following patch seems to work and it retains that it uses spin run under the hood:

diff --git a/.spin/cmds.py b/.spin/cmds.py
index 07e0d6b36..12bb1cbcc 100644
--- a/.spin/cmds.py
+++ b/.spin/cmds.py
@@ -148,8 +148,8 @@ def gdb(python_expr):
 
     """
     util.run(
-        ['gdb', '--args', 'python', '-m', 'spin', 'run',
-         'python', '-P', '-c', python_expr],
+        ['gdb', '-ex', 'set follow-fork-mode child', '--args', 'python', '-m', 'spin', 'run',
+         'python', '-c', python_expr],
         replace=True
     )

Fine with the approach in this PR too if you don't think using spin run is meaningful here.

@ngoldbaum
Copy link
Member

Also my Python install seems to complain about the -P argument you pass it,

Unknown option: -P
usage: python [option] ... [-c cmd | -m mod | file | -] [arg] ...
Try `python -h' for more information.

@stefanv
Copy link
Contributor Author

stefanv commented Jul 5, 2023

Yeah, -W only available in 3.11, so I can go back to "run", let me check.

@stefanv
Copy link
Contributor Author

stefanv commented Jul 5, 2023

OK, take another look please. I have to manipulate the sys.path a bit on Python <3.11 to make it work.

Another question about interface: will you always want to run Python code snippets, or should the command allow for arbitrary commands?

@stefanv stefanv force-pushed the spin-gdb-catch-breakpoint branch from 04f9d49 to 6c24303 Compare July 5, 2023 18:15
@ngoldbaum
Copy link
Member

ngoldbaum commented Jul 5, 2023

For me, I'd want first to check if the argument is a path that exists and then assume that path is a python script, and then don't pass -c in the case, otherwise use -c and assume the argument is a code snippet. I am often debugging longer scripts or have some setup so I can skip breakpoints that would be hit by import numpy as np.

@stefanv stefanv force-pushed the spin-gdb-catch-breakpoint branch from 482fc0e to a752408 Compare July 5, 2023 18:41
@stefanv stefanv changed the title BUG: Launch Python directly so that breakpoint is caught BUG: spin gdb: launch Python directly so that breakpoint is caught Jul 5, 2023
@stefanv
Copy link
Contributor Author

stefanv commented Jul 5, 2023

I think the interface is now a bit more intuitive.
You can specify code with -c like you would for Python, otherwise it handles like gdb.

Copy link
Member

@ngoldbaum ngoldbaum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think it would be nice to have a mode where you just do spin gdb my_script.py and spin handles constructing the appropriate arguments to do gdb --args python my_script.py.

My reason for that is admittedly a little niche: with pyenv I need to do things like gdb --args $(pyenv which python) my_script.py since the python executable in my path is a shim that delegates to the real python executable and gdb needs the real executable. Having a stronger guarantee that the python being executed is the same one executing spin is also nice.

.spin/cmds.py Outdated Show resolved Hide resolved
@stefanv
Copy link
Contributor Author

stefanv commented Jul 5, 2023

OK, done.

@stefanv stefanv force-pushed the spin-gdb-catch-breakpoint branch from afaf7c4 to 44bacd4 Compare July 5, 2023 20:21
Copy link
Member

@ngoldbaum ngoldbaum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost there, with the substantive change I suggested spin gdb test.py "just works" on my setup for a script that imports numpy, then calls os.kill() on itself. 🎉

.spin/cmds.py Outdated Show resolved Hide resolved
.spin/cmds.py Outdated Show resolved Hide resolved
Co-authored-by: Nathan Goldbaum <nathan.goldbaum@gmail.com>
@stefanv
Copy link
Contributor Author

stefanv commented Jul 5, 2023

Let's see!

Copy link
Member

@ngoldbaum ngoldbaum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let’s let someone else test with a different setup before we merge

@stefanv
Copy link
Contributor Author

stefanv commented Jul 7, 2023

Perhaps @mattip or @seberg?

@charris charris changed the title BUG: spin gdb: launch Python directly so that breakpoint is caught BUG: spin gdb: launch Python directly so that breakpoint is caught Jul 7, 2023
@seberg seberg merged commit fd23b35 into numpy:main Jul 8, 2023
57 checks passed
@seberg
Copy link
Member

seberg commented Jul 8, 2023

Possible follow-ups I noticed:

  • Most other commands rebuild I think, it might be nice to keep it consistent.
  • spin gdb without anything dosen't do anything useful, it should maybe use python? (spin python --gdb would also make sense to do that)

We could extend the help to give tips about setting break-points and start things (maybe even slash down the developer help on it and just say to check spin gdb --help?)

Anyway, just noting in case this might spread across many projects really. I might follow-up with running lldb the same way if the system is macos.

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

Successfully merging this pull request may close these issues.

BUG: spin gdb doesn't hit breakpoints
4 participants