[Lldb-commits] [PATCH] D48060: Introduce lldb-framework CMake target and centralize its logic

[Alex Langford via Phabricator via lldb-commits]

xiaobai added inline comments.


================
Comment at: cmake/modules/LLDBFramework.cmake:45
+
+  add_dependencies(lldb-framework liblldb lldb-argdumper lldb-server lldb-framework-headers)
+  add_dependencies(finish_swig lldb-framework)
----------------
labath wrote:
> xiaobai wrote:
> > labath wrote:
> > > Maybe lldb-argdumper and lldb-server dependencies should be handled as a part of INCLUDE_IN_FRAMEWORK argument to add_lldb_executable? Or do you have other plans for that?
> > One of the goals I had in centralizing the framework generation code would be to centralize and make explicit which tools went into the framework. The idea I had was to get rid of the INCLUDE_IN_FRAMEWORK argument to add_lldb_executable. Since add_lldb_executable builds the binary differently when building for the framework (modifying the rpath, changing the output destination and symlinking it to your build tree's bin dir), it should be sufficient just to check for LLDB_BUILD_FRAMEWORK.
> > 
> > What do you think of this?
> Both of the approaches sound reasonable to me. If you want to go this way, then I'm fine with that.
I've begun refactoring to remove `INCLUDE_IN_FRAMEWORK` but I've started thinking about some possibly negative repercussions. I'm wondering what you think of this:

`add_lldb_tool` (which invokes `add_lldb_executable`) can be used to add lldb executables that won't be included in the framework AFAICT (e.g. lldb-mi). What I was going to do was remove `INCLUDE_IN_FRAMEWORK` option so that every tool will get put into the framework and symlinked to `$BUILD_DIR/bin` when you run cmake with `LLDB_BUILD_FRAMEWORK=1`. This will mean that lldb-mi will be put in the framework if you do something like `make lldb-mi` after configuring with `LLDB_BUILD_FRAMEWORK=1`.

In that case, do you think it makes sense to keep `INCLUDE_IN_FRAMEWORK` as an option and use that to build part of the dependency tree for the lldb-framework target? Or do you think this is an unlikely enough example that it shouldn't matter?


https://reviews.llvm.org/D48060