[Lldb-commits] [PATCH] D47278: Remove lldb-private headers when building LLDB.framework with CMake
This revision was automatically updated to reflect the committed changes.
Closed by commit rL333444: Remove lldb-private headers when building
LLDB.framework with CMake (authored by xiaobai, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D47278
Files:
lldb/trunk/source/API/CMakeLists.txt
Index: lldb/trunk/source/API/CMakeLists.txt
===
--- lldb/trunk/source/API/CMakeLists.txt
+++ lldb/trunk/source/API/CMakeLists.txt
@@ -162,8 +162,7 @@
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})
Index: lldb/trunk/source/API/CMakeLists.txt
===
--- lldb/trunk/source/API/CMakeLists.txt
+++ lldb/trunk/source/API/CMakeLists.txt
@@ -162,8 +162,7 @@
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})
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D47278: Remove lldb-private headers when building LLDB.framework with CMake
xiaobai updated this revision to Diff 148688.
xiaobai added a comment.
Updating to reflect changes in r04
https://reviews.llvm.org/D47278
Files:
source/API/CMakeLists.txt
Index: source/API/CMakeLists.txt
===
--- source/API/CMakeLists.txt
+++ source/API/CMakeLists.txt
@@ -162,8 +162,7 @@
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})
Index: source/API/CMakeLists.txt
===
--- source/API/CMakeLists.txt
+++ source/API/CMakeLists.txt
@@ -162,8 +162,7 @@
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})
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D47278: Remove lldb-private headers when building LLDB.framework with CMake
xiaobai added a comment.
In https://reviews.llvm.org/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 https://reviews.llvm.org/D47342
which I believe does what you've suggested.
In https://reviews.llvm.org/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.
https://reviews.llvm.org/D47278
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D47278: Remove lldb-private headers when building LLDB.framework with CMake
labath added a comment. 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` ? https://reviews.llvm.org/D47278 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D47278: Remove lldb-private headers when building LLDB.framework with CMake
clayborg added a comment. 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? https://reviews.llvm.org/D47278 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D47278: Remove lldb-private headers when building LLDB.framework with CMake
xiaobai updated this revision to Diff 148305.
xiaobai added a comment.
Remove SystemInitializerFull.h from framework headers
https://reviews.llvm.org/D47278
Files:
source/API/CMakeLists.txt
Index: source/API/CMakeLists.txt
===
--- source/API/CMakeLists.txt
+++ source/API/CMakeLists.txt
@@ -162,8 +162,9 @@
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)
+ list(REMOVE_ITEM public_headers
+${LLDB_SOURCE_DIR}/include/lldb/API/SystemInitializerFull.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})
Index: source/API/CMakeLists.txt
===
--- source/API/CMakeLists.txt
+++ source/API/CMakeLists.txt
@@ -162,8 +162,9 @@
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)
+ list(REMOVE_ITEM public_headers
+${LLDB_SOURCE_DIR}/include/lldb/API/SystemInitializerFull.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})
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D47278: Remove lldb-private headers when building LLDB.framework with CMake
xiaobai added a comment.
In https://reviews.llvm.org/D47278#1110171, @clayborg wrote:
> In https://reviews.llvm.org/D47278#1110164, @xiaobai wrote:
>
> > In https://reviews.llvm.org/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.
https://reviews.llvm.org/D47278
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D47278: Remove lldb-private headers when building LLDB.framework with CMake
clayborg added a comment.
In https://reviews.llvm.org/D47278#1110164, @xiaobai wrote:
> In https://reviews.llvm.org/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;
}
https://reviews.llvm.org/D47278
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D47278: Remove lldb-private headers when building LLDB.framework with CMake
xiaobai added a comment. In https://reviews.llvm.org/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. https://reviews.llvm.org/D47278 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D47278: Remove lldb-private headers when building LLDB.framework with CMake
clayborg added a comment. 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. https://reviews.llvm.org/D47278 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D47278: Remove lldb-private headers when building LLDB.framework with CMake
xiaobai added a comment. In https://reviews.llvm.org/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. https://reviews.llvm.org/D47278 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D47278: Remove lldb-private headers when building LLDB.framework with CMake
clayborg added a comment. In https://reviews.llvm.org/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 https://reviews.llvm.org/D47278 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D47278: Remove lldb-private headers when building LLDB.framework with CMake
xiaobai added a comment. 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? https://reviews.llvm.org/D47278 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D47278: Remove lldb-private headers when building LLDB.framework with CMake
clayborg added a comment. 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 https://reviews.llvm.org/D47278 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D47278: Remove lldb-private headers when building LLDB.framework with CMake
xiaobai created this revision.
xiaobai added reviewers: compnerd, sas, labath, beanz, zturner.
Herald added a subscriber: mgorny.
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.
https://reviews.llvm.org/D47278
Files:
source/API/CMakeLists.txt
Index: source/API/CMakeLists.txt
===
--- source/API/CMakeLists.txt
+++ source/API/CMakeLists.txt
@@ -162,8 +162,7 @@
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})
Index: source/API/CMakeLists.txt
===
--- source/API/CMakeLists.txt
+++ source/API/CMakeLists.txt
@@ -162,8 +162,7 @@
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})
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
