[PATCH] D127893: [CMake] Fix `FindGRPC.cmake` for setting up gRPC related libraries for macOS+homebrew context

[Argyrios Kyrtzidis via Phabricator via llvm-commits]

akyrtzi 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)
----------------
compnerd wrote:
> 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.
I can give it a try, will this make the search path propagate to all the targets that depend on `grpc++`?


================
Comment at: llvm/cmake/modules/FindGRPC.cmake:79
+      if (ABSL_HOMEBREW_RETURN_CODE EQUAL "0")
+        include_directories(${ABSL_HOMEBREW_PATH}/include)
+      endif()
----------------
compnerd wrote:
> 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?
The gRPC headers include abseil headers.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127893/new/

https://reviews.llvm.org/D127893