Remove lldb-private headers when building LLDB.framework with CMake

[xiaobai]

Summary

Generating LLDB.framework when building with CMake+Ninja will copy the
lldb-private headers because public_headers contains them, even though we try
to make sure they don't get copied by removing root_private_headers from
root_public_headers.

This patch also removes SystemInitializerFull.h from the LLDB.framework headers when building with CMake.

[clayborg]

It would be good to verify all headers that are in the LLDB.framework produced by Xcode builds are also in the LLDB.framework from cmake/ninja

[xiaobai]

I also think that'd be a great idea. The only way I can think to do that would be to build LLDB.framework using both build systems and compare the two. Is there a faster way?

[clayborg]

In D47278#1110092, @xiaobai wrote:

I also think that'd be a great idea. The only way I can think to do that would be to build LLDB.framework using both build systems and compare the two. Is there a faster way?

That is the only guaranteed way as new headers could come along. LLDB.framework doesn't have headers in installed Xcode.app bundles

[xiaobai]

In D47278#1110104, @clayborg wrote:

That is the only guaranteed way as new headers could come along. LLDB.framework doesn't have headers in installed Xcode.app bundles

In that case, adding this test could double build times since we would effectively be building lldb twice. I'm not sure if this is already a pain point, but if it is, then I don't think we should add this as a test right now.

[clayborg]

sorry, not as a test, but just as a way to figure out if we are getting all the needed header files when we modify this framework header file copying code.

[xiaobai]

In D47278#1110155, @clayborg wrote:

sorry, not as a test, but just as a way to figure out if we are getting all the needed header files when we modify this framework header file copying code.

Ah, yeah. I'm in the process of trying to build LLDB.framework with cmake+ninja and get the same thing as when you build with xcodebuild.
As far as headers go, we roughly have parity after this change. CMake copies SystemInitializerFull.h into LLDB.Framework/Headers while Xcode doesn't, but I'm not sure that actually matters that much.

[clayborg]

In D47278#1110164, @xiaobai wrote:

In D47278#1110155, @clayborg wrote:

sorry, not as a test, but just as a way to figure out if we are getting all the needed header files when we modify this framework header file copying code.

Ah, yeah. I'm in the process of trying to build LLDB.framework with cmake+ninja and get the same thing as when you build with xcodebuild.
As far as headers go, we roughly have parity after this change. CMake copies SystemInitializerFull.h into LLDB.Framework/Headers while Xcode doesn't, but I'm not sure that actually matters that much.

It does as we can't expose any lldb_private functions and this exposes lldb_private::SystemInitializerFull which requires lldb_private::SystemInitializerCommon. The only references to stuff inside lldb_private in headers that make it into the LLDB.framework can be forward references like:

namespace lldb_private {
   class Foo;
}

[xiaobai]

In D47278#1110171, @clayborg wrote:

In D47278#1110164, @xiaobai wrote:

In D47278#1110155, @clayborg wrote:

sorry, not as a test, but just as a way to figure out if we are getting all the needed header files when we modify this framework header file copying code.

Ah, yeah. I'm in the process of trying to build LLDB.framework with cmake+ninja and get the same thing as when you build with xcodebuild.
As far as headers go, we roughly have parity after this change. CMake copies SystemInitializerFull.h into LLDB.Framework/Headers while Xcode doesn't, but I'm not sure that actually matters that much.

It does as we can't expose any lldb_private functions and this exposes lldb_private::SystemInitializerFull which requires lldb_private::SystemInitializerCommon. The only references to stuff inside lldb_private in headers that make it into the LLDB.framework can be forward references like:

namespace lldb_private {
   class Foo;
}

I see, thanks for explaining why it matters. I'll update this revision.

[xiaobai]

Remove SystemInitializerFull.h from framework headers

[clayborg]

The issue is actually that SystemInitializerFull.h and SystemInitializerFull.cpp are in the wrong directories. They belong in the "lldb/Initialization" and "Source//Initialization". We should fix this and then some/all of your changes won't be needed?

[labath]

From a layering perspective, it makes sense for SystemInitializerFull to live in the outermost layer, as it's the thing which makes sure liblldb pulls in all required components. Since it is only included from files in source/API (which is as it should be), maybe we could just make it a private header and move the file to source/API/SystemInitializerFull.h ?

[xiaobai]

In D47278#1110777, @labath wrote:

From a layering perspective, it makes sense for SystemInitializerFull to live in the outermost layer, as it's the thing which makes sure liblldb pulls in all required components. Since it is only included from files in source/API (which is as it should be), maybe we could just make it a private header and move the file to source/API/SystemInitializerFull.h?

This makes the most sense to me. I've uploaded D47342 which I believe does what you've suggested.

In D47278#1110333, @clayborg wrote:

The issue is actually that SystemInitializerFull.h and SystemInitializerFull.cpp are in the wrong directories. They belong in the "lldb/Initialization" and "Source//Initialization". We should fix this and then some/all of your changes won't be needed?

Some of them for sure. It's a mistake to add ${LLDB_SOURCE_DIR}/include/lldb/lldb-*.h to public_headers, so that should still be changed.

[xiaobai]

Updating to reflect changes in r333304

source/API/CMakeLists.txt

set_target_properties(liblldb
     )
 endif()
 
 if (LLDB_WRAP_PYTHON)
   add_dependencies(liblldb swig_wrapper)
 endif()
 
 if(LLDB_BUILD_FRAMEWORK)
-  file(GLOB public_headers ${LLDB_SOURCE_DIR}/include/lldb/API/*.h
-      ${LLDB_SOURCE_DIR}/include/lldb/lldb-*.h)
+  file(GLOB public_headers ${LLDB_SOURCE_DIR}/include/lldb/API/*.h)
   file(GLOB root_public_headers ${LLDB_SOURCE_DIR}/include/lldb/lldb-*.h)
   file(GLOB root_private_headers ${LLDB_SOURCE_DIR}/include/lldb/lldb-private*.h)
   list(REMOVE_ITEM root_public_headers ${root_private_headers})
 
   foreach(header ${public_headers} ${root_public_headers} ${LLDB_SOURCE_DIR}/include/lldb/Utility/SharingPtr.h)
     get_filename_component(basename ${header} NAME)
     add_custom_command(OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/FrameworkHeaders/${basename}
                        DEPENDS ${header}