[CMake] Revised RPATH handling

[sgraenitz]

Summary

If we build LLDB.framework, dependant tools need appropriate RPATHs in both locations, the build-tree (for testing) and the install-tree (for deployment). Luckily, CMake can handle it for us: https://gitlab.kitware.com/cmake/community/wikis/doc/cmake/RPATH-handling.

• In the build-tree, tools use the absolute path to the framework's actual output location.
• In the install-tree, tools get a list of RPATHs to look for the framework when deployed.

LLDB_FRAMEWORK_INSTALL_DIR is added to the CMAKE_INSTALL_PREFIX to change the relative location of LLDB.framework in the install-tree.
If it is not empty, it will be added as an additional RPATH to all dependant tools (so they are functional in the install-tree).
If it is empty, LLDB.framework goes to the root and tools will not be functional in the directory structure of the LLVM install-tree.
For historical reasons LLDB_FRAMEWORK_INSTALL_DIR defaults to "Library/Frameworks".

[sgraenitz]

Avoid conflicts: updating diff for recent changes on master

[JDevlieghere]

LGTM with the question answered/addressed. Thanks Stefan, these patches are really great work!

tools/driver/CMakeLists.txt

27 ↗

(On Diff #176956)

Would it make sense/work to do the check inside this function?

[xiaobai]

Looks good to me overall. You also probably probably also invoke lldb_setup_rpaths_framework for the tools included in the framework (argdumper, darwin-debug, debugserver, lldb-server).

[sgraenitz]

Rename function to setup rpaths and extend comments

[sgraenitz]

In D55330#1323525, @xiaobai wrote:

Looks good to me overall. You also probably probably also invoke lldb_setup_rpaths_framework for the tools included in the framework (argdumper, darwin-debug, debugserver, lldb-server).

Actually, these targets don't need the RPATHs. They are going into the framework bundle, yes, but they are linked entirely statically. So they don't need to find the framework at load time and don't need the RPATH, right?

tools/driver/CMakeLists.txt

27 ↗

(On Diff #176956)

Yes would be possible and it would save some lines of code, but this way it's more visible that it's a very special use-case and shouldn't be copy/pasted into each and every tool. For the general cases the default RPATHs from LLVM are exactly what is necessary. I have the impression that there was some confusing about RPATHs in this code before.

I thought about the alternative approach to query the dependencies for each target in lldb_add_executable and apply the RPATH settings to those targets that depend on liblldb (if LLDB_BUILD_FRAMEWORK). However, it feels like a bit too much magic under the hood and also it goes a little bit too far in the direction of building our own dependency management in CMake, which is not what CMake is made for.

I made the function name a little bit more explicit and extended the documentation. I would keep it like this now. What do you think?

lldb/trunk/cmake/modules/AddLLDB.cmake

function(add_lldb_library name)
   # or omit this logic instead.
   if (NOT WIN32)
     set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fPIC")
   endif()
 
   if (PARAM_OBJECT)
     add_library(${name} ${libkind} ${srcs})
   else()
-    llvm_add_library(${name} ${libkind} ${srcs} LINK_LIBS
-                                ${PARAM_LINK_LIBS}
-                                DEPENDS ${PARAM_DEPENDS})
+    if(LLDB_NO_INSTALL_DEFAULT_RPATH)
+      set(pass_NO_INSTALL_RPATH NO_INSTALL_RPATH)
+    endif()
+
+    llvm_add_library(${name} ${libkind} ${srcs}
+      LINK_LIBS ${PARAM_LINK_LIBS}
+      DEPENDS ${PARAM_DEPENDS}
+      ${pass_NO_INSTALL_RPATH}
+    )
 
     if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY OR ${name} STREQUAL "liblldb")
       if (PARAM_SHARED)
         if(${name} STREQUAL "liblldb" AND LLDB_BUILD_FRAMEWORK)
           if(LLDB_FRAMEWORK_INSTALL_DIR)
             set(install_dir ${LLDB_FRAMEWORK_INSTALL_DIR})
           else()
             set(install_dir ".")

 function(add_lldb_executable name)
   cmake_parse_arguments(ARG
     "GENERATE_INSTALL"
     "ENTITLEMENTS"
     "LINK_LIBS;LINK_COMPONENTS"
     ${ARGN}
     )
 
+  if(LLDB_NO_INSTALL_DEFAULT_RPATH)
+    set(pass_NO_INSTALL_RPATH NO_INSTALL_RPATH)
+  endif()
+
   list(APPEND LLVM_LINK_COMPONENTS ${ARG_LINK_COMPONENTS})
-  add_llvm_executable(${name} ${ARG_UNPARSED_ARGUMENTS} ENTITLEMENTS ${ARG_ENTITLEMENTS})
+  add_llvm_executable(${name} ${ARG_UNPARSED_ARGUMENTS}
+    ENTITLEMENTS ${ARG_ENTITLEMENTS}
+    ${pass_NO_INSTALL_RPATH}
+  )
 
   target_link_libraries(${name} PRIVATE ${ARG_LINK_LIBS})
   set_target_properties(${name} PROPERTIES FOLDER "lldb executables")
 
   if(ARG_GENERATE_INSTALL)
     install(TARGETS ${name}
             COMPONENT ${name}
             RUNTIME DESTINATION bin)
function(lldb_append_link_flags target_name new_link_flags)
   # If we had any linker flags, include them first in the new linker flags.
   if(current_link_flags)
     set(new_link_flags "${current_link_flags} ${new_link_flags}")
   endif()
 
   # Now set them onto the target.
   set_target_properties(${target_name} PROPERTIES LINK_FLAGS ${new_link_flags})
 endfunction()
+
+# For tools that depend on liblldb, account for varying directory structures in
+# which LLDB.framework can be used and distributed: In the build-tree we find it
+# by its absolute target path. This is only relevant for running the test suite.
+# In the install step CMake will remove this entry and insert the final RPATHs.
+# These are relative to the file path from where the tool will be loaded on the
+# enduser system.
+#
+# Note that the LLVM install-tree doesn't match the enduser system structure
+# for LLDB.framework, so by default dependent tools will not be functional in
+# their install location. The LLDB_FRAMEWORK_INSTALL_DIR variable allows to fix
+# this. If specified, it causes the install-tree location of the framework to be
+# added as an extra RPATH below.
+#
+function(lldb_setup_framework_rpaths_in_tool name)
+  # In the build-tree, we know the exact path to the binary in the framework.
+  set(rpath_build_tree "$<TARGET_FILE:liblldb>")
+
+  # The installed framework is relocatable and can be in different locations.
+  set(rpaths_install_tree "@loader_path/../../../SharedFrameworks")
+  list(APPEND rpaths_install_tree "@loader_path/../../System/Library/PrivateFrameworks")
+  list(APPEND rpaths_install_tree "@loader_path/../../Library/PrivateFrameworks")
+
+  if(LLDB_FRAMEWORK_INSTALL_DIR)
+    set(rpaths_install_tree "@loader_path/../${LLDB_FRAMEWORK_INSTALL_DIR}")
+  endif()
+
+  # If LLDB_NO_INSTALL_DEFAULT_RPATH was NOT enabled (default), this overwrites
+  # the default settings from llvm_setup_rpath().
+  set_target_properties(${name} PROPERTIES
+    BUILD_WITH_INSTALL_RPATH OFF
+    BUILD_RPATH "${rpath_build_tree}"
+    INSTALL_RPATH "${rpaths_install_tree}"
+  )
+
+  add_dependencies(${name} lldb-framework)
+endfunction()

lldb/trunk/cmake/modules/LLDBConfig.cmake

 
 option(LLDB_DISABLE_PYTHON "Disable Python scripting integration." ${default_disable_python})
 option(LLDB_DISABLE_CURSES "Disable Curses integration." ${default_disable_curses})
 option(LLDB_DISABLE_LIBEDIT "Disable the use of editline." ${default_disable_libedit})
 option(LLDB_RELOCATABLE_PYTHON "Use the PYTHONHOME environment variable to locate Python." OFF)
 option(LLDB_USE_SYSTEM_SIX "Use six.py shipped with system and do not install a copy of it" OFF)
 option(LLDB_USE_ENTITLEMENTS "When codesigning, use entitlements if available" ON)
 option(LLDB_BUILD_FRAMEWORK "Build LLDB.framework (Darwin only)" OFF)
+option(LLDB_NO_INSTALL_DEFAULT_RPATH "Disable default RPATH settings in binaries" OFF)
 
 if(LLDB_BUILD_FRAMEWORK)
   if(NOT APPLE)
     message(FATAL_ERROR "LLDB.framework can only be generated when targeting Apple platforms")
   endif()
   # CMake 3.6 did not correctly emit POST_BUILD commands for Apple Framework targets
   if(CMAKE_VERSION VERSION_LESS 3.7)
     message(FATAL_ERROR "LLDB_BUILD_FRAMEWORK is not supported on CMake < 3.7")

lldb/trunk/tools/driver/CMakeLists.txt

 if ( CMAKE_SYSTEM_NAME MATCHES "Windows" )
   add_definitions( -DIMPORT_LIBLLDB )
 endif()
 
 add_dependencies(lldb
   LLDBOptionsTableGen
   ${tablegen_deps}
 )
+
+if(LLDB_BUILD_FRAMEWORK)
+  lldb_setup_framework_rpaths_in_tool(lldb)
+endif()

lldb/trunk/tools/lldb-mi/CMakeLists.txt

add_lldb_tool(lldb-mi
   LINK_LIBS
     liblldb
     ${host_lib}
     ${extra_libs}
 
   LINK_COMPONENTS
     Support
   )
+
+if(LLDB_BUILD_FRAMEWORK)
+  lldb_setup_framework_rpaths_in_tool(lldb-mi)
+endif()

lldb/trunk/tools/lldb-vscode/CMakeLists.txt

add_lldb_tool(lldb-vscode
   LINK_LIBS
     liblldb
     ${host_lib}
     ${extra_libs}
 
   LINK_COMPONENTS
     Support
   )
+
+if(LLDB_BUILD_FRAMEWORK)
+  lldb_setup_framework_rpaths_in_tool(lldb-vscode)
+endif()