"Bad" signals can't be ignored with _ok_code

[iamjackg]

https://github.com/amoffat/sh/blame/11ea5c852c108b1dc98bc415c6ae5a9419b5f67e/sh.py#L1739

It seems like it's currently impossible to use _ok_code to prevent an exception being raised for "bad" signals. The issue was introduced by this commit back in 2016: e98004c

>>> exit_code = -9
>>> ok_codes = [-9]
>>> SIGNALS_THAT_SHOULD_THROW_EXCEPTION = [9]
>>> success = exit_code in ok_codes
>>> bad_sig = -exit_code in SIGNALS_THAT_SHOULD_THROW_EXCEPTION
>>> not success or bad_sig
True
>>> exit_code not in ok_codes and (exit_code > 0 or -exit_code in
...         SIGNALS_THAT_SHOULD_THROW_EXCEPTION)
False

[ecederstrand]

It's not clear to me what you are actually attempting to do. What is your use case? It looks like you want a process to succeed even though it was SIGKILL'ed:

import signal
import sh

sh.sleep(999, _ok_code=[signal.SIGKILL])

SIGNALS_THAT_SHOULD_THROW_EXCEPTION is not meant for general consumption and cannot be altered.

[iamjackg]

Ah, sorry, I should have specified. You're correct: I want no exception to be raised when I send SIGKILL (or SIGTERM, or whatever) to a specific process. My actual use case:

• start pavumeter in the background to monitor audio levels for a stream
• start parec and ffmpeg in the foreground to record audio from that stream
• when done recording, kill pavumeter and proceed with the rest of the script (without forcing the user to manually close pavumeter)

I'm not suggesting modifying SIGNALS_THAT_SHOULD_THROW_EXCEPTION: that should stay as it is. I would just like the -9 (or -signal.SIGKILL) in _ok_code to be honored, which seemed to be possible until that commit in 2016 (accidentally?) removed the ability to do so.

There's maybe a conversation to be had on how to make signal allowlisting more intuitive, since the only mention of them just being negative exit codes is in the Architecture document, but not allowing them at all feels more confusing to me.

[iamjackg]

SIGNALS_THAT_SHOULD_THROW_EXCEPTION is not meant for general consumption and cannot be altered.

Just to further clarify, the code snippet in the first post was just me using the Python REPL to create a minimal reproduction of the issue, showing that the pre- and post-simplification conditions for the if in get_exc_exit_code_would_raise are not equivalent.

The "simplified" condition returns True, and causes an exception to be raised even if -9 is one of the allowlisted exit codes:

>>> not success or bad_sig
True

Whereas the pre-e98004c condition returns False, allowing me to ignore -9 if I so choose:

>>> exit_code not in ok_codes and (exit_code > 0 or -exit_code in
...         SIGNALS_THAT_SHOULD_THROW_EXCEPTION)
False

[amoffat]

Thanks for sleuthing @iamjackg. It indeed looks like a regression that nobody has noticed in 7 years. I'm kind of surprised there was no test case for this. It sounds like the fix is to revert back to the old logic (but make it easier to read), add a test, and add some documentation. If you agree, would you be willing to take a crack at it?

[iamjackg]

I'd love to, not sure when I'll have time, so if you get to this first by all means go for it.