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
Differential D46521
[llvm] cmake: resolve symlinks in LLVMConfig.cmake Lekensteyn on May 7 2018, 5:59 AM. Authored by
Details
Ensure that symlinks such as /usr/lib/llvm-X.Y/cmake (pointing to Partially addresses PR37128
Diff Detail Event TimelineComment Actions Hi, this patch obsoletes a downstream (Debian) patch (which hard-coded the LLVM_INSTALL_PREFIX before). Could you have a look? Thanks! Comment Actions 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. Comment Actions 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. 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).
If the second approach is used, then this would be the "upgrade" path:
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? Comment Actions 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. Comment Actions 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? Comment Actions
This is not about finding clang in PATH, but about find_package(LLVM) being able to locate the desired CMake configuration (LLVMConfig.cmake). 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?) Comment Actions -- 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. Comment Actions 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. Comment Actions …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. Comment Actions 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. Comment Actions 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! |