[PATCH] D81967: [clang-tools-extra] Prevent linking to duplicate .a libs and dylib

[Michał Górny via Phabricator via cfe-commits]

mgorny marked 2 inline comments as done.
mgorny added inline comments.


================
Comment at: clang-tools-extra/clangd/CMakeLists.txt:105
+
+clang_target_link_libraries(clangDaemon
+  PRIVATE
----------------
sammccall wrote:
> This has split our link dependencies in two, and I'm not really sure why (and so am likely to put future dependencies in the wrong place).
> 
> Can *all* the link dependencies be moved to the clang_target_link_libraries section?
> ((Even if this addresses a problem only seen with clang libs, I'd rather have everything in one place)
No. `clang_target_link_libraries` is only for libraries that are included in `libclang-cpp.so`, so when the latter is built, they are replaced with it entirely. I'm all for improving this (e.g. to automatically replace only these libs that are included in clang-cpp) but I don't really have time to work on this beyond fixing immediate failures.


================
Comment at: clang-tools-extra/clangd/unittests/CMakeLists.txt:123
   clangTidy
-  LLVMSupport
   LLVMTestingSupport
----------------
sammccall wrote:
> why this change? We do depend directly on LLVMSupport, and I'd prefer that to remain explicit rather than pick it up transitively.
It's already listed in `LLVM_LINK_COMPONENTS` which handles using libLLVM correctly. Listing it here results in another copy of LLVMSupport being linked in which caused the test to crash immediately on duplicate command-line options.


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

https://reviews.llvm.org/D81967