From 6d97ae2c4847d286b84faaaccafea1aee0c9a516 Mon Sep 17 00:00:00 2001 From: Cary Phillips Date: Thu, 19 Sep 2024 18:02:02 -0700 Subject: [PATCH] Reoganize pybind11 cmake configuration (#434) This reconfigures the new pybind11 implementation so it can be built in parallel with the existing Boost.Python bindings, and arranges so that the Boost.Python bindings can be deprecated and removed once with new bindings are functional. It's now possible to build the pybind11-based bindings without requiring Boost.Python to be installed. The CMake option ``PYTHON`` specifies whether to build the Boost.Python-based bindings, same as always. The Boost.Python binding source code is under src/python. The new CMake optin ``PYBIND11`` specifies whether to build the pybind11-based bindings. The pybind11 binding source code is under src/pybind11. The bindings build into a module called "pybindimath" and a library called "libPyBindImath.so". These are temporary names until these bindings become official, at which time they'll be renamed to "imath". The new pybind11-based CMake configuration is simplfied, with all the setup inline in src/pybind11/PyBindImath/CMakeLists.txt. It does not rely on any of the sort of CMake functions or setups that the Boost.Python bindings pull from src/python/config. That code is overly complicated, and overly general since it's only used for two libraries (PyImathand PyImathNumpy). Signed-off-by: Cary Phillips --- .github/workflows/ci_workflow.yml | 53 +++++++-- CMakeLists.txt | 11 +- src/pybind11/CMakeLists.txt | 17 +++ src/pybind11/PyBindImath/CMakeLists.txt | 106 ++++++++++++++++++ .../PyBindImath/PyBindImath.h} | 8 +- src/pybind11/PyBindImath/PyBindImath.pc.in | 17 +++ .../PyBindImath/PyBindImathBox.cpp} | 6 +- .../PyBindImath/PyBindImathExport.h} | 14 +-- .../PyBindImath/PyBindImathVec.cpp} | 6 +- .../PyBindImath/pybindimathmodule.cpp} | 12 +- src/python/CMakeLists.txt | 17 +-- src/python/PyBindImath/CMakeLists.txt | 18 --- 12 files changed, 218 insertions(+), 67 deletions(-) create mode 100644 src/pybind11/CMakeLists.txt create mode 100644 src/pybind11/PyBindImath/CMakeLists.txt rename src/{python/PyBindImath/PyImath.h => pybind11/PyBindImath/PyBindImath.h} (64%) create mode 100644 src/pybind11/PyBindImath/PyBindImath.pc.in rename src/{python/PyBindImath/PyImathBox.cpp => pybind11/PyBindImath/PyBindImathBox.cpp} (97%) rename src/{python/PyBindImath/PyImathExport.h => pybind11/PyBindImath/PyBindImathExport.h} (50%) rename src/{python/PyBindImath/PyImathVec.cpp => pybind11/PyBindImath/PyBindImathVec.cpp} (99%) rename src/{python/PyBindImath/imathmodule.cpp => pybind11/PyBindImath/pybindimathmodule.cpp} (94%) delete mode 100644 src/python/PyBindImath/CMakeLists.txt diff --git a/.github/workflows/ci_workflow.yml b/.github/workflows/ci_workflow.yml index 3cfd3783..25a81594 100644 --- a/.github/workflows/ci_workflow.yml +++ b/.github/workflows/ci_workflow.yml @@ -60,7 +60,7 @@ jobs: image: aswf/ci-openexr:${{ matrix.vfx-cy }} strategy: matrix: - build: [1, 2, 3, 4, 5, 6, 7, 8, 9] + build: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11] include: # ------------------------------------------------------------------- # GCC, VFX CY2024 @@ -188,6 +188,35 @@ jobs: python-desc: python3.7.9 vfx-cy: 2021 + # ------------------------------------------------------------------- + # pybind11 and python together + # ------------------------------------------------------------------- + - build: 10 + build-type: Release + build-shared: 'ON' + cxx-standard: 17 + cxx-compiler: g++ + cc-compiler: gcc + compiler-desc: gcc11.2.1 + python: 'ON' + pybind11: 'ON' + python-desc: python3.11 + vfx-cy: 2024 + + # ------------------------------------------------------------------- + # pybind11 w/o python + # ------------------------------------------------------------------- + - build: 11 + build-type: Release + build-shared: 'ON' + cxx-standard: 17 + cxx-compiler: g++ + cc-compiler: gcc + compiler-desc: gcc11.2.1 + python: 'OFF' + pybind11: 'ON' + python-desc: python3.11 + vfx-cy: 2024 env: CXX: ${{ matrix.cxx-compiler }} CC: ${{ matrix.cc-compiler }} @@ -211,6 +240,7 @@ jobs: -DCMAKE_VERBOSE_MAKEFILE:BOOL='OFF' \ -DBUILD_SHARED_LIBS=${{ matrix.build-shared }} \ -DPYTHON=${{ matrix.python }} \ + -DPYBIND11=${{ matrix.pybind11 }} \ -DUSE_PYTHON2=${{ matrix.use-python2 }} working-directory: _build - name: Build @@ -221,12 +251,6 @@ jobs: working-directory: _build - name: Examples run: | - # Confirm the python module loads. Set PYTHONPATH to the - # _install directory of the module (better to find it - # procedurally than hard code a path). - export PYTHONPATH=`find ../_install -name imath.so | xargs dirname` - python -c "import imath;print(imath.__version__)" - # Make sure we can build the tests when configured as a # standalone application linking against the just-installed # Imath library. @@ -252,6 +276,21 @@ jobs: --config ${{ matrix.build-type }} ctest working-directory: _examples + - name: Test Python + run: | + # Confirm the python module loads. Set PYTHONPATH to the + # _install directory of the module (better to find it + # procedurally than hard code a path). + if [[ "${{ matrix.python }}" == "ON" ]]; then + export PYTHONPATH=`find ../_install -name imath.so | xargs dirname` + python -c "import imath;print(imath.__version__)" + fi + if [[ "${{ matrix.pybind11 }}" == "ON" ]]; then + export PYTHONPATH=`find ../_install -name 'pybindimath.*.so' | xargs dirname` + python -c "import pybindimath;print(pybindimath.__version__)" + fi + shell: bash + working-directory: _build - name: Test run: | ctest -T Test ${{ matrix.exclude-tests }} \ diff --git a/CMakeLists.txt b/CMakeLists.txt index 282242dc..6cb90feb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -71,14 +71,13 @@ set(Imath_DIR "${CMAKE_CURRENT_BINARY_DIR}/config" CACHE PATH "" FORCE) file(WRITE "${CMAKE_CURRENT_BINARY_DIR}/config/ImathTargets.cmake" "# Dummy file") option(PYTHON "Set ON to compile boost PyImath bindings") -option(PYBIND11 "Set ON to compile pybind11 PyImath bindings") - -if (PYTHON AND PYBIND11) - message(FATAL_ERROR "Compiling both the boost and pybind11 python bindings at the same time is not currently supported") +if (PYTHON) + add_subdirectory(src/python) endif() -if (PYTHON OR PYBIND11) - add_subdirectory(src/python) +option(PYBIND11 "Set ON to compile pybind11 PyImath bindings") +if (PYBIND11) + add_subdirectory(src/pybind11) endif() option(BUILD_WEBSITE "Set ON to build the readthedocs website source") diff --git a/src/pybind11/CMakeLists.txt b/src/pybind11/CMakeLists.txt new file mode 100644 index 00000000..2b9620eb --- /dev/null +++ b/src/pybind11/CMakeLists.txt @@ -0,0 +1,17 @@ +# SPDX-License-Identifier: BSD-3-Clause +# Copyright Contributors to the OpenEXR Project. + +set(PYBINDIMATH_OVERRIDE_PYTHON_INSTALL_DIR "" CACHE STRING "Override the install location for imath.so and imathnumpy.so modules") + +######################## +## Build related options + +# Suffix to append to root name, this helps with version management +# but can be turned off if you don't care, or otherwise customized +# +set(PYBINDIMATH_LIB_SUFFIX "-${IMATH_VERSION_API}" CACHE STRING "String added to the end of all the libraries") +# This provides a root for the unique name of the library based on +# the version of python being compiled for +set(PYBINDIMATH_LIB_PYTHONVER_ROOT "_Python" CACHE STRING "String added as a root to the identifier of the python version in the libraries") + +add_subdirectory(PyBindImath) diff --git a/src/pybind11/PyBindImath/CMakeLists.txt b/src/pybind11/PyBindImath/CMakeLists.txt new file mode 100644 index 00000000..9c148cda --- /dev/null +++ b/src/pybind11/PyBindImath/CMakeLists.txt @@ -0,0 +1,106 @@ +# SPDX-License-Identifier: BSD-3-Clause +# Copyright Contributors to the OpenEXR Project. + +find_package(Python3 REQUIRED COMPONENTS Interpreter Development) +find_package(pybind11 REQUIRED) + +# +# Source/headers +# + +set(PYBINDIMATH_SOURCES + PyBindImathBox.cpp + PyBindImathVec.cpp +) + +set(PYBINDIMATH_HEADERS + PyBindImathExport.h + PyBindImath.h +) + +# +# shared library, e.g. libPyBindImath_Python3_11-3_2.so.30.3.2.0 +# + +set(PYBINDIMATH_LIBRARY PyBindImath) + +add_library(${PYBINDIMATH_LIBRARY} SHARED ${PYBINDIMATH_SOURCES}) + +target_link_libraries(${PYBINDIMATH_LIBRARY} PRIVATE Imath::Imath pybind11::module) + +# Set include directories +target_include_directories(${PYBINDIMATH_LIBRARY} PRIVATE ${Python3_INCLUDE_DIRS} ${Imath_INCLUDE_DIRS}) + +if(NOT "${PYBINDIMATH_LIB_PYTHONVER_ROOT}" STREQUAL "") + set(pythonver_root "${PYBINDIMATH_LIB_PYTHONVER_ROOT}${Python3_VERSION_MAJOR}_${Python3_VERSION_MINOR}") + message("pythonver_root ${PYBINDIMATH_LIB_PYTHONVER_ROOT}${Python3_VERSION_MAJOR}_${Python3_VERSION_MINOR}") +endif() + +if(BUILD_SHARED_LIBS) + # This creates the so-versioned library symlinks + set_target_properties(${PYBINDIMATH_LIBRARY} PROPERTIES + SOVERSION ${IMATH_LIB_SOVERSION} + VERSION ${IMATH_LIB_VERSION} + OUTPUT_NAME "${PYBINDIMATH_CURLIB_OUTROOT}${PYBINDIMATH_LIBRARY}${pythonver_root}${PYBINDIMATH_LIB_SUFFIX}" + ) +endif() + +# +# python module, e.g. pybindimath.cpython-311-x86_64-linux-gnu.so +# + +set(PYBINDIMATH_MODULE pybindimath) + +pybind11_add_module(pybindimath MODULE pybindimathmodule.cpp $) + +target_link_libraries(${PYBINDIMATH_MODULE} PRIVATE Imath::Imath pybind11::module) + +if(SKBUILD) + set(PYTHON_INSTALL_DIR ${SKBUILD_PLATLIB_DIR}) +else() + set(PYTHON_INSTALL_DIR "lib/python${Python3_VERSION_MAJOR}.${Python3_VERSION_MINOR}/site-packages") +endif() + +if (IMATH_INSTALL) + + # module + + install(TARGETS ${PYBINDIMATH_MODULE} DESTINATION ${PYTHON_INSTALL_DIR} COMPONENT python) + + # shared library + + install(TARGETS ${PYBINDIMATH_LIBRARY} + RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} + LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} + ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR} + INCLUDES DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}) + + if(BUILD_SHARED_LIBS AND (NOT "${IMATH_LIB_SUFFIX}" STREQUAL "") AND IMATH_INSTALL_SYM_LINK) + + # create symlinks for the shared object so versions + + string(TOUPPER "${CMAKE_BUILD_TYPE}" uppercase_CMAKE_BUILD_TYPE) + set(verlibname ${CMAKE_SHARED_LIBRARY_PREFIX}${PYBINDIMATH_LIBRARY}${pythonver_root}${IMATH_LIB_SUFFIX}${CMAKE_${uppercase_CMAKE_BUILD_TYPE}_POSTFIX}${CMAKE_SHARED_LIBRARY_SUFFIX}) + set(baselibname ${CMAKE_SHARED_LIBRARY_PREFIX}${PYBINDIMATH_LIBRARY}${pythonver_root}${CMAKE_${uppercase_CMAKE_BUILD_TYPE}_POSTFIX}${CMAKE_SHARED_LIBRARY_SUFFIX}) + file(CREATE_LINK ${verlibname} ${CMAKE_CURRENT_BINARY_DIR}/${baselibname} SYMBOLIC) + if(WIN32) + install(FILES ${CMAKE_CURRENT_BINARY_DIR}/${baselibname} DESTINATION ${CMAKE_INSTALL_FULL_BINDIR}) + install(CODE "message(STATUS \"Creating symlink ${CMAKE_INSTALL_FULL_BINDIR}/${baselibname} -> ${verlibname}\")") + else() + install(FILES ${CMAKE_CURRENT_BINARY_DIR}/${baselibname} DESTINATION ${CMAKE_INSTALL_FULL_LIBDIR}) + install(CODE "message(STATUS \"Creating symlink ${CMAKE_INSTALL_FULL_LIBDIR}/${baselibname} -> ${verlibname}\")") + endif() + endif() + + # pkgconfig + + set(pcinfile PyBindImath.pc.in) + set(prefix ${CMAKE_INSTALL_PREFIX}) + set(exec_prefix "\${prefix}") + set(libdir "\${exec_prefix}/${CMAKE_INSTALL_LIBDIR}") + set(includedir "\${prefix}/${CMAKE_INSTALL_INCLUDEDIR}") + string(REPLACE ".in" "" pcout ${pcinfile}) + configure_file(${pcinfile} ${CMAKE_CURRENT_BINARY_DIR}/${pcout} @ONLY) + install(FILES ${CMAKE_CURRENT_BINARY_DIR}/${pcout} DESTINATION ${CMAKE_INSTALL_LIBDIR}/pkgconfig) + +endif() diff --git a/src/python/PyBindImath/PyImath.h b/src/pybind11/PyBindImath/PyBindImath.h similarity index 64% rename from src/python/PyBindImath/PyImath.h rename to src/pybind11/PyBindImath/PyBindImath.h index 3769bbef..346f4b63 100644 --- a/src/python/PyBindImath/PyImath.h +++ b/src/pybind11/PyBindImath/PyBindImath.h @@ -10,15 +10,15 @@ #include #include #include -#include "PyImathExport.h" +#include "PyBindImathExport.h" #ifndef _PyBindImath_h_ #define _PyBindImath_h_ -namespace PyImath { +namespace PyBindImath { -PYIMATH_EXPORT void register_imath_vec(pybind11::module& m); -PYIMATH_EXPORT void register_imath_box(pybind11::module& m); +PYBINDIMATH_EXPORT void register_imath_vec(pybind11::module& m); +PYBINDIMATH_EXPORT void register_imath_box(pybind11::module& m); } diff --git a/src/pybind11/PyBindImath/PyBindImath.pc.in b/src/pybind11/PyBindImath/PyBindImath.pc.in new file mode 100644 index 00000000..3acbfc03 --- /dev/null +++ b/src/pybind11/PyBindImath/PyBindImath.pc.in @@ -0,0 +1,17 @@ +## +## SPDX-License-Identifier: BSD-3-Clause +## Copyright Contributors to the OpenEXR Project. +## + +prefix=@prefix@ +exec_prefix=@exec_prefix@ +libdir=@libdir@ +includedir=@includedir@ +pythonver=@pythonver_root@ + +Name: PyBindImath +Description: pybind11-based python bindings for the Imath libraries +Version: @IMATH_VERSION@ +Libs: -L${libdir} -lImath${libsuffix} -lPyBindImath${pythonver$}${libsuffix} + +Cflags: -I${includedir} -I${includedir}/Imath diff --git a/src/python/PyBindImath/PyImathBox.cpp b/src/pybind11/PyBindImath/PyBindImathBox.cpp similarity index 97% rename from src/python/PyBindImath/PyImathBox.cpp rename to src/pybind11/PyBindImath/PyBindImathBox.cpp index 07ac7224..c42d99e3 100644 --- a/src/python/PyBindImath/PyImathBox.cpp +++ b/src/pybind11/PyBindImath/PyBindImathBox.cpp @@ -3,10 +3,10 @@ // Copyright Contributors to the OpenEXR Project. // -#include "PyImath.h" +#include "PyBindImath.h" #include -namespace PyImath { +namespace PyBindImath { template void register_box(pybind11::class_& c) @@ -57,4 +57,4 @@ void register_imath_box(pybind11::module& m) } -} \ No newline at end of file +} diff --git a/src/python/PyBindImath/PyImathExport.h b/src/pybind11/PyBindImath/PyBindImathExport.h similarity index 50% rename from src/python/PyBindImath/PyImathExport.h rename to src/pybind11/PyBindImath/PyBindImathExport.h index d48c5f7e..9b08bb51 100644 --- a/src/python/PyBindImath/PyImathExport.h +++ b/src/pybind11/PyBindImath/PyBindImathExport.h @@ -10,19 +10,19 @@ #if defined(IMATH_DLL) #if defined(PLATFORM_VISIBILITY_AVAILABLE) - #define PYIMATH_EXPORT __attribute__((visibility("default"))) - #define PYIMATH_EXPORT __attribute__((visibility("default"))) + #define PYBINDIMATH_EXPORT __attribute__((visibility("default"))) + #define PYBINDIMATH_EXPORT __attribute__((visibility("default"))) #elif defined(_MSC_VER) - #if defined(PYIMATH_BUILD) - #define PYIMATH_EXPORT __declspec(dllexport) + #if defined(PYBINDIMATH_BUILD) + #define PYBINDIMATH_EXPORT __declspec(dllexport) #else - #define PYIMATH_EXPORT __declspec(dllimport) + #define PYBINDIMATH_EXPORT __declspec(dllimport) #endif #else - #define PYIMATH_EXPORT + #define PYBINDIMATH_EXPORT #endif #else - #define PYIMATH_EXPORT + #define PYBINDIMATH_EXPORT #endif #endif // #ifndef PYBINDIMATHEXPORT_H diff --git a/src/python/PyBindImath/PyImathVec.cpp b/src/pybind11/PyBindImath/PyBindImathVec.cpp similarity index 99% rename from src/python/PyBindImath/PyImathVec.cpp rename to src/pybind11/PyBindImath/PyBindImathVec.cpp index ddb10934..07d453fa 100644 --- a/src/python/PyBindImath/PyImathVec.cpp +++ b/src/pybind11/PyBindImath/PyBindImathVec.cpp @@ -3,11 +3,11 @@ // Copyright Contributors to the OpenEXR Project. // -#include "PyImath.h" +#include "PyBindImath.h" #include #include -namespace PyImath { +namespace PyBindImath { template void register_vec(pybind11::class_& c) @@ -124,4 +124,4 @@ void register_imath_vec(pybind11::module& m) register_vec4(m, "V4f"); } -} \ No newline at end of file +} diff --git a/src/python/PyBindImath/imathmodule.cpp b/src/pybind11/PyBindImath/pybindimathmodule.cpp similarity index 94% rename from src/python/PyBindImath/imathmodule.cpp rename to src/pybind11/PyBindImath/pybindimathmodule.cpp index a210b625..ea4ca8cf 100644 --- a/src/python/PyBindImath/imathmodule.cpp +++ b/src/pybind11/PyBindImath/pybindimathmodule.cpp @@ -3,16 +3,16 @@ // Copyright Contributors to the OpenEXR Project. // -#include "PyImath.h" +#include "PyBindImath.h" #include -PYBIND11_MODULE(imath, m) +PYBIND11_MODULE(pybindimath, m) { - m.doc() = "Imath module"; + m.doc() = "PyBindImath module"; m.attr("__version__") = IMATH_VERSION_STRING; - PyImath::register_imath_vec(m); - PyImath::register_imath_box(m); + PyBindImath::register_imath_vec(m); + PyBindImath::register_imath_box(m); // // Initialize constants @@ -71,4 +71,4 @@ PYBIND11_MODULE(imath, m) m.attr("DBL_LOWEST") = std::numeric_limits::lowest(); m.attr("DBL_EPS") = std::numeric_limits::epsilon(); -} \ No newline at end of file +} diff --git a/src/python/CMakeLists.txt b/src/python/CMakeLists.txt index 57903f7d..734c5b52 100644 --- a/src/python/CMakeLists.txt +++ b/src/python/CMakeLists.txt @@ -159,20 +159,16 @@ if (PYTHON) endif() endif() -if (PYBIND11) - find_package(pybind11 REQUIRED) -endif() - if(Boost_PYTHON_FOUND AND NOT _pyimath_have_perver_boost) # old boost case, I guess we just warn and assume it is python2 (likely) message(WARNING "Ambiguous boost python module found, assuming python ${Python_VERSION_MAJOR}. If you have a new boost library, try cleaning the cmake cache and reconfigure with -DBoost_NO_BOOST_CMAKE=ON") set(PYIMATH_BOOST_PY_COMPONENT python) # set it to a bogus string but not empty so later we don't test against a namespace only target # set(PYIMATH_BOOST_PY3_COMPONENT pythonIgnore) -elseif(NOT _pyimath_have_perver_boost AND NOT pybind11_FOUND) +elseif(NOT _pyimath_have_perver_boost) message(WARNING "Unable to find boost::python library, disabling PyImath. If you believe this is wrong, check the cmake documentation and see if you need to set Boost_ROOT or Boost_NO_BOOST_CMAKE") return() -elseif(NOT pybind11_FOUND) +else() if(TARGET Boost::${PYIMATH_BOOST_PY_COMPONENT}) message(STATUS "Found Python ${Python_VERSION_MAJOR} boost: Boost::${PYIMATH_BOOST_PY_COMPONENT}") elseif(Boost_PYTHON_FOUND OR Boost_${PYIMATH_PY_UPPER}_FOUND) @@ -201,13 +197,8 @@ if(NOT BUILD_SHARED_LIBS) message(WARNING "Forcing python bindings to be built with dynamic libraries") endif() -if ( pybind11_FOUND ) - message(STATUS "PyBind11 PyImath") - add_subdirectory( PyBindImath ) -else() - message(STATUS "Boost PyImath") - add_subdirectory( PyImath ) -endif() +message(STATUS "Boost PyImath") +add_subdirectory( PyImath ) if(TARGET Python2::ImathNumPy OR TARGET Python3::ImathNumPy) add_subdirectory( PyImathNumpy ) diff --git a/src/python/PyBindImath/CMakeLists.txt b/src/python/PyBindImath/CMakeLists.txt deleted file mode 100644 index ccd45473..00000000 --- a/src/python/PyBindImath/CMakeLists.txt +++ /dev/null @@ -1,18 +0,0 @@ -# SPDX-License-Identifier: BSD-3-Clause -# Copyright Contributors to the OpenEXR Project. - -pyimath_define_module(imath - LIBNAME PyBindImath - PRIV_EXPORT PYIMATH_BUILD - CURDIR ${CMAKE_CURRENT_SOURCE_DIR} - LIBSOURCE - PyImathVec.cpp - MODSOURCE - imathmodule.cpp - PyImathVec.cpp - PyImathBox.cpp - HEADERS - PyImathExport.h - DEPENDENCIES - Imath - )