Skip to content

Commit

Permalink
apacheGH-43218: [C++] Resolve Abseil like any other dependency in the…
Browse files Browse the repository at this point in the history
… build system (apache#43219)

### Rationale for this change

The workarounds around Abseil resolution don't seem necessary anymore and they don't work on all possible configurations of the build.

### What changes are included in this PR?

Removal of the `ensure_absl` macro and adding a call to `resolve_dependency` when depending on the Google Cloud SDK (a GCS filesystem dependency) or gRPC (a flight dependency).

### Are these changes tested?

Yes, by me trying different build configurations on my macOS and existing builds in CI.

* GitHub Issue: apache#43218

Lead-authored-by: Felipe Oliveira Carvalho <[email protected]>
Co-authored-by: Sutou Kouhei <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
  • Loading branch information
felipecrv and kou authored Jul 12, 2024
1 parent 6e438e6 commit e65c1e2
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 32 deletions.
3 changes: 1 addition & 2 deletions ci/docker/fedora-39-cpp.dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,7 @@ RUN /arrow/ci/scripts/install_sccache.sh unknown-linux-musl /usr/local/bin

# PYARROW_TEST_GANDIVA=OFF: GH-39695: We need to make LLVM symbols visible in
# Python process explicitly if we use LLVM 17 or later.
ENV absl_SOURCE=BUNDLED \
ARROW_ACERO=ON \
ENV ARROW_ACERO=ON \
ARROW_AZURE=OFF \
ARROW_BUILD_TESTS=ON \
ARROW_DEPENDENCY_SOURCE=SYSTEM \
Expand Down
2 changes: 1 addition & 1 deletion cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -689,7 +689,7 @@ endif()

if("${ARROW_TEST_LINKAGE}" STREQUAL "shared")
if(ARROW_BUILD_TESTS AND NOT ARROW_BUILD_SHARED)
message(FATAL_ERROR "If using shared linkage for unit tests, must also \
message(FATAL_ERROR "If using ARROW_TEST_LINKAGE=shared, must also \
pass ARROW_BUILD_SHARED=on")
endif()
# Use shared linking for unit tests if it's available
Expand Down
57 changes: 28 additions & 29 deletions cpp/cmake_modules/ThirdpartyToolchain.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -2874,33 +2874,6 @@ endmacro()
# ----------------------------------------------------------------------
# Dependencies for Arrow Flight RPC

macro(ensure_absl)
if(NOT absl_FOUND)
if(${absl_SOURCE} STREQUAL "AUTO")
# We can't use resolve_dependency(absl 20211102) to use Abseil
# 20211102 or later because Abseil's CMake package uses "EXACT"
# version match strategy. Our CMake configuration will work with
# Abseil LTS 20211102 or later. So we want to accept Abseil LTS
# 20211102 or later. We need to update
# ARROW_ABSL_REQUIRED_LTS_VERSIONS list when new Abseil LTS is
# released.
set(ARROW_ABSL_REQUIRED_LTS_VERSIONS 20230125 20220623 20211102)
foreach(_VERSION ${ARROW_ABSL_REQUIRED_LTS_VERSIONS})
find_package(absl ${_VERSION})
if(absl_FOUND)
break()
endif()
endforeach()
# If we can't find Abseil LTS 20211102 or later, we use bundled
# Abseil.
if(NOT absl_FOUND)
set(absl_SOURCE "BUNDLED")
endif()
endif()
resolve_dependency(absl)
endif()
endmacro()

macro(build_absl)
message(STATUS "Building Abseil-cpp from source")
set(absl_FOUND TRUE)
Expand Down Expand Up @@ -3845,7 +3818,6 @@ macro(build_grpc)
TRUE
PC_PACKAGE_NAMES
libcares)
ensure_absl()

message(STATUS "Building gRPC from source")

Expand Down Expand Up @@ -4135,12 +4107,40 @@ macro(build_grpc)
endif()
endmacro()

if(ARROW_WITH_GOOGLE_CLOUD_CPP OR ARROW_WITH_GRPC)
set(ARROW_ABSL_REQUIRED_VERSION 20211102)
# Google Cloud C++ SDK and gRPC require Google Abseil
if(ARROW_WITH_GOOGLE_CLOUD_CPP)
set(ARROW_ABSL_CMAKE_PACKAGE_NAME Arrow)
set(ARROW_ABSL_PC_PACKAGE_NAME arrow)
else()
set(ARROW_ABSL_CMAKE_PACKAGE_NAME ArrowFlight)
set(ARROW_ABSL_PC_PACKAGE_NAME arrow-flight)
endif()
resolve_dependency(absl
ARROW_CMAKE_PACKAGE_NAME
${ARROW_ABSL_CMAKE_PACKAGE_NAME}
ARROW_PC_PACKAGE_NAME
${ARROW_ABSL_PC_PACKAGE_NAME}
HAVE_ALT
FALSE
FORCE_ANY_NEWER_VERSION
TRUE
REQUIRED_VERSION
${ARROW_ABSL_REQUIRED_VERSION})
endif()

if(ARROW_WITH_GRPC)
if(NOT ARROW_ENABLE_THREADING)
message(FATAL_ERROR "Can't use gRPC with ARROW_ENABLE_THREADING=OFF")
endif()

set(ARROW_GRPC_REQUIRED_VERSION "1.30.0")
if(absl_SOURCE STREQUAL "BUNDLED" AND NOT gRPC_SOURCE STREQUAL "BUNDLED")
# System gRPC can't be used with bundled Abseil
message(STATUS "Forcing gRPC_SOURCE to BUNDLED because absl_SOURCE is BUNDLED")
set(gRPC_SOURCE "BUNDLED")
endif()
if(NOT Protobuf_SOURCE STREQUAL gRPC_SOURCE)
# ARROW-15495: Protobuf/gRPC must come from the same source
message(STATUS "Forcing gRPC_SOURCE to Protobuf_SOURCE (${Protobuf_SOURCE})")
Expand Down Expand Up @@ -4259,7 +4259,6 @@ macro(build_google_cloud_cpp_storage)
message(STATUS "Only building the google-cloud-cpp::storage component")

# List of dependencies taken from https://github.com/googleapis/google-cloud-cpp/blob/main/doc/packaging.md
ensure_absl()
build_crc32c_once()

# Curl is required on all platforms, but building it internally might also trip over S3's copy.
Expand Down

0 comments on commit e65c1e2

Please sign in to comment.