cython C-API is broken in published wheels (and in the build_wheels.yml CI workflow)

[skirpichev]

The problem is that we bundle libraries.

Then, first, probably we should also supply relevant header files (gmp.h, etc). Next, document how user can specify a custom path (gmpy2.libs) to libraries in the setup.py.

[casevh]

This is a fundamental flaw with binary wheels, a C-API, and gmpy2. An extension that uses gmpy2's CAPI must(*) use the same GMP, MPFR, and MPC libraries. The name-mangling that occurs on Linux and MacOS makes that somewhere between complicated and impossible. The assumption was that Python and the extensions use the same OS provided libraries. So the Cython tests can practically only be done when gmpy2 with the OS provided GMP, MPFR, and MPC libraries.

I think we could require that the development version of gmpy2 can be successfully compiled on the most recent Ubuntu LTS version. A workflow that tests gmpy2 against the most recent Ubuntu LTS and its libraries would be the best place to test Cython and the C-API.

Windows binary wheels are different. I'm currently including the header files, the DLLs, and the LIB definition files MSVC compiler under the gmpy2 directory. It should work with Cython and other extensions but I still need to test it.

(*) "Must" may be too strong of a word. It may work. But we can't guarantee it.

[skirpichev]

The assumption was that Python and the extensions use the same OS provided libraries.

Hmm, I don't think it's impossible. If you put header files to the gmpy2 package - they will be found. To find libraries (gmpy2.libs) - users could use LD_LIBRARY_PATH & set library_dirs option of the Extension class.

A workflow that tests gmpy2 against the most recent Ubuntu LTS and its libraries would be the best place to test Cython and the C-API.

We run these tests (pip_install_gmpy2.yml, but not for Ubuntu yet). But I don't see too much value in untested wheels with C-API for Cython...

[casevh]

I was able to successfully compile and run test_cython on Windows. I need to make some changes to the Windows process. More details to follow.

[skirpichev]

@casevh, if this was about 8a39f52 - I doubt this commit does fix anything. Cython tests works in the pip_install_gmpy2 for Windows too:

gmpy/.github/workflows/pip_install_gmpy2.yml

Lines 82 to 107 in 8a39f52

	windows:
	strategy:
	fail-fast: false
	matrix:
	os: [windows-latest]
	runs-on: ${{ matrix.os }}
	defaults:
	run:
	shell: msys2 {0}
	steps:
	- uses: actions/checkout@v3
	- uses: msys2/setup-msys2@v2
	name: Setup msys2
	with:
	update: true
	install: >-
	gcc
	gmp-devel
	mpfr-devel
	mpc-devel
	python-setuptools
	python-pip
	- run: pip install --upgrade pip
	- run: pip --verbose install -e .[tests]
	- run: pytest test/
	- run: PYTHONPATH=`pwd`/gmpy2 python test_cython/runtests.py

What doesn't work (and this issue is about): cython tests after installation of binary wheels in a clean environment.

[skirpichev]

Ok, I have successful cython tests locally (Linux) after installation of repaired (of repaired :)) wheels:

sk@note:~/src/gmpy/tmp-xxx/lib/python3.12/site-packages/gmpy2.libs $ ls -l
total 2.3M
-rw-r----- 1 sk sk  83K Dec 20 05:05 gmp.h
-rwxr-x--x 1 sk sk 728K Dec 20 03:55 libgmp-e0c82b6b.so.10.5.0
-rwxr-x--x 1 sk sk 660K Dec 20 03:55 libmpc-b4e96b6f.so.3.3.1
lrwxrwxrwx 1 sk sk   24 Dec 20 05:04 libmpc.so -> libmpc-b4e96b6f.so.3.3.1
-rwxr-x--x 1 sk sk 793K Dec 20 03:55 libmpfr-da886a52.so.6.2.1
lrwxrwxrwx 1 sk sk   25 Dec 20 04:09 libmpfr.so -> libmpfr-da886a52.so.6.2.1
lrwxrwxrwx 1 sk sk   25 Dec 20 04:08 libmpfr.so.6 -> libmpfr-da886a52.so.6.2.1
-rw-r----- 1 sk sk  17K Dec 20 04:06 mpc.h
-rw-r----- 1 sk sk  57K Dec 20 04:06 mpfr.h

As you can see, (1) header files were added. And (2) also symlinks to workaround library name mangling. include_dirs/library_dirs of the test_cython extension should include gmpy2.libs:

diff --git a/test_cython/setup_cython.py b/test_cython/setup_cython.py
index ad2de54..1e15895 100644
--- a/test_cython/setup_cython.py
+++ b/test_cython/setup_cython.py
@@ -1,19 +1,15 @@
 from setuptools import Extension, setup
 from Cython.Build import cythonize
-import platform
 import sys
 import os
 import gmpy2
 
