chore: Use system s2geometry - #307
paleolimbot wants to merge 15 commits into
Conversation
| cmake_minimum_required(VERSION 3.14) | ||
|
|
||
| project(r_s2geography LANGUAGES CXX) | ||
|
|
||
| find_package(s2 CONFIG REQUIRED) | ||
|
|
||
| add_library( | ||
| s2geography STATIC | ||
| ../src/s2geography/accessors-geog.cc | ||
| ../src/s2geography/accessors.cc | ||
| ../src/s2geography/build.cc | ||
| ../src/s2geography/coverings.cc | ||
| ../src/s2geography/distance.cc | ||
| ../src/s2geography/geography.cc | ||
| ../src/s2geography/linear-referencing.cc | ||
| ../src/s2geography/predicates.cc) | ||
|
|
||
| set_target_properties(s2geography PROPERTIES POSITION_INDEPENDENT_CODE ON) | ||
| target_link_libraries(s2geography PUBLIC s2::s2) | ||
|
|
||
| # The R sources must use the same S2 headers as this library. Generate a shell- | ||
| # and Make-compatible include fragment from the imported CMake target. | ||
| file( | ||
| GENERATE | ||
| OUTPUT "${CMAKE_BINARY_DIR}/s2_include_flags.txt" | ||
| CONTENT "-isystem $<JOIN:$<REMOVE_DUPLICATES:$<TARGET_PROPERTY:s2::s2,INTERFACE_INCLUDE_DIRECTORIES>>, -isystem >\n") | ||
|
|
||
| # Abseil is split into many interdependent libraries whose names change between | ||
| # releases. Record the link interface resolved by CMake instead of duplicating | ||
| # that dependency list in configure. This is the non-MSVC form of the approach | ||
| # used by Apache Sedona's sedona-s2geography crate. | ||
| set(CMAKE_ECHO_LINK_EXECUTABLE | ||
| "sh -c \"echo <LINK_LIBRARIES> > <TARGET>\"") | ||
|
|
||
| add_executable(linker_flags CMakeLists.txt) | ||
| add_dependencies(linker_flags s2geography) | ||
| target_link_libraries(linker_flags PRIVATE "$<TARGET_FILE:s2geography>" s2::s2) | ||
| set_target_properties(linker_flags PROPERTIES LINKER_LANGUAGE ECHO SUFFIX ".txt") |
There was a problem hiding this comment.
This is pretty much what powers the whole thing. We find_package(s2) instead of vendor it, then trick CMake into regurgitating what it would use to link it so we can use a normalish Makevars.
|
@edzer What do you think of this? Conceptually it is "just" pushing one more library (s2geometry) on to the system, but s2geometry doesn't and will never provide pkgconfig so we have to use CMake. This works great for conda, homebrew, and vcpkg, but as a follow-up we can also ship a vendored copy directly derived from vcpkg that can be optionally built should the user not have s2geometry available and there is no binary (likely on Linux). From CRAN's perspective, getting the "build the vendored s2 and abseil with cmake" part right with the vendored copy will be tricky, but also so is the current approach. Also, they can provide such an installation that conforms to their expectations and/or build flags. The main thing I do not want to do is maintain a patched copy of Abseil or s2geometry. |
This branch has not been deployed
1 participant
This PR gets us building against updated s2geometry (0.14). This uses the same approach as apache/sedona/c/sedona-s2geography: use CMake's find_package() to locate s2geography, the emit the flags required to link it into a text file. The Makevars read the text file and in theory link everything. We have passing CI here for Windows, MacOS, and Linux, although we've rigged CI with a homebrew install of s2geometry and a VCPKG_ROOT pre-populated with a fresh cache.
Follow ups are: