You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by we...@apache.org on 2019/01/23 00:01:37 UTC
[arrow] branch master updated: ARROW-4230: [C++] Fix Flight builds
with gRPC/Protobuf/c-ares
This is an automated email from the ASF dual-hosted git repository.
wesm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/master by this push:
new 6df4c92 ARROW-4230: [C++] Fix Flight builds with gRPC/Protobuf/c-ares
6df4c92 is described below
commit 6df4c92209f03b564b7f1e1addd1791e555b566b
Author: David Li <Da...@twosigma.com>
AuthorDate: Tue Jan 22 18:01:29 2019 -0600
ARROW-4230: [C++] Fix Flight builds with gRPC/Protobuf/c-ares
- Non-vendored, non-CMake gRPC dependencies are accepted via `GRPC_HOME`
- Non-vendored, non-CMake c-ares dependencies are accepted via `CARES_HOME`
- Refactored gRPC dependency configuration into `FindgRPC.cmake`
```shell
# Build with all system dependencies
env GRPC_HOME=/usr/local/Cellar/grpc/1.17.2 CARES_HOME=/usr/local/Cellar/c-ares/1.15.0 PROTOBUF_HOME=/usr/local/Cellar/protobuf/3.6.1.3_1 cmake -DARROW_FLIGHT=ON
# Build with all vendored dependencies
cmake -DARROW_FLIGHT=ON
# Build with CMake gRPC installation
cmake -DgRPC_INSTALL=ON -DgRPC_BUILD_TESTS=OFF -DgRPC_PROTOBUF_PROVIDER=package -DgRPC_ZLIB_PROVIDER=package -DgRPC_CARES_PROVIDER=package -DgRPC_SSL_PROVIDER=package -DOPENSSL_ROOT_DIR=/usr/local/Cellar/openssl/1.0.2q && make -j
cd path/to/arrow
env GRPC_HOME=/usr/local/lib/cmake/grpc CARES_HOME=/usr/local/ cmake -DARROW_FLIGHT=ON -DgRPC_DIR=/usr/local/lib/cmake/grpc
```
Author: David Li <Da...@twosigma.com>
Author: Wes McKinney <we...@apache.org>
Author: David Li <li...@gmail.com>
Closes #3370 from lihalite/grpc-nocmake and squashes the following commits:
b77c3a44f <Wes McKinney> c-ares fixes
8bb701803 <David Li> Fix flight tests to use a valid port number
bdf0bc569 <David Li> Fix build of Flight tests
ea098c5e1 <David Li> Caution against building with system gRPC
ef9100a62 <David Li> Add Debian/Ubuntu instructions to build Flight with system libs
5756129aa <David Li> Sort environment variable entries in ThirdpartyToolchain.cmake
3f7a41cfd <David Li> Use environment variables for CARES_HOME
fe440ba0e <David Li> Explicitly link to static c-ares
942d84a25 <David Li> Fix reference to libgrpc++.a
589ad9de2 <David Li> Explain how to build Flight in README
38a219aae <David Li> Properly depend on Protobuf for Flight protos
1eed959c4 <David Li> Fix building with c-ares
e8c494d33 <David Li> Fix building with vendored/CMake gRPC
33e1282cc <David Li> Make sure to add gRPC include directory
7e8641887 <David Li> Fix name of FindgRPC.cmake to work on case-sensitive systems
0e618882c <David Li> Explicitly link Flight to static gRPC
ee85e8674 <David Li> Move gRPC build configuration to FindGrpc.cmake
d3074ac5f <David Li> Add option for non-vendored, non-CMake gRPC dependency
---
cpp/README.md | 49 ++++++++++++++
cpp/cmake_modules/FindgRPC.cmake | 101 ++++++++++++++++++++++++++++
cpp/cmake_modules/ThirdpartyToolchain.cmake | 43 +++++++++---
cpp/src/arrow/flight/CMakeLists.txt | 33 +++++----
cpp/src/arrow/flight/flight-test.cc | 6 +-
5 files changed, 202 insertions(+), 30 deletions(-)
diff --git a/cpp/README.md b/cpp/README.md
index 5700096..276dbf3 100644
--- a/cpp/README.md
+++ b/cpp/README.md
@@ -315,6 +315,55 @@ ctest -L gandiva
This library is still in Alpha stages, and subject to API changes without
deprecation warnings.
+### Building and developing Flight (optional)
+
+In addition to the Arrow dependencies, Flight requires:
+* gRPC (>= 1.14, roughly)
+* Protobuf (>= 3.6, earlier versions may work)
+* c-ares (used by gRPC)
+
+By default, Arrow will try to download and build these dependencies
+when building Flight.
+
+The optional `flight` libraries and tests can be built by passing
+`-DARROW_FLIGHT=ON`.
+
+```shell
+cmake .. -DARROW_FLIGHT=ON -DARROW_BUILD_TESTS=ON
+make
+```
+
+You can also use existing installations of the extra dependencies.
+When building, set the environment variables `GRPC_HOME` and/or
+`PROTOBUF_HOME` and/or `CARES_HOME`.
+
+You may try using system libraries for gRPC and Protobuf, but these
+are likely to be too old.
+
+On Ubuntu/Debian, you can try:
+
+```shell
+sudo apt-get install libgrpc-dev libgrpc++-dev protobuf-compiler-grpc libc-ares-dev
+```
+
+Note that the version of gRPC in Ubuntu 18.10 is too old; you will
+have to install gRPC from source. (Ubuntu 19.04/Debian Sid may work.)
+
+On macOS, you can try [Homebrew][1]:
+
+```shell
+brew install grpc
+```
+
+You can also install gRPC from source. In this case, you must install
+gRPC to generate the necessary files for CMake to find gRPC:
+
+```shell
+cmake -DgRPC_INSTALL=ON -DgRPC_BUILD_TESTS=OFF -DgRPC_PROTOBUF_PROVIDER=package -DgRPC_ZLIB_PROVIDER=package -DgRPC_CARES_PROVIDER=package -DgRPC_SSL_PROVIDER=package
+```
+
+You can then specify `-DgRPC_DIR` to `cmake`.
+
### API documentation
To generate the (html) API documentation, run the following command in the apidoc
diff --git a/cpp/cmake_modules/FindgRPC.cmake b/cpp/cmake_modules/FindgRPC.cmake
new file mode 100644
index 0000000..edf7286
--- /dev/null
+++ b/cpp/cmake_modules/FindgRPC.cmake
@@ -0,0 +1,101 @@
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+if( NOT "${GRPC_HOME}" STREQUAL "")
+ file (TO_CMAKE_PATH "${GRPC_HOME}" _grpc_path)
+endif()
+
+message (STATUS "GRPC_HOME: ${GRPC_HOME}")
+
+find_package(gRPC CONFIG)
+if (gRPC_FOUND)
+ message (STATUS "Found CMake installation of gRPC")
+ get_property(GRPC_INCLUDE_DIR TARGET gRPC::gpr PROPERTY INTERFACE_INCLUDE_DIRECTORIES)
+ get_property(GPR_STATIC_LIB TARGET gRPC::gpr PROPERTY LOCATION)
+ get_property(GRPC_STATIC_LIB TARGET gRPC::grpc_unsecure PROPERTY LOCATION)
+ get_property(GRPCPP_STATIC_LIB TARGET gRPC::grpc++_unsecure PROPERTY LOCATION)
+ get_property(GRPC_ADDRESS_SORTING_STATIC_LIB
+ TARGET gRPC::address_sorting PROPERTY LOCATION)
+ # Get location of grpc_cpp_plugin so we can pass it to protoc
+ get_property(GRPC_CPP_PLUGIN TARGET gRPC::grpc_cpp_plugin PROPERTY LOCATION)
+else()
+ find_path (GRPC_INCLUDE_DIR grpc/grpc.h HINTS
+ ${_grpc_path}
+ NO_DEFAULT_PATH
+ PATH_SUFFIXES "include")
+
+ set (lib_dirs "lib")
+ if (EXISTS "${_grpc_path}/lib64")
+ set (lib_dirs "lib64" ${lib_dirs})
+ endif ()
+ if (EXISTS "${_grpc_path}/lib/${CMAKE_LIBRARY_ARCHITECTURE}")
+ set (lib_dirs "lib/${CMAKE_LIBRARY_ARCHITECTURE}" ${lib_dirs})
+ endif ()
+
+ find_library (GPR_STATIC_LIB
+ NAMES "${CMAKE_STATIC_LIBRARY_PREFIX}gpr${CMAKE_STATIC_LIBRARY_SUFFIX}"
+ PATHS ${_grpc_path}
+ NO_DEFAULT_PATH
+ PATH_SUFFIXES ${lib_dirs})
+
+ # On Debian/Ubuntu, libaddress_sorting is statically linked.
+ find_library (GRPC_ADDRESS_SORTING_STATIC_LIB
+ NAMES "${CMAKE_STATIC_LIBRARY_PREFIX}address_sorting${CMAKE_STATIC_LIBRARY_SUFFIX}"
+ "${CMAKE_STATIC_LIBRARY_PREFIX}grpc++${CMAKE_STATIC_LIBRARY_SUFFIX}"
+ PATHS ${_grpc_path}
+ NO_DEFAULT_PATH
+ PATH_SUFFIXES ${lib_dirs})
+
+ find_library (GRPC_STATIC_LIB
+ NAMES "${CMAKE_STATIC_LIBRARY_PREFIX}grpc${CMAKE_STATIC_LIBRARY_SUFFIX}"
+ PATHS ${_grpc_path}
+ NO_DEFAULT_PATH
+ PATH_SUFFIXES ${lib_dirs})
+
+ find_library (GRPCPP_STATIC_LIB
+ NAMES "${CMAKE_STATIC_LIBRARY_PREFIX}grpc++${CMAKE_STATIC_LIBRARY_SUFFIX}"
+ PATHS ${_grpc_path}
+ NO_DEFAULT_PATH
+ PATH_SUFFIXES ${lib_dirs})
+
+ find_program(GRPC_CPP_PLUGIN grpc_cpp_plugin protoc-gen-grpc-cpp
+ HINTS ${_grpc_path}
+ NO_DEFAULT_PATH
+ PATH_SUFFIXES "bin")
+endif()
+
+if (GRPC_INCLUDE_DIR AND GPR_STATIC_LIB AND GRPC_ADDRESS_SORTING_STATIC_LIB AND
+ GRPC_STATIC_LIB AND GRPCPP_STATIC_LIB AND GRPC_CPP_PLUGIN)
+ set (gRPC_FOUND TRUE)
+else ()
+ set (gRPC_FOUND FALSE)
+endif ()
+
+if (gRPC_FOUND)
+ message (STATUS "Found the gRPC headers: ${GRPC_INCLUDE_DIR}")
+else()
+ if (_grpc_path)
+ set (GRPC_ERR_MSG "Could not find gRPC. Looked in ${_grpc_path}.")
+ else ()
+ set (GRPC_ERR_MSG "Could not find gRPC in system search paths.")
+ endif()
+
+ if (gRPC_FIND_REQUIRED)
+ message (FATAL_ERROR "${GRPC_ERR_MSG}")
+ else ()
+ message (STATUS "${GRPC_ERR_MSG}")
+ endif ()
+endif()
+
+mark_as_advanced (
+ GRPC_INCLUDE_DIR
+ )
diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake
index e2ed70c..1668c00 100644
--- a/cpp/cmake_modules/ThirdpartyToolchain.cmake
+++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake
@@ -31,6 +31,7 @@ set(THIRDPARTY_DIR "${arrow_SOURCE_DIR}/thirdparty")
if (NOT "$ENV{ARROW_BUILD_TOOLCHAIN}" STREQUAL "")
set(BROTLI_HOME "$ENV{ARROW_BUILD_TOOLCHAIN}")
set(BZ2_HOME "$ENV{ARROW_BUILD_TOOLCHAIN}")
+ set(CARES_HOME "$ENV{ARROW_BUILD_TOOLCHAIN}")
set(DOUBLE_CONVERSION_HOME "$ENV{ARROW_BUILD_TOOLCHAIN}")
set(FLATBUFFERS_HOME "$ENV{ARROW_BUILD_TOOLCHAIN}")
set(GFLAGS_HOME "$ENV{ARROW_BUILD_TOOLCHAIN}")
@@ -69,6 +70,10 @@ if (DEFINED ENV{BZ2_HOME})
set(BZ2_HOME "$ENV{BZ2_HOME}")
endif()
+if (DEFINED ENV{CARES_HOME})
+ set(CARES_HOME "$ENV{CARES_HOME}")
+endif()
+
if (DEFINED ENV{DOUBLE_CONVERSION_HOME})
set(DOUBLE_CONVERSION_HOME "$ENV{DOUBLE_CONVERSION_HOME}")
endif()
@@ -1230,7 +1235,9 @@ if (ARROW_FLIGHT)
set(GRPC_INCLUDE_DIR "${GRPC_PREFIX}/include")
set(GRPC_STATIC_LIBRARY_GPR "${GRPC_BUILD_DIR}/${CMAKE_CFG_INTDIR}/${CMAKE_STATIC_LIBRARY_PREFIX}gpr${CMAKE_STATIC_LIBRARY_SUFFIX}")
set(GRPC_STATIC_LIBRARY_GRPC "${GRPC_BUILD_DIR}/${CMAKE_CFG_INTDIR}/${CMAKE_STATIC_LIBRARY_PREFIX}grpc${CMAKE_STATIC_LIBRARY_SUFFIX}")
- set(GRPC_STATIC_LIBRARY_GRPCPP "${GRPC_BUILD_DIR}/${CMAKE_CFG_INTDIR}/${CMAKE_STATIC_LIBRARY_PREFIX}grpcpp${CMAKE_STATIC_LIBRARY_SUFFIX}")
+ set(GRPC_STATIC_LIBRARY_GRPCPP "${GRPC_BUILD_DIR}/${CMAKE_CFG_INTDIR}/${CMAKE_STATIC_LIBRARY_PREFIX}grpc++${CMAKE_STATIC_LIBRARY_SUFFIX}")
+ set(GRPC_STATIC_LIBRARY_ADDRESS_SORTING "${GRPC_BUILD_DIR}/${CMAKE_CFG_INTDIR}/${CMAKE_STATIC_LIBRARY_PREFIX}address_sorting${CMAKE_STATIC_LIBRARY_SUFFIX}")
+ set(GRPC_STATIC_LIBRARY_CARES "${GRPC_BUILD_DIR}/${CMAKE_CFG_INTDIR}/third_party/cares/cares/lib/${CMAKE_STATIC_LIBRARY_PREFIX}cares${CMAKE_STATIC_LIBRARY_SUFFIX}")
set(GRPC_CMAKE_ARGS -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}
"-DCMAKE_CXX_FLAGS=${EP_CXX_FLAGS}"
"-DCMAKE_C_FLAGS=${EP_C_FLAGS}"
@@ -1245,31 +1252,47 @@ if (ARROW_FLIGHT)
${EP_LOG_OPTIONS}
CMAKE_ARGS ${GRPC_CMAKE_ARGS}
${EP_LOG_OPTIONS})
- include_directories(SYSTEM ${GRPC_INCLUDE_DIR})
+
+ set(GPR_STATIC_LIB "${GRPC_STATIC_LIBRARY_GPR}")
+ set(GRPC_STATIC_LIB "${GRPC_STATIC_LIBRARY_GRPC}")
+ set(GRPCPP_STATIC_LIB "${GRPC_STATIC_LIBRARY_GRPCPP}")
+ set(GRPC_ADDRESS_SORTING_STATIC_LIB "${GRPC_STATIC_LIBRARY_ADDRESS_SORTING}")
+ # XXX(wesm): relying on vendored c-ares provided by gRPC for the time being
+ set(CARES_STATIC_LIB "${GRPC_STATIC_LIBRARY_CARES}")
+ set(GRPC_CPP_PLUGIN "${GRPC_BUILD_DIR}/${CMAKE_CFG_INTDIR}/grpc_cpp_plugin")
else()
- find_package(gRPC CONFIG REQUIRED)
+ find_package(gRPC REQUIRED)
set(GRPC_VENDORED 0)
endif()
- get_property(GPR_STATIC_LIB TARGET gRPC::gpr PROPERTY LOCATION)
+ # If we built gRPC ourselves, we should use its c-ares.
+ if ("${CARES_STATIC_LIB}" STREQUAL "")
+ if (NOT "${CARES_HOME}" STREQUAL "")
+ set(CARES_STATIC_LIB "${CARES_HOME}/lib/${CMAKE_STATIC_LIBRARY_PREFIX}cares_static${CMAKE_STATIC_LIBRARY_SUFFIX}")
+ elseif (c-ares_FOUND)
+ get_property(CARES_STATIC_LIB TARGET c-ares::cares_static PROPERTY LOCATION)
+ endif()
+ endif()
+ message(STATUS "Found the c-ares library: ${CARES_STATIC_LIB}")
+
+ if ("${GRPC_CPP_PLUGIN}" STREQUAL "")
+ message(SEND_ERROR "Please set GRPC_CPP_PLUGIN.")
+ endif()
+
+ include_directories(SYSTEM ${GRPC_INCLUDE_DIR})
+
ADD_THIRDPARTY_LIB(grpc_gpr
STATIC_LIB ${GPR_STATIC_LIB})
- get_property(GRPC_STATIC_LIB TARGET gRPC::grpc_unsecure PROPERTY LOCATION)
ADD_THIRDPARTY_LIB(grpc_grpc
STATIC_LIB ${GRPC_STATIC_LIB})
- get_property(GRPCPP_STATIC_LIB TARGET gRPC::grpc++_unsecure PROPERTY LOCATION)
ADD_THIRDPARTY_LIB(grpc_grpcpp
STATIC_LIB ${GRPCPP_STATIC_LIB})
- get_property(GRPC_ADDRESS_SORTING_STATIC_LIB
- TARGET gRPC::address_sorting PROPERTY LOCATION)
ADD_THIRDPARTY_LIB(grpc_address_sorting
STATIC_LIB ${GRPC_ADDRESS_SORTING_STATIC_LIB})
- # XXX(wesm): relying on vendored c-ares provided by gRPC for the time being
- get_property(CARES_STATIC_LIB TARGET c-ares::cares_static PROPERTY LOCATION)
ADD_THIRDPARTY_LIB(cares
STATIC_LIB ${CARES_STATIC_LIB})
endif()
diff --git a/cpp/src/arrow/flight/CMakeLists.txt b/cpp/src/arrow/flight/CMakeLists.txt
index db2092e..41d49a1 100644
--- a/cpp/src/arrow/flight/CMakeLists.txt
+++ b/cpp/src/arrow/flight/CMakeLists.txt
@@ -21,11 +21,18 @@ add_custom_target(arrow_flight)
ARROW_INSTALL_ALL_HEADERS("arrow/flight")
SET(ARROW_FLIGHT_STATIC_LINK_LIBS
- grpc_grpcpp
- grpc_grpc
- grpc_gpr
- grpc_address_sorting
- cares)
+ grpc_grpcpp_static
+ grpc_grpc_static
+ grpc_gpr_static
+ grpc_address_sorting_static
+ cares_static)
+
+SET(ARROW_FLIGHT_TEST_STATIC_LINK_LIBS
+ arrow_static
+ arrow_flight_static
+ arrow_testing_static
+ ${ARROW_FLIGHT_STATIC_LINK_LIBS}
+ ${PROTOBUF_LIBRARY})
# TODO(wesm): Protobuf shared vs static linking
@@ -38,14 +45,7 @@ set(FLIGHT_GENERATED_PROTO_FILES
"${CMAKE_CURRENT_BINARY_DIR}/Flight.grpc.pb.cc"
"${CMAKE_CURRENT_BINARY_DIR}/Flight.grpc.pb.h")
-if(PROTOBUF_VENDORED)
- set(PROTO_DEPENDS ${FLIGHT_PROTO} protobuf)
-else()
- set(PROTO_DEPENDS ${FLIGHT_PROTO})
-endif()
-
-# Get location of grpc_cpp_plugin so we can pass it to protoc
-get_property(GRPC_CPP_PLUGIN TARGET gRPC::grpc_cpp_plugin PROPERTY LOCATION)
+set(PROTO_DEPENDS ${FLIGHT_PROTO} ${PROTOBUF_LIBRARY})
add_custom_command(
OUTPUT ${FLIGHT_GENERATED_PROTO_FILES}
@@ -79,21 +79,20 @@ ADD_ARROW_LIB(arrow_flight
STATIC_LINK_LIBS arrow_static ${ARROW_FLIGHT_STATIC_LINK_LIBS})
ADD_ARROW_TEST(flight-test
- EXTRA_LINK_LIBS arrow_flight_static ${ARROW_FLIGHT_STATIC_LINK_LIBS}
+ EXTRA_LINK_LIBS ${ARROW_FLIGHT_TEST_STATIC_LINK_LIBS}
LABELS "arrow_flight")
# Build test server for unit tests or benchmarks
if (ARROW_BUILD_TESTS OR ARROW_BUILD_BENCHMARKS)
add_executable(flight-test-server test-server.cc)
target_link_libraries(flight-test-server
- arrow_flight_static
- ${ARROW_FLIGHT_STATIC_LINK_LIBS}
+ ${ARROW_FLIGHT_TEST_STATIC_LINK_LIBS}
gflags_static
gtest_static)
# This is needed for the unit tests
if (ARROW_BUILD_TESTS)
- add_dependencies(flight-test flight-test-server)
+ add_dependencies(arrow-flight-test flight-test-server)
endif()
endif()
diff --git a/cpp/src/arrow/flight/flight-test.cc b/cpp/src/arrow/flight/flight-test.cc
index 2d1b2f8..0389c76 100644
--- a/cpp/src/arrow/flight/flight-test.cc
+++ b/cpp/src/arrow/flight/flight-test.cc
@@ -53,11 +53,11 @@ namespace arrow {
namespace flight {
TEST(TestFlight, StartStopTestServer) {
- TestServer server("flight-test-server", 92385);
+ TestServer server("flight-test-server", 30000);
server.Start();
ASSERT_TRUE(server.IsRunning());
- sleep_for(0.2);
+ std::this_thread::sleep_for(std::chrono::duration<double>(0.2));
ASSERT_TRUE(server.IsRunning());
int exit_code = server.Stop();
@@ -79,7 +79,7 @@ class TestFlightClient : public ::testing::Test {
// void TearDown() {}
void SetUp() {
- port_ = 92358;
+ port_ = 30000;
server_.reset(new TestServer("flight-test-server", port_));
server_->Start();
ASSERT_OK(ConnectClient());