Conversation
There was a problem hiding this comment.
Pull request overview
This pull request refactors the monolithic Debugger class into a mixin-based architecture to enable plugin-style extensions. The refactor maintains backward compatibility while allowing advanced users to subclass Debugger and add custom mixins. The main changes include:
- Split the Debugger class into 9 focused mixins (ExecutionMixin, BreakpointMixin, ConfigurationMixin, etc.)
- Introduced a
DebuggerMetametaclass that detects method name collisions across mixins and creates aliased methods - Updated the
debugger()factory function to accept aclsparameter for custom subclasses - Changed initialization from a two-step pattern (
__init__+post_init_) to direct constructor injection ofInternalDebugger
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| libdebug/debugger/debugger.py | Replaced 1160+ lines with ~40 lines composing mixins using DebuggerMeta metaclass |
| libdebug/debugger/debugger_meta.py | New metaclass that detects and warns about mixin method collisions, creating aliased methods |
| libdebug/debugger/mixins/base.py | Base mixin class declaring _internal_debugger dependency |
| libdebug/debugger/mixins/core.py | Core initialization mixin accepting InternalDebugger in constructor |
| libdebug/debugger/mixins/execution.py | Process lifecycle controls (run, attach, kill, step, etc.) |
| libdebug/debugger/mixins/breakpoints.py | Breakpoint, watchpoint, signal and syscall handling |
| libdebug/debugger/mixins/configuration.py | Configuration properties (argv, env, arch, etc.) |
| libdebug/debugger/mixins/introspection.py | Read-only state accessors (threads, maps, symbols, etc.) |
| libdebug/debugger/mixins/thread_state.py | Main thread state properties (regs, syscall args, etc.) |
| libdebug/debugger/mixins/display.py | Pretty-print methods and representation |
| libdebug/debugger/mixins/gdb.py | GDB migration functionality |
| libdebug/debugger/mixins/snapshot.py | Snapshot creation and loading |
| libdebug/debugger/mixins/init.py | Exports all mixin classes |
| libdebug/debugger/init.py | Exports Debugger and all mixins for public use |
| libdebug/libdebug.py | Updated debugger() factory to accept cls parameter and use new constructor pattern |
| libdebug/debugger/internal_debugger.py | Updated child debugger creation to use new constructor |
| libdebug/init.py | Updated docstring to reflect mixin architecture |
| test/scripts/debugger_mixin_test.py | New test validating metaclass collision detection and aliasing |
| test/scripts/init.py | Added DebuggerMixinTest to test exports |
| test/run_suite.py | Added DebuggerMixinTest to fast test suite |
| newsfragments/291.improvement.md | Release note documenting the refactor |
| README.md | Added section explaining how to extend Debugger with custom mixins |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Should not have committed that file
2780b6a to
7ec0a2f
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
|
|
||
| __all__ = ["AliasTest", "AntidebugEscapingTest", "ArgumentListTest", "AtexitHandlerTest", "AttachDetachTest", "AutoWaitingTest", "BacktraceTest", "BreakpointTest", "BruteTest", "CallbackTest", "ControlFlowTest", "CorruptedELFTest", "CursedBinariesTest", "DeathTest", "DebuggerArgumentTest", "DeepDiveDivisionTest", "ELFUtilsUnitTest", "FinishTest", "FloatingPointTest", "JumpoutTest", "JumpstartTest", "LargeBinarySymTest", "MemoryTest", "MemoryNoFastTest", "MultipleDebuggersTest", "NextTest", "NlinksTest", "PPrintSyscallsTest", "RegisterTest", "RunPipesTest", "SignalCatchTest", "SignalMultithreadTest", "SpeedTest", "SyscallHandleTest", "SyscallHijackTest", "TimeoutTest", "ThreadTest", "Vmwhere1Test", "WatchpointTest", "FindPointersTest", "SymbolTest", "SnapshotsTest", "MultiprocessingTest", "MemoryLeakTest"] | ||
| __all__ = ["AliasTest", "AntidebugEscapingTest", "ArgumentListTest", "AtexitHandlerTest", "AttachDetachTest", "AutoWaitingTest", "BacktraceTest", "BreakpointTest", "BruteTest", "CallbackTest", "ControlFlowTest", "CorruptedELFTest", "CursedBinariesTest", "DebuggerMixinTest", "DeathTest", "DebuggerArgumentTest", "DeepDiveDivisionTest", "ELFUtilsUnitTest", "FinishTest", "FloatingPointTest", "JumpoutTest", "JumpstartTest", "LargeBinarySymTest", "MemoryTest", "MemoryNoFastTest", "MultipleDebuggersTest", "NextTest", "NlinksTest", "PPrintSyscallsTest", "RegisterTest", "RunPipesTest", "SignalCatchTest", "SignalMultithreadTest", "SpeedTest", "SyscallHandleTest", "SyscallHijackTest", "TimeoutTest", "ThreadTest", "Vmwhere1Test", "WatchpointTest", "FindPointersTest", "SymbolTest", "SnapshotsTest", "MultiprocessingTest", "MemoryLeakTest"] |
There was a problem hiding this comment.
The name 'JumpstartTest' is exported by all but is not defined.
| __all__ = ["AliasTest", "AntidebugEscapingTest", "ArgumentListTest", "AtexitHandlerTest", "AttachDetachTest", "AutoWaitingTest", "BacktraceTest", "BreakpointTest", "BruteTest", "CallbackTest", "ControlFlowTest", "CorruptedELFTest", "CursedBinariesTest", "DebuggerMixinTest", "DeathTest", "DebuggerArgumentTest", "DeepDiveDivisionTest", "ELFUtilsUnitTest", "FinishTest", "FloatingPointTest", "JumpoutTest", "JumpstartTest", "LargeBinarySymTest", "MemoryTest", "MemoryNoFastTest", "MultipleDebuggersTest", "NextTest", "NlinksTest", "PPrintSyscallsTest", "RegisterTest", "RunPipesTest", "SignalCatchTest", "SignalMultithreadTest", "SpeedTest", "SyscallHandleTest", "SyscallHijackTest", "TimeoutTest", "ThreadTest", "Vmwhere1Test", "WatchpointTest", "FindPointersTest", "SymbolTest", "SnapshotsTest", "MultiprocessingTest", "MemoryLeakTest"] | |
| __all__ = ["AliasTest", "AntidebugEscapingTest", "ArgumentListTest", "AtexitHandlerTest", "AttachDetachTest", "AutoWaitingTest", "BacktraceTest", "BreakpointTest", "BruteTest", "CallbackTest", "ControlFlowTest", "CorruptedELFTest", "CursedBinariesTest", "DebuggerMixinTest", "DeathTest", "DebuggerArgumentTest", "DeepDiveDivisionTest", "ELFUtilsUnitTest", "FinishTest", "FloatingPointTest", "JumpoutTest", "LargeBinarySymTest", "MemoryTest", "MemoryNoFastTest", "MultipleDebuggersTest", "NextTest", "NlinksTest", "PPrintSyscallsTest", "RegisterTest", "RunPipesTest", "SignalCatchTest", "SignalMultithreadTest", "SpeedTest", "SyscallHandleTest", "SyscallHijackTest", "TimeoutTest", "ThreadTest", "Vmwhere1Test", "WatchpointTest", "FindPointersTest", "SymbolTest", "SnapshotsTest", "MultiprocessingTest", "MemoryLeakTest"] |
No description provided.