bpo-14527: fix (some) _ctypes liffi build problems #20451
bpo-14527: fix (some) _ctypes liffi build problems #20451rupertnash wants to merge 2 commits intopython:mainfrom
Conversation
|
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA). CLA MissingOur records indicate the following people have not signed the CLA: For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. If you have recently signed the CLA, please wait at least one business day You can check yourself to see if the CLA has been received. Thanks again for the contribution, we look forward to reviewing it! |
lapsintra
left a comment
There was a problem hiding this comment.
Worked for my setup.
Before following this commit, I had set PKG_CONFIG_PATH, LD_LIBRARY_PATH, LD_RUN_PATH and LIBRARY_PATH point towards the appropriate locations.
./configure did set correctly LIBFFI_INCLUDEDIR using pkgconfig
Still couldn't build module _ctypes
This commit solved my issue.
FFY00
left a comment
There was a problem hiding this comment.
This patch seems correct to me.
setup.py
Outdated
| if ffi_inc is not None: | ||
| for lib_name in ('ffi', 'ffi_pic'): | ||
| if (self.compiler.find_library_file(self.lib_dirs, lib_name)): | ||
| fullpath = self.compiler.find_library_file(self.lib_dirs + ffi_libdir, lib_name) |
There was a problem hiding this comment.
Why use setup.py heuristics to find libname when you could just let pkgconfig gives the right hint for compilation host/target with pkg-config libffi --libs-only-l like you did with LIBFFI_LIBDIR and LIBFFI_INCLUDEDIR ?
There was a problem hiding this comment.
That is a reasonable question.
The true answer is that while making my changes I tried to keep them as small as practical.
However when I came here to respond, I notice that this library search checks for libffi and libffi_pic, whether or not pkg-config is available on the compilation host.
If we simply create a new configure variable with say:
LIBFFI_LIBNAME="`"$PKG_CONFIG" libffi --libs-only-l 2> /dev/null | sed -e 's/^-l//;s/ *$//'`"
then we will lose the ability to find libffi_pic.{so,a} if there's no pkg-config.
We could set a default like LIBFFI_LIBNAME="ffi ffi_pic" and then have setup.py split it's value on space before looping much as now...
My feeling is to stick as we are, but happy to defer to core team
There was a problem hiding this comment.
whether or not pkg-config is available on the compilation host.
maybe if pkg-config is here then use it fully else use old way ( and good luck for cross compilation setup.py will pick anything in /usr instead of sysroot in most cases ).
pmp-p
left a comment
There was a problem hiding this comment.
my 3.9.1 android/wasm (with HAVE_FFI_PREP_CLOSURE_LOC and ffi git on wasm ) CI is quite happy with that patch, it looks like a good step forward into cross-compilation to me
76c8001 to
8fe5428
Compare
|
Dear devs - I had forgotten about this PR until I had to rebuild python yesterday on a machine with libffi in a non-standard location. Due to some major changes in setup.py around libffi, I've basically had to re-implement my patch. I've done this and force-pushed this branch. (Old commits https://github.com/rupertnash/cpython/tree/fix-ffi-build-old) I would appreciate your comments again and I will have a bit of time in the next week or two to actually get this merged |
|
I needed to make additional changes to pass -Wl,-rpath, to setup.py to get this to work. I wonder why there is support for building with an external SSL, but FFI lags behind. |
|
I'd like to chime in and say this worked for Python 3.10.8 with a slight modification, but the patch would not apply against 3.11.0. It would be super nice if this PR was set up so it would merge cleanly into the main branch. My rationale for that is we use an embedded interpreter that runs on myriad Linux distributions, so we can't count on the system libffi being the same across distributions. Hence we need to use our own built from source libffi and point the python build at that libffi. |
|
Sorry for the delay. I've tested a build with env vars given by I'll close this PR. Sorry again for not getting to it earlier. |
Fundamentally, the problem appears to be that configure uses pkgconfig to find the libffi include directory, while setup.py's detect_ctypes only uses the global list of library directories.
I have made an attempt at fixing this by having configure produce the directory containing libffi (
LIBFFI_LIBDIR) and setup.py use this. However I've hardly any experience with autotools, so I would be very happy to be corrected if this is no use at all.https://bugs.python.org/issue14527