Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't link against libpython on non-Windows targets. #5

Open
wants to merge 1 commit into
base: 2.3.0
from

Conversation

@kyonifer
Copy link

commented Aug 2, 2019

CPython extension modules that are explicitly linked against libpython will cause undefined behavior when being used by a statically linked python interpreter such as anaconda. For this reason, the pybind11 docs recommend not linking against libpython, saying

On Linux and macOS, it’s better to (intentionally) not link against libpython. The symbols will be resolved when the extension library is loaded into a Python binary. This is preferable because you might have several different installations of a given Python version (e.g. the system-provided Python, and one that ships with a piece of commercial software). In this way, the plugin will work with both versions, instead of possibly importing a second Python library into a process that already contains one (which will lead to a segfault).

For similar reasons Python 3.8 is moving to never linking against libpython on unix.

Currently, this wrap uses pkg-configs output, which will often include link flags for libpython. For example, on homebrew:

$ pkg-config python3 --libs
-L/usr/local/opt/python/Frameworks/Python.framework/Versions/3.7/lib -lpython3.7m

Looking at the way other meson projects handle this situation, it seems they solve it by excluding the pkg-config linker flags. See for example this pygobject patch and this pycairo patch. This MR applies a patch similar to the above pycairo patch to not link against libpython explicitly unless on windows.

@sarum9in

This comment has been minimized.

Copy link
Member

commented Aug 6, 2019

I am afraid I am not familiar with this project enough to review this change.

@vedranmiletic added the last build, @jeandet created the wrap. I would defer to them to take a look.

@jeandet

This comment has been minimized.

Copy link
Member

commented Aug 16, 2019

@sarum9in, @kyonifer this looks ok to me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.