Skip to content

Commit

Permalink
ARROW-17051: [C++] Link Flight/gRPC/Protobuf consistently (#13599)
Browse files Browse the repository at this point in the history
If Protobuf/gRPC are used statically, Flight must be as well, or else we can get odd runtime behavior due to the global state in those libraries when Flight SQL is involved (as Flight SQL would then bundle a second copy of Protobuf into its shared library).

Authored-by: David Li <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
  • Loading branch information
lidavidm authored Jul 26, 2022
1 parent 87cefe8 commit bbf249e
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 28 deletions.
2 changes: 0 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ jobs:
# aws-sdk-cpp.
DOCKER_RUN_ARGS: >-
"
-e ARROW_BUILD_STATIC=OFF
-e ARROW_FLIGHT=ON
-e ARROW_GCS=OFF
-e ARROW_MIMALLOC=OFF
Expand Down Expand Up @@ -145,7 +144,6 @@ jobs:
# aws-sdk-cpp.
DOCKER_RUN_ARGS: >-
"
-e ARROW_BUILD_STATIC=OFF
-e ARROW_FLIGHT=ON
-e ARROW_GCS=OFF
-e ARROW_MIMALLOC=OFF
Expand Down
5 changes: 4 additions & 1 deletion ci/docker/ubuntu-18.04-cpp.dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,10 @@ RUN apt-get update -y -q && \
# - thrift is too old
# - utf8proc is too old(v2.1.0)
# - s3 tests would require boost-asio that is included since Boost 1.66.0
ENV ARROW_BUILD_TESTS=ON \
# ARROW-17051: this build uses static Protobuf, so we must also use
# static Arrow to run Flight/Flight SQL tests
ENV ARROW_BUILD_STATIC=ON \
ARROW_BUILD_TESTS=ON \
ARROW_DATASET=ON \
ARROW_DEPENDENCY_SOURCE=SYSTEM \
ARROW_FLIGHT=OFF \
Expand Down
5 changes: 4 additions & 1 deletion ci/docker/ubuntu-20.04-cpp.dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,10 @@ RUN /arrow/ci/scripts/install_ceph.sh
# - flatbuffer is not packaged
# - libgtest-dev only provide sources
# - libprotobuf-dev only provide sources
ENV ARROW_BUILD_TESTS=ON \
# ARROW-17051: this build uses static Protobuf, so we must also use
# static Arrow to run Flight/Flight SQL tests
ENV ARROW_BUILD_STATIC=ON \
ARROW_BUILD_TESTS=ON \
ARROW_DEPENDENCY_SOURCE=SYSTEM \
ARROW_DATASET=ON \
ARROW_FLIGHT=OFF \
Expand Down
5 changes: 4 additions & 1 deletion ci/docker/ubuntu-22.04-cpp.dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,10 @@ RUN /arrow/ci/scripts/install_gcs_testbench.sh default
# - flatbuffer is not packaged
# - libgtest-dev only provide sources
# - libprotobuf-dev only provide sources
ENV ARROW_BUILD_TESTS=ON \
# ARROW-17051: this build uses static Protobuf, so we must also use
# static Arrow to run Flight/Flight SQL tests
ENV ARROW_BUILD_STATIC=ON \
ARROW_BUILD_TESTS=ON \
ARROW_DEPENDENCY_SOURCE=SYSTEM \
ARROW_DATASET=ON \
ARROW_FLIGHT=ON \
Expand Down
2 changes: 1 addition & 1 deletion cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -862,7 +862,7 @@ add_dependencies(arrow_test_dependencies toolchain-tests)

if(ARROW_STATIC_LINK_LIBS)
add_dependencies(arrow_dependencies ${ARROW_STATIC_LINK_LIBS})
if(ARROW_ORC)
if(ARROW_HDFS OR ARROW_ORC)
if(NOT MSVC_TOOLCHAIN)
list(APPEND ARROW_STATIC_LINK_LIBS ${CMAKE_DL_LIBS})
list(APPEND ARROW_STATIC_INSTALL_INTERFACE_LIBS ${CMAKE_DL_LIBS})
Expand Down
42 changes: 21 additions & 21 deletions cpp/src/arrow/flight/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -36,28 +36,28 @@ if(NOT ARROW_GRPC_USE_SHARED)
endif()

set(ARROW_FLIGHT_TEST_INTERFACE_LIBS)
if(ARROW_FLIGHT_TEST_LINKAGE STREQUAL "static")
if(ARROW_BUILD_STATIC)
set(ARROW_FLIGHT_TEST_LINK_LIBS arrow_flight_static)
else()
set(ARROW_FLIGHT_TEST_LINK_LIBS arrow_flight_shared)
endif()
if(ARROW_FLIGHT_TESTING_BUILD_STATIC)
list(APPEND ARROW_FLIGHT_TEST_LINK_LIBS arrow_flight_testing_static)
if(ARROW_BUILD_INTEGRATION OR ARROW_BUILD_TESTS)
if(ARROW_FLIGHT_TEST_LINKAGE STREQUAL "static")
if(NOT ARROW_BUILD_STATIC)
message(STATUS "If static Protobuf or gRPC are used, Arrow must be built statically"
)
message(STATUS "(These libraries have global state, and linkage must be consistent)"
)
message(FATAL_ERROR "Must build Arrow statically to link Flight tests statically")
endif()
set(ARROW_FLIGHT_TEST_LINK_LIBS arrow_flight_static arrow_flight_testing_static)
list(APPEND ARROW_FLIGHT_TEST_LINK_LIBS ${ARROW_TEST_STATIC_LINK_LIBS})
if(ARROW_CUDA)
list(APPEND ARROW_FLIGHT_TEST_INTERFACE_LIBS arrow_cuda_static)
list(APPEND ARROW_FLIGHT_TEST_LINK_LIBS arrow_cuda_static)
endif()
else()
list(APPEND ARROW_FLIGHT_TEST_LINK_LIBS arrow_flight_testing_shared)
endif()
list(APPEND ARROW_FLIGHT_TEST_LINK_LIBS ${ARROW_TEST_LINK_LIBS})
if(ARROW_CUDA)
list(APPEND ARROW_FLIGHT_TEST_INTERFACE_LIBS arrow_cuda_static)
list(APPEND ARROW_FLIGHT_TEST_LINK_LIBS arrow_cuda_static)
endif()
else()
set(ARROW_FLIGHT_TEST_LINK_LIBS arrow_flight_shared arrow_flight_testing_shared
${ARROW_TEST_LINK_LIBS})
if(ARROW_CUDA)
list(APPEND ARROW_FLIGHT_TEST_INTERFACE_LIBS arrow_cuda_shared)
list(APPEND ARROW_FLIGHT_TEST_LINK_LIBS arrow_cuda_shared)
set(ARROW_FLIGHT_TEST_LINK_LIBS arrow_flight_shared arrow_flight_testing_shared
${ARROW_TEST_SHARED_LINK_LIBS})
if(ARROW_CUDA)
list(APPEND ARROW_FLIGHT_TEST_INTERFACE_LIBS arrow_cuda_shared)
list(APPEND ARROW_FLIGHT_TEST_LINK_LIBS arrow_cuda_shared)
endif()
endif()
endif()
list(APPEND
Expand Down
1 change: 0 additions & 1 deletion docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,6 @@ services:
<<: *ccache
CC: clang-${CLANG_TOOLS}
CXX: clang++-${CLANG_TOOLS}
ARROW_BUILD_STATIC: "OFF"
ARROW_ENABLE_TIMING_TESTS: # inherit
ARROW_FUZZING: "ON" # Check fuzz regressions
ARROW_JEMALLOC: "OFF"
Expand Down

0 comments on commit bbf249e

Please sign in to comment.