[PATCH] D85815: [clangd] Warn developers when trying to link system-installed gRPC statically

[Sam McCall via Phabricator via llvm-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Just hijacking to ask for some extra comments :-)



================
Comment at: llvm/cmake/modules/FindGRPC.cmake:1
 # This setup requires gRPC to be built from sources using CMake and installed to
 # ${GRPC_INSTALL_PATH} via -DCMAKE_INSTALL_PREFIX=${GRPC_INSTALL_PATH}.
----------------
can we move this comment inside the if?
currently it looks like it applies to the whole file.

And mention whether we're going to link static or dynamically, or if it depends on BUILD_SHARED_LIBS


================
Comment at: llvm/cmake/modules/FindGRPC.cmake:24
 else()
+  if (NOT BUILD_SHARED_LIBS)
+    message(WARNING "gRPC and Protobuf will be linked dynamically. If you want static linking build gRPC from sources.")
----------------
similarly, can we add a comment here? mentioning the overall "system-installed GRPC" idea,
and then explicitly saying something like

"We always link dynamically in this mode. While the static libraries are usually installed, the CMake files telling us *which* static libraries to link are not".


================
Comment at: llvm/cmake/modules/FindGRPC.cmake:25
+  if (NOT BUILD_SHARED_LIBS)
+    message(WARNING "gRPC and Protobuf will be linked dynamically. If you want static linking build gRPC from sources.")
+  endif()
----------------
nit: comma after "linking"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85815