Skip to content

GH-48074: [C++] Use FetchContent for bundled Abseil - #48075

Merged
kou merged 4 commits into
apache:mainfrom
raulcd:GH-48074
Nov 15, 2025
Merged

GH-48074: [C++] Use FetchContent for bundled Abseil#48075
kou merged 4 commits into
apache:mainfrom
raulcd:GH-48074

Conversation

@raulcd

@raulcdraulcd commented Nov 6, 2025

Copy link
Copy Markdown
Member

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 abseil is the first step to simplify some of them.

What changes are included in this PR?

The general change is moving from ExternalProject to FetchContent. In more detail this gets rid of all the manual definition of targets for all abseil libraries with it's dependency management. This is removing around 900 lines of custom code which is not necessary with 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 abseil. 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. As we have to install abseil for those to find it we also want to not install abseil outside of the build path that's why we override abseil's cmake_install.cmake with no-op.

Are these changes tested?

Yes, the changes are tested locally and on CI.

Are there any user-facing changes?

No

@github-actions

Copy link
Copy Markdown

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

@raulcd

Copy link
Copy Markdown
MemberAuthor

@github-actions crossbow submit -g cpp

@github-actions

Copy link
Copy Markdown

Revision: ffffeb4

Submitted crossbow builds: ursacomputing/crossbow @ actions-f1048dc859

TaskStatus
example-cpp-minimal-build-staticGitHub Actions
example-cpp-minimal-build-static-system-dependencyGitHub Actions
example-cpp-tutorialGitHub Actions
test-build-cpp-fuzzGitHub Actions
test-conda-cppGitHub Actions
test-conda-cpp-valgrindGitHub Actions
test-cuda-cpp-ubuntu-22.04-cuda-11.7.1GitHub Actions
test-debian-12-cpp-amd64GitHub Actions
test-debian-12-cpp-i386GitHub Actions
test-fedora-42-cppGitHub Actions
test-ubuntu-22.04-cppGitHub Actions
test-ubuntu-22.04-cpp-20GitHub Actions
test-ubuntu-22.04-cpp-bundledGitHub Actions
test-ubuntu-22.04-cpp-emscriptenGitHub Actions
test-ubuntu-22.04-cpp-no-threadingGitHub Actions
test-ubuntu-24.04-cppGitHub Actions
test-ubuntu-24.04-cpp-bundled-offlineGitHub Actions
test-ubuntu-24.04-cpp-gcc-13-bundledGitHub Actions
test-ubuntu-24.04-cpp-gcc-14GitHub Actions
test-ubuntu-24.04-cpp-minimal-with-formatsGitHub Actions
test-ubuntu-24.04-cpp-thread-sanitizerGitHub Actions

@raulcd

Copy link
Copy Markdown
MemberAuthor

@github-actions crossbow submit -g cpp

@github-actions

Copy link
Copy Markdown

Revision: ef326eb

Submitted crossbow builds: ursacomputing/crossbow @ actions-b9f2ce761a

TaskStatus
example-cpp-minimal-build-staticGitHub Actions
example-cpp-minimal-build-static-system-dependencyGitHub Actions
example-cpp-tutorialGitHub Actions
test-build-cpp-fuzzGitHub Actions
test-conda-cppGitHub Actions
test-conda-cpp-valgrindGitHub Actions
test-cuda-cpp-ubuntu-22.04-cuda-11.7.1GitHub Actions
test-debian-12-cpp-amd64GitHub Actions
test-debian-12-cpp-i386GitHub Actions
test-fedora-42-cppGitHub Actions
test-ubuntu-22.04-cppGitHub Actions
test-ubuntu-22.04-cpp-20GitHub Actions
test-ubuntu-22.04-cpp-bundledGitHub Actions
test-ubuntu-22.04-cpp-emscriptenGitHub Actions
test-ubuntu-22.04-cpp-no-threadingGitHub Actions
test-ubuntu-24.04-cppGitHub Actions
test-ubuntu-24.04-cpp-bundled-offlineGitHub Actions
test-ubuntu-24.04-cpp-gcc-13-bundledGitHub Actions
test-ubuntu-24.04-cpp-gcc-14GitHub Actions
test-ubuntu-24.04-cpp-minimal-with-formatsGitHub Actions
test-ubuntu-24.04-cpp-thread-sanitizerGitHub Actions

