fix: support both old and new versions of protobuf

CMakeLists.txt

@@ -16,13 +16,18 @@ 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)
 
+add_subdirectory(third_party)
+
+find_package(Protobuf QUIET CONFIG)
+if(NOT ${Protobuf_FOUND})
+ find_package(Protobuf REQUIRED)
+ include_directories(${Protobuf_INCLUDE_DIRS})
+endif()
+
 list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake_modules")
 include(BuildUtils)
 

scripts/setup-ubuntu.sh

@@ -21,6 +21,7 @@ sudo --preserve-env apt install -y \
  libprotobuf-dev \
  libprotobuf23 \
  protobuf-compiler \
+ libabsl-dev \
  clang-format \
  uuid-dev \
  default-jre \

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)

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

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);
 }

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;
  }

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)

src/substrait/textplan/tests/RoundtripTest.cpp

@@ -10,8 +10,6 @@
 #include <gtest/gtest.h>
 #include <protobuf-matchers/protocol-buffer-matchers.h>
 
-#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();
 }