Allow cross-compilation with protobuf #3117
Conversation
✅ Deploy Preview for opentelemetry-cpp-api-docs canceled.
|
| endif() | ||
| include(CMakeDependentOption) |
There was a problem hiding this comment.
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.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3117 +/- ##
==========================================
- Coverage 87.86% 87.84% -0.01%
==========================================
Files 195 195
Lines 6151 6151
==========================================
- Hits 5404 5403 -1
- Misses 747 748 +1 |
| set(PROTOBUF_PROTOC_EXECUTABLE protobuf::protoc) | ||
| if(NOT PROTOBUF_PROTOC_EXECUTABLE) | ||
| # Latest Protobuf imported targets and without legacy module support | ||
| if(TARGET protobuf::protoc) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
For thus project, doesn't this also need precompiled grpc protoc plugin?
|
@tobim Could you please update this PR? Find scripts of some packages are already be added in other PRs. |
The upstream config module from protobuf always declares targets for the protobuf library and protoc of the same platform. This means one of the two is always wrong, as protoc needs to be run on the build machine, but the library must be linked for the "host" platform. A small change in the CMake scaffold allows the user to pass `-DPROTOBUF_PROTOC_EXECUTABLE=<path/to/protoc>` to cmake when creating the build tree.
tobim
force-pushed
the
protobuf-cross-build
branch
from
5c7e514
to
8a48d35
Compare
The upstream config module from protobuf always declares targets for the protobuf library and protoc of the same platform. This means one of the two is always wrong, as protoc needs to be run on the build machine, but the library must be linked for the "host" platform.
A small change in the CMake scaffold allows the user to pass
-DPROTOBUF_PROTOC_EXECUTABLE=<path/to/protoc>to cmake when creating the build tree.Fixes # (issue)
Changes
Please provide a brief description of the changes here.
For significant contributions please make sure you have completed the following items:
CHANGELOG.mdupdated for non-trivial changes