This is an automated email from the ASF dual-hosted git repository.
kou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new 2a2c253c57 GH-36479: [C++][FlightRPC] Use gRPC version detected by
find_package() (#36581)
2a2c253c57 is described below
commit 2a2c253c57b4299a98ee3c7ad84b1d9479e39b8b
Author: Sutou Kouhei <[email protected]>
AuthorDate: Mon Jul 10 11:18:39 2023 +0900
GH-36479: [C++][FlightRPC] Use gRPC version detected by find_package()
(#36581)
### Rationale for this change
We don't need to use `try_compile()` by using gRPC version detected by
`find_package()`.
### What changes are included in this PR?
Use gRPC version detected by `find_package()`.
### Are these changes tested?
Yes.
### Are there any user-facing changes?
No.
* Closes: #36479
Authored-by: Sutou Kouhei <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
---
ci/scripts/cpp_build.sh | 1 -
cpp/cmake_modules/FindgRPCAlt.cmake | 5 +-
cpp/cmake_modules/ThirdpartyToolchain.cmake | 9 ++-
cpp/src/arrow/flight/CMakeLists.txt | 83 +++-------------------
.../arrow/flight/try_compile/check_tls_opts_127.cc | 36 ----------
.../arrow/flight/try_compile/check_tls_opts_132.cc | 36 ----------
.../arrow/flight/try_compile/check_tls_opts_134.cc | 44 ------------
.../arrow/flight/try_compile/check_tls_opts_136.cc | 38 ----------
.../arrow/flight/try_compile/check_tls_opts_143.cc | 37 ----------
9 files changed, 22 insertions(+), 267 deletions(-)
diff --git a/ci/scripts/cpp_build.sh b/ci/scripts/cpp_build.sh
index 9854c5ff16..91a570be97 100755
--- a/ci/scripts/cpp_build.sh
+++ b/ci/scripts/cpp_build.sh
@@ -109,7 +109,6 @@ cmake \
-DARROW_EXTRA_ERROR_CONTEXT=${ARROW_EXTRA_ERROR_CONTEXT:-OFF} \
-DARROW_FILESYSTEM=${ARROW_FILESYSTEM:-ON} \
-DARROW_FLIGHT=${ARROW_FLIGHT:-OFF} \
-
-DARROW_FLIGHT_REQUIRE_TLSCREDENTIALSOPTIONS=${ARROW_FLIGHT_REQUIRE_TLSCREDENTIALSOPTIONS:-OFF}
\
-DARROW_FLIGHT_SQL=${ARROW_FLIGHT_SQL:-OFF} \
-DARROW_FUZZING=${ARROW_FUZZING:-OFF} \
-DARROW_GANDIVA_PC_CXX_FLAGS=${ARROW_GANDIVA_PC_CXX_FLAGS:-} \
diff --git a/cpp/cmake_modules/FindgRPCAlt.cmake
b/cpp/cmake_modules/FindgRPCAlt.cmake
index 4e38605235..c5d8c8a812 100644
--- a/cpp/cmake_modules/FindgRPCAlt.cmake
+++ b/cpp/cmake_modules/FindgRPCAlt.cmake
@@ -33,8 +33,10 @@ pkg_check_modules(GRPCPP_PC grpc++)
if(GRPCPP_PC_FOUND)
set(gRPCAlt_VERSION "${GRPCPP_PC_VERSION}")
set(GRPCPP_INCLUDE_DIRECTORIES ${GRPCPP_PC_INCLUDEDIR})
+ # gRPC's pkg-config file neglects to specify pthreads.
+ find_package(Threads REQUIRED)
if(ARROW_GRPC_USE_SHARED)
- set(GRPCPP_LINK_LIBRARIES ${GRPCPP_PC_LINK_LIBRARIES})
+ set(GRPCPP_LINK_LIBRARIES ${GRPCPP_PC_LINK_LIBRARIES} Threads::Threads)
set(GRPCPP_LINK_OPTIONS ${GRPCPP_PC_LDFLAGS_OTHER})
set(GRPCPP_COMPILE_OPTIONS ${GRPCPP_PC_CFLAGS_OTHER})
else()
@@ -45,6 +47,7 @@ if(GRPCPP_PC_FOUND)
HINTS ${GRPCPP_PC_STATIC_LIBRARY_DIRS})
list(APPEND GRPCPP_LINK_LIBRARIES
"${GRPCPP_LIBRARY_${GRPCPP_LIBRARY_NAME}}")
endforeach()
+ list(APPEND GRPCPP_LINK_LIBRARIES Threads::Threads)
set(GRPCPP_LINK_OPTIONS ${GRPCPP_PC_STATIC_LDFLAGS_OTHER})
set(GRPCPP_COMPILE_OPTIONS ${GRPCPP_PC_STATIC_CFLAGS_OTHER})
endif()
diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake
b/cpp/cmake_modules/ThirdpartyToolchain.cmake
index 4a19e226f7..c9ad4f665b 100644
--- a/cpp/cmake_modules/ThirdpartyToolchain.cmake
+++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake
@@ -3971,7 +3971,7 @@ macro(build_grpc)
endmacro()
if(ARROW_WITH_GRPC)
- set(ARROW_GRPC_REQUIRED_VERSION "1.17.0")
+ set(ARROW_GRPC_REQUIRED_VERSION "1.30.0")
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})")
@@ -3986,10 +3986,17 @@ if(ARROW_WITH_GRPC)
grpc++)
if(GRPC_VENDORED)
+ # Remove "v" from "vX.Y.Z"
+ string(SUBSTRING ${ARROW_GRPC_BUILD_VERSION} 1 -1 ARROW_GRPC_VERSION)
set(GRPCPP_PP_INCLUDE TRUE)
# Examples need to link to static Arrow if we're using static gRPC
set(ARROW_GRPC_USE_SHARED OFF)
else()
+ if(gRPCAlt_VERSION)
+ set(ARROW_GRPC_VERSION ${gRPCAlt_VERSION})
+ else()
+ set(ARROW_GRPC_VERSION ${gRPC_VERSION})
+ endif()
# grpc++ headers may reside in ${GRPC_INCLUDE_DIR}/grpc++ or
${GRPC_INCLUDE_DIR}/grpcpp
# depending on the gRPC version.
get_target_property(GRPC_INCLUDE_DIR gRPC::grpc++
INTERFACE_INCLUDE_DIRECTORIES)
diff --git a/cpp/src/arrow/flight/CMakeLists.txt
b/cpp/src/arrow/flight/CMakeLists.txt
index 917c0c3321..7383a7eec9 100644
--- a/cpp/src/arrow/flight/CMakeLists.txt
+++ b/cpp/src/arrow/flight/CMakeLists.txt
@@ -102,84 +102,21 @@ set(CMAKE_CXX_FLAGS_BACKUP "${CMAKE_CXX_FLAGS}")
string(REPLACE "/WX" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
string(REPLACE "-Werror " " " CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
-# Probe the version of gRPC being used to see if it supports disabling server
-# verification when using TLS.
-# gRPC's pkg-config file neglects to specify pthreads.
-find_package(Threads REQUIRED)
-function(test_grpc_version DST_VAR DETECT_VERSION TEST_FILE)
- if(NOT DEFINED ${DST_VAR})
- message(STATUS "Checking support for TlsCredentialsOptions (gRPC >=
${DETECT_VERSION})..."
- )
- get_property(CURRENT_INCLUDE_DIRECTORIES
- DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
- PROPERTY INCLUDE_DIRECTORIES)
- # ARROW-13881: when detecting support, avoid mismatch between
- # debug flags of gRPC and our probe (which results in LNK2038)
- set(CMAKE_TRY_COMPILE_CONFIGURATION ${CMAKE_BUILD_TYPE})
- try_compile(HAS_GRPC_VERSION ${CMAKE_CURRENT_BINARY_DIR}/try_compile
- SOURCES "${CMAKE_CURRENT_SOURCE_DIR}/try_compile/${TEST_FILE}"
- CMAKE_FLAGS
"-DINCLUDE_DIRECTORIES=${CURRENT_INCLUDE_DIRECTORIES}"
- LINK_LIBRARIES gRPC::grpc++ Threads::Threads
- OUTPUT_VARIABLE TLS_CREDENTIALS_OPTIONS_CHECK_OUTPUT
CXX_STANDARD 11)
- if(HAS_GRPC_VERSION)
- set(${DST_VAR}
- "${DETECT_VERSION}"
- CACHE INTERNAL "The detected (approximate) gRPC version.")
- else()
- message(STATUS "TlsCredentialsOptions (for gRPC ${DETECT_VERSION}) not
found in grpc::experimental."
- )
- if(ARROW_FLIGHT_REQUIRE_TLSCREDENTIALSOPTIONS)
- message(WARNING "Build output:")
- list(APPEND CMAKE_MESSAGE_INDENT "${TEST_FILE}: ")
- message(WARNING ${TLS_CREDENTIALS_OPTIONS_CHECK_OUTPUT})
- list(REMOVE_AT CMAKE_MESSAGE_INDENT -1)
- else()
- message(DEBUG "Build output:")
- list(APPEND CMAKE_MESSAGE_INDENT "${TEST_FILE}: ")
- message(DEBUG ${TLS_CREDENTIALS_OPTIONS_CHECK_OUTPUT})
- list(REMOVE_AT CMAKE_MESSAGE_INDENT -1)
- endif()
- endif()
- endif()
-endfunction()
-
-if(GRPC_VENDORED)
- # v1.35.0 -> 1.35
- string(REGEX MATCH "[0-9]+\\.[0-9]+" GRPC_VERSION
"${ARROW_GRPC_BUILD_VERSION}")
-else()
- test_grpc_version(GRPC_VERSION "1.43" "check_tls_opts_143.cc")
- test_grpc_version(GRPC_VERSION "1.36" "check_tls_opts_136.cc")
- test_grpc_version(GRPC_VERSION "1.34" "check_tls_opts_134.cc")
- test_grpc_version(GRPC_VERSION "1.32" "check_tls_opts_132.cc")
- test_grpc_version(GRPC_VERSION "1.27" "check_tls_opts_127.cc")
- message(STATUS "Found approximate gRPC version: ${GRPC_VERSION}
(ARROW_FLIGHT_REQUIRE_TLSCREDENTIALSOPTIONS=${ARROW_FLIGHT_REQUIRE_TLSCREDENTIALSOPTIONS})"
- )
-endif()
-if(GRPC_VERSION EQUAL "1.27")
-
add_definitions(-DGRPC_NAMESPACE_FOR_TLS_CREDENTIALS_OPTIONS=grpc_impl::experimental)
-elseif(GRPC_VERSION EQUAL "1.32")
-
add_definitions(-DGRPC_NAMESPACE_FOR_TLS_CREDENTIALS_OPTIONS=grpc::experimental)
-elseif(GRPC_VERSION EQUAL "1.34" OR GRPC_VERSION EQUAL "1.35")
+if(ARROW_GRPC_VERSION VERSION_GREATER_EQUAL "1.43")
add_definitions(-DGRPC_USE_TLS_CHANNEL_CREDENTIALS_OPTIONS
- -DGRPC_USE_TLS_CHANNEL_CREDENTIALS_OPTIONS_ROOT_CERTS
-
-DGRPC_NAMESPACE_FOR_TLS_CREDENTIALS_OPTIONS=grpc::experimental)
-elseif(GRPC_VERSION EQUAL "1.36")
+
-DGRPC_NAMESPACE_FOR_TLS_CREDENTIALS_OPTIONS=grpc::experimental
+ -DGRPC_USE_CERTIFICATE_VERIFIER)
+elseif(ARROW_GRPC_VERSION VERSION_GREATER_EQUAL "1.36")
add_definitions(-DGRPC_USE_TLS_CHANNEL_CREDENTIALS_OPTIONS
-DGRPC_NAMESPACE_FOR_TLS_CREDENTIALS_OPTIONS=grpc::experimental)
-elseif((GRPC_VERSION EQUAL "1.43") OR (GRPC_VERSION EQUAL "1.46"))
- # 1.46 is the bundled version
+elseif(ARROW_GRPC_VERSION VERSION_GREATER_EQUAL "1.34")
add_definitions(-DGRPC_USE_TLS_CHANNEL_CREDENTIALS_OPTIONS
-
-DGRPC_NAMESPACE_FOR_TLS_CREDENTIALS_OPTIONS=grpc::experimental
- -DGRPC_USE_CERTIFICATE_VERIFIER)
+ -DGRPC_USE_TLS_CHANNEL_CREDENTIALS_OPTIONS_ROOT_CERTS
+
-DGRPC_NAMESPACE_FOR_TLS_CREDENTIALS_OPTIONS=grpc::experimental)
+elseif(ARROW_GRPC_VERSION VERSION_GREATER_EQUAL "1.32")
+
add_definitions(-DGRPC_NAMESPACE_FOR_TLS_CREDENTIALS_OPTIONS=grpc::experimental)
else()
- message(STATUS "A proper version of gRPC could not be found to support
TlsCredentialsOptions in Arrow Flight."
- )
- message(STATUS "You may need a newer version of gRPC (>= 1.27), or the gRPC
API has changed and Flight must be updated to match."
- )
- if(ARROW_FLIGHT_REQUIRE_TLSCREDENTIALSOPTIONS)
- message(FATAL_ERROR "Halting build since
ARROW_FLIGHT_REQUIRE_TLSCREDENTIALSOPTIONS is set."
- )
- endif()
+
add_definitions(-DGRPC_NAMESPACE_FOR_TLS_CREDENTIALS_OPTIONS=grpc_impl::experimental)
endif()
# </KLUDGE> Restore the CXXFLAGS that were modified above
diff --git a/cpp/src/arrow/flight/try_compile/check_tls_opts_127.cc
b/cpp/src/arrow/flight/try_compile/check_tls_opts_127.cc
deleted file mode 100644
index 11de498991..0000000000
--- a/cpp/src/arrow/flight/try_compile/check_tls_opts_127.cc
+++ /dev/null
@@ -1,36 +0,0 @@
-// Licensed to the Apache Software Foundation (ASF) under one
-// or more contributor license agreements. See the NOTICE file
-// distributed with this work for additional information
-// regarding copyright ownership. The ASF licenses this file
-// to you 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.
-
-// Dummy file for checking if TlsCredentialsOptions exists in
-// the grpc_impl::experimental namespace. gRPC versions 1.27-1.31
-// put it here. This is for supporting disabling server
-// validation when using TLS.
-
-#include <grpc/grpc_security_constants.h>
-#include <grpcpp/grpcpp.h>
-#include <grpcpp/security/tls_credentials_options.h>
-
-static grpc_tls_server_verification_option check(
- const grpc_impl::experimental::TlsCredentialsOptions* options) {
- grpc_tls_server_verification_option server_opt =
options->server_verification_option();
- return server_opt;
-}
-
-int main(int argc, const char** argv) {
- [[maybe_unused]] grpc_tls_server_verification_option opt = check(nullptr);
- return 0;
-}
diff --git a/cpp/src/arrow/flight/try_compile/check_tls_opts_132.cc
b/cpp/src/arrow/flight/try_compile/check_tls_opts_132.cc
deleted file mode 100644
index fa5ba0f43d..0000000000
--- a/cpp/src/arrow/flight/try_compile/check_tls_opts_132.cc
+++ /dev/null
@@ -1,36 +0,0 @@
-// Licensed to the Apache Software Foundation (ASF) under one
-// or more contributor license agreements. See the NOTICE file
-// distributed with this work for additional information
-// regarding copyright ownership. The ASF licenses this file
-// to you 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.
-
-// Dummy file for checking if TlsCredentialsOptions exists in
-// the grpc::experimental namespace. gRPC versions 1.32 and higher
-// put it here. This is for supporting disabling server
-// validation when using TLS.
-
-#include <grpc/grpc_security_constants.h>
-#include <grpcpp/grpcpp.h>
-#include <grpcpp/security/tls_credentials_options.h>
-
-static grpc_tls_server_verification_option check(
- const grpc::experimental::TlsCredentialsOptions* options) {
- grpc_tls_server_verification_option server_opt =
options->server_verification_option();
- return server_opt;
-}
-
-int main(int argc, const char** argv) {
- [[maybe_unused]] grpc_tls_server_verification_option opt = check(nullptr);
- return 0;
-}
diff --git a/cpp/src/arrow/flight/try_compile/check_tls_opts_134.cc
b/cpp/src/arrow/flight/try_compile/check_tls_opts_134.cc
deleted file mode 100644
index 4ee2122ef5..0000000000
--- a/cpp/src/arrow/flight/try_compile/check_tls_opts_134.cc
+++ /dev/null
@@ -1,44 +0,0 @@
-// Licensed to the Apache Software Foundation (ASF) under one
-// or more contributor license agreements. See the NOTICE file
-// distributed with this work for additional information
-// regarding copyright ownership. The ASF licenses this file
-// to you 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.
-
-// Dummy file for checking if TlsCredentialsOptions exists in
-// the grpc::experimental namespace. gRPC starting from 1.34
-// put it here. This is for supporting disabling server
-// validation when using TLS.
-
-#include <grpc/grpc_security_constants.h>
-#include <grpcpp/grpcpp.h>
-#include <grpcpp/security/tls_credentials_options.h>
-
-// Dummy file for checking if TlsCredentialsOptions exists in
-// the grpc::experimental namespace. gRPC starting from 1.34
-// puts it here. This is for supporting disabling server
-// validation when using TLS.
-
-static void check() {
- // In 1.34, there's no parameterless constructor; in 1.36, there's
- // only a parameterless constructor
- auto options =
-
std::make_shared<grpc::experimental::TlsChannelCredentialsOptions>(nullptr);
- options->set_server_verification_option(
- grpc_tls_server_verification_option::GRPC_TLS_SERVER_VERIFICATION);
-}
-
-int main(int argc, const char** argv) {
- check();
- return 0;
-}
diff --git a/cpp/src/arrow/flight/try_compile/check_tls_opts_136.cc
b/cpp/src/arrow/flight/try_compile/check_tls_opts_136.cc
deleted file mode 100644
index 638eec67ba..0000000000
--- a/cpp/src/arrow/flight/try_compile/check_tls_opts_136.cc
+++ /dev/null
@@ -1,38 +0,0 @@
-// Licensed to the Apache Software Foundation (ASF) under one
-// or more contributor license agreements. See the NOTICE file
-// distributed with this work for additional information
-// regarding copyright ownership. The ASF licenses this file
-// to you 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.
-
-// Dummy file for checking if TlsCredentialsOptions exists in
-// the grpc::experimental namespace. gRPC starting from 1.36
-// puts it here. This is for supporting disabling server
-// validation when using TLS.
-
-#include <grpc/grpc_security_constants.h>
-#include <grpcpp/grpcpp.h>
-#include <grpcpp/security/tls_credentials_options.h>
-
-static void check() {
- // In 1.34, there's no parameterless constructor; in 1.36, there's
- // only a parameterless constructor
- auto options =
std::make_shared<grpc::experimental::TlsChannelCredentialsOptions>();
- options->set_server_verification_option(
- grpc_tls_server_verification_option::GRPC_TLS_SERVER_VERIFICATION);
-}
-
-int main(int argc, const char** argv) {
- check();
- return 0;
-}
diff --git a/cpp/src/arrow/flight/try_compile/check_tls_opts_143.cc
b/cpp/src/arrow/flight/try_compile/check_tls_opts_143.cc
deleted file mode 100644
index 2fdaac9d6e..0000000000
--- a/cpp/src/arrow/flight/try_compile/check_tls_opts_143.cc
+++ /dev/null
@@ -1,37 +0,0 @@
-// Licensed to the Apache Software Foundation (ASF) under one
-// or more contributor license agreements. See the NOTICE file
-// distributed with this work for additional information
-// regarding copyright ownership. The ASF licenses this file
-// to you 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.
-
-// Dummy file for checking if TlsCredentialsOptions supports
-// set_verify_server_certs. gRPC starting from 1.43 added this boolean
-// flag as opposed to prior versions which used an enum. This is for
-// supporting disabling server validation when using TLS.
-
-#include <grpc/grpc_security_constants.h>
-#include <grpcpp/grpcpp.h>
-#include <grpcpp/security/tls_credentials_options.h>
-
-static void check() {
- // 1.36 uses an enum; 1.43 uses booleans
- auto options =
std::make_shared<grpc::experimental::TlsChannelCredentialsOptions>();
- options->set_check_call_host(false);
- options->set_verify_server_certs(false);
-}
-
-int main(int argc, const char** argv) {
- check();
- return 0;
-}