@raulcd

Copy link
Copy Markdown
MemberAuthor

@kou I'd like to discuss this with you once you have some time. There are some challenges mixing FetchContent and ExternalProject but with some custom manipulations I was able to make this work. We will be able to get rid of part of the code once we migrate grpc, gcs to FetchContent too but this can be done after this PR to keep things small instead of trying a big PR approach.
This approach works and we are able to get rid of a lot of custom code to manipulate the abseil targets. This will massively simplify upgrading abseil.
I plan to work on grpc, protobuf, gcs migration to FetchContent next and my plan is to update dependencies once the migration is done. Let me know your thoughts.

@raulcd
raulcd requested a review from kouNovember 7, 2025 10:04
@raulcd

Copy link
Copy Markdown
MemberAuthor

@github actions crossbow submit bundled

@raulcd

Copy link
Copy Markdown
MemberAuthor

@github-actions crossbow submit bundled

@github-actions

Copy link
Copy Markdown

Revision: b81600c

Submitted crossbow builds: ursacomputing/crossbow @ actions-3868d3fe73

TaskStatus
test-r-depsource-bundledAzure
test-ubuntu-22.04-cpp-bundledGitHub Actions
test-ubuntu-24.04-cpp-bundled-offlineGitHub Actions
test-ubuntu-24.04-cpp-gcc-13-bundledGitHub Actions

@raulcd

Copy link
Copy Markdown
MemberAuthor

@kou@assignUser I would apprec iate if you could take a look here. I have several PRs aligned after this one, the current plan is to move:
abseil -> c-ares (already on a follow up draft PR) -> protobuf (currently working on it, locally) -> re2 ->grpc

The main problem is that in the interim we have to live with both FetchContent and ExternalProject and their interaction. See the description + comments above.

@kou

kou commented Nov 14, 2025

Copy link
Copy Markdown
Member

Sorry for missing this! (I saw your comment last week but I attended a conference. So I forgot to review this... Sorry...)

I'll review this today.

@@ -2994,920 +3005,146 @@ endmacro()
# Dependencies for Arrow Flight RPC

macro(build_absl)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you use function() not macro() to create a variable scope?

We can remove cleanup_fetchcontent() by this change.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

That was a good idea, moving to function help me clean a bunch of things that were not necessary! Thanks for the suggestion @kou

Comment threadcpp/cmake_modules/ThirdpartyToolchain.cmake
Comment threadcpp/cmake_modules/ThirdpartyToolchain.cmake Outdated
Comment threadcpp/cmake_modules/ThirdpartyToolchain.cmake Outdated
Comment threadcpp/cmake_modules/ThirdpartyToolchain.cmake Outdated
Comment threadcpp/cmake_modules/ThirdpartyToolchain.cmake Outdated
Comment threadcpp/cmake_modules/ThirdpartyToolchain.cmake Outdated
Comment threadcpp/cmake_modules/ThirdpartyToolchain.cmake Outdated
Comment threadcpp/cmake_modules/ThirdpartyToolchain.cmake Outdated
Comment threadcpp/cmake_modules/ThirdpartyToolchain.cmake Outdated
@github-actionsgithub-actionsBot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Nov 14, 2025
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
@github-actionsgithub-actionsBot added awaiting change review Awaiting change review awaiting changes Awaiting changes and removed awaiting changes Awaiting changes awaiting change review Awaiting change review labels Nov 14, 2025
@github-actionsgithub-actionsBot removed the awaiting changes Awaiting changes label Nov 14, 2025
@github-actionsgithub-actionsBot added the awaiting change review Awaiting change review label Nov 14, 2025
@raulcd

