[PATCH] D146918: [CMake] Don't set absolute paths as install runpaths on ELF platforms in llvm_setup_rpath()

[Peter Waller via Phabricator via llvm-commits]

peterwaller-arm added a comment.

> Sure, simply run the readelf commands from this commit message or something similar on the clang toolchain you built, both in your build and install directories, and perhaps with this commit reverted too.

To clarify, I meant to ask if it's correct to say "It's incorrect for `LLVM_LIBRARY_DIR` to be in the install rpath", because I wondered how else the cmake configuration was going to configure this automatically.

For what it's worth, I found this differential by noticing that in the broken build, the `_install_rpath` had been changed relative to the working one and that the `ninja install` phase was removing the rpath from the binaries.

> you can always add it to the configuration yourself for scenarios like yours.

If I understand you correctly then, it is not expected that the `find_package(LLVM)` machinery will produce a functioning binary without also setting `CMAKE_INSTALL_RPATH` to point to the LLVM install directory? Is that right? I suppose that is what I am questioning, I'd have hoped the build system generator would have made that work.

I don't yet fully understand what is or isn't meant to be supported, perhaps you can help? Some questions that come to mind:

- Shouldn't users be setting `LLVM_LIBRARY_DIR` to their install dir rather than the build dir?
- Under what circumstances is it desirable to set `LLVM_LIBRARY_DIR` to the build directory?
- If they're setting it to the build directory rather than the install directory, why is it incorrect to refer to the build directory as has been configured, once installed?
- It seems suspicious to me that it has been necessary to diverge behavior across platforms here ("it was said to break the build on macOS and Windows", from the commit message).

In my mind the problem with asking others to set `CMAKE_INSTALL_RPATH` manually is that this is meant to be a detail encapsulated in the build system by `find_package(LLVM)`, is it not? It's nice if LLVM works as a cmake package like any other, rather than having to special case it. It seems to me this would break other users of `find_package(LLVM)`.

I wonder if there is a better way to support pointing an `LLVM_LIBRARY_DIR` at a build directory (and stripping the rpath at install time) but I don't yet understand the use case well enough.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146918/new/

https://reviews.llvm.org/D146918