[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