Skip to content

Commit c1a8255

Browse files
authored
GH-48316: [C++] Use FetchContent for bundled nlohmann-json (#48320)
### 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. ### What changes are included in this PR? The general change is moving nlohmann-json from `ExternalProject` to `FetchContent`. The expectation is that we will be able to clean up installation once google-cloud-cpp is also migrated. ### Are these changes tested? Yes, the changes are tested locally and on CI. ### Are there any user-facing changes? No Authored-by: Raúl Cumplido <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
1 parent f7159f2 commit c1a8255

File tree

1 file changed

+74
-30
lines changed

1 file changed

+74
-30
lines changed

cpp/cmake_modules/ThirdpartyToolchain.cmake

Lines changed: 74 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -3465,38 +3465,73 @@ function(build_crc32c_once)
34653465
list(POP_BACK CMAKE_MESSAGE_INDENT)
34663466
endfunction()
34673467

3468-
macro(build_nlohmann_json)
3469-
message(STATUS "Building nlohmann-json from source")
3470-
# "Build" nlohmann-json
3471-
set(NLOHMANN_JSON_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/nlohmann_json_ep-install")
3472-
set(NLOHMANN_JSON_INCLUDE_DIR "${NLOHMANN_JSON_PREFIX}/include")
3473-
set(NLOHMANN_JSON_CMAKE_ARGS
3474-
${EP_COMMON_CMAKE_ARGS} "-DCMAKE_INSTALL_PREFIX=<INSTALL_DIR>"
3475-
# google-cloud-cpp requires JSON_MultipleHeaders=ON
3476-
-DJSON_BuildTests=OFF -DJSON_MultipleHeaders=ON)
3468+
function(build_nlohmann_json)
3469+
list(APPEND CMAKE_MESSAGE_INDENT "nlohmann-json: ")
3470+
message(STATUS "Building nlohmann-json from source using FetchContent")
3471+
set(NLOHMANN_JSON_VENDORED
3472+
TRUE
3473+
PARENT_SCOPE)
3474+
set(NLOHMANN_JSON_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/nlohmann_json_fc-install")
3475+
set(NLOHMANN_JSON_PREFIX
3476+
"${NLOHMANN_JSON_PREFIX}"
3477+
PARENT_SCOPE)
34773478

3478-
# We can remove this once we remove -DCMAKE_POLICY_VERSION_MINIMUM=3.5
3479-
# from EP_COMMON_CMAKE_ARGS.
3480-
list(REMOVE_ITEM NLOHMANN_JSON_CMAKE_ARGS -DCMAKE_POLICY_VERSION_MINIMUM=3.5)
3479+
fetchcontent_declare(nlohmann_json
3480+
${FC_DECLARE_COMMON_OPTIONS}
3481+
URL ${NLOHMANN_JSON_SOURCE_URL}
3482+
URL_HASH "SHA256=${ARROW_NLOHMANN_JSON_BUILD_SHA256_CHECKSUM}")
34813483

3482-
set(NLOHMANN_JSON_BUILD_BYPRODUCTS ${NLOHMANN_JSON_PREFIX}/include/nlohmann/json.hpp)
3484+
prepare_fetchcontent()
34833485

3484-
externalproject_add(nlohmann_json_ep
3485-
${EP_COMMON_OPTIONS}
3486-
INSTALL_DIR ${NLOHMANN_JSON_PREFIX}
3487-
URL ${NLOHMANN_JSON_SOURCE_URL}
3488-
URL_HASH "SHA256=${ARROW_NLOHMANN_JSON_BUILD_SHA256_CHECKSUM}"
3489-
CMAKE_ARGS ${NLOHMANN_JSON_CMAKE_ARGS}
3490-
BUILD_BYPRODUCTS ${NLOHMANN_JSON_BUILD_BYPRODUCTS})
3486+
# google-cloud-cpp requires JSON_MultipleHeaders=ON
3487+
set(JSON_BuildTests OFF)
3488+
set(JSON_MultipleHeaders ON)
3489+
set(JSON_Install ON)
3490+
fetchcontent_makeavailable(nlohmann_json)
34913491

3492-
# Work around https://gitlab.kitware.com/cmake/cmake/issues/15052
3493-
file(MAKE_DIRECTORY ${NLOHMANN_JSON_INCLUDE_DIR})
3492+
# google-cloud-cpp requires nlohmann_json to be installed to a known location.
3493+
# We have to do this in two steps to avoid double installation of nlohmann_json
3494+
# when Arrow is installed.
3495+
# This custom target ensures nlohmann_json is built before we install.
3496+
add_custom_target(nlohmann_json_built DEPENDS nlohmann_json::nlohmann_json)
34943497

3495-
add_library(nlohmann_json::nlohmann_json INTERFACE IMPORTED)
3496-
target_include_directories(nlohmann_json::nlohmann_json BEFORE
3497-
INTERFACE "${NLOHMANN_JSON_INCLUDE_DIR}")
3498-
add_dependencies(nlohmann_json::nlohmann_json nlohmann_json_ep)
3499-
endmacro()
3498+
# Disable nlohmann_json's install script after it's built to prevent double installation.
3499+
add_custom_command(OUTPUT "${nlohmann_json_BINARY_DIR}/cmake_install.cmake.saved"
3500+
COMMAND ${CMAKE_COMMAND} -E copy_if_different
3501+
"${nlohmann_json_BINARY_DIR}/cmake_install.cmake"
3502+
"${nlohmann_json_BINARY_DIR}/cmake_install.cmake.saved"
3503+
COMMAND ${CMAKE_COMMAND} -E echo
3504+
"# nlohmann-json install disabled to prevent double installation with Arrow"
3505+
> "${nlohmann_json_BINARY_DIR}/cmake_install.cmake"
3506+
DEPENDS nlohmann_json_built
3507+
COMMENT "Disabling nlohmann-json install to prevent double installation"
3508+
VERBATIM)
3509+
3510+
add_custom_target(nlohmann_json_install_disabled ALL
3511+
DEPENDS "${nlohmann_json_BINARY_DIR}/cmake_install.cmake.saved")
3512+
3513+
# Install nlohmann_json to NLOHMANN_JSON_PREFIX for google-cloud-cpp to find.
3514+
add_custom_command(OUTPUT "${NLOHMANN_JSON_PREFIX}/.nlohmann_json_installed"
3515+
COMMAND ${CMAKE_COMMAND} -E copy_if_different
3516+
"${nlohmann_json_BINARY_DIR}/cmake_install.cmake.saved"
3517+
"${nlohmann_json_BINARY_DIR}/cmake_install.cmake.tmp"
3518+
COMMAND ${CMAKE_COMMAND}
3519+
-DCMAKE_INSTALL_PREFIX=${NLOHMANN_JSON_PREFIX}
3520+
-DCMAKE_INSTALL_CONFIG_NAME=$<CONFIG> -P
3521+
"${nlohmann_json_BINARY_DIR}/cmake_install.cmake.tmp" ||
3522+
${CMAKE_COMMAND} -E true
3523+
COMMAND ${CMAKE_COMMAND} -E touch
3524+
"${NLOHMANN_JSON_PREFIX}/.nlohmann_json_installed"
3525+
DEPENDS nlohmann_json_install_disabled
3526+
COMMENT "Installing nlohmann-json to ${NLOHMANN_JSON_PREFIX} for google-cloud-cpp"
3527+
VERBATIM)
3528+
3529+
# Make nlohmann_json_fc depend on the install completion marker.
3530+
add_custom_target(nlohmann_json_fc
3531+
DEPENDS "${NLOHMANN_JSON_PREFIX}/.nlohmann_json_installed")
3532+
3533+
list(POP_BACK CMAKE_MESSAGE_INDENT)
3534+
endfunction()
35003535
if(ARROW_WITH_NLOHMANN_JSON)
35013536
resolve_dependency(nlohmann_json)
35023537
get_target_property(nlohmann_json_INCLUDE_DIR nlohmann_json::nlohmann_json
@@ -3559,7 +3594,11 @@ macro(build_google_cloud_cpp_storage)
35593594
add_dependencies(google_cloud_cpp_dependencies zlib_ep)
35603595
endif()
35613596
add_dependencies(google_cloud_cpp_dependencies crc32c_fc)
3562-
add_dependencies(google_cloud_cpp_dependencies nlohmann_json::nlohmann_json)
3597+
if(NLOHMANN_JSON_VENDORED)
3598+
add_dependencies(google_cloud_cpp_dependencies nlohmann_json_fc)
3599+
else()
3600+
add_dependencies(google_cloud_cpp_dependencies nlohmann_json::nlohmann_json)
3601+
endif()
35633602

35643603
set(GOOGLE_CLOUD_CPP_STATIC_LIBRARY_STORAGE
35653604
"${GOOGLE_CLOUD_CPP_INSTALL_PREFIX}/lib/${CMAKE_STATIC_LIBRARY_PREFIX}google_cloud_cpp_storage${CMAKE_STATIC_LIBRARY_SUFFIX}"
@@ -4052,9 +4091,14 @@ macro(build_opentelemetry)
40524091
CONFIGURE_COMMAND ""
40534092
INSTALL_COMMAND ""
40544093
EXCLUDE_FROM_ALL OFF)
4094+
if(NLOHMANN_JSON_VENDORED)
4095+
add_dependencies(opentelemetry_dependencies nlohmann_json_fc)
4096+
else()
4097+
add_dependencies(opentelemetry_dependencies nlohmann_json::nlohmann_json)
4098+
endif()
40554099

4056-
add_dependencies(opentelemetry_dependencies nlohmann_json::nlohmann_json
4057-
opentelemetry_proto_ep ${ARROW_PROTOBUF_LIBPROTOBUF})
4100+
add_dependencies(opentelemetry_dependencies opentelemetry_proto_ep
4101+
${ARROW_PROTOBUF_LIBPROTOBUF})
40584102

40594103
# Ensure vendored protobuf is installed before OpenTelemetry builds
40604104
if(PROTOBUF_VENDORED)

0 commit comments

Comments
 (0)