[merge] Control-backspace to drop into the debugger

[John Arbash Meinel]

Martin Pool wrote:
> On 4/17/07, Robert Collins <robertc at robertcollins.net> wrote:
>> On Tue, 2007-04-17 at 15:16 +1000, Martin Pool wrote:
>> >
>> > That puts you into pdb if an error occurs.  This puts you in at any
>> > time of your choosing.  They're different.
>> >
>> > I should maybe explain them in HACKING as well.
>>
>> Well if you do BZR_PDB=1 then hit ctrl-\, won't you be put into pdb ?
> 
> No, because sigquit just terminates Python.  It doesn't get turned
> into an exception like sigint does.
> 
> You can set BZR_PDB and then press C-c, but then you're in postmortem
> debugger mode, and the process is already shutting down. You can
> continue but then the process terminates.
> 
>> I guess I'm wondering if its /desirable/ to have this hook active all th
>> time.
> 
> Well the downsides are:
> 
> - Some more code is loaded unconditionally.  The change to bzr rocks
> benchmark time seems to be in the noise.
> - People may be confused if they hit C-\ accidentally.  I put the
> message in to try to make it clear.
> - Processes may get stuck if they're running noninteractively and get
> a sigquit, but that shouldn't just randomly happen.
> 
> Personally I think it's quite useful to have always available.
> 
> John's very cool --profile-imports showed that I was loading pdb all
> the time when it's not needed, so here's an updated bundle.
> 

I think Robert brings up a good idea of at least being able to disable
this with an env var. Maybe "BZR_SIGQUIT_EXITS" versus
"BZR_SIGQUIT_DEBUGS". (If we wanted just a knob to disable it, or enable
it respectively)


> === added file bzrlib/breakin.py // file-id:breakin.py-20070417043829-so46nevf9
> ... 78u713k-1
> --- /dev/null
> +++ bzrlib/breakin.py
> @@ -0,0 +1,28 @@

v- I think this is just 2007 (unless you started on it last year :)
> +# Copyright (C) 2006, 2007 Canonical Ltd
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA

-- Docstring and 2 blank spaces for the module, and a small docstring
for the _debug() function, please.

> +
> +def _debug(signal_number, interrupted_frame):
> +    import pdb
> +    import sys
> +    sys.stderr.write("** SIGQUIT received, entering debugger\n"
> +            "** Type 'c' to continue or 'q' to stop the process\n")
> +    pdb.set_trace()
> +

^- Is there a way to make sure 'pdb' is running in the original frame,
rather than running in the _debug() frame? (it may be the case already,
I haven't actually tried this)


-- Two spaces and a docstring.

> +def hook_sigquit():
> +    # when sigquit (C-\) is received go into pdb
> +    # XXX: is this meaningful on Windows?
> +    import signal
> +    signal.signal(signal.SIGQUIT, _debug)
> 
> === modified file bzr // last-changed:mbp at sourcefrog.net-20070417043912-2mwgd3h
> ... tam3a5uq2
> --- bzr
> +++ bzr
> @@ -73,6 +73,9 @@
>  import bzrlib.lazy_regex
>  bzrlib.lazy_regex.install_lazy_compile()
>  
> +import bzrlib.breakin
> +bzrlib.breakin.hook_sigquit()
> +
>  import bzrlib.decorators
>  if ('--lsprof' in sys.argv
>      or '--lsprof-file' in sys.argv

^- I'm happy to see this is only enabled by 'bzr' the front-end.

It might be good (at some point) to start factoring out all of the
things the front-end wants to set up. (And at that time we can change
the ~/.bazaar/ignore bogosity)

> 
> === modified file bzrlib/tests/blackbox/test_debug.py // last-changed:mbp at sourc
> ... efrog.net-20070417043912-2mwgd3htam3a5uq2
> --- bzrlib/tests/blackbox/test_debug.py
> +++ bzrlib/tests/blackbox/test_debug.py
> @@ -17,8 +17,12 @@
>  """Blackbox tests for -D debug options"""
>  
>  import os
> +import signal
> +import subprocess
> +import sys
> +import time
>  
> -from bzrlib.tests import TestCase
> +from bzrlib.tests import TestCase, TestSkipped
>  
>  class TestDebugOption(TestCase):
>  
> @@ -30,3 +34,23 @@
>          # here but it may be missing if the source is not in sync with the
>          # pyc file.
>          self.assertContainsRe(err, "Traceback \\(most recent call last\\)")

-- Extra blank line here

> +
> +class TestBreakin(TestCase):
> +
> +    def test_breakin(self):
> +        # Break in to a debugger while bzr is running
> +        # we need to test against a command that will wait for 
> +        # a while -- bzr serve should do
> +        #
> +        # this may not work on windows but i don't think this use of signals
> +        # will work there
> +        if sys.platform == 'win32':
> +            raise TestSkipped('breakin signal not tested on win32')
> +        proc = self.start_bzr_subprocess(['serve'])
> +        time.sleep(.5)
> +        os.kill(proc.pid, signal.SIGQUIT)
> +        time.sleep(.5)
> +        proc.stdin.write('q\n')
> +        err = proc.stderr.read()
> +        self.assertContainsRe(err, r'entering debugger')
> +        proc.wait()

pdb doesn't attach to the terminal? (It only attaches to stdin/stdout?)
Interesting.

Otherwise this seems generally useful to me. (+1)

John
=:->