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

cmake fails to correctly use non-imported protobuf::protoc target #2360

Closed
andremarianiello opened this issue Oct 11, 2023 · 0 comments · Fixed by #2362
Closed

cmake fails to correctly use non-imported protobuf::protoc target #2360

andremarianiello opened this issue Oct 11, 2023 · 0 comments · Fixed by #2362
Labels
bug Something isn't working

Comments

@andremarianiello
Copy link
Contributor

andremarianiello commented Oct 11, 2023

Describe your environment Describe any aspect of your environment relevant to the problem, including your platform, build system, version numbers of installed dependencies, etc. If you're reporting a problem with a specific version of a library in this repo, please check whether the problem has been fixed on main branch.

Rocky Linux 9, cmake 3.27.7, protobuf 21.12

Steps to reproduce
Create a local project (myproject). Add Protobuf and opentelemetry-cpp as dependencies using FetchContent_Declare, with OVERRIDE_FIND_PACKAGE, then add opentelemetry-cpp to the project via find_package/FetchContent_MakeAvailable/add_subdirectory. Then configure and build

cmake_minimum_required(VERSION 3.24)

project(myproject)

include(FetchContent)

set(protobuf_BUILD_TESTS OFF CACHE INTERNAL "" FORCE)
set(protobuf_BUILD_SHARED_LIBS ON CACHE INTERNAL "" FORCE)
FetchContent_Declare(
  Protobuf
  GIT_REPOSITORY "https://github.com/protocolbuffers/protobuf.git"
  GIT_TAG "v21.12"
  OVERRIDE_FIND_PACKAGE
  )

set(BUILD_TESTING OFF CACHE BOOL "" FORCE)
set(WITH_EXAMPLES OFF CACHE BOOL "" FORCE)
set(WITH_OTLP_HTTP ON CACHE BOOL "" FORCE) # triggers the inclusion of protobuf
FetchContent_Declare(
  opentelemetry-cpp
  GIT_REPOSITORY "https://github.com/open-telemetry/opentelemetry-cpp.git"
  GIT_TAG "main"
  )

FetchContent_MakeAvailable(opentelemetry-cpp)

What is the expected behavior?
I would expect a full build to succeed, and I would expect opentelemetry-proto to build using the protoc built from the Declared protobuf package.

What is the actual behavior?
We fail to build opentelemetry-proto protos

[ 27%] [Run]: "" "--proto_path=/root/tmp/build/_deps/opentelemetry-cpp-src/third_party/opentelemetry-proto" "--cpp_out=/root/tmp/build/generated/third_party/opentelemetry-proto" "/root/tmp/build/_deps/opentelemetry-cpp-src/third_party/opentelemetry-proto/opentelemetry/proto/common/v1/common.proto" "/root/tmp/build/_deps/opentelemetry-cpp-src/third_party/opentelemetry-proto/opentelemetry/proto/resource/v1/resource.proto" "/root/tmp/build/_deps/opentelemetry-cpp-src/third_party/opentelemetry-proto/opentelemetry/proto/trace/v1/trace.proto" "/root/tmp/build/_deps/opentelemetry-cpp-src/third_party/opentelemetry-proto/opentelemetry/proto/logs/v1/logs.proto" "/root/tmp/build/_deps/opentelemetry-cpp-src/third_party/opentelemetry-proto/opentelemetry/proto/metrics/v1/metrics.proto" "/root/tmp/build/_deps/opentelemetry-cpp-src/third_party/opentelemetry-proto/opentelemetry/proto/collector/trace/v1/trace_service.proto" "/root/tmp/build/_deps/opentelemetry-cpp-src/third_party/opentelemetry-proto/opentelemetry/proto/collector/logs/v1/logs_service.proto" "/root/tmp/build/_deps/opentelemetry-cpp-src/third_party/opentelemetry-proto/opentelemetry/proto/collector/metrics/v1/metrics_service.proto"
/bin/sh: line 1: --proto_path=/root/tmp/build/_deps/opentelemetry-cpp-src/third_party/opentelemetry-proto: No such file or directory

Notice the first argument is "" which is supposed to be the path to protoc.

Additional context
The top-level CMakeLists.txt puts the path to protoc in PROTOBUF_PROTOC_EXECUTABLE via calls to find_package(Protobuf). However, when find_package is overridden (for any reason - OVERRIDE_FIND_PACKAGE) is just one way) then PROTOBUF_PROTOC_EXECUTABLE is not set. There is code to extract an executable path from an imported protobuf::protoc target, BUT this only works if it is actually an imported target. If that happens to be an ALIAS target, like would be the case when overriding find_package, then PROTOBUF_PROTOC_EXECUTABLE remains unset, causing this problem.

Workaround
You can attempt to work around this problem by defining PROTOBUF_PROTOC_EXECUTABLE yourself. In my example you could use

set(PROTOBUF_PROTOC_EXECUTABLE ${CMAKE_BINARY_DIR}_deps/protobuf-build/protoc)

though

FetchContent_MakeAvailable(Protobuf)
set(PROTOBUF_PROTOC_EXECUTABLE ${protobuf_BINARY_DIR}/protoc)

is probably more correct. Neither way solve the whole problem though. The protoc command will be correct, but protoc itself might not yet exists. This is because passing the path to opentelemetry-proto this way doesn't impose a dependency on protoc. You can workaround this problem by adding

add_dependencies(opentelemetry_proto protobuf::protoc)

but IMO none of these workarounds should be required.

@andremarianiello andremarianiello added the bug Something isn't working label Oct 11, 2023
@github-actions github-actions bot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Oct 11, 2023
@marcalff marcalff removed the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Oct 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants