This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Warn developers when trying to link system-installed gRPC statically
ClosedPublic

Authored by kbobyrev on Aug 12 2020, 1:02 AM.

Diff Detail

Event Timeline

kbobyrev created this revision.Aug 12 2020, 1:02 AM
kbobyrev requested review of this revision.Aug 12 2020, 1:02 AM
sammccall accepted this revision.Aug 13 2020, 7:33 AM

Just hijacking to ask for some extra comments :-)

llvm/cmake/modules/FindGRPC.cmake
1

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

24

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".

25

nit: comma after "linking"

This revision is now accepted and ready to land.Aug 13 2020, 7:33 AM
kbobyrev updated this revision to Diff 285577.Aug 14 2020, 1:21 AM
kbobyrev marked 3 inline comments as done.

Address post-LGTM comments.

This revision was landed with ongoing or failed builds.Aug 14 2020, 1:22 AM
This revision was automatically updated to reflect the committed changes.