[llvm] cmake: resolve symlinks in LLVMConfig.cmake

[Lekensteyn]

Summary

Ensure that symlinks such as /usr/lib/llvm-X.Y/cmake (pointing to
lib/cmake/llvm) are resolved. This ensures that LLVM_INSTALL_PREFIX
becomes /usr/lib/llvm-X.Y instead of /usr.

Partially addresses PR37128

[Lekensteyn]

Hi, this patch obsoletes a downstream (Debian) patch (which hard-coded the LLVM_INSTALL_PREFIX before). Could you have a look? Thanks!

[mgorny]

I don't think I agree with this change. Unless I'm missing something, Debian is using a install layout that is broken by design, and you're now attempting to workaround that by adding symlinks and realpath hacks that are supposed to make the broken design somewhat incidentally work, if only someone doesn't miss one of the fragile connections and accidentally break it.

[Lekensteyn]

To support multiple co-installed versions, Debian/Ubuntu use /usr/lib/llvm-X.Y as prefix instead of /usr. It is however desirable to make one of these versions available to CMake by default and that requires only one symlink per project (LLVM/Clang): /usr/lib/llvm-X.Y/cmake -> /usr/lib/llvm-X.Y/lib/cmake/llvm.
This symlink however requires that the LLVMConfig.cmake script uses its real path to locate other files. The alternative is to require every user to set CMAKE_PREFIX_PATH which in my opinion is unreasonable to ask.

Distributions that do not use this approach include Fedora and Arch Linux. They use /usr as prefix for the latest version (so LLVMConfig.cmake can be found in /usr/lib/cmake/llvm/ as expected by CMake). For older versions, they use a different prefix, /usr/lib/llvm-X.Y.

As a user, I really like the standarized /usr/lib/llvm-X.Y structure (as used by apt.llvm.org) since it makes CI integration with multiple versions easier (set CMAKE_PREFIX_PATH=/usr/lib/llvm-$VERSION without special cases).
Installing everything in /usr/lib/llvm-X.Y has the additional benefit that upgrades are less likely to break things and requires less downloads:

• Initially "clang" depends on "clang-5.0" which installs to /usr/lib/llvm-5.0.
• Another package (say, mesa) is installed which depends on clang 5.0.
• Later, a new LLVM version is released and "clang" depends on "clang-6.0".
• Since the old package still depends on "clang-5.0", it keeps working.

If the second approach is used, then this would be the "upgrade" path:

• Install "clang" which installs to /usr
• Another package (say, mesa) is installed which depends on this Clang version.
• Later, a newer LLVM version is released.
• To upgrade the "clang" package and keep "mesa" working, these are the options:
  • Rebuild "mesa" with the latest clang, upgrade both. (Works only if mesa is actually compatible with the latest LLVM version.)
  • Build a new "clang5.0" package, rebuild mesa to use it. Upgrade clang, mesa and install a new "clang5.0" package. Wastes CPU cycles and data because "clang5.0" is essentially the same as the old "clang", except that files are now in /usr/lib/llvm-5.0.

Given this scenario, I think that the Debian approach is more beneficial if stability/backwards compatibility is important. And the only cost for this is supporting adding a few symlinks in /usr/bin and one in /usr/lib/cmake. Does that seem unreasonable to you? Why do you think that this (installing to /usr/lib/llvm-X.Y?) is broken by design?

[brad.king]

For reference, CMake's own install(EXPORT) does a similar REALPATH trick here. The goal is to overcome cross-prefix symlinks. CMake doesn't actually use the realpath directly, but the realpath of the original installation prefix and the realpath of the location from which the file is loaded are the same, then it uses the original installation prefix instead of the path used to load the file. This achieves robustness against cross-prefix symlinks while still allowing the package to be relocatable.

[mgorny]

Gentoo also supports installing multiple versions (in /usr/lib/llvm/X), and I have never seen any problems close to what you're describing. We're just putting the appropriate bin/ directories in PATH, and CMake finds its files just fine (relatively to PATH). So why do you need a lot of symlink magic to achieve the same effect?

[Lekensteyn]

Gentoo also supports installing multiple versions (in /usr/lib/llvm/X), and I have never seen any problems close to what you're describing. We're just putting the appropriate bin/ directories in PATH, and CMake finds its files just fine (relatively to PATH). So why do you need a lot of symlink magic to achieve the same effect?

This is not about finding clang in PATH, but about find_package(LLVM) being able to locate the desired CMake configuration (LLVMConfig.cmake).
Looking at http://www.portagefilelist.de/site/query/listPackageFiles/?do&category=sys-devel&package=llvm&version=5.0.2#result shows that Gentoo installs /usr/lib/llvm/5/lib64/cmake/llvm/LLVMConfig.cmake.
This should not work with find_package(LLVM) based on the CMake documentation for find_package.

Could you check the test case at https://bugs.llvm.org/show_bug.cgi?id=37128#c7 (see also the extra test in comment 8?)

[mgorny]

-- LLVM_CMAKE_DIR: /usr/lib/llvm/6/lib64/cmake/llvm
-- CLANG_INCLUDE_DIRS: /usr/lib/llvm/6/include

Works just fine. Note that find_package documentation actually mentions scanning locations relative to PATH.

[Lekensteyn]

Oh, by "putting in PATH", you actually mean putting /usr/lib/llvm/6/bin in PATH rather than symlinks in /usr/bin? That would explain why it can locate your LLVM config.

That's a policy decision for Gentoo, but I don't think it should rule out the possibility of having symlinks from /usr/bin/clang-6.0 to /usr/lib/llvm-6.0/bin/clang-6.0 (as is the case on Debian/Ubuntu). That way, PATH can be kept rather simple and requires less lookups.

[mgorny]

…at the cost of breaking CMake, and having to hack LLVM and CMake to make it work again. And I'm pretty sure sooner or later this realpath use will break somebody's use case. Think of a simple example of someone using a temporary symlink in path to relocate LLVM install. By using realpath, you're effectively replacing the canonical location with possibly temporary current location that may no longer be valid afterwards.

[mgorny]

To add a real use case broken by this: Gentoo prior to 17.1 (which includes most of existing installs) used /usr/lib -> lib64 symlink. The canonical LLVM install path for Gentoo is:

/usr/lib/llvm/X/...

With your patch, CMake wrongly replaces it with:

/usr/lib64/llvm/X/...

Which means that if it gets written anywhere, it will become broken after upgrading to 17.1.

Realpath is the worst idea anyone might ever have. It replaces canonical locations with completely unpredictable paths behind user's back.

[Lekensteyn]

A workaround is to check whether the resolved ${LLVM_CONFIG_CMAKE_DIR} exists, and then try REALPATH as fallback, but that is just stacking another hack on it which starts to become really ugly.

I guess that it is best to carry this patch around for distributions that do know that the real path is correct and leave the current behavior in place to avoid regressing on other platforms.

Thank you for your comments!

cmake/modules/CMakeLists.txt

file(COPY .
   FILES_MATCHING PATTERN *.cmake
   PATTERN .svn EXCLUDE
   PATTERN CMakeFiles EXCLUDE
   )
 
 # Generate LLVMConfig.cmake for the install tree.
 set(LLVM_CONFIG_CODE "
 # Compute the installation prefix from this LLVMConfig.cmake file location.
-get_filename_component(LLVM_INSTALL_PREFIX \"\${CMAKE_CURRENT_LIST_FILE}\" PATH)")
+get_filename_component(LLVM_INSTALL_PREFIX \"\${CMAKE_CURRENT_LIST_FILE}\" REALPATH)")
 # Construct the proper number of get_filename_component(... PATH)
 # calls to compute the installation prefix.
-string(REGEX REPLACE "/" ";" _count "${LLVM_INSTALL_PACKAGE_DIR}")
+string(REGEX REPLACE "/" ";" _count "prefix/${LLVM_INSTALL_PACKAGE_DIR}")
 foreach(p ${_count})
   set(LLVM_CONFIG_CODE "${LLVM_CONFIG_CODE}
 get_filename_component(LLVM_INSTALL_PREFIX \"\${LLVM_INSTALL_PREFIX}\" PATH)")
 endforeach(p)
 set(LLVM_CONFIG_INCLUDE_DIRS "\${LLVM_INSTALL_PREFIX}/include")
 set(LLVM_CONFIG_LIBRARY_DIRS "\${LLVM_INSTALL_PREFIX}/lib\${LLVM_LIBDIR_SUFFIX}")
 set(LLVM_CONFIG_CMAKE_DIR "\${LLVM_INSTALL_PREFIX}/${LLVM_INSTALL_PACKAGE_DIR}")
 set(LLVM_CONFIG_BINARY_DIR "\${LLVM_INSTALL_PREFIX}")