[PATCH] D155111: [clangd] Fix build failures observed on build bots for missing libs

[Ahsan Saghir via Phabricator via cfe-commits]

saghir added a comment.

In D155111#4495131 <https://reviews.llvm.org/D155111#4495131>, @mstorsjo wrote:

> To clarify the issue - the kind of builds that seems to be broken is builds with `BUILD_SHARED_LIBS=ON`. The reason is that these libraries are needed is because the `clangd` target includes `$<TARGET_OBJECTS:obj.clangDaemonTweaks>`, so all the dependencies of `clangDaemonTweaks` would need to be included here as well. Please include that in the commit message description. (Is there a way to pull in those instead of duplicating the list?)
>
> This looks mostly ok to me, but it does add slightly more libraries than what's needed. As the list of libraries that now are linked into `clangdMain` is the list of libraries that previously was linked for the two components that now are `clangd` and `clangdMain`, so some of the dependencies only need to be moved, not duplicated.
>
> A more minimal set of dependencies, which seems to link successfully with `BUILD_SHARED_LIBS=ON`, is achieved with this diff on top of current git main:
>
>   diff --git a/clang-tools-extra/clangd/tool/CMakeLists.txt b/clang-tools-extra/clangd/tool/CMakeLists.txt
>   index ddf9c2488819..6c21175d7687 100644
>   --- a/clang-tools-extra/clangd/tool/CMakeLists.txt
>   +++ b/clang-tools-extra/clangd/tool/CMakeLists.txt
>   @@ -26,11 +26,7 @@ clang_target_link_libraries(clangdMain
>      clangBasic
>      clangFormat
>      clangFrontend
>   -  clangLex
>   -  clangSema
>      clangTooling
>   -  clangToolingCore
>   -  clangToolingRefactoring
>      clangToolingSyntax
>      )
>    
>   @@ -44,7 +40,20 @@ target_link_libraries(clangdMain
>      ${CLANGD_XPC_LIBS}
>      )
>    
>   +clang_target_link_libraries(clangd
>   +  PRIVATE
>   +  clangAST
>   +  clangBasic
>   +  clangLex
>   +  clangSema
>   +  clangToolingCore
>   +  clangToolingRefactoring
>   +  clangToolingSyntax
>   +  )
>   +
>    target_link_libraries(clangd
>      PRIVATE
>      clangdMain
>   +  clangDaemon
>   +  clangdSupport
>      )
>
> Not sure if it's good hygiene to only link specifically to exactly those libraries that are needed and nothing else, or if it's just making things slightly more brittle?

Thanks for reviewing and providing suggestions. I have put up a follow up patch for review: https://reviews.llvm.org/D155540


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155111