[CMake] Update FindGPRC to use add_llvm_library

[steven_wu]

Summary

add_clang_library is not available in components other than clang and
clang-tool-extras. Trying to use this module elsewhere will cause cmake
error. add_clang_library doesn't seem necessary for clangd's GRPC.
Change it to a more generic add_llvm_library so it is easier to build
other downstream projects with GRPC that doesn't depend on clang.

[steven_wu]

@sammccall any concerns for this? If not, I will just commit and see if breaks anything.

[sammccall]

Hi Steven,

Sorry for missing this, I've been out on medical leave & vacation.
I don't see any big problems here, the usage of add_clang_library seems to be mostly passthrough to add_llvm_library.

I'm not certain about all the platforms/build modes but agree that seeing if it breaks something is reasonable.
That said, this sort of change can break somewhat obscure configurations and may need a fix/revert at some later point, which could be awkward if you have downstream dependencies but nothing in-tree to guarantee this works.

(I'm curious what you plan to use grpc for, if that's public)

llvm/cmake/modules/FindGRPC.cmake

1

There's a mention of clangd here that should probably be made more generic too

45

another mention of clangd

[steven_wu]

In D131593#3725476, @sammccall wrote:

Hi Steven,

Sorry for missing this, I've been out on medical leave & vacation.
I don't see any big problems here, the usage of add_clang_library seems to be mostly passthrough to add_llvm_library.

Thanks for looking! I pulled the trigger and didn't see anything broken from bots yet. If there are any downstream bots I don't have access that you seen broken, please let me know and I can help investigating.

I'm not certain about all the platforms/build modes but agree that seeing if it breaks something is reasonable.
That said, this sort of change can break somewhat obscure configurations and may need a fix/revert at some later point, which could be awkward if you have downstream dependencies but nothing in-tree to guarantee this works.

(I'm curious what you plan to use grpc for, if that's public)

This is just a cleanup that I am preparing/experimenting with CAS integration. (See RFC: https://discourse.llvm.org/t/rfc-add-an-llvm-cas-library-and-experiment-with-fine-grained-caching-for-builds). I was playing with some solutions to integrate 3rd party CAS with the LLVM CAS interface we are proposing and want to use gRPC to connect with other CAS solutions as an option.

I currently have a PR open on Apple Github to do some experiments with GRPC CAS: https://github.com/apple/llvm-project/pull/5033. The plan is to upstream this GRPC protocol unless there are objections. There are some dependencies we need to upstream first before we can get there.

I will cleanup the option names that mentions clangd in a followup.

[steven_wu]

Cleanup here: https://reviews.llvm.org/rG07c2f592a69eba5cd644d3467a4d427ed6262f47

[rymiel]

@sammccall

That said, this sort of change can break somewhat obscure configurations and may need a fix/revert at some later point

Could you please have a look at https://github.com/llvm/llvm-project/issues/58075? I feel like my configuration isn't obscure, I'm simply trying to use LLVM as a CMake library, but I doubt myself because something that simple should have been caught by other people.

[sammccall]

In D131593#3847986, @rymiel wrote:

@sammccall

That said, this sort of change can break somewhat obscure configurations and may need a fix/revert at some later point

Could you please have a look at https://github.com/llvm/llvm-project/issues/58075? I feel like my configuration isn't obscure, I'm simply trying to use LLVM as a CMake library, but I doubt myself because something that simple should have been caught by other people.

@steven_wu are you able to take a look at this?

I'm not familiar with consuming LLVM as a library through CMake.

A glance through the macros suggests maybe we want to pass BUILDTREE_ONLY to add_llvm_library, but that's a complete guess.

[steven_wu]

@sammccall move the discussion to the bug report. Let me know if you have any opinion.

llvm/cmake/modules/FindGRPC.cmake

 option(ENABLE_GRPC_REFLECTION "Link clangd-index-server to gRPC Reflection library" OFF)
 
 # FIXME(kirillbobyrev): Check if gRPC and Protobuf headers can be included at
 # configure time.
 find_package(Threads REQUIRED)
 if (GRPC_INSTALL_PATH)
   # This setup requires gRPC to be built from sources using CMake and installed
   # to ${GRPC_INSTALL_PATH} via -DCMAKE_INSTALL_PREFIX=${GRPC_INSTALL_PATH}.
   # Libraries will be linked according to gRPC build policy which generates
else()
   # usually installed, the CMake files telling us *which* static libraries to
   # link are not.
   if (NOT BUILD_SHARED_LIBS)
     message(NOTICE "gRPC and Protobuf will be linked dynamically. If you want static linking, build gRPC from sources with -DBUILD_SHARED_LIBS=Off.")
   endif()
   find_program(GRPC_CPP_PLUGIN grpc_cpp_plugin)
   find_program(PROTOC protoc)
   if (NOT GRPC_CPP_PLUGIN OR NOT PROTOC)
     message(FATAL_ERROR "gRPC C++ Plugin and Protoc must be on $PATH for Clangd remote index build.")
   endif()
   # On macOS the libraries are typically installed via Homebrew and are not on
   # the system path.
   set(GRPC_OPTS "")
   set(PROTOBUF_OPTS "")
   set(GRPC_INCLUDE_PATHS "")
   if (${APPLE})
     find_program(HOMEBREW brew)
if (PROTO_GRPC)
     list(APPEND GeneratedProtoHeader "${CMAKE_CURRENT_BINARY_DIR}/${Basename}.grpc.pb.h")
   endif()
   add_custom_command(
         OUTPUT ${GeneratedProtoSource} ${GeneratedProtoHeader}
         COMMAND ${PROTOC}
         ARGS ${Flags} "${ProtoSourceAbsolutePath}"
         DEPENDS "${ProtoSourceAbsolutePath}")
 
-  add_clang_library(${LibraryName} ${GeneratedProtoSource}
+  add_llvm_library(${LibraryName} ${GeneratedProtoSource}
     PARTIAL_SOURCES_INTENDED
     LINK_LIBS PUBLIC grpc++ protobuf)
 
   # Ensure dependency headers are generated before dependent protos are built.
   # DEPENDS arg is a list of "Foo.proto". While they're logically relative to
   # the source dir, the generated headers we need are in the binary dir.
   foreach(ImportedProto IN LISTS PROTO_DEPENDS)
     # Foo.proto -> Foo.pb.h