sagemath / cysignals

cysignals: interrupt and signal handling for Cython
GNU Lesser General Public License v3.0
44 stars 23 forks source link

Cython complaints about place of nogil #174

Closed malb closed 10 months ago

malb commented 11 months ago

Those failures look more like a problem with the testing code than this PR?

dimpase commented 11 months ago

oops, sorry, it needs more work. It's the usual Python packaging mess :-(

dimpase commented 11 months ago

in https://github.com/sagemath/cysignals/actions/runs/5885338281/job/15961620534?pr=174 there is a compiler error, check it out (it's macOS with Python 3.11/clang)

dimpase commented 11 months ago

On Ubuntu, it builds, but test fails:

Compiling cysignals_example.pyx because it changed.
[1/1] Cythonizing cysignals_example.pyx
running build_ext
building 'cysignals_example' extension
creating build
creating build/temp.linux-x86_64-cpython-311
gcc -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -fPIC -I/home/runner/work/cysignals/cysignals/tmp/site-packages/cysignals -I/opt/hostedtoolcache/Python/3.11.4/x64/include/python3.11 -c cysignals_example.cpp -o build/temp.linux-x86_64-cpython-311/cysignals_example.o
cysignals_example.cpp:3657:14: warning: ‘long int* __pyx_f_17cysignals_example_safe_range_long(long int)’ defined but not used [-Wunused-function]
 3657 | static long *__pyx_f_17cysignals_example_safe_range_long(long __pyx_v_count) {
      |              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/cysignals/pselect.pyx
creating build/lib.linux-x86_64-cpython-311
g++ -shared -Wl,--rpath=/opt/hostedtoolcache/Python/3.11.4/x64/lib -Wl,--rpath=/opt/hostedtoolcache/Python/3.11.4/x64/lib build/temp.linux-x86_64-cpython-311/cysignals_example.o -L/opt/hostedtoolcache/Python/3.11.4/x64/lib -o build/lib.linux-x86_64-cpython-311/cysignals_example.cpython-311-x86_64-linux-gnu.so
src/cysignals/pysignals.pyx
src/cysignals/signals.pyx
src/cysignals/tests.pyx
Doctest src/cysignals/tests.pyx terminated. Timeout limit exceeded (>600s)
make[1]: *** [Makefile:97: check-prefix-doctest] Error 1
dimpase commented 11 months ago

@malb - also locally, make check-all fails with

...

export PYTHONUSERBASE="`pwd`/tmp/user" && python3 -B rundoctests.py src/cysignals/*.pyx
Doctesting 5 files.
...
src/cysignals/tests.pyx
**********************************************************************
File "src/cysignals/tests.pyx", line 958, in tests.pyx
Failed example:
    try:
        test_sig_occurred_live_exception()
    except RuntimeError:
        pass
Expected:
    RuntimeError: Aborted
Got nothing
malb commented 11 months ago

Ah, right, so it's a bigger job :(

dimpase commented 11 months ago

@malb - the main branch exhibits the same error in check-all.

tornaria commented 10 months ago

Could you just merge the first commit and bump version? It seems very safe and it really solves a compatibility issue (I think cysignals built with 0.29 will work with cython 3, but with a warning).

It's possible to build cysignals with cython 3 using legacy_implicit_noexcept=True (see https://github.com/void-linux/void-packages/blob/master/srcpkgs/python3-cysignals/patches/cython3-legacy.patch). However that option won't work on cython 2. In order to support both cython 0.29 and cython 3 it is necessary to add explicit noexcept clauses everywhere it's needed.

mkoeppe commented 10 months ago

Let's close this as a duplicate of #176