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

Can not use prebuilt protobuf of parent project with cmake. #740

Closed
owent opened this issue May 11, 2021 · 4 comments · Fixed by #742
Closed

Can not use prebuilt protobuf of parent project with cmake. #740

owent opened this issue May 11, 2021 · 4 comments · Fixed by #742
Labels
bug Something isn't working

Comments

@owent
Copy link
Member

owent commented May 11, 2021

Describe your environment
CMake, Windows

Steps to reproduce
Having a prebuilt of protobuf.

What is the expected behavior?
Do not trigger vcpkg.

What is the actual behavior?
Using vcpkg to installl protobuf again.

Additional context
Should use Protobuf_FOUND instead of protobuf_FOUND .

@owent owent added the bug Something isn't working label May 11, 2021
owent pushed a commit to owent/opentelemetry-cpp that referenced this issue May 11, 2021
Signed-off-by: owent <owent@tencent.com>
@owent owent mentioned this issue May 11, 2021
3 tasks
owent added a commit to owent/opentelemetry-cpp that referenced this issue May 11, 2021
Signed-off-by: owent <owent@tencent.com>
owent added a commit to owent/opentelemetry-cpp that referenced this issue May 11, 2021
Signed-off-by: owent <admin@owent.net>
owent added a commit to owent/opentelemetry-cpp that referenced this issue May 11, 2021
Signed-off-by: owent <admin@owent.net>
@ThomsonTan
Copy link
Contributor

@owent if you are using a prebuilt protobuf, how about gRPC? I think gRPC will be needed for most cases when protobuf is used in this repo.

@owent
Copy link
Member Author

owent commented May 12, 2021

@owent if you are using a prebuilt protobuf, how about gRPC? I think gRPC will be needed for most cases when protobuf is used in this repo.

@ThomsonTan Using -DgRPC_PROTOBUF_PROVIDER=package, and add install path to CMAKE_PREFIX_PATH or set Protobuf_DIR then we can use our prebuilt protobuf.So does other dependencies of gRPC.The document is here

I create a script toolset to build all denpendencies of gRPC.(https://github.com/atframework/cmake-toolset)
And I also build all dependencies of opentelemetry with it(except of ms-gsl, which is include by submodule instead of find_package).

The script to build opentelemetry-cpp is here

@maxgolov
Copy link
Contributor

It might be different depending on what version of CMake you use. I've seen it with Gtest_FOUND and gtest_FOUND, which is different depending on whether you use newer or older package finder. Could it be something like this, can we make both checks?

owent added a commit to owent/opentelemetry-cpp that referenced this issue May 12, 2021
Signed-off-by: owent <admin@owent.net>
@owent
Copy link
Member Author

owent commented May 12, 2021

It might be different depending on what version of CMake you use. I've seen it with Gtest_FOUND and gtest_FOUND, which is different depending on whether you use newer or older package finder. Could it be something like this, can we make both checks?

Thanks, I checked the document of cmake just now and found FindProtobuf.cmake will set Protobuf_FOUND from cmake 3.6 and FindGTest.cmake will set GTest_FOUND from cmake 3.20 in the legacy MODULE mode.

I have add codes to check PROTOBUF_FOUND and use Protobuf_PROTOC_EXECUTABLE to overwrite PROTOBUF_PROTOC_EXECUTABLE when it exists. But I also found both GTEST_FOUND and GTest_FOUND are already checked.

I think the case should be the same as the package name passed into find_package, so we should use GTest_FOUND instead of gtest_FOUND and it's already the right way.

owent added a commit to owent/opentelemetry-cpp that referenced this issue May 14, 2021
Signed-off-by: owent <admin@owent.net>
lalitb pushed a commit that referenced this issue May 14, 2021
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.

3 participants