[Lldb-commits] [lldb] f6ce39c - [lldb] Remove tools copied into LLDB.framework before install

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2023-01-10T11:22:47-08:00
New Revision: f6ce39cf1d1d80699e13cd1422f60d428e5cf0ec

URL: https://github.com/llvm/llvm-project/commit/f6ce39cf1d1d80699e13cd1422f60d428e5cf0ec
DIFF: https://github.com/llvm/llvm-project/commit/f6ce39cf1d1d80699e13cd1422f60d428e5cf0ec.diff

LOG: [lldb] Remove tools copied into LLDB.framework before install

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. This patch
adds a custom target (lldb-framework-cleanup) that will be run before
the install phase and remove those files from LLDB.framework in the
build tree.

Differential revision: https://reviews.llvm.org/D141021

Added: 
    

Modified: 
    lldb/cmake/modules/AddLLDB.cmake
    lldb/cmake/modules/LLDBConfig.cmake
    lldb/source/API/CMakeLists.txt

Removed: 
    


################################################################################
diff  --git a/lldb/cmake/modules/AddLLDB.cmake b/lldb/cmake/modules/AddLLDB.cmake
index ea52b47d6d727..a5be4afb40fb0 100644
--- a/lldb/cmake/modules/AddLLDB.cmake
+++ b/lldb/cmake/modules/AddLLDB.cmake
@@ -243,6 +243,16 @@ function(lldb_add_to_buildtree_lldb_framework name subdir)
     COMMAND ${CMAKE_COMMAND} -E copy $<TARGET_FILE:${name}> ${copy_dest}
     COMMENT "Copy ${name} to ${copy_dest}"
   )
+
+  # Create a custom target to remove the copy again from LLDB.framework in the
+  # build tree.
+  # Intentionally use remove_directory because the target can be a either a
+  # file or directory and using remove_directory is 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.

diff  --git a/lldb/cmake/modules/LLDBConfig.cmake b/lldb/cmake/modules/LLDBConfig.cmake
index f3f1103d244f6..ebb1eec8464a6 100644
--- a/lldb/cmake/modules/LLDBConfig.cmake
+++ b/lldb/cmake/modules/LLDBConfig.cmake
@@ -101,6 +101,20 @@ if(LLDB_BUILD_FRAMEWORK)
   # 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 targets (binaries, directories) that were
+  # copied into LLDB.framework in the build tree.
+  #
+  # These targets need to be removed before the install phase because otherwise
+  # because otherwise they may overwrite already installed binaries with the
+  # wrong RPATH (i.e. build RPATH instead of install RPATH).
+  #
+  # This target needs to be created here (rather than in API/CMakeLists.txt)
+  # because add_lldb_tool creates the custom rules to copy the binaries before
+  # the framework target exists and that's the only place where this is
+  # tracked.
+  add_custom_target(lldb-framework-cleanup
+    COMMENT "Cleaning up build-tree frameworks in preparation for install")
 endif()
 
 if(APPLE AND CMAKE_GENERATOR STREQUAL Xcode)

diff  --git a/lldb/source/API/CMakeLists.txt b/lldb/source/API/CMakeLists.txt
index dd73ba56afef7..f3c8814fcc82d 100644
--- a/lldb/source/API/CMakeLists.txt
+++ b/lldb/source/API/CMakeLists.txt
@@ -211,4 +211,9 @@ endif()
 
 if(LLDB_BUILD_FRAMEWORK)
   include(LLDBFramework)
+
+  add_dependencies(install-liblldb
+    lldb-framework-cleanup)
+  add_dependencies(install-liblldb-stripped
+    lldb-framework-cleanup)
 endif()