diff --git a/CMakeLists.txt b/CMakeLists.txt index c6df8ce1..bd7fe7e6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -16,13 +16,25 @@ option( "Enable substrait-cpp tests. This will enable all other build options automatically." ON) -find_package(Protobuf REQUIRED) -include_directories(${PROTOBUF_INCLUDE_DIRS}) - -add_subdirectory(third_party) +# Local files come first. include_directories(include) include_directories(src) +# TODO: Simplify once we can require cmake 3.27 (where CONFIG is default). + +# Due to packaging changes we use the combined protobuf/absl packaging if +# available otherwise we fallback to the older protobuf method. +find_package(Protobuf QUIET CONFIG) +if(${Protobuf_FOUND}) + set(ABSL_INCLUDED_WITH_PROTOBUF ON) +else() + find_package(Protobuf REQUIRED) + include_directories(${Protobuf_INCLUDE_DIRS}) + set(ABSL_INCLUDED_WITH_PROTOBUF OFF) +endif() + +add_subdirectory(third_party) + list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake_modules") include(BuildUtils) diff --git a/src/substrait/common/CMakeLists.txt b/src/substrait/common/CMakeLists.txt index dac9006c..846d077f 100644 --- a/src/substrait/common/CMakeLists.txt +++ b/src/substrait/common/CMakeLists.txt @@ -1,7 +1,5 @@ # SPDX-License-Identifier: Apache-2.0 -find_package(fmt) - add_library(substrait_common Exceptions.cpp) target_link_libraries(substrait_common fmt::fmt-header-only) diff --git a/src/substrait/proto/CMakeLists.txt b/src/substrait/proto/CMakeLists.txt index d8dc327c..0c5d5a62 100644 --- a/src/substrait/proto/CMakeLists.txt +++ b/src/substrait/proto/CMakeLists.txt @@ -82,7 +82,7 @@ add_library(substrait_proto ${PROTO_SRCS} ${PROTO_HDRS} ProtoUtils.cpp ProtoUtils.h) # Include the protobuf library as a dependency to use this class. -target_link_libraries(substrait_proto ${PROTOBUF_LIBRARIES}) +target_link_libraries(substrait_proto protobuf::libprotobuf) # Make sure we can see our own generated include files. target_include_directories(substrait_proto diff --git a/src/substrait/textplan/converter/LoadBinary.cpp b/src/substrait/textplan/converter/LoadBinary.cpp index 69d98007..787ab054 100644 --- a/src/substrait/textplan/converter/LoadBinary.cpp +++ b/src/substrait/textplan/converter/LoadBinary.cpp @@ -70,8 +70,9 @@ PlanOrErrors loadFromJson(std::string_view json) { auto status = google::protobuf::util::JsonStringToMessage( std::string{usableJson}, &plan); if (!status.ok()) { - return PlanOrErrors({fmt::format( - "Failed to parse Substrait JSON: {}", status.message().ToString())}); + std::string msg{status.message()}; + return PlanOrErrors( + {fmt::format("Failed to parse Substrait JSON: {}", msg)}); } return PlanOrErrors(plan); } diff --git a/src/substrait/textplan/parser/SubstraitPlanRelationVisitor.cpp b/src/substrait/textplan/parser/SubstraitPlanRelationVisitor.cpp index 9ed04f84..abdb98b2 100644 --- a/src/substrait/textplan/parser/SubstraitPlanRelationVisitor.cpp +++ b/src/substrait/textplan/parser/SubstraitPlanRelationVisitor.cpp @@ -790,8 +790,7 @@ std::any SubstraitPlanRelationVisitor::visitExpressionFunctionUse( expr.mutable_scalar_function()->set_function_reference(funcReference); for (const auto& exp : ctx->expression()) { if (endsWith(exp->getText(), "_enum")) { - auto str = exp->getText(); - str = absl::StripSuffix(str, "_enum"); + std::string str{absl::StripSuffix(exp->getText(), "_enum")}; expr.mutable_scalar_function()->add_arguments()->set_enum_(str); continue; } diff --git a/src/substrait/textplan/tests/CMakeLists.txt b/src/substrait/textplan/tests/CMakeLists.txt index 16da68ba..24be6792 100644 --- a/src/substrait/textplan/tests/CMakeLists.txt +++ b/src/substrait/textplan/tests/CMakeLists.txt @@ -36,7 +36,6 @@ if(${SUBSTRAIT_CPP_ROUNDTRIP_TESTING}) parse_result_matchers protobuf-matchers fmt::fmt-header-only - absl::strings gmock gtest gtest_main) diff --git a/src/substrait/textplan/tests/RoundtripTest.cpp b/src/substrait/textplan/tests/RoundtripTest.cpp index 0d5949ca..4cf2e3ea 100644 --- a/src/substrait/textplan/tests/RoundtripTest.cpp +++ b/src/substrait/textplan/tests/RoundtripTest.cpp @@ -10,8 +10,6 @@ #include #include -#include "absl/strings/str_split.h" -#include "gmock/gmock.h" #include "substrait/textplan/SymbolTablePrinter.h" #include "substrait/textplan/converter/LoadBinary.h" #include "substrait/textplan/converter/ParseBinary.h" @@ -28,10 +26,12 @@ namespace io::substrait::textplan { namespace { std::string addLineNumbers(const std::string& text) { + std::stringstream input{text}; std::stringstream result; int lineNum = 0; - for (absl::string_view sp : absl::StrSplit(text, '\n')) { - result << std::setw(4) << ++lineNum << " " << sp << std::endl; + std::string line; + while (std::getline(input, line, '\n')) { + result << std::setw(4) << ++lineNum << " " << line << std::endl; } return result.str(); } diff --git a/third_party/CMakeLists.txt b/third_party/CMakeLists.txt index 6ace2da8..7b8d8e60 100644 --- a/third_party/CMakeLists.txt +++ b/third_party/CMakeLists.txt @@ -1,7 +1,9 @@ # SPDX-License-Identifier: Apache-2.0 -set(ABSL_PROPAGATE_CXX_STD ON) -add_subdirectory(abseil-cpp) +if(NOT ${ABSL_INCLUDED_WITH_PROTOBUF}) + set(ABSL_PROPAGATE_CXX_STD ON) + add_subdirectory(abseil-cpp) +endif() set(BUILD_TZ_LIB ON) add_subdirectory(datetime) diff --git a/third_party/abseil-cpp b/third_party/abseil-cpp index c8a2f925..c2435f83 160000 --- a/third_party/abseil-cpp +++ b/third_party/abseil-cpp @@ -1 +1 @@ -Subproject commit c8a2f92586fe9b4e1aff049108f5db8064924d8e +Subproject commit c2435f8342c2d0ed8101cb43adfd605fdc52dca2