Merge lp:~spiv/bzr/i-hate-signals into lp:bzr

Abstaining so I get CCed until I have a spare moment to look at this properly.

Of somewhat related interest on python-dev today:
<http://mail.python.org/pipermail/python-dev/2010-April/099165.html>
<http://bugs.python.org/issue7978>

I filed <http://bugs.python.org/issue8354> about this.

I should clarify that this patch doesn't fully workaround that bug, but it helps in the case that there's some IO between delivery of signals (so that my signal handler gets a chance to redo the siginterrupt call that Python undoes). The only relevant signal for current bzr is SIGWINCH, so IO just needs to happen more often than a window resize event, which is plausible — it did fix the immediate issue that I ran into as a user.

Okay, been through this, the upstream bug, and the signal module C code.
Diagnosis looks fine, as does the proposed patch to the C signal handler in the Python source, would be good to get that landed.

One thing I'm not clear on is how effective the workaround in this merge will be. As the Python handler won't run till the IO function returns, and we've prevented it returning early, there can (and in common cases will be) whole seconds elapsed before say, `sock.read(65536)` completes. Do window managers fire multiple SIGWINCHes if a terminal emulator is dragged wider, or do they tend to wait for the release?

It probably depends on the window manager: some have you drag an outline (or static snapshot) of the window, some do "live" resizing (and some can be configured either way). For me the most common way I resize a terminal is by maximising or unmaximising a GNOME Terminal, which thankfully only seems to trigger one SIGWINCH per (un)maximise.

There can be other ways to trigger a resize too, like opening a second tab in a maximised GNOME Terminal window: the creation of the tab widget consumes some screen real estate, so that's a terminal resize too. Opening a second tab and (un)maximising are relatively frequent operations for me.

I'm sure there are many real-world cases this workaround can't fix, sadly. But I can say that it definitely improved behaviour for me in some real-world cases, e.g. a 'bzr pull' over plain HTTP.

Tangentially, we do have the option to say that sigwinch is just too
much trouble and instead poll the window size when we need it. But
polling every time we update progress may be too often, and doing it
infrequently may cause mess when the window is resized.

Alternatively we could hook that signal from C, and make it just
update a global accessible from python.

--
Martin <http://launchpad.net/~mbp/>

Okay, so there are common situations this workaround-for-a-workaround will help with then. Can't test here, but diff looks fine to me. Oh, and I sympathise with the branch name.

Martin P: I think avoiding SIGWINCH, or at least a Python handler for it, is worth exploring. Although there's a reasonable-looking fix attached to the upstream bug, which would make the existing Python SIGWINCH handler just fine.

In the meantime, I'll merge this partial workaround because it is better than the status quo.