Fix rpath to libclang archive on mac with external llvm

[Chilledheart]

The impact on current build.py usage is minimal because build.py doesn't support external LLVM root. You can modify build.py to support it by add something like this cmake_args.append( '-DPATH_TO_LLVM_ROOT=/Users/hky/clang+llvm-17.0.6-arm64-apple-darwin22.0' )

1. tested with downloaded libclang

➜  ycmd git:(master) otool -l ycm_core.cpython-310-darwin.so|grep -A3 LC_RPATH
          cmd LC_RPATH
      cmdsize 96
         path /Users/hky/.vim/plugged/YouCompleteMe/third_party/ycmd/cpp/../third_party/clang/lib (offset 12)
Load command 18

Okay, because the dylib lies inside LIBCLANG_DIR 2) tested with system libclang

➜  ycmd git:(master) otool -l ycm_core.cpython-310-darwin.so|grep -A3 LC_RPATH
          cmd LC_RPATH
      cmdsize 96
         path /Users/hky/.vim/plugged/YouCompleteMe/third_party/ycmd/cpp/../third_party/clang/lib (offset 12)
Load command 18
          cmd LC_RPATH
      cmdsize 104
         path /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib (offset 12)
Load command 19

Okay, because the dylib lies outside LIBCLANG_DIR and next RPATH is correct. 3) tested with external libclang

➜  ycmd git:(master) ✗ otool -l ycm_core.cpython-310-darwin.so|grep -A3 LC_RPATH
          cmd LC_RPATH
      cmdsize 96
         path /Users/hky/.vim/plugged/YouCompleteMe/third_party/ycmd/cpp/../third_party/clang/lib (offset 12)
Load command 18
          cmd LC_RPATH
      cmdsize 72
         path /Users/hky/clang+llvm-17.0.6-arm64-apple-darwin22.0/lib (offset 12)
Load command 19

Okay, because the dylib lies outside LIBCLANG_DIR and next RPATH is correct.

---

This change is 

[Chilledheart]

For the external LLVM root's usage, copying the dylib to LIBCLANG_DIR is wrong because the dylib is bound with the clang subdirectory (called resource dir in clang's terminology) under /path/to/llvm/root/lib directory which contains important things such as builtin headers which might not match the one inside LIBCLANG_DIR.

[codecov[bot]]

Codecov Report

Merging #1729 (40e3c81) into master (7bc5d08) will not change coverage. The diff coverage is n/a.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1729   +/-   ##
=======================================
  Coverage   95.44%   95.44%           
=======================================
  Files          83       83           
  Lines        8166     8166           
  Branches      163      163           
=======================================
  Hits         7794     7794           
  Misses        322      322           
  Partials       50       50           

[bstaletic]

@puremourning You're the one with a macOS. Can you take a look at this?

[puremourning]

Can you provide a description of the problem this solves, and steps to reproduce

[Chilledheart]

Steps to reproduce

0. Use a mac (Linux doesn't have such an issue because it is capable to links with the full path instead of rpath)
1. Build or download an LLVM binary tarball such as the one from LLVM official site https://github.com/llvm/llvm-project/releases/download/llvmorg-17.0.6/clang+llvm-17.0.6-arm64-apple-darwin22.0.tar.xz
2. Install/Extract the binary tarball to ~/clang+llvm-17.0.6-arm64-apple-darwin22.0
3. Modify the build.py to support linking with this external llvm (libclang inside)

diff --git a/build.py b/build.py
index 1e0f9a20..b84e3c81 100755
--- a/build.py
+++ b/build.py
@@ -618,7 +618,7 @@ def GetCmakeArgs( parsed_args ):
     cmake_args.append( '-DUSE_CLANG_TIDY=ON' )

   if parsed_args.system_libclang:
-    cmake_args.append( '-DUSE_SYSTEM_LIBCLANG=ON' )
+    cmake_args.append( '-DPATH_TO_LLVM_ROOT=/Users/hky/clang+llvm-17.0.6-arm64-apple-darwin22.0' )

   if parsed_args.enable_debug:
     cmake_args.append( '-DCMAKE_BUILD_TYPE=Debug' )

4. Run the build.py to get ycm compiled and linked with the external llvm (libclang inside)

./build.py --ninja --verbose --clang-completer --go-completer --system-libclang

5. Check it with vim (You can use the :YcmToggleLogs to get the error)
6. Check the ycm dylib with otool -l and you will find the libclang.dylib doesn't present inside the first rpath.

[puremourning]

does this issue occur when using EXTRA_CMAKE_ARGS as described here, as opposed to hacking build.py: https://github.com/ycm-core/YouCompleteMe/wiki/Troubleshooting-steps-for-ycmd-server-SHUT-DOWN#if-you-used-the-full-installation-guide ?

[Chilledheart]

I think it is the fastest way to use external llvm path cmake option by modifying build.py while building ycm doesn't depend on build.py (just a helper to build faster), so it is possible to run cmake directly for end users.

I didn't try extra cmake flags because if it downloads the libclang archive that will conflict with external llvm path option.

Typed from my phone.

[puremourning]

I can't reproduce the issue. Here's what I did (Intel Mac):

 5012  EXTRA_CMAKE_ARGS="-DPATH_TO_LLVM_ROOT=$(brew --prefix llvm)" ./build.py --clang-completer --system-libclang --verbose
 5013  dtvim --cmd 'let g:ycm_use_clangd=0' test.cpp

Completion works.

-- Resolve completions: On demand
-- Client logfile: /var/folders/mr/p_8nf2d90gvd72mh012fqlg00000gn/T/ycm_bhqk74eq.log
-- Server Python interpreter: /usr/local/opt/[email protected]/bin/python3.11
-- Server Python version: 3.11.6
-- Server has Clang support compiled in: True

The reason I am nitpicking here is that we have had a number of changes in this area and every now and again someone comes along and says it doesn't work, but never fully explains why, and I am very keen not to regress it.

Also, I'm keen not to spend too much time on the legacy libclang support due to it being deprecated in favour of clangd which doesn't have this issue.

[Chilledheart]

It is fine if you don't merge it. I saw a mess in the code conflicting with each other. As far as I can tell, on msvc it is expected to have libclang copy from external llvm path but it isn't the case for other platforms.

It is a minimal change I could do. But for a better future and more maintainable code base, it might be greater to reduce the conflicting code and do a code refactor. After that, running full tests on every platform to make sure things working.

[puremourning]

It's not that I'm against merging it (and TBH I don't quite understand your point about the codebase), but that when we merge a change to the build it's our responsiblity to understand that it works, how it works, and why because we need to maintain it forever. I'm very grateful for the contribution, but I need to understand it better before moving on.

[Chilledheart]

For clangd, I guess it is a different story. From my experience, Libclang (c interface) is much faster and uses less memory for me(e.g. Xcode still ships libclang).

In the latest releases of llvm, it even ships with libclang(cpp) dylib (c++ interface) in the releases tarball. Maybe it is encouraging to adopt c++ interface directly?

[puremourning]

libclang lacks significant features vs clangd; we have not intention to invest further in it.