Add a functional test for restore_signals=True. #51
Conversation
It wasn't being compiled as anything useful due to a missing include of signal.h. This also adds the missing functional test that would have caught this. Needs pushing to upstream CPython as well.
gpshead
changed the title
|
it was being compiled properly in most environments, just not one we had. this adds a functional test and makes sure signal.h is explicitly included. though in many environments this already happens. |
|
@gpshead, did you file a bpo for this? We would like to get this fix for both python and python-subprocess32 in Fedora and RHEL. |
ChangeLog
Outdated
| ----------------- | ||
| 2018-06-04 3.5.2 (not yet released) | ||
| ----------------- | ||
| * Fixed: restore_signals=True was a no-op due to a missing #include. |
There was a problem hiding this comment.
Is this true for everywhere, or only on some platforms?
| @@ -2,6 +2,12 @@ | |||
| This file is #included by _posixsubprocess.c and the functions | |||
| are declared static to avoid exposing them outside this module. */ | |||
|
|
|||
| /* _posixsubprocess_config.h was already included by _posixsubprocess.c | |||
| * which is #include'ing us despite the .c name. HAVE_SIGNAL_H comes | |||
| * from there. Yes, confusing! */ | |||
There was a problem hiding this comment.
Yes this is confusing. If this module is not compiled separately but included by _posixsubprocess.c, why not name this _posixsubprocess_helpers.c? I guess we keep it as is because this is the way it was in upstream?
There was a problem hiding this comment.
Upstream CPython doesn't have this file, the helpers file contains things that are built in to the Python 3 interpreter.
|
I am unable to reproduce this as a bug upstream. It does the right thing. Test added regardless: python/cpython#7414 |
It wasn't being compiled as anything useful due to a missing
include of signal.h. This also adds the missing functional test
that would have caught this. Needs pushing to upstream CPython
as well.
Reported by Tom Knych.