Code review comment for lp:~spiv/bzr/sigwinch-without-signalmodule-583941

Revision history for this message
Martin Packman (gz) wrote :

Thanks for writing this, it's interesting to see. It's unfortunate we can't do the work that actually needs to happen in response to the signal (setting the COLUMNS envvar) as it's not an atomic operation. So, this essentially moves us back to polling, at which point, why bother with signals at all? Moving the width variable to a Python int wouldn't help either.

A few misc comments on the patch:

+if compiler.has_function('sigaction'):

Think that should be instead:

    if compiler.has_function('sigaction', includes=['signal.h']):

Also, putting it in the not-win32 section would at least mean no stderr kipple for me. ;)

Have been reading ncurses source, and they check for SA_RESTART on top of sigaction. Probably not something to worry about, have fewer platforms and less history with Bazaar.

review: Abstain

« Back to merge proposal