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
Cygwin does not provide siginfo_t.si_band #65284
Comments
Folks, Is Cygwin supported by Python? We met the following error when compiling Python 3.3.2 on Cygwin 1.7.17: gcc -c -Wno-unused-result -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -I. -IInclude -I./Include -DPy_BUILD_CORE -o Modules/main.o Modules/main.c Any lights would be appreciated. Thanks, |
This code is conditional, it is surrounded by: #if defined(HAVE_SIGWAITINFO) || defined(HAVE_SIGTIMEDWAIT) Does Windows support sigwaitinfo() or sigtimedwait()? I would be surprised. You used configure? Check pyconfig.h for these two constants. |
cygwin is not officially supported. We do accept patches that improve cygwin support when they are narrowly targted and well motivated. |
Victor, I did do ./configure, without any parameters. Should I use it with "--without-signal-module" option? Will all signals be disabled if this option is specified? From Python document, it mentions Windows support the following signals: The following are found in pyconfig.h.in 804 /* Define to 1 if you have the Br, |
dellair jie: would you like to work on a patch? It's fine if not, but it may then be that there is no resolution to this issue for the coming months or years (unless somebody else volunteers to write a patch). Running configure should have created pyconfig.h. Please report what this file has; pyconfig.h.in is not configured. |
Yes, my pleasure. After configure on Cygwin, there is pyconfig.h generated, variables are as following: The error disappeared once changed HAVE_SIGWAITINFO to undef. The patches provided seem only for 3.2.2, not sure if it is ok to apply them directly on 3.3.2? |
Applied the patch 0001-CYGWIN-issue13756-Python-make-fail-on-cygwin.patch in case: http://bugs.python.org/issue13756 The build failed with parser module: Could anyone shed some lights please? Br, |
dellair jie: please focus on one issue at a time. The issue you reported here is the compile error with si_band; I will not discuss any other problems you bring up in this issue. Note that editing pyconfig.h is not a solution. It's a generated file, so it must not be edited. To diagnose this further, add the -dD option to the preprocessor output of signalmodule.c, i.e. run gcc -dD -Wno-unused-result -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -I. -IInclude -I./Include -DPy_BUILD_CORE -E ./Modules/signalmodule.c and attach the resulting file (or else study the output yourself). BTW, please also understand that Python 3.3 is no longer maintained; it would be good if you could run this porting project on Python 3.4 instead. |
Martin, Certainly running on 3.4 now. Will try to look into it tomorrow if no immediate follow up post. Br, |
Can you try to comment si_band code in signalmodule.c, and then run test_signal to see if sigwaitinfo works on Windows? test_sigwaitinfo() uses signal.alarm(1): does it work on Windows (with Cygwin)? (sigwaitinfo doc says "Availability: Unix.") |
dellair: According to the compiler output, ./pyconfig.h has #define HAVE_SIGWAIT 1
#define HAVE_SIGWAITINFO 1 So why did you say (in msg215221) that the file had /* #undef HAVE_SIGTIMEDWAIT */ (i.e. where do the comment signs come from) |
Martin, Thanks for the analysis! The following values came from Python 3.3.2, the #define HAVE_SIGWAITINFO 1 was without comment sign, I commented it out for the build to go through. The output was from 3.4, haven't checked the values yet, might be different. Will check it tomorrow. Br, |
STINNER Victor wrote:
Sure. It seems to me it is extracted from bpo-3871 . |
dellair jie wrote:
See bpo-18637 |
Martin, Here is the values presented on Python 3.4.0, in fact they are the same as 3.3.2, please note the key difference.
/* #undef HAVE_SIGTIMEDWAIT */
#define HAVE_SIGWAIT 1
#define HAVE_SIGWAITINFO 1
The SIGTIMEDWAIT is undef while the SIGWAIT is defined.
The patch Victor found (https://github.com/Alexpux/MSYS2-packages/blob/master/python3/3.3.2-cygwin-siginfo.patch) to modify sigalmodule.c works on both 3.3.2 and 3.4 (at least the build went through the sign failure.)
So shall we close this issue? Petrov, Br, |
Nope. Could you please try the test described in msg215249? We might integrate https://github.com/Alexpux/MSYS2-packages/blob/master/python3/3.3.2-cygwin-siginfo.patch but it would be better to fix sigwaitinfo() if it works without si_band. |
Victor, Would you please be more specific on test_signal? Br, |
Please try to compile Python 3.4 with the attached cygwin_si_band.patch applied, and then run test_signal. I would like to know if all tests pass, and if not, which tests are failing. I also would like to know if test_sigwaitinfo() runs on Cygwin. I'm not sure that signal.alarm(1) is available on Windows, whereas test_sigwaitinfo() uses this function. Or you can test manually sigwaitinfo(). Example on Windows using SIGINT and CTRL+c to send a signal: >>> import signal
>>> info = signal.sigwaitinfo({signal.SIGINT})
^C
>>> info
signal.struct_siginfo(si_signo=2, si_code=128, si_errno=0, si_pid=0, si_uid=0, si_status=3, si_band=0) I don't know how to test sigwaitinfo() on Windows. |
Victor, I suppose that Python needs to be built successfully before running test_signal. Should I open another Issue to track it? As long as the build goes through completely on Cygwin, I will go back and do the signal test. Br, |
Finally got it compiled on Cygwin! :) Victor, I am still having issue with the "make test" on Cygwin, hence can only do some manual testing: Output:
>>> info = signal.sigwaitinfo({signal.SIGINT})
^C
>>>
>>> info = signal.struct_siginfo((2, 128, 0, 0, 0, 3))
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: signal.struct_siginfo() takes an at least 7-sequence (6-sequence given)
>>> info = signal.struct_siginfo((2, 128, 0, 0, 0, 3, 0))
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: signal.struct_siginfo() takes an at most 6-sequence (7-sequence given)
>>> help(signal.struct_siginfo)
| | Data descriptors defined here: Feel free to let me know if more testing is required. Br, |
I wrote improved patch to remove the 'si_band'. This patch modifies the 'n_in_sequence' to conform to the number of structure members. $ python3.4.exe
Python 3.4.3 (default, Mar 2 2015, 22:23:56)
[GCC 4.9.2] on cygwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import signal
>>> signal.struct_siginfo([1, 2, 3, 4, 5, 6])
signal.struct_siginfo(si_signo=1, si_code=2, si_errno=3, si_pid=4, si_uid=5, si_status=6)
>>> signal.sigwaitinfo({signal.SIGINT})
^C
signal.struct_siginfo(si_signo=2, si_code=6, si_errno=0, si_pid=3352, si_uid=1000, si_status=0)
>>> signal.alarm(1); signal.sigwaitinfo({signal.SIGALRM})
0
signal.struct_siginfo(si_signo=14, si_code=4, si_errno=0, si_pid=5816, si_uid=1000, si_status=1629189728) |
Could you please use a define like SIGINFO_HAS_SI_BAND? Something like: #if defined(HAVE_SIGINFO) && !defined(__CYGWIN__)
/* Issue python/cpython#65284: In Cygwin, siginfo_t does not have si_band field. */
# define SIGINFO_HAS_SI_BAND
#endif And please generate patches not the git format. Otherwise, Rietveld is unable to generated the "review" link. |
Victor, In that case, May I edit configure script to generate the HAVE_* defines?
|
Does Cygwin use configure? If yes, go for configure. You can copy/paste my recent change for dirent.d_type field.
Are you using git or hg? If you use hg, just change the config to not use |
New patch uses configure script to set the compile condition for struct_siginfo.si_band. Could you make sure of having the si_band in another platform? |
I've run into this recently. Is there anything I can do to shepherd this issue toward a resolution status? |
Review the patch, confirm that it works for you (please indicate what cygwin and OS version, etc you test on). Also review this issue and make sure all open concerns have been addressed by the current patch. |
Hello, I'm writer for past patch. New patch changes: |
Oh, I made a mistake that is checking the si_band field without signal header. |
Thanks Masayuki for the updated patch. I agree, the new approach looks better. I will review the patch more carefully and test it soon. |
I've reviewed masamoto's last patch 3.5-issue21085-struct_siginfo-2.patch It applies cleanly and allows the signals module to compile on Cygwin64 2.5.1 / Windows 10. I attached versions of the patch that apply cleanly to tip, 3.4, and 3.3 as well. However, even with this issue resolved it can't be fully tested because there are other open issues preventing a successful build on Cygwin, including bpo-25658 and bpo-13756 Would it be ok to commit this even though it doesn't resolve all build issues? Or should I try to cobble together a meta-issue of current known build issues and make sure they all have working patches first? |
I think it would be reasonable to apply it, unless other devs object, given your confirmation that it solves this particular problem (and, looking at the code, it won't break anything...which the buildbots will confirm). This will only be applied to 3.5 and 3.6, since this is not a security issue. I've updated the versions accordingly. |
New changeset 6874928eae4b by Zachary Ware in branch 'default': |
Thanks for the patch, masamoto, and thanks for the review and rebase, erik.bray! I only applied this to 3.7. It's much more open to experimentation currently, and we have a way to go before Python can even be built on Cygwin. Once we reach a more stable point (say, the full test suite can be run, even if not everything passes), we can evaluate whether backporting all the changes it took to get there is worthwhile. |
New changeset 5673cf852daa by Zachary Ware in branch 'default': |
Thanks for the merge--targeting 3.7 for now and thinking about backporting later sounds fine to me. I'm also in the process of getting a Cygwin buildbot for Python up and running, but it's been slow going. That said, having a Cygwin buildbot is also high priority for my own (non-Python-specific) work. So once I have that up I will happily maintain a buildbot for Python on Cygwin. |
When you're ready to get your bot set up, ping me and I'll get you set up on the master side. Thanks! |
In 3.6 and 3.7 The Same Error is back on line 960 with gcc 6.3.0 on cygwin64. This issue is not entirely fixed even with the absolute latest commits. $ make
gcc -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-pro
totypes -std=c99 -Wextra -Wno-unused-result -Wno-unused-parameter -Wno-missin
g-field-initializers -I. -I./Include -DPy_BUILD_CORE -c ./Modules/signalmo
dule.c -o Modules/signalmodule.o
In file included from ./Include/Python.h:85:0,
from ./Modules/signalmodule.c:6:
./Modules/signalmodule.c: In function ‘fill_siginfo’:
./Modules/signalmodule.c:960:60: error: ‘siginfo_t {aka struct <anonymous>}’ has
no member named ‘si_band’; did you mean ‘si_pid’?
PyStructSequence_SET_ITEM(result, 6, PyLong_FromLong(si->si_band));
^
./Include/tupleobject.h:62:75: note: in definition of macro ‘PyTuple_SET_ITEM’
#define PyTuple_SET_ITEM(op, i, v) (((PyTupleObject *)(op))->ob_item[i] = v)
^
./Modules/signalmodule.c:960:5: note: in expansion of macro ‘PyStructSequence_SE
T_ITEM’
PyStructSequence_SET_ITEM(result, 6, PyLong_FromLong(si->si_band));
^~~~~~~~~~~~~~~~~~~~~~~~~
make: *** [Makefile:1741: Modules/signalmodule.o] Error 1 |
Decorater, since this issue was already closed, could you open a new one? And when you do, please add me to the nosy list. I'm still planning to get a Cygwin built bot up for Python, I've just had other various priorities. |
Misc/NEWS
so that it is managed by towncrier #552Note: 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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: