Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFE: Add signal.strsignal(): string describing a signal #66864

Closed
dolda2000 mannequin opened this issue Oct 19, 2014 · 19 comments
Closed

RFE: Add signal.strsignal(): string describing a signal #66864

dolda2000 mannequin opened this issue Oct 19, 2014 · 19 comments
Labels
3.8 only security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@dolda2000
Copy link
Mannequin

dolda2000 mannequin commented Oct 19, 2014

BPO 22674
Nosy @birkenfeld, @pitrou, @vstinner, @benjaminp, @ned-deily, @bitdancer, @vajrasky, @seirl
PRs
  • bpo-22674: signal: add strsignal() #6017
  • bpo-22674: fix test_strsignal on OSX #6085
  • Files
  • strsignal.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2018-03-12.19:03:28.289>
    created_at = <Date 2014-10-19.19:12:54.060>
    labels = ['3.8', 'type-feature', 'library']
    title = 'RFE: Add signal.strsignal(): string describing a signal'
    updated_at = <Date 2018-03-12.19:03:28.288>
    user = 'https://bugs.python.org/Dolda2000'

    bugs.python.org fields:

    activity = <Date 2018-03-12.19:03:28.288>
    actor = 'pitrou'
    assignee = 'none'
    closed = True
    closed_date = <Date 2018-03-12.19:03:28.289>
    closer = 'pitrou'
    components = ['Library (Lib)']
    creation = <Date 2014-10-19.19:12:54.060>
    creator = 'Dolda2000'
    dependencies = []
    files = ['37018']
    hgrepos = []
    issue_num = 22674
    keywords = ['patch']
    message_count = 19.0
    messages = ['229691', '229695', '229697', '229698', '229717', '229726', '229738', '230026', '304290', '304335', '313390', '313650', '313660', '313661', '313663', '313665', '313667', '313670', '313679']
    nosy_count = 10.0
    nosy_names = ['georg.brandl', 'pitrou', 'vstinner', 'benjamin.peterson', 'ned.deily', 'r.david.murray', 'cvrebert', 'Dolda2000', 'vajrasky', 'antoine.pietri']
    pr_nums = ['6017', '6085']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue22674'
    versions = ['Python 3.8']

    @dolda2000
    Copy link
    Mannequin Author

    dolda2000 mannequin commented Oct 19, 2014

    Like it says on the tin, it would be nice if strsignal(3) were added to the signal module.

    @dolda2000 dolda2000 mannequin added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Oct 19, 2014
    @bitdancer
    Copy link
    Member

    This seems like a reasonable request to me. Do you want to propose a patch?

    @benjaminp
    Copy link
    Contributor

    A dictionary, signal number -> signal name might be more friendly.

    @bitdancer
    Copy link
    Member

    Yeah, the thinnest possible exposure of the strsignal API wouldn't really be that sensible for Python. But making the OS information available *somehow* makes sense.

    @birkenfeld
    Copy link
    Member

    Is it possible to determine the range of signal numbers? Otherwise it would be a guessing game where to stop querying when filling up the dictionary.

    A problem is also that if the signal number is not valid, the return value of strsignal() is unspecified, *and* there is no way to check for this situation because "no errors are defined".

    @vstinner
    Copy link
    Member

    I don't think that a strsignal() is required, signals now have a name attribute!

    Python 3.5.0a0 (default:07ae7bc06af0, Oct 16 2014, 09:46:01) 
    >>> import signal
    >>> signal.SIGINT
    <Signals.SIGINT: 2>
    >>> signal.SIGINT.name
    'SIGINT'

    @birkenfeld
    Copy link
    Member

    Nice. However, strsignal() returns not just SIGINT, but "Interrupted" etc.

    @vajrasky
    Copy link
    Mannequin

    vajrasky mannequin commented Oct 26, 2014

    Here is the preliminary patch. It's a thin wrapper of strsignal.

    Some issues and things:

    1. About Benjamin Peterson's request, what is the name of the dictionary supposed to be? Is everyone okay with Benjamin's suggestion?

    2. About George Brandl's question: "Is it possible to determine the range of signal numbers?" We have a heuristic algorithm:

    #ifndef NSIG
    # if defined(_NSIG)
    #  define NSIG _NSIG            /* For BSD/SysV */
    # elif defined(_SIGMAX)
    #  define NSIG (_SIGMAX + 1)    /* For QNX */
    # elif defined(SIGMAX)
    #  define NSIG (SIGMAX + 1)     /* For djgpp */
    # else
    #  define NSIG 64               /* Use a reasonable default value */
    # endif
    #endif
    
    if (sig_num < 1 || sig_num >= NSIG) {
            PyErr_SetString(PyExc_ValueError,
                            "signal number out of range");
            return NULL;
    }
    1. For the unknown signal, what is the description should be? "Unknown signal" like c function returns or None?

    2. What is the name of the function that wrap strsignal should be? I use strsignal for now. I just don't think it is appropriate.

    @seirl
    Copy link
    Mannequin

    seirl mannequin commented Oct 12, 2017

    Hey everyone,

    We would like to have that feature for a project where we have to use ctypes to achieve that. I don't know the answers to vajrasky's questions but I just wanted to chime in to say having this feature would be greatly appreciated. I can also work on the code if you need any help.

    Thanks!

    @vstinner
    Copy link
    Member

    1. For the unknown signal, what is the description should be? "Unknown signal" like c function returns or None?

    Hum, Linux returns "Unknown signal 12345". I propose to use this behaviour on all platforms (which provide strsignal()).

    @vstinner vstinner changed the title strsignal() missing from signal module RFE: Add signal.strsignal(): string describing a signal Oct 13, 2017
    @seirl
    Copy link
    Mannequin

    seirl mannequin commented Mar 7, 2018

    I updated Vajrasky's patch to rebase it onto master, use the clinic argument parser and improve the docs. I made a PR on GitHub so the review can be easier than a patch.

    I left a Co-Authored-By field so I'm not stealing the ownership of this patch :-P

    @pitrou pitrou added the 3.8 only security fixes label Mar 12, 2018
    @pitrou pitrou closed this as completed Mar 12, 2018
    @pitrou
    Copy link
    Member

    pitrou commented Mar 12, 2018

    This is now pushed. Thank you Antoine!

    @ned-deily
    Copy link
    Member

    test_strsignal is failing on macOS.

    ======================================================================
    FAIL: test_strsignal (test.test_signal.PosixTests)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/Users/nad/Projects/PyDev/active/dev/3x/source/Lib/test/test_signal.py", line 61, in test_strsignal
        self.assertEqual(signal.strsignal(signal.SIGINT), "Interrupt")
    AssertionError: 'Interrupt: 2' != 'Interrupt'
    - Interrupt: 2
    ?          

    + Interrupt

    Also:
    http://buildbot.python.org/all/#/builders/14/builds/779/steps/4/logs/stdio
    http://buildbot.python.org/all/#/builders/93/builds/435/steps/4/logs/stdio

    @ned-deily ned-deily reopened this Mar 12, 2018
    @pitrou
    Copy link
    Member

    pitrou commented Mar 12, 2018

    Ned, this is why I'd like bpo-33048 to be solved :-) Having to rely on the buildbot fleet for bugfix iteration is not convenient at all.

    @seirl
    Copy link
    Mannequin

    seirl mannequin commented Mar 12, 2018

    Yes, sorry, the issue is that we decided with pitrou to remove the osx specific handling.

    The fix should be:

    diff --git a/Lib/test/test_signal.py b/Lib/test/test_signal.py
    index fbb12a5b67..ae0351e992 100644
    --- a/Lib/test/test_signal.py
    +++ b/Lib/test/test_signal.py
    @@ -58,8 +58,10 @@ class PosixTests(unittest.TestCase):
             self.assertEqual(signal.getsignal(signal.SIGHUP), hup)
     
         def test_strsignal(self):
    -        self.assertEqual(signal.strsignal(signal.SIGINT), "Interrupt")
    -        self.assertEqual(signal.strsignal(signal.SIGTERM), "Terminated")
    +        self.assertTrue(signal.strsignal(signal.SIGINT)
    +                        .startswith("Interrupt"))
    +        self.assertTrue(signal.strsignal(signal.SIGTERM)
    +                        .startswith("Terminated"))
     
         # Issue 3864, unknown if this affects earlier versions of freebsd also
         def test_interprocess_signal(self):

    Should I submit a new PR for this?

    @pitrou
    Copy link
    Member

    pitrou commented Mar 12, 2018

    Should I submit a new PR for this?

    Please do.

    @seirl
    Copy link
    Mannequin

    seirl mannequin commented Mar 12, 2018

    Done, #6085

    As I said in the PR body, I can't test it myself, I don't have an OSX VM setup.

    @ned-deily
    Copy link
    Member

    Ned, this is why I'd like bpo-33048 to be solved :-) Having to rely on the buildbot fleet for bugfix iteration is not convenient at all.

    I want to see it solved, too. :-) But there are other core-devs out there who are in a better position to solve it at the moment; it's not particularly a macOS problem; it's a problem with using Homebrew and Travis, neither one of which I'm all that familiar with and which others have set up. And I'm in the middle of trying to get some releases and other stuff out the door. So, I'm not going to be able to spend time on it right now. Sorry!

    @pitrou
    Copy link
    Member

    pitrou commented Mar 12, 2018

    New changeset 019f5b3 by Antoine Pitrou (Antoine Pietri) in branch 'master':
    bpo-22674: fix test_strsignal on OSX (GH-6085)
    019f5b3

    @pitrou pitrou closed this as completed Mar 12, 2018
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.8 only security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    6 participants