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.

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

Permit building python bindings separately from gz-math library #636

Merged
merged 8 commits into from
Oct 28, 2024
Merged

Permit building python bindings separately from gz-math library #636

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
646de60
Permit to build bindings against an external gz-math
traversaro Sep 26, 2024
d2bae80
Fix python install path
scpeters Oct 11, 2024
a670b65
Reference brew install instructions in tutorial
scpeters Oct 14, 2024
54978b9
Add details about building python bindings
scpeters Oct 14, 2024
cb32516
Use same required cmake version
scpeters Oct 18, 2024
63d16a8
Remove unneeded call to enable_testing()
scpeters Oct 18, 2024
b475bdc
Merge branch 'gz-math8' into scpeters/build_python_bindings_separately
scpeters Oct 18, 2024
06b8ab8
Find pybind11 in just one place
scpeters Oct 18, 2024
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
18 changes: 17 additions & 1 deletion src/python_pybind11/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,19 @@
# Detect if we are doing a standalone build of the bindings, using an external gz-math
if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
cmake_minimum_required(VERSION 3.16)
set(GZ_MATH_VER 8)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm afraid about keeping this number in sync with bumps. As a workaround we can put a comment in the main CMakeLists.txt pointing to here. In the mid-term, it would be nice to integrate our CMake code to grab the version/name from git directly, probably using something like https://github.com/LecrisUT/CMakeExtraUtils/blob/main/cmake/DynamicVersion.md

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not a big fan of using git for version information as fails for tarballs. Something a bit ugly but effective could be to grep the major version from the parent folder CMakeLists.txt .

Copy link
Member Author

Choose a reason for hiding this comment

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

or we could extract the version numbers from package.xml, since we also have a workflow to keep those versions in sync

Copy link
Member Author

Choose a reason for hiding this comment

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

prototype for extracting version numbers from package.xml: #639

Copy link
Member Author

Choose a reason for hiding this comment

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

I've also opened gazebosim/gz-cmake#456 adding the python script from #639 and a cmake helper function. If you don't mind approving this PR for now so we can start to fix CI, I will follow-up by using the gz-cmake helper once it has been merged and released

project(gz-math${GZ_MATH_VER}-python VERSION ${GZ_MATH_VER})
find_package(Python3 COMPONENTS Interpreter Development REQUIRED)
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice if we can move the find_package logic from the main CMakeLists.txt to here so we don't have a duplicate logic.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've moved the find_package(pybind11) call here in 06b8ab8 using the CMAKE_REQUIRE_FIND_PACKAGE_pybind11 variable to support finding it as an optional or required package depending on the context.

It's tricky to move the find_package(Python3) call to this folder because the Development component can be optional or required, depending on the context, but the FindPython3 module doesn't support CMAKE_REQUIRE_FIND_PACKAGE_* for its components. So I just left the find_package(Python3) code as is

find_package(pybind11 REQUIRED)
find_package(gz-math${PROJECT_VERSION_MAJOR} REQUIRED)
set(PROJECT_LIBRARY_TARGET_NAME "gz-math${PROJECT_VERSION_MAJOR}::gz-math${PROJECT_VERSION_MAJOR}")
include(GNUInstallDirs)
include(CTest)
if(BUILD_TESTING)
scpeters marked this conversation as resolved.
Show resolved Hide resolved
enable_testing()
endif()
endif()

message(STATUS "Building pybind11 interfaces")
set(BINDINGS_MODULE_NAME "math${PROJECT_VERSION_MAJOR}")
# Split from main extension and converted to pybind11
Expand Down Expand Up @@ -81,7 +97,7 @@ if(USE_SYSTEM_PATHS_FOR_PYTHON_INSTALLATION)
endif()
else()
# If not a system installation, respect local paths
set(GZ_PYTHON_INSTALL_PATH ${GZ_LIB_INSTALL_DIR}/python)
set(GZ_PYTHON_INSTALL_PATH ${CMAKE_INSTALL_LIBDIR}/python)
endif()

set(GZ_PYTHON_INSTALL_PATH "${GZ_PYTHON_INSTALL_PATH}/gz")
Expand Down
39 changes: 37 additions & 2 deletions tutorials/installation.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ you should use `ign-math<#>`.

### macOS

On macOS, add OSRF packages:
On macOS, after installing the [Homebrew package manager](https://brew.sh),
add OSRF packages:
```
/bin/bash -c "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)"
brew tap osrf/simulation
```

Expand Down Expand Up @@ -92,6 +92,13 @@ The optional Eigen component of Gazebo Math requires:
sudo apt-get install libeigen3-dev
```

The optional Python bindings of Gazebo Math require:

* [Pybind11](https://pybind11.readthedocs.io/en/stable/index.html). Refer to the [Pybind11 Documentation](https://pybind11.readthedocs.io/en/stable/installing.html) for installation instructions. On Ubuntu systems, `apt-get` can be used to install Pybind11:
```
sudo apt-get install python3-pybind11
```

The optional Ruby tests of Gazebo Math require:

* [Ruby](https://www.ruby-lang.org/). Refer to the [Ruby Documentation](https://www.ruby-lang.org/downloads/) for installation instructions. On Ubuntu systems `apt-get` can be used to install Ubuntu Package `ruby-dev`:
Expand Down Expand Up @@ -228,6 +235,34 @@ The optional Eigen component of Gazebo Math requires:
cmake --install . --config Release
```

### cmake parameters

| Name | Type | Default | Description |
|-----------------|------|---------|--------------------------------------------|
| `SKIP_PYBIND11` | BOOL | OFF | Set to ON to skip building python bindings |

### Building Python bindings separately from main library

If you want to build Python bindings separately from the main gz-math library
(for example if you want to build Python bindings for multiple versions of Python),
you can invoke cmake on the `src/python_pybind11` folder instead of the root folder.
Specify the path to the python executable with which you wish to build bindings
in the `Python3_EXECUTABLE` cmake variable.
Specify the install path for the bindings in the `CMAKE_INSTALL_PREFIX`
variable, and be sure to set your `PYTHONPATH` accordingly after install.

```bash
cd gz-math
mkdir build_python3
cd build_python3
cmake ../src/python_pybind11 \
-DPython3_EXECUTABLE=/usr/local/bin/python3.12 \
-DCMAKE_INSTALL_PREFIX=<prefix>
```

See the homebrew [gz-math8 formula](https://github.com/osrf/homebrew-simulation/blob/ccda47647ed9aeb38f0ea1ec8804fd1501058de1/Formula/gz-math8.rb#L12-L52)
for an example of building bindings for multiple versions of Python.

# Documentation

API and tutorials can be found at [https://gazebosim.org/libs/math](https://gazebosim.org/libs/math).
Expand Down