ARROW-17104: [C++] Workaround gRPC ABI issues

gRPC's ABI unfortunately depends on whether NDEBUG is enabled or not. When installing gRPC using a package manager (Homebrew, conda...), most often it is built with NDEBUG enabled. Some hackery is then needed to avoid missing symbols on Arrow debug builds... Should hopefully also fix ARROW-16520.
apache · Jul 18, 2022 · a6a445c · a6a445c
1 parent 4db3222
commit a6a445c
Show file tree

Hide file tree

Showing 18 changed files with 106 additions and 3 deletions.
diff --git a/cpp/cmake_modules/DefineOptions.cmake b/cpp/cmake_modules/DefineOptions.cmake
@@ -234,8 +234,6 @@ if(ARROW_DEFINE_OPTIONS)
 
   define_option(ARROW_DATASET "Build the Arrow Dataset Modules" OFF)
 
-  define_option(ARROW_SUBSTRAIT "Build the Arrow Substrait Consumer Module" OFF)
-
   define_option(ARROW_FILESYSTEM "Build the Arrow Filesystem Layer" OFF)
 
   define_option(ARROW_FLIGHT
@@ -284,6 +282,8 @@ if(ARROW_DEFINE_OPTIONS)
 
   define_option(ARROW_SKYHOOK "Build the Skyhook libraries" OFF)
 
+  define_option(ARROW_SUBSTRAIT "Build the Arrow Substrait Consumer Module" OFF)
+
   define_option(ARROW_TENSORFLOW "Build Arrow with TensorFlow support enabled" OFF)
 
   define_option(ARROW_TESTING "Build the Arrow testing libraries" OFF)

diff --git a/cpp/src/arrow/engine/substrait/expression_internal.h b/cpp/src/arrow/engine/substrait/expression_internal.h
@@ -26,6 +26,8 @@
 #include "arrow/engine/substrait/visibility.h"
 #include "arrow/type_fwd.h"
 
+#include "arrow/util/protobuf_abi_fixup_internal.h"  // before any PB headers
+
 #include "substrait/algebra.pb.h"  // IWYU pragma: export
 
 namespace arrow {

diff --git a/cpp/src/arrow/engine/substrait/plan_internal.h b/cpp/src/arrow/engine/substrait/plan_internal.h
@@ -23,6 +23,8 @@
 #include "arrow/engine/substrait/visibility.h"
 #include "arrow/type_fwd.h"
 
+#include "arrow/util/protobuf_abi_fixup_internal.h"  // before any PB headers
+
 #include "substrait/plan.pb.h"  // IWYU pragma: export
 
 namespace arrow {

diff --git a/cpp/src/arrow/engine/substrait/relation_internal.h b/cpp/src/arrow/engine/substrait/relation_internal.h
@@ -25,6 +25,8 @@
 #include "arrow/engine/substrait/visibility.h"
 #include "arrow/type_fwd.h"
 
+#include "arrow/util/protobuf_abi_fixup_internal.h"  // before any PB headers
+
 #include "substrait/algebra.pb.h"  // IWYU pragma: export
 
 namespace arrow {

diff --git a/cpp/src/arrow/engine/substrait/serde.cc b/cpp/src/arrow/engine/substrait/serde.cc
@@ -23,6 +23,8 @@
 #include "arrow/engine/substrait/type_internal.h"
 #include "arrow/util/string_view.h"
 
+#include "arrow/util/protobuf_abi_fixup_internal.h"  // before any PB headers
+
 #include <google/protobuf/descriptor.h>
 #include <google/protobuf/io/zero_copy_stream_impl_lite.h>
 #include <google/protobuf/message.h>
@@ -347,6 +349,8 @@ Result<std::string> SubstraitToJSON(util::string_view type_name, const Buffer& b
   return out;
 }
 
+DEFINE_ABI_FIXUP_FOR_PROTOBUF()
+
 }  // namespace internal
 }  // namespace engine
 }  // namespace arrow
diff --git a/cpp/src/arrow/engine/substrait/type_internal.h b/cpp/src/arrow/engine/substrait/type_internal.h
@@ -25,6 +25,8 @@
 #include "arrow/engine/substrait/visibility.h"
 #include "arrow/type_fwd.h"
 
+#include "arrow/util/protobuf_abi_fixup_internal.h"  // before any PB headers
+
 #include "substrait/type.pb.h"  // IWYU pragma: export
 
 namespace arrow {

diff --git a/cpp/src/arrow/flight/flight_internals_test.cc b/cpp/src/arrow/flight/flight_internals_test.cc
@@ -21,6 +21,8 @@
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
+#include "arrow/util/protobuf_abi_fixup_internal.h"  // before any PB headers
+
 #include "arrow/flight/client_cookie_middleware.h"
 #include "arrow/flight/client_middleware.h"
 #include "arrow/flight/cookie_internal.h"

diff --git a/cpp/src/arrow/flight/sql/client.cc b/cpp/src/arrow/flight/sql/client.cc
@@ -20,6 +20,8 @@
 
 #include "arrow/flight/sql/client.h"
 
+#include "arrow/util/protobuf_abi_fixup_internal.h"  // before any PB headers
+
 #include <google/protobuf/any.pb.h>
 
 #include "arrow/buffer.h"

diff --git a/cpp/src/arrow/flight/sql/client_test.cc b/cpp/src/arrow/flight/sql/client_test.cc
@@ -21,11 +21,14 @@
 #include "arrow/flight/client.h"
 
 #include <gmock/gmock.h>
-#include <google/protobuf/any.pb.h>
 #include <gtest/gtest.h>
 
 #include <utility>
 
+#include "arrow/util/protobuf_abi_fixup_internal.h"  // before any PB headers
+
+#include <google/protobuf/any.pb.h>
+
 #include "arrow/buffer.h"
 #include "arrow/flight/sql/api.h"
 #include "arrow/flight/sql/protocol_internal.h"

diff --git a/cpp/src/arrow/flight/sql/protocol_internal.cc b/cpp/src/arrow/flight/sql/protocol_internal.cc
@@ -21,3 +21,15 @@
 // ensure our header gets included (and Protobuf will not insert the
 // include for you)
 #include "arrow/flight/sql/FlightSql.pb.cc"  // NOLINT
+
+namespace arrow {
+namespace flight {
+namespace sql {
+namespace internal {
+
+DEFINE_ABI_FIXUP_FOR_PROTOBUF()
+
+}  // namespace internal
+}  // namespace sql
+}  // namespace flight
+}  // namespace arrow
diff --git a/cpp/src/arrow/flight/sql/protocol_internal.h b/cpp/src/arrow/flight/sql/protocol_internal.h
@@ -19,6 +19,8 @@
 // This addresses platform-specific defines, e.g. on Windows
 #include "arrow/flight/platform.h"  // IWYU pragma: keep
 
+#include "arrow/util/protobuf_abi_fixup_internal.h"  // before any PB headers
+
 // This header holds the Flight SQL definitions.
 
 #include "arrow/flight/sql/visibility.h"

diff --git a/cpp/src/arrow/flight/sql/server.cc b/cpp/src/arrow/flight/sql/server.cc
@@ -23,6 +23,8 @@
 
 #include "arrow/flight/sql/server.h"
 
+#include "arrow/util/protobuf_abi_fixup_internal.h"  // before any PB headers
+
 #include <google/protobuf/any.pb.h>
 
 #include "arrow/buffer.h"

diff --git a/cpp/src/arrow/flight/transport/grpc/customize_grpc.h b/cpp/src/arrow/flight/transport/grpc/customize_grpc.h
@@ -24,6 +24,8 @@
 #include "arrow/flight/type_fwd.h"
 #include "arrow/util/config.h"
 
+#include "arrow/util/protobuf_abi_fixup_internal.h"  // before any PB headers
+
 // Silence protobuf warnings
 #ifdef _MSC_VER
 #pragma warning(push)

diff --git a/cpp/src/arrow/flight/transport/grpc/grpc_client.cc b/cpp/src/arrow/flight/transport/grpc/grpc_client.cc
@@ -26,6 +26,9 @@
 #include <utility>
 
 #include "arrow/util/config.h"
+
+#include "arrow/util/protobuf_abi_fixup_internal.h"  // before any PB headers
+
 #ifdef GRPCPP_PP_INCLUDE
 #include <grpcpp/grpcpp.h>
 #if defined(GRPC_NAMESPACE_FOR_TLS_CREDENTIALS_OPTIONS)

diff --git a/cpp/src/arrow/flight/transport/grpc/grpc_server.cc b/cpp/src/arrow/flight/transport/grpc/grpc_server.cc
@@ -26,6 +26,9 @@
 #include <utility>
 
 #include "arrow/util/config.h"
+
+#include "arrow/util/protobuf_abi_fixup_internal.h"  // before any PB headers
+
 #ifdef GRPCPP_PP_INCLUDE
 #include <grpcpp/grpcpp.h>
 #else

diff --git a/cpp/src/arrow/flight/transport/grpc/serialization_internal.h b/cpp/src/arrow/flight/transport/grpc/serialization_internal.h
@@ -22,6 +22,8 @@
 
 #include <memory>
 
+#include "arrow/util/protobuf_abi_fixup_internal.h"  // before any PB headers
+
 #include "arrow/flight/protocol_internal.h"
 #include "arrow/flight/transport/grpc/protocol_grpc_internal.h"
 #include "arrow/flight/type_fwd.h"

diff --git a/cpp/src/arrow/flight/transport/grpc/util_internal.cc b/cpp/src/arrow/flight/transport/grpc/util_internal.cc
@@ -239,6 +239,12 @@ ::grpc::Status ToGrpcStatus(const Status& arrow_status, ::grpc::ServerContext* c
   return status;
 }
 
+namespace internal {
+
+DEFINE_ABI_FIXUP_FOR_PROTOBUF()
+
+}  // namespace internal
+
 }  // namespace grpc
 }  // namespace transport
 }  // namespace flight

