[WIP] protobuf config search mode #1385
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1385 +/- ##
==========================================
+ Coverage 85.36% 85.77% +0.42%
==========================================
Files 151 151
Lines 4362 4362
==========================================
+ Hits 3723 3741 +18
+ Misses 639 621 -18
|
| @@ -292,10 +292,10 @@ endif() | |||
|
|
|||
| if(WITH_OTLP) | |||
| set(protobuf_MODULE_COMPATIBLE ON) | |||
| find_package(Protobuf) | |||
| find_package(Protobuf PATHS ${EXTERNAL_PROTOBUF_DIR}) | |||
There was a problem hiding this comment.
Why we need this? External protobuf and grpc can be found by add installation path into CMAKE_FIND_ROOT_PATH and CMAKE_MODULE_PATH ,or by setting Protobuf_DIR/Protobuf_ROOT and gRPC_DIR/gRPC_ROOT ?
See https://cmake.org/cmake/help/latest/command/find_package.html#id4 for details
There was a problem hiding this comment.
Thanks @owent, I used your suggestion on the issue, and it works without my changes.
I have another issue now. The files for this library
opentelemetry-cpp/cmake/opentelemetry-proto.cmake
Lines 191 to 203 in 54abc27
otherwise I have to close this PR.
There was a problem hiding this comment.
Sorry, I don't really get the point about the files are not available when calling make. These files are generated by add_custom_command(OUTPUT [files] above.
This command should run when OUTPUT files are expired or not found and when they are depended to build any target.
If you are writing a standalone Makefile, I think it's required to write a similar TARGET of Makefile to generate these files.
There was a problem hiding this comment.
The problem is that, the files are not there and custome_command is ran on build time not config time. on build time when I use 16 cores to build most of the times the PB files are not generated before the compile of proto library. I think this can be solved by using execute_process.
There was a problem hiding this comment.
Why we need the generated files at config time?If we want a standalone target to just generate these files, but not compile them. I think the better way is adding a custom target by add_custom_target(generate_opentelemetry_proto SOURCES [source files...]) and run cmake --build . --target generate_opentelemetry_proto after configure is done.
There was a problem hiding this comment.
thanks @owent. I'm going to close this PR.
| if(WITH_OTLP_GRPC OR (NOT DEFINED WITH_OTLP_GRPC AND NOT DEFINED | ||
| CACHE{WITH_OTLP_GRPC})) | ||
| find_package(gRPC) | ||
| find_package(gRPC PATHS ${EXTERNAL_GRPC_DIR}) |
There was a problem hiding this comment.
Haven't seen where ${EXTERNAL_GRPC_DIR} is defined?
There was a problem hiding this comment.
this was not defined. I' going to close the PR as the changes are not needed.
|
Closing the PR as no change is needed here. |
Fixes #1384 (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