Skip to content

Commit f8ae141

Browse files
authored
GH-48178: [C++] Use FetchContent for bundled RE2 (#48179)
### Rationale for this change As a follow up of requiring a minimum CMake version >= 3.25 we discussed moving our dependencies from ExternalProject to FetchContent. This can simplify our third party dependency management. Moving RE2 is the next step before moving Protobuf and gRPC. ### What changes are included in this PR? The general change is moving from `ExternalProject` to `FetchContent`. It also add some required integration due to other dependencies, like gRPC, using `ExternalProject`. We not only have to build but also install in order for those other dependencies to find RE2. This causes some timing issues between config, build, install that requires us to create a custom target to depend on so the other dependencies find abseil. ### Are these changes tested? Yes, the changes are tested locally and on CI. ### Are there any user-facing changes? No * GitHub Issue: #48178 Authored-by: Raúl Cumplido <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
1 parent 2fb2f79 commit f8ae141

File tree

1 file changed

+65
-31
lines changed

1 file changed

+65
-31
lines changed

cpp/cmake_modules/ThirdpartyToolchain.cmake

Lines changed: 65 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2799,37 +2799,66 @@ if(ARROW_WITH_ZSTD)
27992799
endif()
28002800

28012801
# ----------------------------------------------------------------------
2802-
# RE2 (required for Gandiva)
2802+
# RE2 (required for Gandiva and gRPC)
28032803

2804-
macro(build_re2)
2805-
message(STATUS "Building RE2 from source")
2806-
set(RE2_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/re2_ep-install")
2807-
set(RE2_STATIC_LIB
2808-
"${RE2_PREFIX}/lib/${CMAKE_STATIC_LIBRARY_PREFIX}re2${CMAKE_STATIC_LIBRARY_SUFFIX}")
2804+
function(build_re2)
2805+
list(APPEND CMAKE_MESSAGE_INDENT "RE2: ")
2806+
message(STATUS "Building RE2 from source using FetchContent")
2807+
set(RE2_VENDORED
2808+
TRUE
2809+
PARENT_SCOPE)
2810+
set(RE2_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/re2_fc-install")
2811+
set(RE2_PREFIX
2812+
"${RE2_PREFIX}"
2813+
PARENT_SCOPE)
28092814

2810-
set(RE2_CMAKE_ARGS ${EP_COMMON_CMAKE_ARGS} "-DCMAKE_INSTALL_PREFIX=${RE2_PREFIX}")
2815+
fetchcontent_declare(re2
2816+
URL ${RE2_SOURCE_URL}
2817+
URL_HASH "SHA256=${ARROW_RE2_BUILD_SHA256_CHECKSUM}")
2818+
prepare_fetchcontent()
28112819

2812-
externalproject_add(re2_ep
2813-
${EP_COMMON_OPTIONS}
2814-
INSTALL_DIR ${RE2_PREFIX}
2815-
URL ${RE2_SOURCE_URL}
2816-
URL_HASH "SHA256=${ARROW_RE2_BUILD_SHA256_CHECKSUM}"
2817-
CMAKE_ARGS ${RE2_CMAKE_ARGS}
2818-
BUILD_BYPRODUCTS "${RE2_STATIC_LIB}")
2819-
2820-
file(MAKE_DIRECTORY "${RE2_PREFIX}/include")
2821-
add_library(re2::re2 STATIC IMPORTED)
2822-
set_target_properties(re2::re2 PROPERTIES IMPORTED_LOCATION "${RE2_STATIC_LIB}")
2823-
target_include_directories(re2::re2 BEFORE INTERFACE "${RE2_PREFIX}/include")
2824-
2825-
add_dependencies(re2::re2 re2_ep)
2826-
set(RE2_VENDORED TRUE)
2827-
# Set values so that FindRE2 finds this too
2828-
set(RE2_LIB ${RE2_STATIC_LIB})
2829-
set(RE2_INCLUDE_DIR "${RE2_PREFIX}/include")
2830-
2831-
list(APPEND ARROW_BUNDLED_STATIC_LIBS re2::re2)
2832-
endmacro()
2820+
# Unity build causes some build errors
2821+
set(CMAKE_UNITY_BUILD OFF)
2822+
fetchcontent_makeavailable(re2)
2823+
2824+
# This custom target ensures re2 is built before we install
2825+
add_custom_target(re2_built DEPENDS re2::re2)
2826+
2827+
add_custom_command(OUTPUT "${re2_BINARY_DIR}/cmake_install.cmake.saved"
2828+
COMMAND ${CMAKE_COMMAND} -E copy_if_different
2829+
"${re2_BINARY_DIR}/cmake_install.cmake"
2830+
"${re2_BINARY_DIR}/cmake_install.cmake.saved"
2831+
COMMAND ${CMAKE_COMMAND} -E echo
2832+
"# RE2 install disabled to prevent double installation with Arrow"
2833+
> "${re2_BINARY_DIR}/cmake_install.cmake"
2834+
DEPENDS re2_built
2835+
COMMENT "Disabling RE2 install to prevent double installation"
2836+
VERBATIM)
2837+
2838+
add_custom_target(re2_install_disabled ALL
2839+
DEPENDS "${re2_BINARY_DIR}/cmake_install.cmake.saved")
2840+
2841+
# Install RE2 to RE2_PREFIX for gRPC to find
2842+
add_custom_command(OUTPUT "${RE2_PREFIX}/.re2_installed"
2843+
COMMAND ${CMAKE_COMMAND} -E copy_if_different
2844+
"${re2_BINARY_DIR}/cmake_install.cmake.saved"
2845+
"${re2_BINARY_DIR}/cmake_install.cmake.tmp"
2846+
COMMAND ${CMAKE_COMMAND} -DCMAKE_INSTALL_PREFIX=${RE2_PREFIX}
2847+
-DCMAKE_INSTALL_CONFIG_NAME=$<CONFIG> -P
2848+
"${re2_BINARY_DIR}/cmake_install.cmake.tmp" ||
2849+
${CMAKE_COMMAND} -E true
2850+
COMMAND ${CMAKE_COMMAND} -E touch "${RE2_PREFIX}/.re2_installed"
2851+
DEPENDS re2_install_disabled
2852+
COMMENT "Installing RE2 to ${RE2_PREFIX} for gRPC"
2853+
VERBATIM)
2854+
2855+
add_custom_target(re2_fc DEPENDS "${RE2_PREFIX}/.re2_installed")
2856+
2857+
set(ARROW_BUNDLED_STATIC_LIBS
2858+
${ARROW_BUNDLED_STATIC_LIBS} re2::re2
2859+
PARENT_SCOPE)
2860+
list(POP_BACK CMAKE_MESSAGE_INDENT)
2861+
endfunction()
28332862

28342863
if(ARROW_WITH_RE2)
28352864
resolve_dependency(re2
@@ -3234,7 +3263,7 @@ macro(build_grpc)
32343263
endif()
32353264

32363265
if(RE2_VENDORED)
3237-
add_dependencies(grpc_dependencies re2_ep)
3266+
add_dependencies(grpc_dependencies re2_fc)
32383267
endif()
32393268

32403269
add_dependencies(grpc_dependencies ${ARROW_PROTOBUF_LIBPROTOBUF} c-ares::cares
@@ -3255,8 +3284,13 @@ macro(build_grpc)
32553284
get_filename_component(GRPC_CARES_ROOT "${GRPC_CARES_INCLUDE_DIR}" DIRECTORY)
32563285
endif()
32573286

3258-
get_target_property(GRPC_RE2_INCLUDE_DIR re2::re2 INTERFACE_INCLUDE_DIRECTORIES)
3259-
get_filename_component(GRPC_RE2_ROOT "${GRPC_RE2_INCLUDE_DIR}" DIRECTORY)
3287+
# For FetchContent RE2, use the install prefix directly
3288+
if(RE2_VENDORED)
3289+
set(GRPC_RE2_ROOT "${RE2_PREFIX}")
3290+
else()
3291+
get_target_property(GRPC_RE2_INCLUDE_DIR re2::re2 INTERFACE_INCLUDE_DIRECTORIES)
3292+
get_filename_component(GRPC_RE2_ROOT "${GRPC_RE2_INCLUDE_DIR}" DIRECTORY)
3293+
endif()
32603294

32613295
# Put Abseil, etc. first so that local directories are searched
32623296
# before (what are likely) system directories

0 commit comments

Comments
 (0)