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

bpo-36721: Add --embed option to python-config #13500

Merged
merged 4 commits into from May 23, 2019
Merged

Conversation

@vstinner
Copy link
Member

vstinner commented May 22, 2019

To embed Python into an application, a new --embed option must be
passed to "python3-config --libs --embed" to get "-lpython3.8" (link
the application to libpython). To support Python 3.7 and older, try
"python3-config --libs" (without --embed) on failure.

Add a pkg-config "python-3.8-embed" module to embed Python into an
application: "pkg-config python-3.8-embed --libs" includes
"-lpython3.8". To support Python 3.7 and older, try "pkg-config
python-X.Y --libs" (without "-embed") on failure (replace X.Y with
the Python version).

On the other hand, "pkg-config python3.8 --libs" no longer contains
"-lpython3.8". C extensions must not be linked to libpython (except
on Android, case handled by the script); this change is backward
incompatible on purpose.

"make install" now also installs "python-3.8-embed.pc".

https://bugs.python.org/issue36721

@vstinner

This comment has been minimized.

Copy link
Member Author

vstinner commented May 22, 2019

I tested with:

./configure CFLAGS="-O0" --enable-shared --prefix=/opt/py38
make
make install

Commands without -lpython, build a C extension:

$ /opt/py38/bin/python3.8-config --libs
 -lcrypt -lpthread -ldl  -lutil -lm -lm 

# empty output (expected result),
$ pkg-config python-3.8 --libs 

Commands with -lpython, embed Python into an applicaton:

$ /opt/py38/bin/python3.8-config --libs --embed
-lpython3.8 -lcrypt -lpthread -ldl  -lutil -lm -lm 

$ pkg-config python-3.8-embed --libs 
-L/opt/py38/lib -lpython3.8
@vstinner

This comment has been minimized.

Copy link
Member Author

vstinner commented May 22, 2019

@hroncok @xdegaye: Would you mind to review (may also test?) this change? I marked https://bugs.python.org/issue36721 as a release blocker.

@vstinner

This comment has been minimized.

Copy link
Member Author

vstinner commented May 22, 2019

cc @doko42

Doc/whatsnew/3.8.rst Outdated Show resolved Hide resolved
Doc/whatsnew/3.8.rst Outdated Show resolved Hide resolved
To embed Python into an application, a new --embed option must be
passed to "python3-config --libs --embed" to get "-lpython3.8" (link
the application to libpython). To support both 3.8 and older, try
"python3-config --libs --embed" first and fallback to "python3-config
--libs" (without --embed) if the previous command fails.

Add a pkg-config "python-3.8-embed" module to embed Python into an
application: "pkg-config python-3.8-embed --libs" includes
"-lpython3.8".  To support both 3.8 and older, try "pkg-config
python-X.Y-embed --libs" first and fallback to "pkg-config python-X.Y
--libs" (without --embed) if the previous command fails (replace
"X.Y" with the Python version).

On the other hand, "pkg-config python3.8 --libs" no longer contains
"-lpython3.8". C extensions must not be linked to libpython (except
on Android, case handled by the script); this change is backward
incompatible on purpose.

"make install" now also installs "python-3.8-embed.pc".
@vstinner vstinner force-pushed the vstinner:embed branch from d0e89be to a588843 May 22, 2019
@vstinner

This comment has been minimized.

Copy link
Member Author

vstinner commented May 22, 2019

I amended (and rebased) my commit to be able to update the commit to address @hroncok review.

@xdegaye

This comment has been minimized.

Copy link
Contributor

xdegaye commented May 22, 2019

As expected, all python-config commands print the argument to link against libpython on Android when python is built with this PR (in case you wonder, pthread is not printed as it is part of libc on Android):

generic_x86_64:/data/local/tmp/python $ sh bin/python3.8-config --libs
-lpython3.8d -ldl  -lm -lm
generic_x86_64:/data/local/tmp/python $ sh bin/python3.8-config --libs --embed
-lpython3.8d -ldl  -lm -lm
generic_x86_64:/data/local/tmp/python $ bin/python lib/python3.8/config-3.8d/python-config.py --libs
-lpython3.8d -ldl -lm -lm
generic_x86_64:/data/local/tmp/python $ bin/python lib/python3.8/config-3.8d/python-config.py --libs --embed
-lpython3.8d -ldl -lm -lm

pkg-config is not installed, so could not be tested.

Copy link
Contributor

xdegaye left a comment

Just a nit-pick

Misc/python-config.sh.in Outdated Show resolved Hide resolved
@vstinner

This comment has been minimized.

Copy link
Member Author

vstinner commented May 22, 2019

As expected, all python-config commands print the argument to link against libpython on Android when python is built with this PR (...)

Yeah, it's the expected behavior. But I asked you to double check just in case :-) I'm not confident when I modify the build system. Thanks!

@hroncok

This comment has been minimized.

Copy link
Contributor

hroncok commented May 22, 2019

Should we try to implement the waf part before this is merged to see how possible it is?

@vstinner

This comment has been minimized.

Copy link
Member Author

vstinner commented May 22, 2019

Should we try to implement the waf part before this is merged to see how possible it is?

Are you talking about the code to handle Python 3.8 and older version? The fallback described in the documentation should be trivial to implement, no? Why would it be not possible?

@hroncok

This comment has been minimized.

Copy link
Contributor

hroncok commented May 22, 2019

It should be entirely possible. It was just an idea.

@vstinner vstinner merged commit 0a8e572 into python:master May 23, 2019
5 checks passed
5 checks passed
Azure Pipelines PR #20190522.136 succeeded
Details
bedevere/issue-number Issue number 36721 found
Details
bedevere/news News entry found in Misc/NEWS.d
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@vstinner vstinner deleted the vstinner:embed branch May 23, 2019
@vstinner

This comment has been minimized.

Copy link
Member Author

vstinner commented May 23, 2019

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/

@hroncok

This comment has been minimized.

Copy link
Contributor

hroncok commented May 24, 2019

This is what it gave me in Fedora, feels a bit inconsistent:

   /usr/lib64/pkgconfig/python-3.8-embed.pc
   /usr/lib64/pkgconfig/python-embed-3.8d.pc
   /usr/lib64/pkgconfig/python3-embed.pc
@vstinner

This comment has been minimized.

Copy link
Member Author

vstinner commented May 24, 2019

Should be fixed by PR #13551 :-)

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

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.