diff --git a/cpp/src/arrow/util/protobuf_abi_fixup_internal.h b/cpp/src/arrow/util/protobuf_abi_fixup_internal.h
@@ -0,0 +1,52 @@
+// 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.
+
+#pragma once
+
+// TODO explain this
+
+#ifndef NDEBUG
+
+#ifdef GOOGLE_PROTOBUF_METADATA_LITE_H__
+#error "Protobuf already included, fixup can't be applied!"
+#endif
+#define SAVED_NDEBUG 0
+#define NDEBUG
+
+#else
+
+#define SAVED_NDEBUG 1
+
+#endif
+
+#include <google/protobuf/metadata_lite.h>
+
+#ifndef GOOGLE_PROTOBUF_METADATA_LITE_H__
+#error "Protobuf metadata internal not included!"
+#endif
+
+#if !SAVED_NDEBUG
+#undef NDEBUG
+#endif
+
+#define DEFINE_ABI_FIXUP_FOR_PROTOBUF()                                 \
+  volatile ::google::protobuf::internal::InternalMetadata* dummy_pb_md; \
+                                                                        \
+  void AbiFixupForProtobuf() {                                          \
+    using ::google::protobuf::internal::InternalMetadata;               \
+    dummy_pb_md->~InternalMetadata();                                   \
+  }