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

[dbus] add absl support for higher version protobuf #2130

Merged
merged 1 commit into from
Feb 1, 2024

Conversation

TE-N-ElvenWang
Copy link
Contributor

Protobuf which higher than 22 need link absl. Add the linked shared library to pass the build.

@TE-N-ElvenWang
Copy link
Contributor Author

This patch fixed below build error when the system/SDK using higher version of protobuf:

armv8a-poky-linux/otbr/1.0/recipe-sysroot-native/usr/bin/aarch64-poky-linux/../../libexec/aarch64-poky-linux/gcc/aarch64-poky-linux/13.2.0/ld: src/proto/libotbr-proto.a(capabilities.pb.cc.o): undefined reference to symbol '_ZN4absl12lts_2023080212log_internal17MakeCheckOpStringIllEEPNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEET_T0_PKc'
armv8a-poky-linux/otbr/1.0/recipe-sysroot-native/usr/bin/aarch64-poky-linux/../../libexec/aarch64-poky-linux/gcc/aarch64-poky-linux/13.2.0/ld: armv8a-poky-linux/otbr/1.0/recipe-sysroot/usr/lib64/libabsl_log_internal_check_op.so.2308.0.0: error adding symbols: DSO missing from command line
collect2: error: ld returned 1 exit status

@TE-N-ElvenWang TE-N-ElvenWang force-pushed the protobuf_absl_support branch 6 times, most recently from 406be9a to b064c14 Compare December 14, 2023 02:41
@TE-N-ElvenWang TE-N-ElvenWang force-pushed the protobuf_absl_support branch 2 times, most recently from ead2c64 to 809971e Compare December 24, 2023 09:21
@TE-N-ElvenWang
Copy link
Contributor Author

@jwhui Could you please help on this? Since there don't have any reviewer added.
Thanks!

@jwhui jwhui requested review from bukepo and morningboata January 24, 2024 23:59
Comment on lines 10 to 11
if ("${Protobuf_VERSION}" MATCHES [[[0-9]+.([0-9]+).[0-9]+]])
if(CMAKE_MATCH_1 GREATER_EQUAL 22)
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to use built-in VERSION_LESS?

Suggested change
if ("${Protobuf_VERSION}" MATCHES [[[0-9]+.([0-9]+).[0-9]+]])
if(CMAKE_MATCH_1 GREATER_EQUAL 22)
if (NOT "${Protobuf_VERSION}" VERSION_LESS 22)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bukepo Thank you for your suggestion. Use built-in "VERSION_LESS" to check the version of protobuf. Please see the updated change.

@jwhui jwhui requested a review from bukepo January 25, 2024 22:45
src/proto/CMakeLists.txt Outdated Show resolved Hide resolved
@TE-N-ElvenWang
Copy link
Contributor Author

Hi @bukepo , could you please give a look at the comments reply in this PR? Thanks!

src/proto/CMakeLists.txt Outdated Show resolved Hide resolved
Protobuf which higher than 22 need link absl. Add the linked shared
library to pass the build.

Signed-off-by: Haoran Wang <[email protected]>
@jwhui jwhui requested a review from bukepo February 1, 2024 00:26
Copy link
Member

@bukepo bukepo left a comment

Choose a reason for hiding this comment

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

LGTM:+1:

Copy link
Member

@jwhui jwhui left a comment

Choose a reason for hiding this comment

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

Thanks! 👍🏼

@jwhui jwhui merged commit afe85cc into openthread:main Feb 1, 2024
30 checks passed
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.

4 participants