[Lldb-commits] [PATCH] D24749: [CMake] Initial support for LLDB.framework

[Todd Fiala via lldb-commits]

tfiala added a comment.

I'd say the install rpath change is probably worth doing on the first round.  I think the top-level concept for frameworks is interesting but would be fine to come in as another change.

I'd like to start being able to use this.


================
Comment at: cmake/modules/AddLLDB.cmake:75
@@ -74,1 +74,3 @@
       if (PARAM_SHARED)
+        set(out_dir lib${LLVM_LIBDIR_SUFFIX})
+        if(${name} STREQUAL "liblldb" AND LLDB_BUILD_FRAMEWORK)
----------------
labath wrote:
> I am wondering whether this wouldn't be cleaner if instead of matching on the library name, we made Framework a top-level concept, like PARAM_SHARED, and PARAM_OBJECT is.
@labath that seems to make sense.

================
Comment at: cmake/modules/AddLLDB.cmake:115
@@ +114,3 @@
+    else()
+      set_property(TARGET ${name} APPEND_STRING PROPERTY
+                   LINK_FLAGS " -rpath @loader_path/../${LLDB_FRAMEWORK_INSTALL_DIR}")
----------------
labath wrote:
> It  should be possible to achieve this with the `INSTALL_RPATH` target property. I think that is much cleaner. <https://cmake.org/cmake/help/v3.0/prop_tgt/INSTALL_RPATH.html>
Yep, that seems like the more appropriate solution.

================
Comment at: source/API/CMakeLists.txt:9
@@ -8,1 +8,3 @@
 
+option(LLDB_BUILD_FRAMEWORK "Build the Darwin LLDB.framework" Off)
+
----------------
labath wrote:
> beanz wrote:
> > tfiala wrote:
> > > zturner wrote:
> > > > Is there any reason to have this off when building on OSX?  Is there any value in having it not be an option at all, but just if you're on Darwin, you just always get a Framework?
> > > I mentioned to Chris that I wasn't sure if the Google folks (or maybe Dawn) that build with the CMake build might want to keep it building the old way.
> > > 
> > > I'd be all for only building only the framework way with CMake if nobody else particularly cared.  That would simplify things.  But I don't want to force it if anybody else cares.
> > > 
> > > Our goal is to get the CMake-based build producing the same exact thing as our Xcode-based build.
> > Since this is new functionality that changes the build system behavior it is always best to leave it optional and off by default so that it can be properly tested and won't impact consumers. For users who don't set `LLDB_BUILD_FRAMEWORK=On` this patch should be NFC.
> > 
> > Additionally since OS X is Unix it would be undesirable to force OS X to always build a framework because some users may prefer unix-style builds (me being one of them). Whether or not the default build should be a framework I think is a question we can ask ourselves later once this patch is fully tested and after subsequent patches to fix the install targets. 
> Thank you for thinking about us Todd. For us, it makes our downstream scripts a bit simpler when the darwin and linux builds produce the same layout, but we're not particularly married to them. If it makes things significantly simpler, we would be fine with dropping it (but please do let us know if you intend to do that).
> Thank you for thinking about us Todd. 

> If it makes things significantly simpler, we would be fine with dropping it (but please do let us know if you intend to do that).

I think we'll see how it goes with it being optional up front, both so we can get it polished up and so we can see what kind of complexity we introduce for supporting both ways.  If it all goes well, we'll talk about simplifying with a single delivery style on macOS.


https://reviews.llvm.org/D24749