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

Please do not link with libpython #253

Closed
yarda opened this issue May 19, 2020 · 17 comments
Closed

Please do not link with libpython #253

yarda opened this issue May 19, 2020 · 17 comments
Assignees

Comments

@yarda
Copy link
Contributor

yarda commented May 19, 2020

Python extensions now don't need to link with libpython [1]. We are now compiling Hamlib in Fedora with:

make PYTHON_LIBS=""

and it seems to work OK.

[1] python/cpython#12946

@N0NB
Copy link
Contributor

N0NB commented May 23, 2020

So far as I know the main libhamlib library does not link to libpython.

Having built the Hamlib Python binding, I see that $PREFIX/lib/python2.7/site-packages/_Hamlib.so does link to libpython2.7.so.1.0 on my Debian Buster installation. Isn't this necessary?

Or, are you saying such linking is unnecessary for Python 3.0?

Again, such linking should only occur when the Python bindings are specifically built.

@yarda
Copy link
Contributor Author

yarda commented May 23, 2020

Yes, for the python extension it's not necessary to link with libpython for python-3.8 or newer, upstream PR: python/cpython#12946

Python 3.8 release notes:
https://docs.python.org/3/whatsnew/3.8.html

On Unix, C extensions are no longer linked to libpython except on Android and Cygwin. It is now
possible for a statically linked Python to load a C extension built using a shared library Python.
(Contributed by Victor Stinner in bpo-21536.)

I think you should use pkgconfig.

I.e. pkgconfig for python-3.7:

$ cat /usr/lib64/pkgconfig/python3.pc
# See: man pkg-config
prefix=/usr
exec_prefix=/usr
libdir=/usr/lib64
includedir=/usr/include

Name: Python
Description: Python library
Requires: 
Version: 3.7
Libs.private: -lcrypt -lpthread -ldl  -lutil
Libs: -L${libdir} -lpython3.7m
Cflags: -I${includedir}/python3.7m

And pkgconfig for python-3.8:

$ cat /usr/lib64/pkgconfig/python3.pc
# See: man pkg-config
prefix=/usr
exec_prefix=/usr
libdir=/usr/lib64
includedir=/usr/include

Name: Python
Description: Build a C extension for Python
Requires:
Version: 3.8
Libs.private: -lcrypt -lpthread -ldl  -lutil -lm
Libs:
Cflags: -I${includedir}/python3.8

Notice there is no -lpython, i.e.:

$ pkg-config --libs python3 #python-3.7
-lpython3.7m 
$ pkg-config --libs python3 #python-3.8

@N0NB
Copy link
Contributor

N0NB commented May 23, 2020

Thanks for that information!

@mdblack98
Copy link
Contributor

mdblack98 commented May 23, 2020 via email

@yarda
Copy link
Contributor Author

yarda commented May 23, 2020

Well, I am not python devel, so I don't know the details, but I think the main advantage is that you could load the extension with different interpreter. I think if you link explicitly with the -lpython and don't use the --as-needed linker flag there will be just unnecessary dep.

Could you use something like the following?:

pkg-config --libs python3 || PYTHON_LIBS=-lpython...

It would use pkg-config if available if not, the classic code.

@N0NB
Copy link
Contributor

N0NB commented May 23, 2020

There is already a dependency on pkg-config, Mike, at least as noted in configure.ac. Right now we don't use it directly to configure make variables though some of the included AX_* m4 macro packages do call it.

@mdblack98
Copy link
Contributor

mdblack98 commented May 23, 2020 via email

@yarda
Copy link
Contributor Author

yarda commented May 23, 2020

With the pkg-config --libs python3 || construct pkg-config is optional, i.e. people having the pkg-config would benefit from it, people not having it will build as before.

@mdblack98
Copy link
Contributor

mdblack98 commented May 23, 2020 via email

@yarda
Copy link
Contributor Author

yarda commented May 23, 2020

For me as the user the benefit is that I could run the extension with different interpreters (even statically linked ones) and don't need to recompile it. The binary without the unnecessary dep is not just a bit smaller but it also loads a bit faster.

@N0NB
Copy link
Contributor

N0NB commented May 23, 2020

Probably the easiest thing for us to do is to simply assign a nul string to PYTHON_LIBS if the Python version is 3.8 or higher.

N0NB added a commit to N0NB/Hamlib that referenced this issue May 24, 2020
Per issue Hamlib#253 when building
Python binding for version 3.8 or later, set PYTHON_LIBS="".
@N0NB
Copy link
Contributor

N0NB commented May 24, 2020

yarda, please check the linked PR and see if it works for you. I have tested it on my Debian 10 desktop with Python 2.7.16 and 3.7.3 as well as my laptop running Debian Testing and Python 2.7.18 and 3.8.3. libpython is linked in all versions except 3.8 and the version test should prevent linking with later versions as well.

@N0NB N0NB self-assigned this May 24, 2020
@mdblack98
Copy link
Contributor

mdblack98 commented May 24, 2020 via email

@N0NB
Copy link
Contributor

N0NB commented May 24, 2020

Okay, not a PR but a commit to my personal repository. It shows up in the Web page of Issue 253.

@N0NB
Copy link
Contributor

N0NB commented May 27, 2020

Nothing heard from Yarda, so assuming all is well so merged a279ab1 and am closing this issue.

@N0NB N0NB closed this as completed May 27, 2020
@yarda
Copy link
Contributor Author

yarda commented May 27, 2020

Sorry, I haven't noticed the github notification.

Thanks, LGTM, I will use the patch in Fedora downstream. The check seems to be for python 3.8, but I guess you will update it once python 3.9 will get stable.

One thing that concerns me is the Python upstream release note:
"C extensions are no longer linked to libpython except on Android and Cygwin."
So it seems to me that Android and Cygwin (if you support these platforms) still needs -lpython. The reason behind is unknown to me.

N0NB added a commit that referenced this issue May 27, 2020
Per issue #253 when building
Python binding for version 3.8 or later, set PYTHON_LIBS="".
@N0NB
Copy link
Contributor

N0NB commented May 27, 2020 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants