google / s2geometry

Computational geometry and spatial indexing on the sphere
http://s2geometry.io/
Apache License 2.0
2.32k stars 308 forks source link

Linking examples fails because of missing abseil libs #375

Closed tpikonen closed 3 months ago

tpikonen commented 3 months ago

I compiled and installed s2geometry 0.11.1 on Debian trixie and then tried to compile the examples in doc/examples, but that failed in the linking step because of missing abseil symbols.

I could fix this in cmake with the diff below, but is it really necessary now to add the abseil libs manually to all projects using s2geometry? At least this was not needed with 0.10.0 (or maybe with an older version of abseil?).


diff --git a/doc/examples/CMakeLists.txt b/doc/examples/CMakeLists.txt
index d5858e0..edd6792 100644
--- a/doc/examples/CMakeLists.txt
+++ b/doc/examples/CMakeLists.txt
@@ -1,4 +1,5 @@
+find_package(absl REQUIRED)
 add_executable(point_index point_index.cc)
-target_link_libraries(point_index LINK_PUBLIC s2testing s2)
+target_link_libraries(point_index LINK_PUBLIC s2testing s2 absl::flags_internal absl::flags_reflection absl::log_internal_check_op)
 add_executable(term_index term_index.cc)
-target_link_libraries(term_index LINK_PUBLIC s2testing s2)
+target_link_libraries(term_index LINK_PUBLIC s2testing s2 absl::flags_internal absl::flags_reflection absl::log_internal_check_op)
``
tpikonen commented 3 months ago

On further look, part of the linking problem is that the s2geometry installs its .cmake files to /usr/share/s2, where they are not found. The diff below installs them to <libdir>/cmake/s2. This allows linking of s2 in Debian in standard way by adding find_package(s2 REQUIRED) and target_link_libraries(<...> s2::s2) to the cmake files of the application.

diff --git a/CMakeLists.txt b/CMakeLists.txt
index edb6a20..0d7707d 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -438,13 +438,15 @@ install(TARGETS ${S2_TARGETS}
 # Create an export type "s2Targets" detailing the targetable artifacts created
 # by this project.
 install(TARGETS s2 EXPORT s2Targets)
-# Install the export targets as a CMake config file in the share/s2 folder so
+# Install the export targets as a CMake config file in the cmake/s2 folder so
 # that they can referenced  by downstream projects as `s2::s2` after a
 # successful `find_package` call.
+set(CMAKE_INSTALL_CMAKEDIR "${CMAKE_INSTALL_LIBDIR}/cmake/s2")
+message("CMAKE_INSTALL_CMAKEDIR:" ${CMAKE_INSTALL_CMAKEDIR})
 install(EXPORT s2Targets
         NAMESPACE s2::
         FILE s2Targets.cmake
-        DESTINATION share/s2/)
+        DESTINATION "${CMAKE_INSTALL_CMAKEDIR}")

 if (BUILD_TESTS)
   if (NOT GOOGLETEST_ROOT)
@@ -606,7 +608,7 @@ include(CMakePackageConfigHelpers)
 # Generate the config file that includes the exports.
 configure_package_config_file(${CMAKE_CURRENT_SOURCE_DIR}/Config.cmake.in
   "${CMAKE_CURRENT_BINARY_DIR}/s2Config.cmake"
-  INSTALL_DESTINATION "share/s2/"
+  INSTALL_DESTINATION "${CMAKE_INSTALL_CMAKEDIR}"
   NO_SET_AND_CHECK_MACRO
   NO_CHECK_REQUIRED_COMPONENTS_MACRO)
-install(FILES ${CMAKE_CURRENT_BINARY_DIR}/s2Config.cmake DESTINATION "share/s2/")
+install(FILES ${CMAKE_CURRENT_BINARY_DIR}/s2Config.cmake DESTINATION "${CMAKE_INSTALL_CMAKEDIR}")
tpikonen commented 3 months ago

The linking problems are caused by other issues in the example CMakeLists.txt, but it would not be a bad idea to install the *.cmake files under the library dir anyway.