Added option to hide importlib frames in stack

[wronglink]

I've made some research and here are the results:

1. As big part of python import system since python3.3 is mostly implemented in python, we now catch some importlib frames.
2. Actually it's a well known bug, and they have also made a hack over it. They added a special function _call_with_frames_removed that marks you code for import.c machinery that it must be cleaned up after __import__ call. But I guess it doesn't work for our case, because we interrupt code inside the import.

I think that best solution for us is to add optional stack clean-up of importlib trash frames at the view level. It would hide some inner python magic for most of users, but will also leave a way to debug importlib machinery (for example when custom import Finders and Loaders are being developed).

Before:

After:

Fixes #109

[asmeurer]

Hiding them for everything seems like a good solution, especially since they don't even have source to show.

[asmeurer]

I tested it and it appears to work.

Should we add the setting to the preferences UI?

[asmeurer]

Actually the behavior of import pudb.b is still a little weird in Python 3. The debugger stops on the import pudb.b line itself, instead of on the next line (so you have to press s for pudb to appear in the variables view). Also, the automatic marking of the line as a breakpoint doesn't work until the second time it is hit (where the above works as well). The correct behavior appears in Python 2.

However, this does make it at least usable in Python 3, which it wasn't before, so I'm +1 to merge as is, unless you have any ideas on the above issues.

[wronglink]

Should we add the setting to the preferences UI?

Maybe yes. Or at least mention the setting at README.

Actually the behavior of import pudb.b is still a little weird in Python 3

I think I need to do some investigation on these bugs. I didn't notice them before =) But I don't think it's related to current PR.

[asmeurer]

I think it has to do with the behavior that we are hiding in this PR. The debugger still thinks that it is being stopped inside the import, not after it. In master if you import pudb.b and then move up the stack to the module you see the same thing.

[asmeurer]

Actually, I misspoke above. You have to press n to get pudb to appear. If you press s (step in), it apparently does nothing, because you are stepping around in the hidden import code. If you press it enough times it eventually gets through it. So I'm actually a little weary about this change. It hides importlib in the UI, but the debugger itself is still going through it. Then again, it is highlighting the import pudb.b line itself in the debugger, so maybe no one will try using s there.

[wronglink]

@asmeurer I got it. I need some time to try to provide the correct behaviour. I'd like to ask you not to merge this PR until I understand how I can hack it.

[wronglink]

Hi there! I've finally get it work. The problem was with that we set a breakpoint at set_trace on a current frame line. But as the frame stack was cleaned up later, we didn't set the breakpoint at right place (we set it several frames lower at some importlib._bootstrap frames). So now I cleanup the stack and take the most top frame, and assume that it's the current one.

[asmeurer]

Does this still rely on the importlib hiding in the UI? I think we should remove that if it isn't used.

[wronglink]

Yes it does. I don't see any way to clean-up importlib frames from system stack (and also, don't think it's good idea). What we do:

0. Override the Bdb.get_stack method, and clean-up all hidden frames (if needed) there.
1. Detect the top unhidden frame at the set_trace call (actually, I call self.get_stack and get the top frame from there).
2. Send the cleaned-up stack to the UI, so there will be no imporlib trash and no way to accidentally go to importlib frame.

[asmeurer]

When is this empty?

[wronglink]

Well. Normally it shouldn't. But what if it would happen that all of frames were called from importlib._bootstrap? Sounds weird, but I don't think we have guarantee that stack list has at least one element.

[asmeurer]

OK, the check is fine. I agree it shouldn't happen, unless you do something really weird (like inject a set_trace in the importlib._bootstrap code). Might make the code easier to read if you inverted the check to avoid indenting the whole block:

if not stack:
    return
...

[wronglink]

Done. Thanks for idea.

[asmeurer]

OK, I tested it and it seems to work. I have no preference about having the flag in the settings, but we should probably at least document it (we have real docs now, so you can add it somewhere in there).

[wronglink]

I have no preference about having the flag in the settings

The flag is necessary if somebody wants to debug imporlib frames (for example a custom import finder or loader). In that case he would be able to switch to extreme-pro-mode :-)

so you can add it somewhere in there

Okay, I'll do that.

[asmeurer]

In that case he would be able to switch to extreme-pro-mode :-)

You would have to be really pro, since the importlib code is frozen, so PuDB shows no source code for it, meaning you either have to insert some code for it or debug blind.

[inducer]

