[Lldb-commits] [PATCH] D141021: [lldb] Add lldb-framework-cleanup target

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: bulbazord, mib.
Herald added a project: All.
JDevlieghere requested review of this revision.

CMake supports building Framework bundles for Apple platforms. We rely
on this functionality to create LLDB.framework. From CMake's
perspective, a framework is associated with a single target. In reality,
this is often not the case. In addition to a main library (liblldb) the
frameworks often contains associated resources that are their own CMake
targets, such as debugserver and lldb-argdumper.

When building and using the framework to run the test suite, those
binaries are expected to be in LLDB.framework. We have a function
(lldb_add_to_buildtree_lldb_framework) that copies those targets into
the framework.

When CMake installs a framework, it copies over the content of the
framework directory and "installs" the associated target. In addition to
copying the target, installing also means updating the RPATH. However,
the RPATH is only updated for the target associated with the framework.
Everything else is naively copied over, including executables or
libraries that have a different build and install RPATH. This means that
those tools need to have their own install rules.

If the framework is installed
first, the aforementioned tools are copied over with build RPATHs from
the build tree. And when the tools themselves are installed, the
binaries get overwritten and the RPATHs updated to the install RPATHs.

The problem is that CMake provides no guarantees when it comes to the
order in which components get installed. If those tools get installed
first, the inverse happens and the binaries get overwritten with the
ones that have build RPATHs.

lldb_add_to_buildtree_lldb_framework has a comment correctly stating
that those copied binaries should be removed before install.
Unfortunately, there's nothing in place today to do that. This patch
adds a custom target (lldb-framework-cleanup) that must be run before
the install phase when building LLDB.framework.

Because CMake does not allow you to specify an install order, there's no
way to make this happen automatically. Making this a dependency of the
install target is not sufficient as the target needs to be build before
the install phase. Making it a install phase code has the same issue as
I described above: there's no guarantee it will run before another
install component or before any other install steps within the same
component.


https://reviews.llvm.org/D141021

Files:
  lldb/cmake/modules/AddLLDB.cmake
  lldb/cmake/modules/LLDBConfig.cmake


Index: lldb/cmake/modules/LLDBConfig.cmake
===================================================================
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -101,6 +101,13 @@
   # Essentially, emit the framework's dSYM outside of the framework directory.
   set(LLDB_DEBUGINFO_INSTALL_PREFIX ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin CACHE STRING
       "Directory to emit dSYM files stripped from executables and libraries (Darwin Only)")
+
+  # Custom target to remove the tools (binaries) that were copied into
+  # LLDB.framework in the build tree. These binaries need to be removed because
+  # otherwise they're copied as part of the install phase of the framework
+  # without getting their RPATHs fixed up.
+  add_custom_target(lldb-framework-cleanup
+    COMMENT "Cleaning up build-tree frameworks in preparation for install")
 endif()
 
 if(APPLE AND CMAKE_GENERATOR STREQUAL Xcode)
Index: lldb/cmake/modules/AddLLDB.cmake
===================================================================
--- lldb/cmake/modules/AddLLDB.cmake
+++ lldb/cmake/modules/AddLLDB.cmake
@@ -243,6 +243,15 @@
     COMMAND ${CMAKE_COMMAND} -E copy $<TARGET_FILE:${name}> ${copy_dest}
     COMMENT "Copy ${name} to ${copy_dest}"
   )
+
+  # Create a target to remove the target again before the install phase. We
+  # intentionally use remove_directory because the target can be a directory
+  # and it's harmless for files.
+  add_custom_target(${name}-cleanup
+    COMMAND ${CMAKE_COMMAND} -E remove_directory ${copy_dest}
+    COMMENT "Removing ${name} from LLDB.framework")
+  add_dependencies(lldb-framework-cleanup
+    ${name}-cleanup)
 endfunction()
 
 # Add extra install steps for dSYM creation and stripping for the given target.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D141021.486431.patch
Type: text/x-patch
Size: 1765 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20230105/32845922/attachment.bin>