fix: support both old and new versions of protobuf

The version numbering scheme for the protobuf compiler was updated which causes a mismatch warning to appear for versions of cmake less than 3.27.  Later versions of protobuf are bundled with their own copy of abseil.  Since protobuf always required abseil anyway, this fix also updates the code to rely on that instead of our own submodule.  This should lead to greater consistency (especially with new protobuf code leaning on an experimental logging feature that won't be available in older versions of abseil).  If the newer CONFIG-based method of findpackage does not work, we now fall back to the older method of using findpackage.  The version of cmake that we require is still 3.20 as 3.27 isn't available in enough locations to advance to that minimum.

.gitmodules

@@ -10,9 +10,6 @@
 [submodule "third_party/fmt"]
  path = third_party/fmt
  url = https://github.com/fmtlib/fmt
-[submodule "third_party/abseil-cpp"]
- path = third_party/abseil-cpp
- url = https://github.com/abseil/abseil-cpp.git
 [submodule "third_party/datetime"]
  path = third_party/datetime
  url = https://github.com/HowardHinnant/date.git

CMakeLists.txt

@@ -16,13 +16,22 @@ 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(absl REQUIRED)
+#include_directories(${ABSL_INCLUDE_DIRS})
+
+find_package(Protobuf REQUIRED CONFIG)
+#include_directories(${PROTOBUF_INCLUDE_DIRS})
+
+message(STATUS "include dirs are ${Protobuf_INCLUDE_DIRS}")
+message(STATUS "include dirs are ${PROTOBUF_INCLUDE_DIRS}")
+
+
 list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake_modules")
 include(BuildUtils)
 

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

@@ -71,7 +71,7 @@ PlanOrErrors loadFromJson(std::string_view json) {
  std::string{usableJson}, &plan);
  if (!status.ok()) {
  return PlanOrErrors({fmt::format(
- "Failed to parse Substrait JSON: {}", status.message().ToString())});
+ "Failed to parse Substrait JSON: {}", status.message())});
  }
  return PlanOrErrors(plan);
 }

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

third_party/CMakeLists.txt

@@ -1,8 +1,5 @@
 # SPDX-License-Identifier: Apache-2.0
 
-set(ABSL_PROPAGATE_CXX_STD ON)
-add_subdirectory(abseil-cpp)
-
 set(BUILD_TZ_LIB ON)
 add_subdirectory(datetime)