Python bindings should use -undefined dynamic_lookup when Py_ENABLE_SHARED is 0
#92
Comments
|
Hmm I might have seen something similar to this before both in pono and CVC4. In the past, it had to do with the Python binary that was found not being the same version as the default Python libraries that Cython found to link against. I think it specifically showed up because of miniconda actually. I never found a good solution for this, but can look again. I would appreciate anything that you notice. We just had people build without miniconda and make sure they were using the right Python version. |
|
Hmm, that makes sense, it might be related to which Python library it's finding for the build (sounds like it's not the miniconda one). I'll poke around to see if I can get it to point to that one. |
|
I think it's related to this: conda-forge/python-feedstock#272. Still looking into it |
When you configure, it should print out the Python binary and libraries that it found. Although, I'm looking at some previous correspondence, and it seems like even if we forced it to be the same we were still getting the segfault. So I'm leaning towards it being the static linking issue linked above. |
|
Just to confirm, the configure script seems to be picking up the right python: I'll try playing around with the linker flags to see if I can fix it |
|
Poking around the cmake scripts, I don't see any references to python libraries/linking directly. Could this be inside the Cython package logic? |
|
Inside |
|
Ah, it's probably happening inside |
|
I tried removing the explicit linking of the libpython in |
|
Success! Turns out I had to add So it looks like the issue is resolved by fixing the linking logic. I can help look into how to configure the cmake build to do this automatically, but for now I'm going to move forward with a local build that works. We may have to submit a patch to scikit build which I think owns the cmake package for the python/cython extensions |
|
For now, in case anyone needs the workaround, in |
leonardt
changed the title
-undefined dynamic_lookup when Py_ENABLE_SHARED is 0
|
Note from the thread that makai linked above, the |
|
Thanks for looking into this @leonardt! It's much appreciated. I'll poke around a bit more to understand if this is something we can resolve in CMake scripts or if it needs to be changed in scikit. |
|
It looks like they have some logic related to this: but I'm not sure if it's used in the package flow for this build |
|
Hmm, seems like it should be using it: https://github.com/scikit-build/scikit-build/blob/afa978814325336ac318316ded64e3a82cf3bcf7/skbuild/resources/cmake/FindPythonExtensions.cmake#L427 I wonder if I'm using an older version of cmake that doesn't have this logic? |
|
Actually that line was added 4 years ago so that seems unlikely |
|
Yeah that's a while ago. What version of cmake are you using? I could always bump up the required version if that is the problem after all. |
Fixes stanford-centaur/pono#92 It seems that the `target_link_libraries_with_dynamic_lookup` logic doesn't properly discover this requirement on my version of macOS (10.15.6) and conda (conda 4.8.3). This uses the cmake command from conda-forge/python-feedstock#272 to read Py_ENABLE_SHARED. If this config var isn't set, we use `-undefined dynamic_lookup` rather than linking the python library directly
|
Fixed by stanford-centaur/smt-switch#107 |
Walked through the README steps and ran into a seg fault trying to run the test suite:
I traced the issue to the
import ponostep, here's a backtrace produced by lldb:Have you seen anything like this before? I'm going to try rebuilding from scratch to make sure I didn't mess up a step.
The text was updated successfully, but these errors were encountered: