Some refactoring to remove unnecessary linkage to python libs (Academ…

…ySoftwareFoundation#2807) Use pybind11 functions to set up python module, it's got superior logic to what we were doing. Bump pybind11 version default in build_pybind11.bash Suppress clang warnings about python register use
lgritz · Jan 13, 2021 · 309c29f · 309c29f
1 parent fa59ef6
commit 309c29f
Show file tree

Hide file tree

Showing 5 changed files with 24 additions and 17 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -398,6 +398,7 @@ jobs:
           CMAKE_CXX_STANDARD: 14
           PYTHON_VERSION: 3.7
           USE_SIMD: avx2,f16c
+          PYBIND11_VERSION: v2.6.1
           OPENCOLORIO_VERSION: 1c624651b7
           # Pick an OCIO commit that includes a warning fix we need for clang
         run: |

diff --git a/src/build-scripts/build_pybind11.bash b/src/build-scripts/build_pybind11.bash
@@ -7,7 +7,7 @@ set -ex
 
 # Repo and branch/tag/commit of pybind11 to download if we don't have it yet
 PYBIND11_REPO=${PYBIND11_REPO:=https://github.com/pybind/pybind11.git}
-PYBIND11_VERSION=${PYBIND11_VERSION:=v2.4.3}
+PYBIND11_VERSION=${PYBIND11_VERSION:=v2.5.0}
 
 # Where to put pybind11 repo source (default to the ext area)
 PYBIND11_SRC_DIR=${PYBIND11_SRC_DIR:=${PWD}/ext/pybind11}

diff --git a/src/cmake/pythonutils.cmake b/src/cmake/pythonutils.cmake
@@ -69,24 +69,28 @@ macro (setup_python_module)
         set_property (SOURCE ${lib_SOURCES} APPEND_STRING PROPERTY COMPILE_FLAGS " -Wno-macro-redefined ")
     endif ()
 
-    # Add the library itself
-    add_library (${target_name} MODULE ${lib_SOURCES})
+    pybind11_add_module(${target_name} ${lib_SOURCES})
 
+#    # Add the library itself
+#    add_library (${target_name} MODULE ${lib_SOURCES})
+#
     # Declare the libraries it should link against
-    target_link_libraries (${target_name} PRIVATE
-                           Python::Python pybind11::pybind11
-                           ${lib_LIBS} ${SANITIZE_LIBRARIES})
-    if (APPLE)
-        set_target_properties (${target_name} PROPERTIES LINK_FLAGS "-undefined dynamic_lookup")
-    endif ()
+    target_link_libraries (${target_name}
+                           PRIVATE ${lib_LIBS} ${SANITIZE_LIBRARIES})
+
+#    if (APPLE)
+#        set_target_properties (${target_name} PROPERTIES LINK_FLAGS "-undefined dynamic_lookup")
+#    endif ()
+
 
     # Exclude the 'lib' prefix from the name
     if (NOT PYLIB_LIB_PREFIX)
         target_compile_definitions(${target_name}
                                    PRIVATE "PYMODULE_NAME=${lib_MODULE}")
         set_target_properties (${target_name} PROPERTIES
                                OUTPUT_NAME ${lib_MODULE}
-                               PREFIX "")
+                               # PREFIX ""
+                               )
     else ()
         target_compile_definitions(${target_name}
                                    PRIVATE "PYMODULE_NAME=Py${lib_MODULE}")
@@ -95,6 +99,7 @@ macro (setup_python_module)
                                PREFIX lib)
     endif ()
 
+    # This is only needed for SpComp2
     if (PYLIB_INCLUDE_SONAME)
         if (VERBOSE)
             message(STATUS "Setting Py${lib_MODULE} SOVERSION to: ${SOVERSION}")
@@ -104,11 +109,11 @@ macro (setup_python_module)
             SOVERSION ${SOVERSION} )
     endif()
 
-    if (WIN32)
-        set_target_properties (${target_name} PROPERTIES
-                               DEBUG_POSTFIX "_d"
-                               SUFFIX ".pyd")
-    endif()
+#    if (WIN32)
+#        set_target_properties (${target_name} PROPERTIES
+#                               DEBUG_POSTFIX "_d"
+#                               SUFFIX ".pyd")
+#    endif()
 
     # In the build area, put it in lib/python so it doesn't clash with the
     # non-python libraries of the same name (which aren't prefixed by "lib"

diff --git a/src/python/CMakeLists.txt b/src/python/CMakeLists.txt
@@ -3,7 +3,8 @@
 # https://github.com/OpenImageIO/oiio/blob/master/LICENSE.md
 
 # from pythonutils.cmake
-checked_find_package (pybind11 2.4.2 REQUIRED)
+checked_find_package (pybind11 REQUIRED
+                      VERSION_MIN 2.4.2)
 
 
 file (GLOB python_srcs *.cpp)

diff --git a/src/python/py_oiio.h b/src/python/py_oiio.h
@@ -7,7 +7,7 @@
 
 // Python.h uses the 'register' keyword, don't warn about it being
 // deprecated in C++17.
-#if (__cplusplus >= 201703L && defined(__GNUC__))
+#if (__cplusplus >= 201703L && defined(__GNUC__)) || defined(__clang__)
 #    pragma GCC diagnostic ignored "-Wregister"
 #endif