This is an archive of the discontinued LLVM Phabricator instance.

[CMake] Revised RPATH handling
ClosedPublic

Authored by sgraenitz on Dec 5 2018, 8:23 AM.

Details

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".

Diff Detail

Repository
rL LLVM

Event Timeline

sgraenitz created this revision.Dec 5 2018, 8:23 AM
sgraenitz updated this revision to Diff 176956.Dec 6 2018, 5:12 AM

Avoid conflicts: updating diff for recent changes on master

JDevlieghere accepted this revision.Dec 7 2018, 8:59 AM

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?

This revision is now accepted and ready to land.Dec 7 2018, 8:59 AM

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 updated this revision to Diff 177512.Dec 10 2018, 7:46 AM

Rename function to setup rpaths and extend comments

sgraenitz marked 2 inline comments as done.Dec 10 2018, 9:02 AM

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?

This revision was automatically updated to reflect the committed changes.