sagemath / cysignals

cysignals: interrupt and signal handling for Cython. Source repository for https://pypi.org/project/cysignals/
GNU Lesser General Public License v3.0
43 stars 23 forks source link

Fix build when SIGSTKSZ is not a constant. #151

Closed kliem closed 2 years ago

kliem commented 3 years ago

This is intended to fix #150 as done in https://github.com/rr-debugger/rr/issues/2916.

enriqueartal commented 2 years ago

I used kliem:fix_non_constant_MINSIGSTKSZ to creat a cysignals package to be able to compile Sagemath in Fedora 35. The test of src/sage/rings/polynomial/polynomial_zmod_flint.pyx complains about a ) in line 68 of cysignals-CSI; with the correction the test is successful

kliem commented 2 years ago

I used kliem:fix_non_constant_MINSIGSTKSZ to creat a cysignals package to be able to compile Sagemath in Fedora 35. The test of src/sage/rings/polynomial/polynomial_zmod_flint.pyx complains about a ) in line 68 of cysignals-CSI; with the correction the test is successful

I requested the fixes to cysignals-CSI already in #149.

enriqueartal commented 2 years ago

A test of a giac file did not pass, with a complain to cysignals but I do not know if it is related with this fix.

./sage -t --long --warn-long 131.6 --random-seed=0 src/sage/libs/giac/init.py Running doctests with ID 2021-11-06-22-37-54-f7ebb02c. Git branch: u/mkoeppe/test_ticketpython_3_10_development_releases Using --optional=build,dochtml,fedora,pip,sage,sage.geometry.polyhedron,sage.rings.real_double,sage_spkg Doctesting 1 file. sage -t --long --warn-long 131.6 --random-seed=0 src/sage/libs/giac/init__.py


File "src/sage/libs/giac/init.py", line 18, in sage.libs.giac Failed example: B = gb_giac(I.gens()) # random Exception raised: Traceback (most recent call last): File "/usr/local/sage-310/local/var/lib/sage/venv-python3.10/lib64/python3.10/site-packages/sage/doctest/forker.py", line 694, in _run self.compile_and_execute(example, compiler, test.globs) File "/usr/local/sage-310/local/var/lib/sage/venv-python3.10/lib64/python3.10/site-packages/sage/doctest/forker.py", line 1088, in compile_and_execute exec(compiled, globs) File "<doctest sage.libs.giac[3]>", line 1, in B = gb_giac(I.gens()) # random File "/usr/local/sage-310/local/var/lib/sage/venv-python3.10/lib64/python3.10/site-packages/sage/libs/giac/init.py", line 118, in wrapper return func(*args, *kwds) File "/usr/local/sage-310/local/var/lib/sage/venv-python3.10/lib64/python3.10/site-packages/sage/libs/giac/init.py", line 353, in groebner_basis gb_giac = F.gbasis(list(var_names), giac_order) File "sage/libs/giac/auto-methods.pxi", line 6442, in sage.libs.giac.giac.GiacMethods_base.gbasis (build/cythonized/sage/libs/giac/giac.cpp:56012) return GiacMethods['gbasis'](self,args) File "sage/libs/giac/giac.pyx", line 2102, in sage.libs.giac.giac.GiacFunction.call (build/cythonized/sage/libs/giac/giac.cpp:153070) sig_on() RuntimeError: Aborted

kliem commented 2 years ago

I think the giac failure is not related. It's just wrapped in sig_on ... sig_off, so it jumps back to sig_on and raises an exception then, which is exactly what should happen. (At least from cysignals point of view, giac shouldn't fail of course).

kliem commented 2 years ago

I used kliem:fix_non_constant_MINSIGSTKSZ to creat a cysignals package to be able to compile Sagemath in Fedora 35. The test of src/sage/rings/polynomial/polynomial_zmod_flint.pyx complains about a ) in line 68 of cysignals-CSI; with the correction the test is successful

I requested the fixes to cysignals-CSI already in #149.

However, it also makes sense to have the fixes here. #149 is not as urgent as this issue.

dimpase commented 2 years ago

should we merge this?

kliem commented 2 years ago

should we merge this?

I think so and make a release pretty soon. I just closed #139, so that we can run some tests.

kliem commented 2 years ago

The testsuite fails apparently. Have to wait until the artifacts are uploaded.

More importantly my test that I added to configure does not work at all. Any help is welcome.

dimpase commented 2 years ago

What's the branch you're testing?

dimpase commented 2 years ago

Some of the runs seem to work, no?

kliem commented 2 years ago

Yes, but configure.ac isn't doing what it is supposed to do. Fedora 35 fails with exactly the problem we are trying to fix.

enriqueartal commented 2 years ago

Some of the runs seem to work, no?

I can confirm that it is possible to build a running copy of sage (from 9.4 with python 3.9 and from develop with python 3.10). There are more tickets involved (to be able to build in F35) and some tests fail, but I can use it in the daily work.

kliem commented 2 years ago

Some of the runs seem to work, no?

I can confirm that it is possible to build a running copy of sage (from 9.4 with python 3.9 and from develop with python 3.10). There are more tickets involved (to be able to build in F35) and some tests fail, but I can use it in the daily work.

I believe that the current state of the branch is not working, because configure.ac is not working.

You can try this with cloning this rep and just run make configure and ./configure. I'm guessing that the last test outputs:

checking whether MINSIGSTKSZ is constant... yes

Not what I was aiming for, because on Fedora 35 it is not constant. On the other hand, if I do slight modifications, it fails for me, e.g.

diff --git a/configure.ac b/configure.ac
index 003a5bc..552b71a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -229,7 +229,7 @@ AC_MSG_CHECKING([whether MINSIGSTKSZ is constant])
 AC_COMPILE_IFELSE([AC_LANG_SOURCE(
     [
     #include <signal.h>
-    static char alt_stack[MINSIGSTKSZ];
+    static char alt_stack[MINSIGSTKSZ + 5120];
     ])],
     dnl YES
     [AC_MSG_RESULT([yes])]

So I'm gathering that it is not testing, what it is supposed to test.

enriqueartal commented 2 years ago

Yes, but configure.ac isn't doing what it is supposed to do. Fedora 35 fails with exactly the problem we are trying to fix.

I just bundled your branch and pass it to the build of sage in F35 (I got errors if I build it directly, there are some hacks in spkg-install.in

enriqueartal commented 2 years ago

I do not understand what happens, I have errors when building outside sage; I attach the log of the sage installation cysignals_inside_sage.log

kliem commented 2 years ago

I do not understand what happens, I have errors when building outside sage; I attach the log of the sage installation cysignals_inside_sage.log

Does the latest push work for you. As you probably guessed, your config.log should say: checking whether MINSIGSTKSZ is constant... no

enriqueartal commented 2 years ago

Have you checked the patch in the src.rpm package for Fedora 35. There are several ones, maybe this one is relevant. It builds, though using your configure.ac, it still says it is a constant.

--- src/cysignals/implementation.c.orig 2021-05-27 14:03:35.653774857 -0600 +++ src/cysignals/implementation.c 2021-05-27 14:19:42.225312505 -0600 @@ -445,7 +445,8 @@ static void setup_alt_stack(void) /* Static space for the alternate signal stack. The size should be

kliem commented 2 years ago

it seems some pairs of [ ] are missing in the test program. Moreover iirc include statements come apart from the rest of the source, like in a different parameter?

Thank you. This seems to be exactly the problem: https://www.gnu.org/software/autoconf/manual/autoconf-2.65/html_node/Generating-Sources.html

Edit: Should have checked with make configure. What it compiled was

static char alt_stackMINSIGSTKSZ;

Of course this is going to work independent of whether MINSIGSTKSZ is a constant.

enriqueartal commented 2 years ago

Now it builds for me in Fedora 35. I will try now inside sage. Thanks!

kliem commented 2 years ago

As can be observed by the checks of #152, the changes here did not break anything.

But we do fix Fedora 35 and other new systems.

dimpase commented 2 years ago

A new release then?

kliem commented 2 years ago

Yes, I'm just trying to figure out a two bug fixes yet. The macos test suite doesn't work at all and ubuntu-trusty fails. If there are no easy solutions to them, I'll just leave it at that.

I'm running the doctests verbose to see if I can figure out the ubuntu-trusty issue:

https://github.com/kliem/cysignals/actions/runs/1498523090

The original logs were not useful at all:

ulimit 2>/dev/null -s 1024; python3 -B rundoctests.py src/cysignals/*.pyx
cd example && python3 setup.py clean build
Doctesting 5 files.
src/cysignals/alarm.pyx
running clean
running build
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-3.9
gcc -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -Wno-unused -g -O2 -g -O2 -Wp,-U_FORTIFY_SOURCE -fPIC -I/sage/local/var/lib/sage/venv-python3.9.7/lib/python+++3.9/site-packages/cysignals -I/sage/local/var/lib/sage/venv-python3.9.7/include/python3.9 -c cysignals_example.cpp -o build/temp.linux-x86_64-3.9/cysignals_example.o
creating build/lib.linux-x86_64-3.9
g++ -shared -Wl,-rpath-link,/sage/local/lib -L/sage/local/lib -Wl,-rpath,/sage/local/lib -Wl,-rpath-link,/sage/local/lib -L/sage/local/lib -Wl,-rpath,/sage/local/lib -Wl,-rpath+++-link,/sage/local/var/lib/sage/venv-python3.9.7/lib -Wl,-rpath,/sage/local/var/lib/sage/venv-python3.9.7/lib -L. -L/sage/local/lib -Wl,-rpath-link,/sage/local/lib -L/sage/local/+++lib -Wl,-rpath,/sage/local/lib -Wl,-rpath-link,/sage/local/lib -L/sage/local/lib -Wl,-rpath,/sage/local/lib -Wl,-rpath-link,/sage/local/lib -L/sage/local/lib -Wl,-rpath,/sage/lo+++cal/lib -Wl,-rpath-link,/sage/local/lib -L/sage/local/lib -Wl,-rpath,/sage/local/lib -g -O2 -Wp,-U_FORTIFY_SOURCE build/temp.linux-x86_64-3.9/cysignals_example.o -L/sage/local/v+++ar/lib/sage/venv-python3.9.7/lib -o build/lib.linux-x86_64-3.9/cysignals_example.cpython-39-x86_64-linux-gnu.so -lpari
src/cysignals/pselect.pyx
src/cysignals/pysignals.pyx
src/cysignals/signals.pyx
src/cysignals/tests.pyx
make[3]: *** [check-doctest] Error 1
make[3]: Target `check-install' not remade because of errors.

154 attempts to fix the mac test suite problem. If it doesn't work, it doesn't work.

dimpase commented 2 years ago

Does this happen locally on macOS too, or it's GitHub actions-only problem?

On Wed, 24 Nov 2021, 09:17 Jonathan Kliem, @.***> wrote:

Yes, I'm just trying to figure out a two bug fixes yet. The macos test suite doesn't work at all and ubuntu-trusty fails. If there are no easy solutions to them, I'll just leave it at that.

I'm running the doctests verbose to see if I can figure out the ubuntu-trusty issue:

https://github.com/kliem/cysignals/actions/runs/1498523090

The original logs were not useful at all:

ulimit 2>/dev/null -s 1024; python3 -B rundoctests.py src/cysignals/*.pyx cd example && python3 setup.py clean build Doctesting 5 files. src/cysignals/alarm.pyx running clean running build 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-3.9 gcc -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -Wno-unused -g -O2 -g -O2 -Wp,-U_FORTIFY_SOURCE -fPIC -I/sage/local/var/lib/sage/venv-python3.9.7/lib/python+++3.9/site-packages/cysignals -I/sage/local/var/lib/sage/venv-python3.9.7/include/python3.9 -c cysignals_example.cpp -o build/temp.linux-x86_64-3.9/cysignals_example.o creating build/lib.linux-x86_64-3.9 g++ -shared -Wl,-rpath-link,/sage/local/lib -L/sage/local/lib -Wl,-rpath,/sage/local/lib -Wl,-rpath-link,/sage/local/lib -L/sage/local/lib -Wl,-rpath,/sage/local/lib -Wl,-rpath+++-link,/sage/local/var/lib/sage/venv-python3.9.7/lib -Wl,-rpath,/sage/local/var/lib/sage/venv-python3.9.7/lib -L. -L/sage/local/lib -Wl,-rpath-link,/sage/local/lib -L/sage/local/+++lib -Wl,-rpath,/sage/local/lib -Wl,-rpath-link,/sage/local/lib -L/sage/local/lib -Wl,-rpath,/sage/local/lib -Wl,-rpath-link,/sage/local/lib -L/sage/local/lib -Wl,-rpath,/sage/lo+++cal/lib -Wl,-rpath-link,/sage/local/lib -L/sage/local/lib -Wl,-rpath,/sage/local/lib -g -O2 -Wp,-U_FORTIFY_SOURCE build/temp.linux-x86_64-3.9/cysignals_example.o -L/sage/local/v+++ar/lib/sage/venv-python3.9.7/lib -o build/lib.linux-x86_64-3.9/cysignals_example.cpython-39-x86_64-linux-gnu.so -lpari src/cysignals/pselect.pyx src/cysignals/pysignals.pyx src/cysignals/signals.pyx src/cysignals/tests.pyx make[3]: *** [check-doctest] Error 1 make[3]: Target `check-install' not remade because of errors.

154 https://github.com/sagemath/cysignals/pull/154 attempts to fix the

mac test suite problem. If it doesn't work, it doesn't work.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/sagemath/cysignals/pull/151#issuecomment-977684068, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJXYHFI3GTTXQLXJE2OO33UNSURXANCNFSM5E47TOBQ .

kliem commented 2 years ago

I don't know. I don't have a macOS.