-
-
Notifications
You must be signed in to change notification settings - Fork 30.7k
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
Add pkg-config python-3.8-embed and --embed to python3.8-config #80902
Comments
The bpo-21536 modified how C extensions are built: they are no longer linked to libpython. The problem is that when an application embeds Python: the application wants to be linked to libpython. commit 8c3ecc6 (HEAD -> master, upstream/master)
I chose to modify distutils, python-config (shell) and python-config.py (Python), but *not* Misc/python.pc (pkg-config). Previously, we had: $ pkg-config python-3.7 --libs
-lpython3.7m
$ python3.7-config --libs
-lpython3.7m -lcrypt -lpthread -ldl -lutil -lm
$ python3.7-config.py --libs
-lpython3.7m -lcrypt -lpthread -ldl -lutil -lm Now, we get: $ pkg-config python-3.8 --libs
-lpython3.8
$ python3.8-config --libs
-lcrypt -lpthread -ldl -lutil -lm -lm
$ python-config.py --libs
-lcrypt -lpthread -ldl -lutil -lm -lm python-config and python-config.py can now be used to build C extensions, but not to build an application embedding Python. pkg-config should not be used to build a C extenstion since it links to libpython, but we don't want to do that (see bpo-21536). I'm not sure that different tools should return different results. I propose:
On Windows, we already provide different binaries for embedded Python with "-embed" suffix:
|
I don't understand the need for this. AFAICT, the purpose of python-config is to provide configuration info for embedding Python (bpo-1161914 and https://docs.python.org/3/extending/embedding.html#compiling-and-linking-under-unix-like-systems). At some point, a pkg-config template was added (in bpo-3585) but it seems to duplicate the purpose of python-config and thus is also intended for use in embedding Python (although we don't seem to document it, it is familiar to users of automake). I don't know what other purposes either python-config or the pkg-config template serve other than for embedding. AFAIK, they should not be used for building C extension modules. But admittedly this area is not well-documented. In any case, I think pkg-config and python-config should return the same values for similar parameters. Anyone else have an opinion? |
As a note, waf seems to use python3-config for both embedded and extension modules. Currently, embedded is broken. See for example https://bugzilla.redhat.com/show_bug.cgi?id=1711638 |
I mark this issue as a release blocker: bpo-21536 basically broke the compilation of all applications which embed Python. IMHO this issue is the best solution to fix it.
Oh. Let me explain this issue differently. There are two use cases for libpython:
My bpo-21536 broke waf "pyembed" which fails to detect Python. waf "pyembed" creates a C program which calls Py_Initialize(), but the linker fails to locate Py_Initialize() symbol. To embed Python into an application, we really need to link the application to libpython: we need -lpython3.8. In Python 3.7, "pyext" and "pyembed" use cases were covered both by default "python3.7-config --libs" and "pkg-config python3.7 --libs" configuration. In Python 3.8, we now have to distinguish both use cases and provide *different* configuration depending if binary must be linked to libpython or not. More info about waf breakage in my waf bug report: FYI the waf breakage was discovered by trying to build libtdb on Fedora Rawhide with Python 3.8a4, it's a dependency of Samba which embeds Python: |
"AFAICT, the purpose of python-config is to provide configuration info for embedding Python" If that's the intention, then at least it's not used as such. It's also used to build/configure extensions using automake/cmake based build systems. There is one tool, and two different use cases. I think Victor's suggestion is appropriate |
Even if I'm not confident in my change (add --embed option), I chose to merge it anyway since at least "waf" build system is broken by my other changes (no longer link C extensions to libpython). I would like to get this change into Python 3.8 beta1 to attempt to fix most applications embedding Python. Anyway, if something goes wrong, we still have plenty of time to decide what to do before 3.8.0 final, scheduled for 2019-10-21: https://www.python.org/dev/peps/pep-0569/ -- Since I merged my change, I reset the priority from Release Blocker to normal. |
Miro commented my PR: """ /usr/lib64/pkgconfig/python-3.8-embed.pc I installed Python in release mode: $ ./configure --enable-shared --prefix=/opt/py38 CFLAGS="-O0" && make && make install It gives me: vstinner@apu$ ls -l /opt/py38/lib/pkgconfig/ Debug build: $ ./configure --enable-shared --prefix=/opt/py38dbg --with-pydebug CFLAGS="-O0" && make && make install It gives me: vstinner@apu$ ls -l /opt/py38dbg/lib/pkgconfig/ Oh! "python-embed-3.8d.pc" is a broken symbolic link to "python-embed-3.8.pc" (which doesn't exist). Ok, now I get it: I messed up in names of symbolic links :-) I wrote PR 13551 which gives me the following files for a debug build: vstinner@apu$ ls -l /opt/py38dbg/lib/pkgconfig/*.pc The format is now: "python" "version" "embed suffix". |
By the way, I'm not sure that the current layout of .pc files. The name "module" give a different configuration. Is that correct? $ grep ^Libs: /opt/py38/lib/pkgconfig/python-3.8-embed.pc
Libs: -L${libdir} -lpython3.8
$ grep ^Libs: /opt/py38dbg/lib/pkgconfig/python-3.8-embed.pc
Libs: -L${libdir} -lpython3.8d |
The initial issue has been fixed. As far as I know, all known issues have been fixed, so I close the issue. Thanks Miro for the help in reviews and tests! FYI waf is already fixed for the future Python 3.8 beta1! It now attempts to use --embed for its "pyembed" config ;-) Miro wrote a fix: https://gitlab.com/ita1024/waf/merge_requests/2236 |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: