Skip to content

Allow cross-compilation with protobuf #3117

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Allow cross-compilation with protobuf #3117

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 13 additions & 12 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -419,20 +419,21 @@ if(WITH_OTLP_GRPC
endif()
endif()
endif()
# Latest Protobuf imported targets and without legacy module support
if(TARGET protobuf::protoc)
project_build_tools_get_imported_location(PROTOBUF_PROTOC_EXECUTABLE
protobuf::protoc)
# If protobuf::protoc is not a imported target, then we use the target
# directly for fallback
if(NOT PROTOBUF_PROTOC_EXECUTABLE)
set(PROTOBUF_PROTOC_EXECUTABLE protobuf::protoc)
if(NOT PROTOBUF_PROTOC_EXECUTABLE)
# Latest Protobuf imported targets and without legacy module support
if(TARGET protobuf::protoc)
Copy link
Member

@owent owent Nov 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We do not build protobuf::protoc when cross compilation beforem but it's reasonable to support cross compilation when it's built.
Could you please also check and set PROTOBUF_PROTOC_EXECUTABLE?
PROTOBUF_PROTOC_EXECUTABLE is for the old version of cmake, and now it's PROTOBUF_PROTOC_EXECUTABLE.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The intent of this change is so that one cat supply -DPROTOBUF_PROTOC_EXECUTABLE=<path/to/protoc> when invoking cmake on the command line.
I assume you mean I should also make -DProtobuf_PROTOC_EXECUTABLE possible, but I don't see a good reason for it. Having to options that do the same thing just creates confusion.

Copy link

Choose a reason for hiding this comment

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

For thus project, doesn't this also need precompiled grpc protoc plugin?

project_build_tools_get_imported_location(PROTOBUF_PROTOC_EXECUTABLE
protobuf::protoc)
# If protobuf::protoc is not a imported target, then we use the target
# directly for fallback
if(NOT PROTOBUF_PROTOC_EXECUTABLE)
set(PROTOBUF_PROTOC_EXECUTABLE protobuf::protoc)
endif()
elseif(Protobuf_PROTOC_EXECUTABLE)
# Some versions of FindProtobuf.cmake uses mixed case instead of uppercase
set(PROTOBUF_PROTOC_EXECUTABLE ${Protobuf_PROTOC_EXECUTABLE})
endif()
elseif(Protobuf_PROTOC_EXECUTABLE)
# Some versions of FindProtobuf.cmake uses mixed case instead of uppercase
set(PROTOBUF_PROTOC_EXECUTABLE ${Protobuf_PROTOC_EXECUTABLE})
endif()
include(CMakeDependentOption)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is unrelated to the rest of the change, but seeing that the module isn't used anywhere in the project I thought I might as well clean it up.


message(STATUS "PROTOBUF_PROTOC_EXECUTABLE=${PROTOBUF_PROTOC_EXECUTABLE}")
set(SAVED_CMAKE_CXX_CLANG_TIDY ${CMAKE_CXX_CLANG_TIDY})
Expand Down