I'm sorry to say I'm not a fan of this endeavor. The frames are there, and pretending they aren't is going to involve handling all sorts of poorly tested corner cases. (What if you step into a hidden frame? Out of one? Return through one?) I don't know importlib well, and I don't have time to learn. So I'd rather not be on the hook for maintaining code that I don't understand in detail.

[wronglink]

The frames are there, and pretending they aren't

Of course, that's an ugly hack, but I don't see any other way to fix the bug. And it's also the way that, that cpython solves this problem.

What if you step into a hidden frame?

You will get to the top unhidden one.

Out of one? Return through one?

The same. That is why I'm overriding the get_stack method. To get rid of trash frames as early as I can.

[wronglink]

@inducer I think that PuDB codebase has a bug in import pudb.b feature on python 3+. The bug is that we get unexpected frames at breakpoint. I see 3 ways to deal with it:

1. to fix it
2. to remove such feature (frankly speaking, it worked as importlib hack and had some magic from the start)
3. to do nothing

As for me, I'd vote for the first one. And I'm open to discuss any other way to fix it. The least desired for me is to pretend that it's not a bug, but a feature.

I'd like to know what is your point of view?

[asmeurer]

From my point of view, the ideal way to fix the bug would be to leave the stack frames alone, but move the debugger to the right frame on pudb.b. I don't know how easy it is to do this, though (I never figured it out). The fix here isn't this ideal fix, but it does work, and the impact of the workaround is minimal (or at least, minimized).

[wronglink]

the ideal way to fix the bug would be to leave the stack frames alone, but move the debugger to the right frame on pudb.b.

The same for me. At first I thought that we just need to calculate the set_trace depth and go back from current frame not 3, but several more. It didn't work. I think the problem is that importlib runs code through several context managers. And they add frames to stack (I guess on __exit__ call) after we figured out what frame we want to go to. That's why the stack is slightly different when we set trace, and when we run the debugger code. That is why I switched to another tactics with hiding importlib frames.

I also tried to wrap the __import__ function the same way that pudb.b does for the next imports, and don't let it run the importlib. But I don't see any way to do that on the fly (to change the __import__ behaviour during the __import__ call. With pytest-pudb I had a luck because pytest runs plugins hooks before users code and I could wrap importlib before user import pudb.b at first time.

[inducer]

Agree with @asmeurer that pudb.b should be where this gets fixed.

The same for me. At first I thought that we just need to calculate the set_trace depth and go back from current frame not 3, but several more. It didn't work.

What do you mean by "it didn't work"? Can we recognize the "right" frame (or the frame before that) by the function or module names?

[wronglink]

What do you mean by "it didn't work"?

At first I've played with stack depth in pudb.b set_trace call. This is the 0 level depth stack (the last frame is on pudb.b.set_trace():

set_trace b.py:21
<module> b.py:24
_call_with_frames_removed <frozen importlib._bootstrap>:205
exec_module [SourceFileLoader] <frozen importlib._bootstrap_external>:678
_load_unlocked <frozen importlib._bootstrap>:655
_find_and_load_unlocked <frozen importlib._bootstrap>:950
_find_and_load <frozen importlib._bootstrap>:961
test test.py:5
<module> test.py:20

Here is the current pudb.b 2 levels depth. It's quite obvious that we just go 2 frames back:

_call_with_frames_removed <frozen importlib._bootstrap>:205
exec_module [SourceFileLoader] <frozen importlib._bootstrap_external>:678
_load_unlocked <frozen importlib._bootstrap>:655
_find_and_load_unlocked <frozen importlib._bootstrap>:950
_find_and_load <frozen importlib._bootstrap>:961
test test.py:5
<module> test.py:20

It seems that we can skip these 5 imporlib frames and with 7 level depth will get to the test() function in test.py file (that is our actual target). But as I wrote it doesn't work. From the 4-th stack back we stuck in the context manager __exit__ frame, and it doesn't meter if we go back 4, 5 or more frames:

__exit__ [_installed_safely] <frozen importlib._bootstrap>:304
_load_unlocked <frozen importlib._bootstrap>:655
_find_and_load_unlocked <frozen importlib._bootstrap>:950
_find_and_load <frozen importlib._bootstrap>:961
test test.py:5
<module> test.py:20

[asmeurer]

I just noticed today that bdb has a builtin skip functionality for skipping filenames based on a glob. Perhaps we should just use that.

[sisrfeng]

Hi, some 2 years later, how can we hide importlib now ?
Thx!