From a9729bc56481568c03c22f198c604f9aaeebd36f Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Thu, 5 Dec 2024 17:46:31 +0100 Subject: [PATCH 1/8] GH-44954: [C++][CI] Silence protobuf-generated deprecations --- cpp/cmake_modules/ThirdpartyToolchain.cmake | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index 35ad4089e7f..266d80f67f1 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -2061,10 +2061,13 @@ macro(build_substrait) # Missing dll-interface: list(APPEND SUBSTRAIT_SUPPRESSED_FLAGS "/wd4251") - elseif(CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang" OR CMAKE_CXX_COMPILER_ID STREQUAL - "Clang") - # Protobuf generated files trigger some errors on CLANG TSAN builds - list(APPEND SUBSTRAIT_SUPPRESSED_FLAGS "-Wno-error=shorten-64-to-32") + else() + list(APPEND SUBSTRAIT_SUPPRESSED_FLAGS "-Wno-deprecated") + if(CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang" OR CMAKE_CXX_COMPILER_ID STREQUAL + "Clang") + # Protobuf generated files trigger some errors on CLANG TSAN builds + list(APPEND SUBSTRAIT_SUPPRESSED_FLAGS "-Wno-error=shorten-64-to-32") + endif() endif() set(SUBSTRAIT_SOURCES) @@ -2115,7 +2118,9 @@ macro(build_substrait) set(SUBSTRAIT_INCLUDES ${SUBSTRAIT_CPP_DIR} ${PROTOBUF_INCLUDE_DIR}) add_library(substrait STATIC ${SUBSTRAIT_SOURCES}) - set_target_properties(substrait PROPERTIES POSITION_INDEPENDENT_CODE ON) + set_target_properties(substrait + PROPERTIES POSITION_INDEPENDENT_CODE ON + COMPILE_OPTIONS "${SUBSTRAIT_SUPPRESSED_FLAGS}") target_include_directories(substrait PUBLIC ${SUBSTRAIT_INCLUDES}) target_link_libraries(substrait PUBLIC ${ARROW_PROTOBUF_LIBPROTOBUF}) add_dependencies(substrait substrait_gen) From cd490a60f1d49dc419c4a2985b62684fb4b3ae8c Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Fri, 6 Dec 2024 02:52:17 +0100 Subject: [PATCH 2/8] suppress deprecation with pragma in headers --- cpp/src/arrow/engine/substrait/expression_internal.h | 3 +++ .../arrow/engine/substrait/extended_expression_internal.h | 3 +++ cpp/src/arrow/engine/substrait/plan_internal.h | 3 +++ cpp/src/arrow/engine/substrait/relation_internal.h | 3 +++ cpp/src/arrow/engine/substrait/test_plan_builder.cc | 5 ++++- cpp/src/arrow/engine/substrait/util_internal.h | 3 +++ 6 files changed, 19 insertions(+), 1 deletion(-) diff --git a/cpp/src/arrow/engine/substrait/expression_internal.h b/cpp/src/arrow/engine/substrait/expression_internal.h index 2ce2ee76af2..58b6a1af596 100644 --- a/cpp/src/arrow/engine/substrait/expression_internal.h +++ b/cpp/src/arrow/engine/substrait/expression_internal.h @@ -27,8 +27,11 @@ #include "arrow/engine/substrait/type_fwd.h" #include "arrow/engine/substrait/visibility.h" #include "arrow/result.h" +#include "arrow/util/macros.h" +ARROW_SUPPRESS_DEPRECATION_WARNING #include "substrait/algebra.pb.h" // IWYU pragma: export +ARROW_UNSUPPRESS_DEPRECATION_WARNING namespace arrow { namespace engine { diff --git a/cpp/src/arrow/engine/substrait/extended_expression_internal.h b/cpp/src/arrow/engine/substrait/extended_expression_internal.h index 81bc4b87451..2451ad12b5e 100644 --- a/cpp/src/arrow/engine/substrait/extended_expression_internal.h +++ b/cpp/src/arrow/engine/substrait/extended_expression_internal.h @@ -28,8 +28,11 @@ #include "arrow/engine/substrait/visibility.h" #include "arrow/result.h" #include "arrow/status.h" +#include "arrow/util/macros.h" +ARROW_SUPPRESS_DEPRECATION_WARNING #include "substrait/extended_expression.pb.h" // IWYU pragma: export +ARROW_UNSUPPRESS_DEPRECATION_WARNING namespace arrow { namespace engine { diff --git a/cpp/src/arrow/engine/substrait/plan_internal.h b/cpp/src/arrow/engine/substrait/plan_internal.h index 737e65b7e2e..641b06b5ad6 100644 --- a/cpp/src/arrow/engine/substrait/plan_internal.h +++ b/cpp/src/arrow/engine/substrait/plan_internal.h @@ -27,8 +27,11 @@ #include "arrow/engine/substrait/visibility.h" #include "arrow/result.h" #include "arrow/status.h" +#include "arrow/util/macros.h" +ARROW_SUPPRESS_DEPRECATION_WARNING #include "substrait/plan.pb.h" // IWYU pragma: export +ARROW_UNSUPPRESS_DEPRECATION_WARNING namespace arrow { namespace engine { diff --git a/cpp/src/arrow/engine/substrait/relation_internal.h b/cpp/src/arrow/engine/substrait/relation_internal.h index a436f1770d7..f2f6e3246fc 100644 --- a/cpp/src/arrow/engine/substrait/relation_internal.h +++ b/cpp/src/arrow/engine/substrait/relation_internal.h @@ -28,8 +28,11 @@ #include "arrow/engine/substrait/type_fwd.h" #include "arrow/engine/substrait/visibility.h" #include "arrow/result.h" +#include "arrow/util/macros.h" +ARROW_SUPPRESS_DEPRECATION_WARNING #include "substrait/algebra.pb.h" // IWYU pragma: export +ARROW_UNSUPPRESS_DEPRECATION_WARNING namespace arrow { namespace engine { diff --git a/cpp/src/arrow/engine/substrait/test_plan_builder.cc b/cpp/src/arrow/engine/substrait/test_plan_builder.cc index f38f7ece9a7..ad44ee0db28 100644 --- a/cpp/src/arrow/engine/substrait/test_plan_builder.cc +++ b/cpp/src/arrow/engine/substrait/test_plan_builder.cc @@ -31,8 +31,11 @@ #include "arrow/status.h" #include "arrow/table.h" #include "arrow/type_fwd.h" +#include "arrow/util/macros.h" -#include "substrait/algebra.pb.h" +ARROW_SUPPRESS_DEPRECATION_WARNING +#include "substrait/algebra.pb.h" // IWYU pragma: export +ARROW_UNSUPPRESS_DEPRECATION_WARNING namespace arrow { namespace engine { diff --git a/cpp/src/arrow/engine/substrait/util_internal.h b/cpp/src/arrow/engine/substrait/util_internal.h index 627ad1126df..44fd38419cb 100644 --- a/cpp/src/arrow/engine/substrait/util_internal.h +++ b/cpp/src/arrow/engine/substrait/util_internal.h @@ -24,10 +24,13 @@ #include "arrow/engine/substrait/visibility.h" #include "arrow/result.h" #include "arrow/util/hashing.h" +#include "arrow/util/macros.h" #include "arrow/util/unreachable.h" +ARROW_SUPPRESS_DEPRECATION_WARNING #include "substrait/algebra.pb.h" // IWYU pragma: export #include "substrait/plan.pb.h" // IWYU pragma: export +ARROW_UNSUPPRESS_DEPRECATION_WARNING namespace arrow { namespace engine { From c1e10f9ff5a8166209c580c9435e4847b9f3fcae Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Fri, 6 Dec 2024 03:56:58 +0100 Subject: [PATCH 3/8] suppress flightsql.pb.h --- cpp/src/arrow/flight/sql/protocol_internal.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cpp/src/arrow/flight/sql/protocol_internal.h b/cpp/src/arrow/flight/sql/protocol_internal.h index ce50ad2f61b..09bfe32582a 100644 --- a/cpp/src/arrow/flight/sql/protocol_internal.h +++ b/cpp/src/arrow/flight/sql/protocol_internal.h @@ -18,9 +18,12 @@ // This addresses platform-specific defines, e.g. on Windows #include "arrow/flight/platform.h" // IWYU pragma: keep +#include "arrow/util/macros.h" // This header holds the Flight SQL definitions. #include "arrow/flight/sql/visibility.h" +ARROW_SUPPRESS_DEPRECATION_WARNING #include "arrow/flight/sql/FlightSql.pb.h" // IWYU pragma: export +ARROW_UNSUPPRESS_DEPRECATION_WARNING From 9918b38112503b47ef0d1aa5a2cb85e29f0d3dde Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Fri, 6 Dec 2024 09:59:15 +0100 Subject: [PATCH 4/8] Use target_compile_options --- cpp/cmake_modules/ThirdpartyToolchain.cmake | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index 266d80f67f1..39962b31d31 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -2118,9 +2118,8 @@ macro(build_substrait) set(SUBSTRAIT_INCLUDES ${SUBSTRAIT_CPP_DIR} ${PROTOBUF_INCLUDE_DIR}) add_library(substrait STATIC ${SUBSTRAIT_SOURCES}) - set_target_properties(substrait - PROPERTIES POSITION_INDEPENDENT_CODE ON - COMPILE_OPTIONS "${SUBSTRAIT_SUPPRESSED_FLAGS}") + set_target_properties(substrait PROPERTIES POSITION_INDEPENDENT_CODE ON) + target_compile_options(substrait PRIVATE "${SUBSTRAIT_SUPPRESSED_FLAGS}") target_include_directories(substrait PUBLIC ${SUBSTRAIT_INCLUDES}) target_link_libraries(substrait PUBLIC ${ARROW_PROTOBUF_LIBPROTOBUF}) add_dependencies(substrait substrait_gen) From 105b13e4a6b1eea821c3dacad3fdbc6ae6d1cda6 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Fri, 6 Dec 2024 10:24:49 +0100 Subject: [PATCH 5/8] Suppress more PB warnings --- cpp/src/arrow/flight/sql/client.cc | 3 +++ cpp/src/arrow/flight/sql/server.cc | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/cpp/src/arrow/flight/sql/client.cc b/cpp/src/arrow/flight/sql/client.cc index 536bc67fc58..cd722677a5a 100644 --- a/cpp/src/arrow/flight/sql/client.cc +++ b/cpp/src/arrow/flight/sql/client.cc @@ -829,6 +829,8 @@ Status FlightSqlClient::Rollback(const FlightCallOptions& options, return results->Drain(); } +// ActionCancelQuery{Request,Result} are deprecated +ARROW_SUPPRESS_DEPRECATION_WARNING ::arrow::Result FlightSqlClient::CancelQuery( const FlightCallOptions& options, const FlightInfo& info) { flight_sql_pb::ActionCancelQueryRequest cancel_query; @@ -855,6 +857,7 @@ ::arrow::Result FlightSqlClient::CancelQuery( } return Status::IOError("Server returned unknown result ", result.result()); } +ARROW_UNSUPPRESS_DEPRECATION_WARNING Status FlightSqlClient::Close() { return impl_->Close(); } diff --git a/cpp/src/arrow/flight/sql/server.cc b/cpp/src/arrow/flight/sql/server.cc index 5f6154a576b..7e8c2cc2f79 100644 --- a/cpp/src/arrow/flight/sql/server.cc +++ b/cpp/src/arrow/flight/sql/server.cc @@ -337,6 +337,8 @@ arrow::Result ParseActionBeginTransactionRequest( return result; } +// ActionCancelQueryRequest is deprecated +ARROW_SUPPRESS_DEPRECATION_WARNING arrow::Result ParseActionCancelQueryRequest( const Action& action) { pb::sql::ActionCancelQueryRequest command; @@ -346,6 +348,7 @@ arrow::Result ParseActionCancelQueryRequest( ARROW_ASSIGN_OR_RAISE(result.info, FlightInfo::Deserialize(command.info())); return result; } +ARROW_UNSUPPRESS_DEPRECATION_WARNING arrow::Result ParseActionCreatePreparedStatementRequest(const Action& action) { @@ -468,6 +471,8 @@ arrow::Result PackActionResult(const FlightEndpoint& endpoint) { return endpoint.SerializeToBuffer(); } +// ActionCancelQueryResult is deprecated +ARROW_SUPPRESS_DEPRECATION_WARNING arrow::Result PackActionResult(CancelResult result) { pb::sql::ActionCancelQueryResult pb_result; switch (result) { @@ -487,6 +492,7 @@ arrow::Result PackActionResult(CancelResult result) { } return PackActionResult(pb_result); } +ARROW_UNSUPPRESS_DEPRECATION_WARNING arrow::Result PackActionResult(ActionCreatePreparedStatementResult result) { pb::sql::ActionCreatePreparedStatementResult pb_result; From 6b0bdcfebdfb9f3f548af1702e758997a8d0cc4e Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Fri, 6 Dec 2024 10:45:41 +0100 Subject: [PATCH 6/8] More suppressions yet --- cpp/src/arrow/flight/sql/protocol_internal.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cpp/src/arrow/flight/sql/protocol_internal.cc b/cpp/src/arrow/flight/sql/protocol_internal.cc index 0d5e3c4c60b..1ebfaa107e5 100644 --- a/cpp/src/arrow/flight/sql/protocol_internal.cc +++ b/cpp/src/arrow/flight/sql/protocol_internal.cc @@ -14,6 +14,7 @@ // KIND, either express or implied. See the License for the // specific language governing permissions and limitations +ARROW_SUPPRESS_DEPRECATION_WARNING #include "arrow/flight/sql/protocol_internal.h" // NOTE(lidavidm): Normally this is forbidden, but on Windows to get @@ -21,3 +22,4 @@ // ensure our header gets included (and Protobuf will not insert the // include for you) #include "arrow/flight/sql/FlightSql.pb.cc" // NOLINT +ARROW_UNSUPPRESS_DEPRECATION_WARNING From 73b3514130fa3d5c1fc7463fbcc31003eca3cd69 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Fri, 6 Dec 2024 11:08:11 +0100 Subject: [PATCH 7/8] Attempt at fixing --- cpp/src/arrow/flight/sql/client.cc | 1 + cpp/src/arrow/flight/sql/protocol_internal.cc | 2 ++ cpp/src/arrow/flight/sql/server.cc | 1 + 3 files changed, 4 insertions(+) diff --git a/cpp/src/arrow/flight/sql/client.cc b/cpp/src/arrow/flight/sql/client.cc index cd722677a5a..fe087cc947d 100644 --- a/cpp/src/arrow/flight/sql/client.cc +++ b/cpp/src/arrow/flight/sql/client.cc @@ -31,6 +31,7 @@ #include "arrow/ipc/reader.h" #include "arrow/result.h" #include "arrow/util/logging.h" +#include "arrow/util/macros.h" namespace flight_sql_pb = arrow::flight::protocol::sql; diff --git a/cpp/src/arrow/flight/sql/protocol_internal.cc b/cpp/src/arrow/flight/sql/protocol_internal.cc index 1ebfaa107e5..509f6f27ce0 100644 --- a/cpp/src/arrow/flight/sql/protocol_internal.cc +++ b/cpp/src/arrow/flight/sql/protocol_internal.cc @@ -14,6 +14,8 @@ // KIND, either express or implied. See the License for the // specific language governing permissions and limitations +#include "arrow/util/macros.h" + ARROW_SUPPRESS_DEPRECATION_WARNING #include "arrow/flight/sql/protocol_internal.h" diff --git a/cpp/src/arrow/flight/sql/server.cc b/cpp/src/arrow/flight/sql/server.cc index 7e8c2cc2f79..f68d884c621 100644 --- a/cpp/src/arrow/flight/sql/server.cc +++ b/cpp/src/arrow/flight/sql/server.cc @@ -31,6 +31,7 @@ #include "arrow/flight/sql/sql_info_internal.h" #include "arrow/type.h" #include "arrow/util/checked_cast.h" +#include "arrow/util/macros.h" #define PROPERTY_TO_OPTIONAL(COMMAND, PROPERTY) \ COMMAND.has_##PROPERTY() ? std::make_optional(COMMAND.PROPERTY()) : std::nullopt From 87b45e3fd4e30b031d5b51d68705b3e5d54972e7 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Fri, 6 Dec 2024 16:47:47 +0100 Subject: [PATCH 8/8] Add comments --- cpp/cmake_modules/ThirdpartyToolchain.cmake | 1 + cpp/src/arrow/engine/substrait/expression_internal.h | 1 + cpp/src/arrow/engine/substrait/extended_expression_internal.h | 1 + cpp/src/arrow/engine/substrait/plan_internal.h | 1 + cpp/src/arrow/engine/substrait/relation_internal.h | 1 + cpp/src/arrow/engine/substrait/test_plan_builder.cc | 1 + cpp/src/arrow/engine/substrait/util_internal.h | 1 + cpp/src/arrow/flight/sql/protocol_internal.cc | 1 + 8 files changed, 8 insertions(+) diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index 39962b31d31..a7d509de10e 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -2062,6 +2062,7 @@ macro(build_substrait) # Missing dll-interface: list(APPEND SUBSTRAIT_SUPPRESSED_FLAGS "/wd4251") else() + # GH-44954: silence [[deprecated]] declarations in protobuf-generated code list(APPEND SUBSTRAIT_SUPPRESSED_FLAGS "-Wno-deprecated") if(CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang" OR CMAKE_CXX_COMPILER_ID STREQUAL "Clang") diff --git a/cpp/src/arrow/engine/substrait/expression_internal.h b/cpp/src/arrow/engine/substrait/expression_internal.h index 58b6a1af596..a9f8949c232 100644 --- a/cpp/src/arrow/engine/substrait/expression_internal.h +++ b/cpp/src/arrow/engine/substrait/expression_internal.h @@ -29,6 +29,7 @@ #include "arrow/result.h" #include "arrow/util/macros.h" +// GH-44954: silence [[deprecated]] declarations in protobuf-generated code ARROW_SUPPRESS_DEPRECATION_WARNING #include "substrait/algebra.pb.h" // IWYU pragma: export ARROW_UNSUPPRESS_DEPRECATION_WARNING diff --git a/cpp/src/arrow/engine/substrait/extended_expression_internal.h b/cpp/src/arrow/engine/substrait/extended_expression_internal.h index 2451ad12b5e..45f89c8610b 100644 --- a/cpp/src/arrow/engine/substrait/extended_expression_internal.h +++ b/cpp/src/arrow/engine/substrait/extended_expression_internal.h @@ -30,6 +30,7 @@ #include "arrow/status.h" #include "arrow/util/macros.h" +// GH-44954: silence [[deprecated]] declarations in protobuf-generated code ARROW_SUPPRESS_DEPRECATION_WARNING #include "substrait/extended_expression.pb.h" // IWYU pragma: export ARROW_UNSUPPRESS_DEPRECATION_WARNING diff --git a/cpp/src/arrow/engine/substrait/plan_internal.h b/cpp/src/arrow/engine/substrait/plan_internal.h index 641b06b5ad6..f2e7ded5f01 100644 --- a/cpp/src/arrow/engine/substrait/plan_internal.h +++ b/cpp/src/arrow/engine/substrait/plan_internal.h @@ -29,6 +29,7 @@ #include "arrow/status.h" #include "arrow/util/macros.h" +// GH-44954: silence [[deprecated]] declarations in protobuf-generated code ARROW_SUPPRESS_DEPRECATION_WARNING #include "substrait/plan.pb.h" // IWYU pragma: export ARROW_UNSUPPRESS_DEPRECATION_WARNING diff --git a/cpp/src/arrow/engine/substrait/relation_internal.h b/cpp/src/arrow/engine/substrait/relation_internal.h index f2f6e3246fc..2a96d0024e6 100644 --- a/cpp/src/arrow/engine/substrait/relation_internal.h +++ b/cpp/src/arrow/engine/substrait/relation_internal.h @@ -30,6 +30,7 @@ #include "arrow/result.h" #include "arrow/util/macros.h" +// GH-44954: silence [[deprecated]] declarations in protobuf-generated code ARROW_SUPPRESS_DEPRECATION_WARNING #include "substrait/algebra.pb.h" // IWYU pragma: export ARROW_UNSUPPRESS_DEPRECATION_WARNING diff --git a/cpp/src/arrow/engine/substrait/test_plan_builder.cc b/cpp/src/arrow/engine/substrait/test_plan_builder.cc index ad44ee0db28..724c58277e7 100644 --- a/cpp/src/arrow/engine/substrait/test_plan_builder.cc +++ b/cpp/src/arrow/engine/substrait/test_plan_builder.cc @@ -33,6 +33,7 @@ #include "arrow/type_fwd.h" #include "arrow/util/macros.h" +// GH-44954: silence [[deprecated]] declarations in protobuf-generated code ARROW_SUPPRESS_DEPRECATION_WARNING #include "substrait/algebra.pb.h" // IWYU pragma: export ARROW_UNSUPPRESS_DEPRECATION_WARNING diff --git a/cpp/src/arrow/engine/substrait/util_internal.h b/cpp/src/arrow/engine/substrait/util_internal.h index 44fd38419cb..d812bbf7b85 100644 --- a/cpp/src/arrow/engine/substrait/util_internal.h +++ b/cpp/src/arrow/engine/substrait/util_internal.h @@ -27,6 +27,7 @@ #include "arrow/util/macros.h" #include "arrow/util/unreachable.h" +// GH-44954: silence [[deprecated]] declarations in protobuf-generated code ARROW_SUPPRESS_DEPRECATION_WARNING #include "substrait/algebra.pb.h" // IWYU pragma: export #include "substrait/plan.pb.h" // IWYU pragma: export diff --git a/cpp/src/arrow/flight/sql/protocol_internal.cc b/cpp/src/arrow/flight/sql/protocol_internal.cc index 509f6f27ce0..984e7822233 100644 --- a/cpp/src/arrow/flight/sql/protocol_internal.cc +++ b/cpp/src/arrow/flight/sql/protocol_internal.cc @@ -16,6 +16,7 @@ #include "arrow/util/macros.h" +// GH-44954: silence [[deprecated]] declarations in protobuf-generated code ARROW_SUPPRESS_DEPRECATION_WARNING #include "arrow/flight/sql/protocol_internal.h"