Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vendor/extras: Fix building with newer protobuf versions #120

Merged
merged 2 commits into from
Jul 7, 2023

Conversation

Biswa96
Copy link
Collaborator

@Biswa96 Biswa96 commented Jul 4, 2023

Fixes #119

@munix9
Copy link
Contributor

munix9 commented Jul 5, 2023

So the whole locally tested patch looks like this:

diff -ruN a/vendor/CMakeLists.txt b/vendor/CMakeLists.txt
--- a/vendor/CMakeLists.txt
+++ b/vendor/CMakeLists.txt
@@ -73,6 +73,8 @@
 pkg_check_modules(libzstd REQUIRED IMPORTED_TARGET libzstd)
 
+find_package(Protobuf CONFIG)
 find_package(Protobuf REQUIRED)
+set(PROTOBUF_LIBRARIES protobuf::libprotobuf)
 set(THREADS_PREFER_PTHREAD_FLAG ON)
 find_package(Threads REQUIRED)
 
diff -ruN a/vendor/extras/libjsonpb/parse/jsonpb.cpp b/vendor/extras/libjsonpb/parse/jsonpb.cpp
--- a/vendor/extras/libjsonpb/parse/jsonpb.cpp
+++ b/vendor/extras/libjsonpb/parse/jsonpb.cpp
@@ -50,8 +50,10 @@
   if (!status.ok()) {
 #if GOOGLE_PROTOBUF_VERSION < 3016000
     return MakeError<std::string>(status.error_message().as_string());
-#else
+#elif GOOGLE_PROTOBUF_VERSION < 4022000
     return MakeError<std::string>(status.message().as_string());
+#else
+    return MakeError<std::string>(std::string(status.message()));
 #endif
   }
   return ErrorOr<std::string>(std::move(json));
@@ -67,8 +69,10 @@
   if (!status.ok()) {
 #if GOOGLE_PROTOBUF_VERSION < 3016000
     return MakeError<std::monostate>(status.error_message().as_string());
-#else
+#elif GOOGLE_PROTOBUF_VERSION < 4022000
     return MakeError<std::monostate>(status.message().as_string());
+#else
+    return MakeError<std::monostate>(std::string(status.message()));
 #endif
   }
   if (!message->ParseFromString(binary)) {

Tested in openSUSE Tumbleweed with protobuf 21.12 (old) and 22.5 (new).

This fixes the following linking errors

vendor/libliblpdump.a(dynamic_partitions_device_info.pb.cc.o):
undefined reference to symbol '_ZN4absl12lts_2023012512log_internal21CheckOpMessageBuilder7ForVar2Ev'
/usr/lib/libabsl_log_internal_check_op.so.2301.0.0: error adding symbols: DSO missing from command line

The find_package was called twice because protobuf-config.cmake file
was not present in versions older than 22.0. Also the Config mode is
required to get the private abseil-cpp libraries.
@Biswa96 Biswa96 marked this pull request as ready for review July 5, 2023 14:09
Copy link
Contributor

@munix9 munix9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Biswa96 Biswa96 requested review from anatol and nmeum July 7, 2023 05:37
@Biswa96 Biswa96 merged commit 16cba04 into nmeum:master Jul 7, 2023
12 checks passed
@Biswa96 Biswa96 deleted the vendor-extas-protobuf branch July 7, 2023 17:40
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compilation issue on openSUSE Tumbleweed
3 participants