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

adding CMAKE_TOOLCHAIN_FILE for source ExternalProject #374

Merged
merged 1 commit into from
May 10, 2021

Conversation

okhowang
Copy link
Contributor

@okhowang okhowang commented May 8, 2021

adding CMAKE_TOOLCHAIN_FILE for source ExternalProject

@rizsotto
Copy link
Owner

rizsotto commented May 9, 2021

Hey @okhowang , thanks for making this PR. I might ask, what problem it solves?

@okhowang
Copy link
Contributor Author

okhowang commented May 9, 2021

adding CMAKE_TOOLCHAIN_FILE for source ExternalProject

I use vcpkg for dependency. It use CMAKE_TOOLCHAIN_FILE to setup some environment. but Bear don't pass it to ExternalProject_Add.

prefer using find_package for gPRC

grpc's pc file is lake of dependencies of grpc, for exmaple: upb and other. it means that grpc's pkg-config is poor-maintained.
but it has well-maintained cmake config.

@rizsotto
Copy link
Owner

rizsotto commented May 9, 2021

adding CMAKE_TOOLCHAIN_FILE for source ExternalProject

I use vcpkg for dependency. It use CMAKE_TOOLCHAIN_FILE to setup some environment. but Bear don't pass it to ExternalProject_Add.

This sounds ok.

prefer using find_package for gPRC

grpc's pc file is lake of dependencies of grpc, for exmaple: upb and other. it means that grpc's pkg-config is poor-maintained.
but it has well-maintained cmake config.

This is more a package policy. Many distributions do not installs the CMake config files, but all installs the pkg-config ones... So, when it starts to do the if-then-else for this dependency it might be harder to support people. It might have a good logs during the build... But need to check if the third_party/grpc/CMakeLists.txt is using this or not.

Can I have these two as a separate PRs? I would merge the CMAKE_TOOLCHAIN_FILE change, but need to think about the gRPC CMake change. Could you make another PR with the CMAKE_TOOLCHAIN_FILE change?

@okhowang okhowang changed the title build: prefer using find_package for gPRC adding CMAKE_TOOLCHAIN_FILE for source ExternalProject May 10, 2021
@okhowang
Copy link
Contributor Author

okhowang commented May 10, 2021

Can I have these two as a separate PRs?

I have made this PR doing work about CMAKE_TOOLCHAIN_FILE only.

grpc's PR is here

@rizsotto rizsotto merged commit dfa9e26 into rizsotto:master May 10, 2021
@rizsotto
Copy link
Owner

Thanks @okhowang !!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants