Skip to content

fix: android CMake support #5733

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jun 18, 2025
+13 −1
Merged

fix: android CMake support #5733

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions tools/pybind11NewTools.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,17 @@ if(TARGET ${_Python}::Python)
endif()

if(TARGET ${_Python}::Module)
# On Android, older versions of CMake don't know that modules need to link against
# libpython, so Python::Module will be an INTERFACE target with no associated library
# files.
get_target_property(module_target_type ${_Python}::Module TYPE)
if(ANDROID AND module_target_type STREQUAL INTERFACE_LIBRARY)
set_property(
TARGET ${_Python}::Module
APPEND
PROPERTY INTERFACE_LINK_LIBRARIES "${${_Python}_LIBRARIES}")
endif()
Comment on lines +246 to +255
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just looked at the original problem in vcpkg (pybind11 2.6.13) and came to a similiar fixup, however injected on top of find_package(Python). With 3.0.0, I found android broken again for a different reason, and now I'm here with questions:

older versions of CMake don't know that modules need to link against libpython

Has this issue been reported to CMake? I cannot see any relevant changes, so I don't understand how this is limited to "older versions".

IMO the situation is similar to Windows DLLs: no undefined symbols. And this is how it is handled for Windows:
https://github.com/Kitware/CMake/blob/6754d2f9b8d5ba24b87db232a0aedca8d15c707d/Modules/FindPython/Support.cmake#L1517-L1522
https://github.com/Kitware/CMake/blob/6754d2f9b8d5ba24b87db232a0aedca8d15c707d/Modules/FindPython/Support.cmake#L4327-L4335
There is no trace of ANDROID in this file.

set_property(TARGET ${_Python}::Module ...)

IMO it should be avoided to modify imported targets from other CMake modules. In particular, I don't see why it is not sufficient to modifiy pybind11::module.

INTERFACE_LINK_LIBRARIES "${${_Python}_LIBRARIES}"

This is the spot which causes trouble in vcpkg now. ${${_Python}_LIBRARIES} is initialized for multi-config use in target_link_libraries, with debug and optimized keywords which are not allowed when setting the property directly.
My initial fixup is to use $<LINK_ONLY:${_Python}::Python>.
Alternatively, is possible to use target_link_libraries(pybind11::module INTERFACE ${${_Python}_LIBRARIES})

Disclaimer: I'm a python rookie.
vcpkg WIP: microsoft/vcpkg#46539

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set_property(TARGET ${_Python}::Module ...)

IMO it should be avoided to modify imported targets from other CMake modules. In particular, I don't see why it is not sufficient to modifiy pybind11::module.

... Maybe the goal is to fix the following?

  Python_add_library(MyModule2 src2.cpp)
  target_link_libraries(MyModule2 PRIVATE pybind11::headers)

Copy link

@dg0yt dg0yt Jul 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing with the openimageio port, using ${${_Python}_LIBRARIES} still fails because the actual variable Python3_LIBRARIES is not set in the scope of this line of code.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, the openimageio issue is from looking for Interpreter Development.Module components if not on Windows. Development should be at least in OPTIONAL_COMPONENTS.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, it was me who originally wrote this code in #5714, but I didn't subscribe to this PR so I didn't notice your comments. Sorry about that.

Has this issue been reported to CMake? I cannot see any relevant changes, so I don't understand how this is limited to "older versions".

I'm currently working on a fix which I'll submit to CMake, and then I'll post a link here so you can review it. In the long run, this should allow all of these workarounds to be removed.

IMO it should be avoided to modify imported targets from other CMake modules. In particular, I don't see why it is not sufficient to modifiy pybind11::module.

... Maybe the goal is to fix the following?

  Python_add_library(MyModule2 src2.cpp)
  target_link_libraries(MyModule2 PRIVATE pybind11::headers)

Yes, that was the reason. This pattern is recommended in pybind11's documentation, so it will be used by many Python packages.

${${_Python}_LIBRARIES} is initialized for multi-config use in target_link_libraries, with debug and optimized keywords which are not allowed when setting the property directly.

Alternatively, is possible to use target_link_libraries(pybind11::module INTERFACE ${${_Python}_LIBRARIES})

Thanks, that seems like a good solution. I'll test it, and if it works, I'll submit it as another pybind11 PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's continue the discussion in the new PR:


set_property(
TARGET pybind11::module
APPEND
Expand Down
3 changes: 2 additions & 1 deletion tools/pybind11Tools.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,8 @@ target_link_libraries(
pybind11::module
INTERFACE
pybind11::python_link_helper
"$<$<OR:$<PLATFORM_ID:Windows>,$<PLATFORM_ID:Cygwin>>:pybind11::_ClassicPythonLibraries>")
"$<$<OR:$<PLATFORM_ID:Windows>,$<PLATFORM_ID:Cygwin>,$<PLATFORM_ID:Android>>:pybind11::_ClassicPythonLibraries>"
)

target_link_libraries(pybind11::embed INTERFACE pybind11::pybind11
pybind11::_ClassicPythonLibraries)
Expand Down
Loading