Git Product home page Git Product logo

Comments (9)

casevh avatar casevh commented on June 24, 2024

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.

from gmpy.

skirpichev avatar skirpichev commented on June 24, 2024

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...

from gmpy.

casevh avatar casevh commented on June 24, 2024

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.

from gmpy.

skirpichev avatar skirpichev commented on June 24, 2024

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

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.

from gmpy.

skirpichev avatar skirpichev commented on June 24, 2024

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.

from gmpy.

casevh avatar casevh commented on June 24, 2024

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.

from gmpy.

skirpichev avatar skirpichev commented on June 24, 2024

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.

from gmpy.

casevh avatar casevh commented on June 24, 2024

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?

from gmpy.

skirpichev avatar skirpichev commented on June 24, 2024

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.

from gmpy.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.