rizsotto / Bear

Bear is a tool that generates a compilation database for clang tooling.
GNU General Public License v3.0
4.79k stars 314 forks source link

adding CMAKE_TOOLCHAIN_FILE for source ExternalProject #374

Closed okhowang closed 3 years ago

okhowang commented 3 years ago

adding CMAKE_TOOLCHAIN_FILE for source ExternalProject

rizsotto commented 3 years ago

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

okhowang commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

Thanks @okhowang !!!