Cython 3 regressions

[heinezen]

We added Cython 3.0.0 support in https://github.com/SFTtech/openage/pull/1523 but we still have to check for regressions introduced by the new version. Cython 3 is a huge update, so there could be several implications for speed and functionality.

We'll have to check each cython module and can compare the results in this issue.

[zoli111]

This definitely broke Cython <3.0.0

I tried to find out the problems with the CI (or at least Ubuntu, since Linux is what I'm familiar with), so I booted up an Ubuntu 22.04 VM, and experienced the following:

• Cmake couldn't find Toml11. I was able to fix this in a dirty way, by running sudo ln /usr/share/cmake/toml11/toml11Targets.cmake /lib/x86_64-linux-gnu/cmake/toml11/toml11Targets.cmake. After that, I was able to successfully run ./configure.
• When I tried to run make, I got Cython errors. Then I checked cython3 --version, it's 0.29.28. So I used git checkout to switch to a commit before the Cython3 fixes were merged, and then I was able to compile the project.

[heinezen]

@zoli111 For me, old Cython 0.29.32 worked just fine. That's what I tested with at least. The CI uses 0.29.36 with no complains too.

Can you post the errors that you got?

[zoli111]

Here is the error message:

[8/8] Cythonizing openage/main/main_cpp.pyx

Error compiling Cython file:
------------------------------------------------------------
...

cdef extern from "traceback.h":
    void _PyTraceback_Add(const char *funcname, const char *filename, int lineno)

cdef void PyTraceback_Add(const char *functionname, const char *filename, int lineno) noexcept with gil:
                                                                                     ^
------------------------------------------------------------

openage/cppinterface/exctranslate.pyx:50:86: Syntax error in C variable declaration
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/Cython/Build/Dependencies.py", line 1274, in cythonize_one_helper
    return cythonize_one(*m)
  File "/usr/lib/python3/dist-packages/Cython/Build/Dependencies.py", line 1250, in cythonize_one
    raise CompileError(None, pyx_file)
Cython.Compiler.Errors.CompileError: openage/cppinterface/exctranslate.pyx

Error compiling Cython file:
------------------------------------------------------------
...
import atexit
import builtins
import importlib

cdef void xincref(PyObject *ptr) noexcept with gil:
                                ^
------------------------------------------------------------

openage/cppinterface/pyobject.pyx:60:33: Syntax error in C variable declaration
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/Cython/Build/Dependencies.py", line 1274, in cythonize_one_helper
    return cythonize_one(*m)
  File "/usr/lib/python3/dist-packages/Cython/Build/Dependencies.py", line 1250, in cythonize_one
    raise CompileError(None, pyx_file)
Cython.Compiler.Errors.CompileError: openage/cppinterface/pyobject.pyx
multiprocessing.pool.RemoteTraceback: 
"""
Traceback (most recent call last):
  File "/usr/lib/python3.10/multiprocessing/pool.py", line 125, in worker
    result = (True, func(*args, **kwds))
  File "/usr/lib/python3.10/multiprocessing/pool.py", line 48, in mapstar
    return list(map(*args))
  File "/usr/lib/python3/dist-packages/Cython/Build/Dependencies.py", line 1274, in cythonize_one_helper
    return cythonize_one(*m)
  File "/usr/lib/python3/dist-packages/Cython/Build/Dependencies.py", line 1250, in cythonize_one
    raise CompileError(None, pyx_file)
Cython.Compiler.Errors.CompileError: openage/cppinterface/exctranslate.pyx
"""

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/usr/lib/python3.10/runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/usr/lib/python3.10/runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "/home/zoli/openage/buildsystem/cythonize.py", line 190, in <module>
    main()
  File "/home/zoli/openage/buildsystem/cythonize.py", line 162, in main
    cythonize_wrapper(modules, **cythonize_args)
  File "/home/zoli/openage/buildsystem/cythonize.py", line 90, in cythonize_wrapper
    cythonize(src_modules, **kwargs)
  File "/usr/lib/python3/dist-packages/Cython/Build/Dependencies.py", line 1118, in cythonize
    result.get(99999)  # seconds
  File "/usr/lib/python3.10/multiprocessing/pool.py", line 774, in get
    raise self._value
Cython.Compiler.Errors.CompileError: openage/cppinterface/exctranslate.pyx
make[3]: *** [CMakeFiles/cythonize.dir/build.make:118: py/cythonize_timefile] Error 1
make[2]: *** [CMakeFiles/Makefile2:3432: CMakeFiles/cythonize.dir/all] Error 2
make[1]: *** [Makefile:166: all] Error 2
make: *** [Makefile:36: build] Error 2

[heinezen]

Looks like the noexcept keyword was added in 0.29.31 : https://cython.readthedocs.io/en/latest/src/changes.html#id84

I think we can afford to not support older Cython versions, since pip can always fetch the newest version anyway. But we need to update our dependency list.

@zoli111 Do you maybe want to do that in another contribution? :D

[zoli111]

The CI uses 0.29.36 with no complains too.

I played around with the CI a bit in my own branch. I found that libtoml11-dev isn't even installed. After installing it and applying my makeshift fix to make Cmake find Toml11, the compilation could be started.

Then I got the same Cython error. I also checked the Cython version, and it's 0.29.28.

You can check the results at my GitHub Actions.

[heinezen]

I played around with the CI a bit in my own branch. I found that libtoml11-dev isn't even installed. After installing it and applying my makeshift fix to make Cmake find Toml11, the compilation could be started.

Oh sorry, I misunderstood you and was talking about our build CI (we use a different CI for PRs than GitHub action to support faster caching).

You're right about the GitHub actions, I'm going to start upgrading them now. Architecture changes were holding up CI changes, but packages like toml11 should definitely be part of the pipeline now.

[heinezen]

@zoli111 I've updated our docker and CI builds as well as the build requirements to Cython 0.29.31. Also fixed the missing toml libraries :)