-ON_WINDOWS = platform.system() == 'Windows'
 
 extensions = [
     Extension("test_cython", ["test_cython.pyx"],
-                include_dirs=sys.path + ([os.path.dirname(gmpy2.__file__)] if ON_WINDOWS else []),
-                library_dirs=sys.path + ([os.path.dirname(gmpy2.__file__)] if ON_WINDOWS else []),
-                libraries=['mpc','mpfr','gmp']
-            )
-]
+              include_dirs=sys.path + ([os.path.dirname(gmpy2.__file__), gmpy2.__path__[0] + '/../gmpy2.libs/']),
+              library_dirs=sys.path + ([os.path.dirname(gmpy2.__file__), gmpy2.__path__[0] + '/../gmpy2.libs/']),
+              libraries=['mpc','mpfr','gmp'])]
 
 setup(
     name="cython_gmpy_test",

But, probably, we should instead set PYTHONPATH to gmpy2.libs.

Similar workaround should work for MacOS (no name mangling, but a different directory: .dylibs) and Windows.

[casevh]

I've been debugging this while you were making also debugging it. I converged to roughly the same changes as you did.

Just some comments.

• Please don't use symlinks. They aren't well supported in (at least) Windows binary wheels and the Windows environment. While they work as expected in the MSYS2 environment, the contents don't survive getting zipped into the wheel format and deployed on a typical Windows file system. That's why gmpy2\gmpy2.h and gmpy2\gmpy2.pxd are corrupted in Windows wheels. We need to delete the symlink'ed files and copy the real files over from /src in setup.py. Or something similar. (I develop on a Windows server system that support symlinks, but copying the gmpy2 source tree to a Windows 11 VM corrupted the symlinks.)

• The only supportable way that Cython extensions can interoperate with gmpy2 is if the Cython extensions use the identical GMP, MPFR, and MPC libraries as gmpy2. Name mangling is required on Linux since we can't shadow the OS provided libraries. So on Linux, gmpy2 and Cython must both use the OS provided libraries or they must both use the same name-mangled libraries. I've chosen to not mangle the names on Windows since there not likely to be any conflicts with similarly named files. And names aren't mangled by default on WIndows.

• I debugged this years ago. GMP supports changing the memory allocation functions. MPFR uses the GMP allocation functions to return strings. Debugging crashes caused by different memory allocation functions in the same process is not something I plan to repeat.

• I found a Windows specific bug in gmpy2.pxd that I still need to address. "ctypedef long mp_limb_t" should be "ctypedef long long mp_limb_t" on 64-bit Windows.

The combination of a C-API for gmpy2, name-mangling of libraries in a binary wheel, libraries that maintain their own state, and the preference to use binary wheels versus Linux pre-compiled versions or compiling from source is a challenging problem.

Thanks for working on the fixes. I'll look at them tomorrow.

[skirpichev]

Please don't use symlinks.

Actually they aren't symlinks anymore after wheel pack (on the Linux host, but I believe - on any platform, as zipfile doesn't support symlinks).

Name mangling is required on Linux since we can't shadow the OS provided libraries.

Yes, I realize there are reasons for this. On another hand I doubt it's too easy to interfere with the OS provided libraries (gmpy2.libs is a very non-standard place to search for headers/libraries).

So on Linux, gmpy2 and Cython must both use the OS provided libraries or they must both use the same name-mangled libraries.

That's something we could document. Other third-party cython extensions, intended to interoperate with gmpy2 could use libraries from our wheels just as shown above. I will work on above solution for Linux/MacOS wheels and hopefully will come with a pr in a few days.

An easy way: remove cython-related stuff from binary wheels and warn users that to have cython interface - they must build the package from sources.

[casevh]

Actually they aren't symlinks anymore after wheel pack (on the Linux host, but I believe - on any platform, as zipfile doesn't support symlinks).

And on Windows, "gmpy2.h" just contains "../src/gmpy2.h". It's technically not a symlink anymore...

Yes, I realize there are reasons for this. On another hand I doubt it's too easy to interfere with the OS provided libraries (gmpy2.libs is a very non-standard place to search for headers/libraries).

Is it possible do disable the name mangling when creating a Linux binary wheel? If so, could we use one consistent set of name-mangled libraries for an entire release cycle? For Windows, I plan to use the same GMP, MPFR, and MPC libraries for the entire 2.2.x release cycle. A change to those libraries would force a version change.

That's something we could document. Other third-party cython extensions, intended to interoperate with gmpy2 could use libraries from our wheels just as shown above. I will work on above solution for Linux/MacOS wheels and hopefully will come with a pr in a few days.

An easy way: remove cython-related stuff from binary wheels and warn users that to have cython interface - they must build the package from sources.

Remove the C-API when a binary wheel is created? Or is it possible to identify if gmpy2 was distributed as a binary wheel (look for gmpy2.libs??) and not import _C_API into the main gmpy2 namespace?

[skirpichev]

And on Windows, "gmpy2.h" just contains "../src/gmpy2.h".

Hmm, seems to be a platform-specific issue.

Is it possible do disable the name mangling when creating a Linux binary wheel?

This is non-opt feature of auditwheel.

Remove the C-API when a binary wheel is created?

Just remove headers. Then we can decide to not import C-API at runtime.