This is an archive of the discontinued LLVM Phabricator instance.

Make lldb tools dependent on liblldb target when building LLDB.framework with CMake
AbandonedPublic

Authored by xiaobai on Jun 5 2018, 2:28 PM.

Details

Summary

When you build LLDB.framework with cmake, the liblldb target is built
as a framework. When lldb tools are built with INCLUDE_IN_FRAMEWORK, then
LLDB.framework should depend on those tools. This means making liblldb depend on
those tools.

Event Timeline

xiaobai created this revision.Jun 5 2018, 2:28 PM
sas added a comment.Jun 5 2018, 4:02 PM

Don't we risk creating circular dependencies with this? What happens when a tool depends on liblldb? you'll have theTool depend on liblldb and liblldb depend on theTool.

In D47801#1123051, @sas wrote:

Don't we risk creating circular dependencies with this? What happens when a tool depends on liblldb? you'll have theTool depend on liblldb and liblldb depend on theTool.

The same thought has occurred to me, but I believe this should be fine. No existing INCLUDE_IN_FRAMEWORK tool links to liblldb, just it's individual components, which is fine. And this is probably a state that we should maintain for the future.

What I am thinking about is how to reconcile this with the code in tools/driver/CMakeLists.txt which tries to do a very similar thing by adding dependencies from lldb->{lldb-server,debugserver}. Ideally, we shouldn't need to do this sort of thing twice. And indeed, since lldb driver depends on liblldb, with this patch the branches we are adding to lldb would already be present implicitly. The only catch there is we would need to make this dependency unconditional, but that is probably a good thing anyway, as it makes the dependency graph more predictable.

So, how about we do this instead:

  • rename INCLUDE_IN_FRAMEWORK to something more neutral (USED_BY_LIBLLDB or whatever)
  • make the liblldb -> tool dependency not conditioned by LLDB_BUILD_FRAMEWORK
  • remove the lldb->tool dependencies altogether

WDYT?

No existing INCLUDE_IN_FRAMEWORK tool links to liblldb, just it's individual components, which is fine. And this is probably a state that we should maintain for the future.

I think I agree with you here.

So, how about we do this instead:

  • rename INCLUDE_IN_FRAMEWORK to something more neutral (USED_BY_LIBLLDB or whatever)
  • make the liblldb -> tool dependency not conditioned by LLDB_BUILD_FRAMEWORK
  • remove the lldb->tool dependencies altogether

WDYT?

I think that this a good short-term solution. My concern is your suggestion doesn't reflect the actual dependencies, but conveniently errs on the side of "build it if we think we might need it". I'm not against your suggestion, but I'd like to see if we can come up with something better.
One idea I had was to introduce another target for the framwork itself, e.g. lldbFramework, which gets built if LLDB_BUILD_FRAMEWORK is set. It would depend on liblldb and all the necessary tools, headers, etc, that the framework would need. That way liblldb can depend only on what it needs to build instead of treating it as both the library and the entire framework. How do you feel about this?

sas added a comment.Jun 6 2018, 11:35 AM

One idea I had was to introduce another target for the framwork itself, e.g. lldbFramework, which gets built if LLDB_BUILD_FRAMEWORK is set. It would depend on liblldb and all the necessary tools, headers, etc, that the framework would need. That way liblldb can depend only on what it needs to build instead of treating it as both the library and the entire framework. How do you feel about this?

I think this is the cleanest way of handling this. If @labath agrees and if it's not too much of a pain to implement in cmake, we should do this.

  • rename INCLUDE_IN_FRAMEWORK to something more neutral (USED_BY_LIBLLDB or whatever)
  • make the liblldb -> tool dependency not conditioned by LLDB_BUILD_FRAMEWORK
  • remove the lldb->tool dependencies altogether

WDYT?

I think that this a good short-term solution. My concern is your suggestion doesn't reflect the actual dependencies, but conveniently errs on the side of "build it if we think we might need it". I'm not against your suggestion, but I'd like to see if we can come up with something better.
One idea I had was to introduce another target for the framwork itself, e.g. lldbFramework, which gets built if LLDB_BUILD_FRAMEWORK is set. It would depend on liblldb and all the necessary tools, headers, etc, that the framework would need. That way liblldb can depend only on what it needs to build instead of treating it as both the library and the entire framework. How do you feel about this?

It all comes down to how do you want to view the "liblldb" cmake target. Whether it is just "things that are needed to build liblldb.so binary itself", or whether "everything needed to make liblldb functional". In the second POV, it is natural to have liblldb depend on lldb-server, since lldb-server is necessary to have a fully functional liblldb. However, I can see reasons against that too, so I am fine with your proposed solution.

The thing I would suggest, if it is not too much trouble, is again to make the new target framework-agnostic. I.e., in a LLDB_BUILD_FRAMEWORK build, it would build everything that goes into a framework and package it. Otherwise, it would just be a convenient way to refer to liblldb and everything that is needed to make it functional. The reason I'm suggesting that is that then we could remove the additional dependency management when building lldb driver by just making the driver depend on this new target.

  • rename INCLUDE_IN_FRAMEWORK to something more neutral (USED_BY_LIBLLDB or whatever)
  • make the liblldb -> tool dependency not conditioned by LLDB_BUILD_FRAMEWORK
  • remove the lldb->tool dependencies altogether

WDYT?

I think that this a good short-term solution. My concern is your suggestion doesn't reflect the actual dependencies, but conveniently errs on the side of "build it if we think we might need it". I'm not against your suggestion, but I'd like to see if we can come up with something better.
One idea I had was to introduce another target for the framwork itself, e.g. lldbFramework, which gets built if LLDB_BUILD_FRAMEWORK is set. It would depend on liblldb and all the necessary tools, headers, etc, that the framework would need. That way liblldb can depend only on what it needs to build instead of treating it as both the library and the entire framework. How do you feel about this?

The thing I would suggest, if it is not too much trouble, is again to make the new target framework-agnostic. I.e., in a LLDB_BUILD_FRAMEWORK build, it would build everything that goes into a framework and package it. Otherwise, it would just be a convenient way to refer to liblldb and everything that is needed to make it functional. The reason I'm suggesting that is that then we could remove the additional dependency management when building lldb driver by just making the driver depend on this new target.

I think I can try to make that happen.

xiaobai abandoned this revision.Jun 6 2018, 1:03 PM

Going to go with the direction we discussed here. This shouldn't be needed.