This revision was automatically updated to reflect the committed changes.
Closed by commit rL293686: [CMake] Add accurate dependency specifications
(authored by cbieneman).
Changed prior to commit:
https://reviews.llvm.org/D29333?vs=86461&id=86481#toc
Repository:
rL LLVM
https://reviews.llv
beanz added a comment.
@labath, we'll have to see if CMake's handling is good enough. What CMake
actually does is repeat the full chain of the cycle, so it would be something
like adding `-lX -lY -lX -lY` in your example. This doesn't work for certain
pathological cases, but CMake actually has
labath added a comment.
In https://reviews.llvm.org/D29333#661994, @beanz wrote:
> In https://reviews.llvm.org/D29333#661979, @labath wrote:
>
> > I was thinking about that as well. I am not sure if it will work if we
> > actually need multiple iterations of the loop to get all the dependencies
beanz added a comment.
In https://reviews.llvm.org/D29333#661979, @labath wrote:
> I was thinking about that as well. I am not sure if it will work if we
> actually need multiple iterations of the loop to get all the dependencies
> converging, but it may be worth trying out.
The way it is sup
labath accepted this revision.
labath added a comment.
In https://reviews.llvm.org/D29333#661960, @beanz wrote:
> Thankfully CMake will not complain about circular dependencies in static
> archive targets. If it did, we'd really be in trouble because the LLDB
> dependencies graph has *lots* of
beanz added a comment.
@zturner, absolutely! Thanks!
I expect this patch and the next one to be pretty safe because I'm not actually
removing the existing dependency specifications, just adding new explicit ones.
https://reviews.llvm.org/D29333
__
zturner accepted this revision.
zturner added a comment.
This revision is now accepted and ready to land.
ok, lgtm with the caveat that if it breaks anything on Windows we might have to
revert until we figure it out.
https://reviews.llvm.org/D29333
___
beanz added a comment.
Thankfully CMake will not complain about circular dependencies in static
archive targets. If it did, we'd really be in trouble because the LLDB
dependencies graph has *lots* of circular dependencies. Actually, I think CMake
even handles them properly on Linux, which shoul
zturner added a comment.
As long as you considered the tradeoffs then and you like this better, that's
fine then.
Is CMake going to complain about circular dependencies? For example,
Breakpoint depends on Core and Core depends on Breakpoint.
https://reviews.llvm.org/D29333
___
beanz added a comment.
@zturner That mechanism does work, however I generally find that it is cleaner
to pass named parameters into CMake functions which provides a bit of a
self-documenting API, as opposed to relying on known named variables. The
named-parameter was introduced for LLD, and I t
zturner added a comment.
Any reason this doesn't use the same strategy as LLVM? I.e. at the top of the
`CMakeLists.txt` file, write something like `set(LLVM_LINK_COMPONENTS Foo Bar
Baz)`, then just call `add_lldb_library` etc.
https://reviews.llvm.org/D29333
___
beanz updated this revision to Diff 86461.
beanz added a comment.
Updates based on post commit feedback from labath.
He was right on both points. LLVM libs should be done through LINK_COMPONENTS,
and I messed up lldb-server's dependencies.
https://reviews.llvm.org/D29333
Files:
source/Break
labath added a comment.
I like where this is going. I am not sure about a couple of details though.
Please see comments.
Comment at: source/Breakpoint/CMakeLists.txt:32
+lldbPluginObjCLanguage
+LLVMSupport
)
Shouldn't this be: `LINK_COMPONENTS Suppo
13 matches
Mail list logo
