Skip to content

Commit

Permalink
gh-99942: correct the pkg-config/python-config flags for cygwin/android
Browse files Browse the repository at this point in the history
On shared build configurations, Cygwin and Android need to link to
libpython. See bpo-21536.

This was corrected in distutils with an explicit check to only link when
libpython is built shared, but implemented in configure.ac
unconditionally. The correct approach is to follow distutils, which
includes a comment regarding why:

- on Android, a shared libpython is RTLD_LOCAL, and thus available only
  to the main executable, not exported to loaded modules, but a static
  libpython *is* the main executable and thus exported

- on Cygwin, symbols in shared libraries must be resolved at link time

It's actually not clear to me what to do for Cygwin. Cygwin doesn't
actually build static libpython successfully. If it did, then extensions
probably need to be linked to the import library for the executable,
rather than the full static library. So omitting this here is probably
correct, but incomplete. Either way, it won't matter until other fixes
are made.
  • Loading branch information
eli-schwartz committed Jan 12, 2023
1 parent d9dff4c commit f40ce64
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
On Android, in a static build, python-config in embed mode no longer
incorrectly reports a library to link to.
2 changes: 1 addition & 1 deletion configure

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -5830,7 +5830,7 @@ AC_MSG_RESULT($LDVERSION)

# On Android and Cygwin the shared libraries must be linked with libpython.
AC_SUBST(LIBPYTHON)
if test -n "$ANDROID_API_LEVEL" -o "$MACHDEP" = "cygwin"; then
if test "$PY_ENABLE_SHARED" = "1" && ( test -n "$ANDROID_API_LEVEL" || test "$MACHDEP" = "cygwin"); then
LIBPYTHON="-lpython${VERSION}${ABIFLAGS}"
else
LIBPYTHON=''
Expand Down

0 comments on commit f40ce64

Please sign in to comment.