Copy link
Copy Markdown
MemberAuthor

@github-actions crossbow submit bundled

@github-actionsgithub-actionsBot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels Nov 14, 2025
@github-actions

Copy link
Copy Markdown

Revision: 541ca51

Submitted crossbow builds: ursacomputing/crossbow @ actions-46baf386d7

TaskStatus
test-r-depsource-bundledAzure
test-ubuntu-22.04-cpp-bundledGitHub Actions
test-ubuntu-24.04-cpp-bundled-offlineGitHub Actions
test-ubuntu-24.04-cpp-gcc-13-bundledGitHub Actions

@koukou added the CI: Extra Run extra CI label Nov 14, 2025
@kou

kou commented Nov 14, 2025

Copy link
Copy Markdown
Member

@github-actions crossbow submit -g cpp

@github-actions

Copy link
Copy Markdown

Revision: 541ca51

Submitted crossbow builds: ursacomputing/crossbow @ actions-f8f2fc4819

TaskStatus
example-cpp-minimal-build-staticGitHub Actions
example-cpp-minimal-build-static-system-dependencyGitHub Actions
example-cpp-tutorialGitHub Actions
test-build-cpp-fuzzGitHub Actions
test-conda-cppGitHub Actions
test-conda-cpp-valgrindGitHub Actions
test-cuda-cpp-ubuntu-22.04-cuda-11.7.1GitHub Actions
test-debian-12-cpp-amd64GitHub Actions
test-debian-12-cpp-i386GitHub Actions
test-fedora-42-cppGitHub Actions
test-ubuntu-22.04-cppGitHub Actions
test-ubuntu-22.04-cpp-20GitHub Actions
test-ubuntu-22.04-cpp-bundledGitHub Actions
test-ubuntu-22.04-cpp-emscriptenGitHub Actions
test-ubuntu-22.04-cpp-no-threadingGitHub Actions
test-ubuntu-24.04-cppGitHub Actions
test-ubuntu-24.04-cpp-bundled-offlineGitHub Actions
test-ubuntu-24.04-cpp-gcc-13-bundledGitHub Actions
test-ubuntu-24.04-cpp-gcc-14GitHub Actions
test-ubuntu-24.04-cpp-minimal-with-formatsGitHub Actions
test-ubuntu-24.04-cpp-thread-sanitizerGitHub Actions

kou
kou approved these changes Nov 15, 2025

@koukou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1

@kou
kou merged commit de02131 into apache:mainNov 15, 2025
71 of 74 checks passed
@koukou removed the awaiting changes Awaiting changes label Nov 15, 2025
@github-actionsgithub-actionsBot added the awaiting merge Awaiting merge label Nov 15, 2025
@conbench-apache-arrow

Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit de02131.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 22 possible false positives for unstable benchmarks that are known to sometimes produce them.

@jonkeanejonkeane mentioned this pull request Nov 25, 2025
Mottl pushed a commit to Mottl/arrow that referenced this pull request May 26, 2026
### 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 abseil is the first step to simplify some of them.
### What changes are included in this PR?
The general change is moving from `ExternalProject` to `FetchContent`. In more detail this gets rid of all the manual definition of targets for all abseil libraries with it's dependency management. This is removing around 900 lines of custom code which is not necessary with 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 abseil. 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. As we have to install abseil for those to find it we also want to not install abseil outside of the build path that's why we override abseil's `cmake_install.cmake` with no-op.
### Are these changes tested?
Yes, the changes are tested locally and on CI.
### Are there any user-facing changes?
No
* GitHub Issue: apache#48074
Lead-authored-by: Raúl Cumplido <raulcumplido@gmail.com>
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting mergeAwaiting mergeCI: ExtraRun extra CIComponent: C++

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@raulcd@kou