Skip to content

Conversation

@raulcd
Copy link
Member

@raulcd raulcd commented Nov 6, 2025

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

github-actions bot commented Nov 6, 2025

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

@raulcd
Copy link
Member Author

raulcd commented Nov 6, 2025

@github-actions crossbow submit -g cpp

@github-actions
Copy link

github-actions bot commented Nov 6, 2025

Revision: ffffeb4

Submitted crossbow builds: ursacomputing/crossbow @ actions-f1048dc859

Task Status
example-cpp-minimal-build-static GitHub Actions
example-cpp-minimal-build-static-system-dependency GitHub Actions
example-cpp-tutorial GitHub Actions
test-build-cpp-fuzz GitHub Actions
test-conda-cpp GitHub Actions
test-conda-cpp-valgrind GitHub Actions
test-cuda-cpp-ubuntu-22.04-cuda-11.7.1 GitHub Actions
test-debian-12-cpp-amd64 GitHub Actions
test-debian-12-cpp-i386 GitHub Actions
test-fedora-42-cpp GitHub Actions
test-ubuntu-22.04-cpp GitHub Actions
test-ubuntu-22.04-cpp-20 GitHub Actions
test-ubuntu-22.04-cpp-bundled GitHub Actions
test-ubuntu-22.04-cpp-emscripten GitHub Actions
test-ubuntu-22.04-cpp-no-threading GitHub Actions
test-ubuntu-24.04-cpp GitHub Actions
test-ubuntu-24.04-cpp-bundled-offline GitHub Actions
test-ubuntu-24.04-cpp-gcc-13-bundled GitHub Actions
test-ubuntu-24.04-cpp-gcc-14 GitHub Actions
test-ubuntu-24.04-cpp-minimal-with-formats GitHub Actions
test-ubuntu-24.04-cpp-thread-sanitizer GitHub Actions

@raulcd
Copy link
Member Author

raulcd commented Nov 6, 2025

@github-actions crossbow submit -g cpp

@github-actions
Copy link

github-actions bot commented Nov 6, 2025

Revision: ef326eb

Submitted crossbow builds: ursacomputing/crossbow @ actions-b9f2ce761a

Task Status
example-cpp-minimal-build-static GitHub Actions
example-cpp-minimal-build-static-system-dependency GitHub Actions
example-cpp-tutorial GitHub Actions
test-build-cpp-fuzz GitHub Actions
test-conda-cpp GitHub Actions
test-conda-cpp-valgrind GitHub Actions
test-cuda-cpp-ubuntu-22.04-cuda-11.7.1 GitHub Actions
test-debian-12-cpp-amd64 GitHub Actions
test-debian-12-cpp-i386 GitHub Actions
test-fedora-42-cpp GitHub Actions
test-ubuntu-22.04-cpp GitHub Actions
test-ubuntu-22.04-cpp-20 GitHub Actions
test-ubuntu-22.04-cpp-bundled GitHub Actions
test-ubuntu-22.04-cpp-emscripten GitHub Actions
test-ubuntu-22.04-cpp-no-threading GitHub Actions
test-ubuntu-24.04-cpp GitHub Actions
test-ubuntu-24.04-cpp-bundled-offline GitHub Actions
test-ubuntu-24.04-cpp-gcc-13-bundled GitHub Actions
test-ubuntu-24.04-cpp-gcc-14 GitHub Actions
test-ubuntu-24.04-cpp-minimal-with-formats GitHub Actions
test-ubuntu-24.04-cpp-thread-sanitizer GitHub Actions

@raulcd
Copy link
Member Author

raulcd commented Nov 7, 2025

@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 kou November 7, 2025 10:04
@raulcd
Copy link
Member Author

raulcd commented Nov 10, 2025

@github actions crossbow submit bundled

@raulcd
Copy link
Member Author

raulcd commented Nov 13, 2025

@github-actions crossbow submit bundled

@github-actions
Copy link

Revision: b81600c

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

Task Status
test-r-depsource-bundled Azure
test-ubuntu-22.04-cpp-bundled GitHub Actions
test-ubuntu-24.04-cpp-bundled-offline GitHub Actions
test-ubuntu-24.04-cpp-gcc-13-bundled GitHub Actions

@raulcd
Copy link
Member Author

raulcd commented Nov 13, 2025

@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.

@raulcd raulcd requested a review from assignUser November 13, 2025 10:06
@kou
Copy link
Member

kou commented Nov 14, 2025

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.

# ----------------------------------------------------------------------
# Dependencies for Arrow Flight RPC

macro(build_absl)
Copy link
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
Member Author

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

@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Nov 14, 2025
@github-actions github-actions bot 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-actions github-actions bot removed the awaiting changes Awaiting changes label Nov 14, 2025
@github-actions github-actions bot added the awaiting change review Awaiting change review label Nov 14, 2025
@raulcd
Copy link
Member Author

raulcd commented Nov 14, 2025

@github-actions crossbow submit bundled

@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels Nov 14, 2025
@github-actions
Copy link

Revision: 541ca51

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

Task Status
test-r-depsource-bundled Azure
test-ubuntu-22.04-cpp-bundled GitHub Actions
test-ubuntu-24.04-cpp-bundled-offline GitHub Actions
test-ubuntu-24.04-cpp-gcc-13-bundled GitHub Actions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants