Skip to content

Commit bc51a76

Browse files
authored
apacheGH-48091: [C++] Use FetchContent for bundled c-ares (apache#48092)
### 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 heavily simplify our third party dependency management. Moving c-ares is the next step before moving 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 c-ares. 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: apache#48091 Authored-by: Raúl Cumplido <[email protected]> Signed-off-by: Raúl Cumplido <[email protected]>
1 parent ef280b1 commit bc51a76

File tree

1 file changed

+76
-31
lines changed

1 file changed

+76
-31
lines changed

cpp/cmake_modules/ThirdpartyToolchain.cmake

Lines changed: 76 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2950,33 +2950,67 @@ if(ARROW_WITH_UTF8PROC)
29502950
resolve_dependency(${utf8proc_resolve_dependency_args})
29512951
endif()
29522952

2953-
macro(build_cares)
2954-
message(STATUS "Building c-ares from source")
2955-
set(CARES_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/cares_ep-install")
2956-
set(CARES_INCLUDE_DIR "${CARES_PREFIX}/include")
2957-
2958-
# If you set -DCARES_SHARED=ON then the build system names the library
2959-
# libcares_static.a
2960-
set(CARES_STATIC_LIB
2961-
"${CARES_PREFIX}/lib/${CMAKE_STATIC_LIBRARY_PREFIX}cares${CMAKE_STATIC_LIBRARY_SUFFIX}"
2962-
)
2953+
function(build_cares)
2954+
list(APPEND CMAKE_MESSAGE_INDENT "c-ares: ")
2955+
message(STATUS "Building c-ares from source using FetchContent")
2956+
set(CARES_VENDORED
2957+
TRUE
2958+
PARENT_SCOPE)
2959+
set(CARES_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/cares_fc-install")
2960+
set(CARES_PREFIX
2961+
"${CARES_PREFIX}"
2962+
PARENT_SCOPE)
29632963

2964-
set(CARES_CMAKE_ARGS "${EP_COMMON_CMAKE_ARGS}" "-DCMAKE_INSTALL_PREFIX=${CARES_PREFIX}"
2965-
-DCARES_SHARED=OFF -DCARES_STATIC=ON)
2964+
fetchcontent_declare(cares
2965+
URL ${CARES_SOURCE_URL}
2966+
URL_HASH "SHA256=${ARROW_CARES_BUILD_SHA256_CHECKSUM}")
29662967

2967-
externalproject_add(cares_ep
2968-
${EP_COMMON_OPTIONS}
2969-
URL ${CARES_SOURCE_URL}
2970-
URL_HASH "SHA256=${ARROW_CARES_BUILD_SHA256_CHECKSUM}"
2971-
CMAKE_ARGS ${CARES_CMAKE_ARGS}
2972-
BUILD_BYPRODUCTS "${CARES_STATIC_LIB}")
2968+
prepare_fetchcontent()
29732969

2974-
file(MAKE_DIRECTORY ${CARES_INCLUDE_DIR})
2970+
set(CARES_SHARED OFF)
2971+
set(CARES_STATIC ON)
2972+
set(CARES_INSTALL ON)
2973+
set(CARES_BUILD_TOOLS OFF)
2974+
set(CARES_BUILD_TESTS OFF)
2975+
fetchcontent_makeavailable(cares)
2976+
2977+
# gRPC requires c-ares to be installed to a known location.
2978+
# We have to do this in two steps to avoid double installation of c-ares
2979+
# when Arrow is installed.
2980+
# This custom target ensures c-ares is built before we install
2981+
add_custom_target(cares_built DEPENDS c-ares::cares)
2982+
2983+
# Disable c-ares's install script after it's built to prevent double installation
2984+
add_custom_command(OUTPUT "${cares_BINARY_DIR}/cmake_install.cmake.saved"
2985+
COMMAND ${CMAKE_COMMAND} -E copy_if_different
2986+
"${cares_BINARY_DIR}/cmake_install.cmake"
2987+
"${cares_BINARY_DIR}/cmake_install.cmake.saved"
2988+
COMMAND ${CMAKE_COMMAND} -E echo
2989+
"# c-ares install disabled to prevent double installation with Arrow"
2990+
> "${cares_BINARY_DIR}/cmake_install.cmake"
2991+
DEPENDS cares_built
2992+
COMMENT "Disabling c-ares install to prevent double installation"
2993+
VERBATIM)
2994+
2995+
add_custom_target(cares_install_disabled ALL
2996+
DEPENDS "${cares_BINARY_DIR}/cmake_install.cmake.saved")
2997+
2998+
# Install c-ares to CARES_PREFIX for gRPC to find
2999+
add_custom_command(OUTPUT "${CARES_PREFIX}/.cares_installed"
3000+
COMMAND ${CMAKE_COMMAND} -E copy_if_different
3001+
"${cares_BINARY_DIR}/cmake_install.cmake.saved"
3002+
"${cares_BINARY_DIR}/cmake_install.cmake.tmp"
3003+
COMMAND ${CMAKE_COMMAND} -DCMAKE_INSTALL_PREFIX=${CARES_PREFIX}
3004+
-DCMAKE_INSTALL_CONFIG_NAME=$<CONFIG> -P
3005+
"${cares_BINARY_DIR}/cmake_install.cmake.tmp" ||
3006+
${CMAKE_COMMAND} -E true
3007+
COMMAND ${CMAKE_COMMAND} -E touch "${CARES_PREFIX}/.cares_installed"
3008+
DEPENDS cares_install_disabled
3009+
COMMENT "Installing c-ares to ${CARES_PREFIX} for gRPC"
3010+
VERBATIM)
29753011

2976-
add_library(c-ares::cares STATIC IMPORTED)
2977-
set_target_properties(c-ares::cares PROPERTIES IMPORTED_LOCATION "${CARES_STATIC_LIB}")
2978-
target_include_directories(c-ares::cares BEFORE INTERFACE "${CARES_INCLUDE_DIR}")
2979-
add_dependencies(c-ares::cares cares_ep)
3012+
# Make cares_fc depend on the install completion marker
3013+
add_custom_target(cares_fc DEPENDS "${CARES_PREFIX}/.cares_installed")
29803014

29813015
if(APPLE)
29823016
# libresolv must be linked from c-ares version 1.16.1
@@ -2985,10 +3019,11 @@ macro(build_cares)
29853019
"${LIBRESOLV_LIBRARY}")
29863020
endif()
29873021

2988-
set(CARES_VENDORED TRUE)
2989-
2990-
list(APPEND ARROW_BUNDLED_STATIC_LIBS c-ares::cares)
2991-
endmacro()
3022+
set(ARROW_BUNDLED_STATIC_LIBS
3023+
${ARROW_BUNDLED_STATIC_LIBS} c-ares::cares
3024+
PARENT_SCOPE)
3025+
list(POP_BACK CMAKE_MESSAGE_INDENT)
3026+
endfunction()
29923027

29933028
# ----------------------------------------------------------------------
29943029
# Dependencies for Arrow Flight RPC
@@ -3136,7 +3171,9 @@ function(build_absl)
31363171
# This is due to upstream absl::cctz issue
31373172
# https://github.com/abseil/abseil-cpp/issues/283
31383173
find_library(CoreFoundation CoreFoundation)
3139-
set_property(TARGET absl::time
3174+
# When ABSL_ENABLE_INSTALL is ON, the real target is "time" not "absl_time"
3175+
# Cannot use set_property on alias targets (absl::time is an alias)
3176+
set_property(TARGET time
31403177
APPEND
31413178
PROPERTY INTERFACE_LINK_LIBRARIES ${CoreFoundation})
31423179
endif()
@@ -3189,7 +3226,7 @@ macro(build_grpc)
31893226
add_dependencies(grpc_dependencies absl_fc)
31903227
endif()
31913228
if(CARES_VENDORED)
3192-
add_dependencies(grpc_dependencies cares_ep)
3229+
add_dependencies(grpc_dependencies cares_fc)
31933230
endif()
31943231

31953232
if(GFLAGS_VENDORED)
@@ -3208,8 +3245,16 @@ macro(build_grpc)
32083245
get_filename_component(GRPC_PB_ROOT "${GRPC_PROTOBUF_INCLUDE_DIR}" DIRECTORY)
32093246
get_target_property(GRPC_Protobuf_PROTOC_LIBRARY ${ARROW_PROTOBUF_LIBPROTOC}
32103247
IMPORTED_LOCATION)
3211-
get_target_property(GRPC_CARES_INCLUDE_DIR c-ares::cares INTERFACE_INCLUDE_DIRECTORIES)
3212-
get_filename_component(GRPC_CARES_ROOT "${GRPC_CARES_INCLUDE_DIR}" DIRECTORY)
3248+
3249+
# For FetchContent c-ares, use the install prefix directly
3250+
if(CARES_VENDORED)
3251+
set(GRPC_CARES_ROOT "${CARES_PREFIX}")
3252+
else()
3253+
get_target_property(GRPC_CARES_INCLUDE_DIR c-ares::cares
3254+
INTERFACE_INCLUDE_DIRECTORIES)
3255+
get_filename_component(GRPC_CARES_ROOT "${GRPC_CARES_INCLUDE_DIR}" DIRECTORY)
3256+
endif()
3257+
32133258
get_target_property(GRPC_RE2_INCLUDE_DIR re2::re2 INTERFACE_INCLUDE_DIRECTORIES)
32143259
get_filename_component(GRPC_RE2_ROOT "${GRPC_RE2_INCLUDE_DIR}" DIRECTORY)
32153260

0 commit comments

Comments
 (0)