Skip to content

GH-50005: [C++] Use FetchContent for RapidJSON#50006

Open
kou wants to merge 2 commits into
apache:mainfrom
kou:cmake-rapidjson-fetchcontent
Open

GH-50005: [C++] Use FetchContent for RapidJSON#50006
kou wants to merge 2 commits into
apache:mainfrom
kou:cmake-rapidjson-fetchcontent

Conversation

@kou
Copy link
Copy Markdown
Member

@kou kou commented May 21, 2026

Rationale for this change

If we use ExternalProject and CMAKE_FIND_USE_PACKAGE_REGISTRY https://cmake.org/cmake/help/latest/variable/CMAKE_FIND_USE_PACKAGE_REGISTRY.html and running cmake twice like #48801 (comment), RapidJSONConfig.cmake in ${BUILD_DIR}/rapidjson_ep/src/rapidjson_ep-install/lib/cmake/RapidjSON/. It's not intentional.

What changes are included in this PR?

Use FetchContent instead of ExternalProject.

Are these changes tested?

Yes.

Are there any user-facing changes?

Yes.

Copilot AI review requested due to automatic review settings May 21, 2026 00:44
@github-actions
Copy link
Copy Markdown

⚠️ GitHub issue #50005 has been automatically assigned in GitHub to PR creator.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates Arrow C++’s third-party dependency build logic to vendor RapidJSON via CMake FetchContent instead of ExternalProject, addressing unintended RapidJSONConfig.cmake side effects when the package registry is enabled and improving behavior with newer CMake versions.

Changes:

  • Replace RapidJSON’s ExternalProject_Add build/install flow with FetchContent_Declare + FetchContent_MakeAvailable.
  • Update RapidJSON include path wiring to reference the fetched source tree rather than an install prefix.
  • Convert build_rapidjson from a macro to a function and propagate RAPIDJSON_VENDORED via PARENT_SCOPE.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 2799 to 2802
add_library(RapidJSON INTERFACE IMPORTED)
target_include_directories(RapidJSON INTERFACE "${RAPIDJSON_INCLUDE_DIR}")
add_dependencies(RapidJSON rapidjson_ep)
target_include_directories(RapidJSON INTERFACE "${rapidjson_SOURCE_DIR}/include")
add_dependencies(RapidJSON rapidjson)

@kou kou force-pushed the cmake-rapidjson-fetchcontent branch from a184453 to 9c0c088 Compare May 21, 2026 01:59
@raulcd raulcd added CI: Extra: Package: Linux Run extra Linux Packages CI CI: Extra: R Run extra R CI labels May 21, 2026
Comment thread cpp/cmake_modules/ThirdpartyToolchain.cmake
@github-actions github-actions Bot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels May 22, 2026
Co-authored-by: Raúl Cumplido <raulcumplido@gmail.com>
@github-actions github-actions Bot added awaiting change review Awaiting change review and removed CI: Extra: Package: Linux Run extra Linux Packages CI awaiting changes Awaiting changes labels May 22, 2026
@kou kou added the CI: Extra: Package: Linux Run extra Linux Packages CI label May 22, 2026
@github-actions github-actions Bot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels May 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting changes Awaiting changes CI: Extra: Package: Linux Run extra Linux Packages CI CI: Extra: R Run extra R CI Component: C++

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants