[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 , @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 
> `$`, 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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

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 `$`, 
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155111

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Ahsan Saghir via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG915659bfa1e9: [clangd] Fix build failures observed on build 
bots for missing libs (authored by saghir).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155111

Files:
  clang-tools-extra/clangd/tool/CMakeLists.txt


Index: clang-tools-extra/clangd/tool/CMakeLists.txt
===
--- clang-tools-extra/clangd/tool/CMakeLists.txt
+++ clang-tools-extra/clangd/tool/CMakeLists.txt
@@ -44,7 +44,23 @@
   ${CLANGD_XPC_LIBS}
   )
 
+clang_target_link_libraries(clangd
+  PRIVATE
+  clangAST
+  clangBasic
+  clangFormat
+  clangFrontend
+  clangLex
+  clangSema
+  clangTooling
+  clangToolingCore
+  clangToolingRefactoring
+  clangToolingSyntax
+  )
+
 target_link_libraries(clangd
   PRIVATE
   clangdMain
+  clangDaemon
+  clangdSupport
   )


Index: clang-tools-extra/clangd/tool/CMakeLists.txt
===
--- clang-tools-extra/clangd/tool/CMakeLists.txt
+++ clang-tools-extra/clangd/tool/CMakeLists.txt
@@ -44,7 +44,23 @@
   ${CLANGD_XPC_LIBS}
   )
 
+clang_target_link_libraries(clangd
+  PRIVATE
+  clangAST
+  clangBasic
+  clangFormat
+  clangFrontend
+  clangLex
+  clangSema
+  clangTooling
+  clangToolingCore
+  clangToolingRefactoring
+  clangToolingSyntax
+  )
+
 target_link_libraries(clangd
   PRIVATE
   clangdMain
+  clangDaemon
+  clangdSupport
   )
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Nemanja Ivanovic via Phabricator via cfe-commits]

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

Lets get this committed to unblock the bots and if it is not the 
correct/desired fix, the author can subsequently provide the more appropriate 
fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155111

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Ahsan Saghir via Phabricator via cfe-commits]

saghir added a comment.

@mstorsjo @ivanmurashko Can you please take a look at this? This is blocking 
our builds on the PowerPC buildbots. Thanks a lot for your time!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155111

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Ahsan Saghir via Phabricator via cfe-commits]

saghir added a comment.

This is to fix failures in the build bot: 
https://lab.llvm.org/buildbot/#/builders/57/builds/28356


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155111

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Ahsan Saghir via Phabricator via cfe-commits]

saghir created this revision.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
saghir requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

This was broken by 56ac9d46a7c1468d587ccec02a781e52d0bb298a 
.
There were some changes made to fix it in
a20d57e83441a69fa2bab86593b18cc0402095d2 
, but that 
did not quite
fix everything.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155111

Files:
  clang-tools-extra/clangd/tool/CMakeLists.txt


Index: clang-tools-extra/clangd/tool/CMakeLists.txt
===
--- clang-tools-extra/clangd/tool/CMakeLists.txt
+++ clang-tools-extra/clangd/tool/CMakeLists.txt
@@ -44,7 +44,23 @@
   ${CLANGD_XPC_LIBS}
   )
 
+clang_target_link_libraries(clangd
+  PRIVATE
+  clangAST
+  clangBasic
+  clangFormat
+  clangFrontend
+  clangLex
+  clangSema
+  clangTooling
+  clangToolingCore
+  clangToolingRefactoring
+  clangToolingSyntax
+  )
+
 target_link_libraries(clangd
   PRIVATE
   clangdMain
+  clangDaemon
+  clangdSupport
   )


Index: clang-tools-extra/clangd/tool/CMakeLists.txt
===
--- clang-tools-extra/clangd/tool/CMakeLists.txt
+++ clang-tools-extra/clangd/tool/CMakeLists.txt
@@ -44,7 +44,23 @@
   ${CLANGD_XPC_LIBS}
   )
 
+clang_target_link_libraries(clangd
+  PRIVATE
+  clangAST
+  clangBasic
+  clangFormat
+  clangFrontend
+  clangLex
+  clangSema
+  clangTooling
+  clangToolingCore
+  clangToolingRefactoring
+  clangToolingSyntax
+  )
+
 target_link_libraries(clangd
   PRIVATE
   clangdMain
+  clangDaemon
+  clangdSupport
   )
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits