[PATCH] D79387: [clangd] Fix remote index build for macOS with Homebrew-installed gRPC and Protobuf

[Kirill Bobyrev via Phabricator via llvm-commits]

kbobyrev added inline comments.


================
Comment at: llvm/cmake/modules/FindGRPC.cmake:24
   find_program(PROTOC protoc)
+  if (${APPLE})
+    find_program(HOMEBREW brew)
----------------
sammccall wrote:
> sammccall wrote:
> > This could use some comments,  "fall back to homebrew-installed libraries, which typically aren't on the system path."
> this block should only run if we didn't already find the proto tools, right?
Not really, the proto tools are in `$PATH`, but include directories and libraries aren't in default search paths.


================
Comment at: llvm/cmake/modules/FindGRPC.cmake:42
 endif()
 
 # Proto headers are generated in ${CMAKE_CURRENT_BINARY_DIR}.
----------------
sammccall wrote:
> at this point it would be nice to have have a cmake check that protoc and grpc_cpp_plugin both exist, and that we can include the proto and grpc headers.
> This would ensure we get errors at configure time instead of build time.
> 
> (unrelated though, separate change if you want to do this)
Good point! I've added a check for protoc and grpc_cpp_plugin and a FIXME regarding the ability to include headers. I'll do that in a separate patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79387