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

Add macros for generating pybind11 bindings #400

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
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
98 changes: 98 additions & 0 deletions cmake/GzPython.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,101 @@ find_package(Python3 ${GZ_PYTHON_VERSION} QUIET)
if(Python3_EXECUTABLE AND NOT PYTHON_EXECUTABLE)
set(PYTHON_EXECUTABLE ${Python3_EXECUTABLE})
endif()

# Macro Definitions #

#################################################
# Insert options required to control the include of pybind11
#
# Usage:
#
# gz_add_pybind_project_settings()
#
# Variables defined by this macro:
#
# SKIP_PYBIND11 Skip generating of Python bindings if OFF
# USE_SYSTEM_PATHS_FOR_PYTHON_INSTALLATION Install modules in default system path if ON
# USE_DIST_PACKAGES_FOR_PYTHON Use dist-package over site-package for module installation if ON
#
macro(gz_add_pybind_project_settings)
Copy link
Member

Choose a reason for hiding this comment

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

apologies for not reviewing this sooner

I actually deprecated GzPython in #431; I think these new macros should be in a new file like GzPybind.cmake

set(skip_pybind11_default_value OFF)
if (MSVC)
# We're disabling pybind11 by default on Windows because they
# don't have active CI on them for now.
set(skip_pybind11_default_value ON)
endif()

option(SKIP_PYBIND11
"Skip generating Python bindings via pybind11"
${skip_pybind11_default_value})

include(CMakeDependentOption)

cmake_dependent_option(USE_SYSTEM_PATHS_FOR_PYTHON_INSTALLATION
"Install Python modules in standard system paths in the system"
OFF "NOT SKIP_PYBIND11" OFF)

cmake_dependent_option(USE_DIST_PACKAGES_FOR_PYTHON
"Use dist-packages instead of site-package to install Python modules"
OFF "NOT SKIP_PYBIND11" OFF)
endmacro()


#################################################
# Search for pybind11 and its dependencies if SKIP_PYBIND11 is not set
#
# Usage:
#
# gz_add_pybind_project_settings() <- required!
# gz_search_for_pybind()
#
# Variables defined by this macro:
#
# Python3_* Python result variables from FindPython3 module
# pybind11_* pybind11 result variables from pybind11 module
#
macro(gz_search_for_pybind)
if (SKIP_PYBIND11)
message(STATUS "SKIP_PYBIND11 set - disabling python bindings")
else()
find_package(Python3 ${GZ_PYTHON_VERSION} QUIET COMPONENTS Interpreter Development)
Copy link
Contributor

Choose a reason for hiding this comment

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

gazebosim/gz-math#588 updated the code in gz-math to use OPTIONAL_COMPONENTS here.
https://github.com/gazebosim/gz-math/blob/ef2211c6292929ea6a8471815cf83315c1b891cb/CMakeLists.txt#L102-L104

Mind updating this to match?

cc @scpeters

Copy link
Member

Choose a reason for hiding this comment

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

I think I haven't been completely consistent with using COMPONENTS or OPTIONAL_COMPONENTS

Copy link
Member

Choose a reason for hiding this comment

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

ok, in gz-math8 and sdformat15, I used made finding the Python3 Interpreter REQUIRED and used OPTIONAL_COMPONENTS to find Development, while we didn't apply this logic in gz-transport or gz-sim, probably because I didn't realize the inconsistency

I think I do recommend the approach used in gz-math and sdformat, but I also recognize the lack of flexibility in finding packages from a macro. I would suggest calling find_package(Python3 ...) from within the macro if NOT Python3_Development_FOUND, which would allow a package to use their own call to find_package(Python3) but fall back to a good default if it hasn't been searched for yet


if (NOT Python3_FOUND)
GZ_BUILD_WARNING ("Python is missing: Python interfaces are disabled.")
message (STATUS "Searching for Python - not found.")
else()
message (STATUS "Searching for Python - found version ${Python3_VERSION}.")

set(PYBIND11_PYTHON_VERSION 3)
find_package(pybind11 CONFIG QUIET)

if (${pybind11_FOUND})
message (STATUS "Searching for pybind11 - found version ${pybind11_VERSION}.")
else()
GZ_BUILD_WARNING ("pybind11 is missing: Python interfaces are disabled.")
message (STATUS "Searching for pybind11 - not found.")
endif()
endif()
endif()
endmacro()


#################################################
# Add the given directory to the CMake project and thus process the inner CMakeLists.txt
# which should contain the pybind module (e.g. pybind11_add_module) instructions.
#
# Usage:
#
# gz_add_pybind_project_settings() <- required!
# gz_search_for_pybind() <- required!
# gz_add_pybind_directory(my_bindings)
#
# Argument required by this macro:
# DIRECTORY Path to the folder with pybind module(s)
#
macro(gz_add_pybind_directory DIRECTORY)
if (pybind11_FOUND AND NOT SKIP_PYBIND11)
# Bindings subdirectory
add_subdirectory(${DIRECTORY})
endif()
endmacro()