[PATCH] D127893: [CMake] Fix `FindGRPC.cmake` for setting up gRPC related libraries for macOS+homebrew context
Saleem Abdulrasool via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Jun 15 12:59:14 PDT 2022
compnerd added inline comments.
================
Comment at: llvm/cmake/modules/FindGRPC.cmake:71
if (GRPC_HOMEBREW_RETURN_CODE EQUAL "0")
include_directories(${GRPC_HOMEBREW_PATH}/include)
list(APPEND GRPC_OPTS PATHS ${GRPC_HOMEBREW_PATH}/lib NO_DEFAULT_PATH)
----------------
I realize that this is something that exists prior to your change, but it seems better to actually do this as `target_include_directories(grpc++ PUBLIC ${GRPC_HOMEBREW_PATH}/include)` which avoids the extra includes from being put into all targets.
================
Comment at: llvm/cmake/modules/FindGRPC.cmake:79
+ if (ABSL_HOMEBREW_RETURN_CODE EQUAL "0")
+ include_directories(${ABSL_HOMEBREW_PATH}/include)
+ endif()
----------------
Why is the header search path for all targets being modified to add abseil? Is that a dependency for some other target that is being imported here? Perhaps we should add it to that particular library and let CMake propagate it properly?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127893/new/
https://reviews.llvm.org/D127893
More information about the llvm-commits
mailing list