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

python extension_module should not link to python library on macOS and Linux #4117

Closed
nirbheek opened this issue Sep 3, 2018 · 8 comments
Closed
Labels

Comments

@nirbheek
Copy link
Member

nirbheek commented Sep 3, 2018

Continuing from https://gitlab.gnome.org/GNOME/pygobject/issues/253#note_307567

python.extension_module() needs to be passed python_dep explicitly, which makes sense because meson can't always know what python the project wants to use, but it should not explicitly link to the Python so or dylib (but it should link to the dll) since it's a shared_module().

Currently projects need to use python_dep.partial_dependency(compile_args: true) on non-Windows platforms, but we should handle this automatically.

CCing @MathieuDuponchelle @tschoonj

@nirbheek nirbheek added the bug label Sep 3, 2018
@SoapGentoo
Copy link
Member

Isn't this playing with fire? What if the python 2 installation you linked against was compiled with UCS2 support, and the one you're using at runtime is compiled with UCS4 support? Isn't this mixing slightly dangerous, and not linking to the interpreter even more so?

@lazka
Copy link
Contributor

lazka commented Sep 5, 2018

I've found some upstream notes here: https://www.python.org/dev/peps/pep-0513/#libpythonx-y-so-1 There are some links to mailing list/bug tracker discussions at the end of that section.

@lazka
Copy link
Contributor

lazka commented Sep 5, 2018

https://github.com/python/cpython/blob/e42b705188271da108de42b55d9344642170aa2b/Lib/distutils/command/build_ext.py#L695

Looks like it depends on the platform and whether Python is build in shared mode (the default on Fedora, not on Debian)

@nirbheek
Copy link
Member Author

nirbheek commented Sep 5, 2018

So I guess we should duplicate that logic?

@lazka
Copy link
Contributor

lazka commented Sep 5, 2018

And Debian patches that logic away: https://sources.debian.org/src/python3.6/3.6.6-4/debian/patches/ext-no-libpython-link.diff/ So there is no way for us to know .. :(

edit: I guess this would work:

def links_against_libpython():
    from distutils.core import Distribution, Extension
    cmd = Distribution().get_command_obj("build_ext")
    return bool(cmd.get_libraries(Extension("dummy", [])))

print(links_against_libpython())

@nirbheek
Copy link
Member Author

nirbheek commented Sep 5, 2018

Hum. I guess the trend is towards not explicitly linking to libpython on Linux and macOS. I think that would be a good start, and we can change that based on feedback from distros?

We could also enable that for MSVC based on what distutils is doing. We already have a test for this, so we should know if it works.

@lazka
Copy link
Contributor

lazka commented Sep 5, 2018

Hum. I guess the trend is towards not explicitly linking to libpython on Linux and macOS. I think that would be a good start, and we can change that based on feedback from distros?

👍

gnomesysadmins pushed a commit to GNOME/pygobject that referenced this issue Sep 6, 2018
While on Windows it needs to link against it, on Unix systems
it doesn't have to if the process loading it has all the symbols
already loaded (python executable itself, or something dlopening
libpython with RTLD_GLOBAL before).

While on Fedora things get linked by default that's not the case with
Debian based distros and macOS, so most software has to deal with it
anyway and we ignore Fedora here for now.

Ideally this should get handled in meson itself, see:
mesonbuild/meson#4117
lazka added a commit to pygobject/pycairo that referenced this issue Sep 7, 2018
gnomesysadmins pushed a commit to GNOME/pygobject that referenced this issue Sep 7, 2018
While on Windows it needs to link against it, on Unix systems
it doesn't have to if the process loading it has all the symbols
already loaded (python executable itself, or something dlopening
libpython with RTLD_GLOBAL before).

While on Fedora things get linked by default that's not the case with
Debian based distros and macOS, so most software has to deal with it
anyway and we ignore Fedora here for now.

Ideally this should get handled in meson itself, see:
mesonbuild/meson#4117
lazka added a commit to lazka/meson that referenced this issue Sep 15, 2018
…or Windows. Fixes mesonbuild#4117

Windows requires things to be linked, on macOS distutils doesn't link by default.
On Linux etc. things are not so clear, some distros patch distutils to not link,
some don't. In addition the manylinux wheels spec disallows linking against libpython.

Just assume for now that not linking on non-Windows works.
lazka added a commit to lazka/meson that referenced this issue Sep 15, 2018
…or Windows. Fixes mesonbuild#4117

Windows requires things to be linked, on macOS distutils doesn't link by default.
On Linux etc. things are not so clear, some distros patch distutils to not link,
some don't. In addition the manylinux wheels spec prohibits linking against libpython.

Just assume for now that not linking on non-Windows works.
@lazka
Copy link
Contributor

lazka commented Sep 15, 2018

I've opened a PR: #4187

lazka added a commit to lazka/meson that referenced this issue Sep 16, 2018
…or Windows. Fixes mesonbuild#4117

Windows requires things to be linked, on macOS distutils doesn't link by default.
On Linux etc. things are not so clear, some distros patch distutils to not link,
some don't. In addition the manylinux wheels spec prohibits linking against libpython.

Just assume for now that not linking on non-Windows works.
lazka added a commit to lazka/meson that referenced this issue Dec 6, 2018
…does too. Fixes mesonbuild#4117

Windows requires things to be linked, on macOS distutils doesn't link by default.
On Linux etc. things are not so clear, some distros like Debian patch distutils to not link,
some don't. In addition the manylinux wheels spec prohibits linking against libpython
and upstream is thinking about changing the default:
https://bugs.python.org/issue34814

Call into distutils to figure out what distutils does and in case it doesn't link
against libpython replace the passed in Python dependency with a partial one.
gnomesysadmins pushed a commit to GNOME/gobject-introspection that referenced this issue Dec 17, 2018
tbeloqui pushed a commit to pexip/meson that referenced this issue Aug 22, 2019
…does too. Fixes mesonbuild#4117

Windows requires things to be linked, on macOS distutils doesn't link by default.
On Linux etc. things are not so clear, some distros like Debian patch distutils to not link,
some don't. In addition the manylinux wheels spec prohibits linking against libpython
and upstream is thinking about changing the default:
https://bugs.python.org/issue34814

Call into distutils to figure out what distutils does and in case it doesn't link
against libpython replace the passed in Python dependency with a partial one.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants