CMake linking and install improvements

[badshah400]

Hi,
I am a packager for openSUSE and am trying to build RPM packages for pybind11_abseil. We found that we need a few changes to the cmake scripts to improve the package finding logic, linking to the right version of Python, and installation of headers. I list the patches we use one by one in this description, so that you may have a look and see if they are right for your project. If you agree, I can submit PRs for the changes.

Pass FIND_PACKAGE_ARGS to FetchContent_Declare

This enables the detection of system absl and pybind11 and goes on to download them if they are not. Important for packaging on openSUSE because RPMs are built inside a no-network access VM.

Note: This will bump the CMake minimum version to 3.24.

---
 CMakeLists.txt |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Index: pybind11_abseil-202402.0/CMakeLists.txt
===================================================================
--- pybind11_abseil-202402.0.orig/CMakeLists.txt
+++ pybind11_abseil-202402.0/CMakeLists.txt
@@ -17,16 +17,18 @@ if(CMAKE_VERSION VERSION_GREATER_EQUAL 3
 endif()
 
 FetchContent_Declare(
-  abseil-cpp
+  absl
   URL https://github.com/abseil/abseil-cpp/archive/refs/tags/20230802.0.tar.gz
   URL_HASH
-    SHA256=59d2976af9d6ecf001a81a35749a6e551a335b949d34918cfade07737b9d93c5)
+    SHA256=59d2976af9d6ecf001a81a35749a6e551a335b949d34918cfade07737b9d93c5
+  FIND_PACKAGE_ARGS)
 
 FetchContent_Declare(
   pybind11
-  URL https://github.com/pybind/pybind11/archive/refs/heads/master.tar.gz)
+  URL https://github.com/pybind/pybind11/archive/refs/heads/master.tar.gz
+  FIND_PACKAGE_ARGS)
 
-FetchContent_MakeAvailable(abseil-cpp pybind11)
+FetchContent_MakeAvailable(absl pybind11)
 
 set(TOP_LEVEL_DIR ${CMAKE_CURRENT_LIST_DIR})
 include_directories(${TOP_LEVEL_DIR} ${pybind11_INCLUDE_DIRS})

Link against the correct Python library

This is needed in any case for our packages to avoid undefined reference to Py* errors when linking, but additionally it helps to build against the correct version of Python and Pybind11 if there are multiple ones installed in the system. On openSUSE, it is natural to have simultaneously python3.10, python3.11, and python3.12 and the corresponding pybind11, numpy, python-absl installed in the system. This helps pick the right pybind11 to go with the choice of python executable.

---
 CMakeLists.txt                 |    3 +++
 pybind11_abseil/CMakeLists.txt |    1 +
 2 files changed, 4 insertions(+)

Index: pybind11_abseil-202402.0/CMakeLists.txt
===================================================================
--- pybind11_abseil-202402.0.orig/CMakeLists.txt
+++ pybind11_abseil-202402.0/CMakeLists.txt
@@ -16,6 +16,9 @@ if(CMAKE_VERSION VERSION_GREATER_EQUAL 3
   cmake_policy(SET CMP0135 NEW)
 endif()
 
+# Needs to be called *before* looking for pybind11 to ensure pybind11 uses the same python
+find_package(Python COMPONENTS Interpreter Development)
+
 FetchContent_Declare(
   absl
   URL https://github.com/abseil/abseil-cpp/archive/refs/tags/20230802.0.tar.gz
Index: pybind11_abseil-202402.0/pybind11_abseil/CMakeLists.txt
===================================================================
--- pybind11_abseil-202402.0.orig/pybind11_abseil/CMakeLists.txt
+++ pybind11_abseil-202402.0/pybind11_abseil/CMakeLists.txt
@@ -1,5 +1,6 @@
 add_subdirectory(compat)
 add_subdirectory(cpp_capsule_tools)
+link_libraries(${Python_LIBRARIES})
 
 # absl_casters ============================================================
 add_library(absl_casters INTERFACE)

Install headers

Apparently the current CMake scripts do not install the headers to the system, only the libraries. Headers are nonetheless needed for devs using C++ interface of pybind11_abseil. This patch adds the necessary include dirs to the install destination.

---
 CMakeLists.txt |    7 +++++++
 1 file changed, 7 insertions(+)

Index: pybind11_abseil-202402.0/CMakeLists.txt
===================================================================
--- pybind11_abseil-202402.0.orig/CMakeLists.txt
+++ pybind11_abseil-202402.0/CMakeLists.txt
@@ -37,3 +37,11 @@ set(TOP_LEVEL_DIR ${CMAKE_CURRENT_LIST_D
 include_directories(${TOP_LEVEL_DIR} ${pybind11_INCLUDE_DIRS})
 
 add_subdirectory(pybind11_abseil)
+
+if(CMAKE_INSTALL_PYDIR)
+  install(DIRECTORY pybind11_abseil
+          TYPE INCLUDE
+          FILES_MATCHING PATTERN "*.h"
+          PATTERN tests EXCLUDE
+          PATTERN requirements EXCLUDE)
+endif()

Suggestions, comments welcome. Thanks in advance. Thank you also for the very useful library.

Edit: Slightly improved patch for headers' installation: exclude test and requirements dirs.

[StefanBruens]

There are apparently more issues when using an out-of-tree pybind11_abseil:

1. Missing implementations of ImportStatusModule. This is located in import_status_module.cc, but that is not part of any exported shared library. The easiest solution would probably be to make this an inline implementation in the header, as it is fairly small.
2. The INTERFACE libraries are not exported, e.g. pybind11_abseil::absl_caster.
3. No CMake Config file generated/installed, also no Target definititions for the interfaces.
4. The two python extensions (status.so, ok_status_singleton.so) should be MODULE, not SHARED libraries. This would probably covered by usage of the appropriate pybind11 CMake macro.

[StefanBruens]

And even more:

The status_caster and statusor_caster libraries are not actually INTERFACE libraries, as these depend on several compiled source files which are not available outside the source tree.

Makes me wonder if the *.cc files should not be just inlined into the header files. Currently, the *cc files will be compiled to static libraries, and be linked statically into any python extension making use of it.