Wrong libLLVM symbolic links generated for 18.1

[finagolfin]

I'm guessing this is related to bumping the minor version for the first time in a long time:

> ls -l /data/data/com.termux/files/usr/lib/libLLVM*so*
lrwxrwxrwx 1 u0_a318 u0_a318        18 Mar  9 20:46 /data/data/com.termux/files/usr/lib/libLLVM-18.so -> libLLVM.so.18.1
-rw------- 1 u0_a318 u0_a318 114521080 Mar  9 12:39 /data/data/com.termux/files/usr/lib/libLLVM.so

We build with -DLLVM_LINK_LLVM_DYLIB=ON to make sure this shared library is included, which is then linked against by the Rust compiler and so on also. I think the relevant CMake config is here, which I notice @tstellar recently modified.

[tstellar]

What should the symlinks look like? Also, in my builds libLLVM.so is a symlink to libLLVM.so.18.1, I'm not sure why you are seeing libLLVM.so as a regular file.

[finagolfin]

This is what they look like for the current LLVM 17 package that we distribute, built with the exact set of CMake build flags I linked above:

> ls -l data/data/com.termux/files/usr/lib/libLLVM*so
lrwxrwxrwx 1 u0_a318 u0_a318        18 Nov 28 21:26 data/data/com.termux/files/usr/lib/libLLVM-17.0.6.so -> libLLVM-17.so
-rw------- 1 u0_a318 u0_a318 115962784 Nov 28 21:26 data/data/com.termux/files/usr/lib/libLLVM-17.so
lrwxrwxrwx 1 u0_a318 u0_a318        18 Nov 28 21:26 data/data/com.termux/files/usr/lib/libLLVM.so -> libLLVM-17.so

What do you see after building LLVM 18 with the shared library?

[tstellar]

$ ls -l libLLVM*so
lrwxrwxrwx. 1 fedora fedora 15 Mar  9 21:04 libLLVM-18.so -> libLLVM.so.18.1
lrwxrwxrwx. 1 fedora fedora 15 Mar  9 21:04 libLLVM.so -> libLLVM.so.18.1

[h-vetinari]

In our builds on conda-forge, #82647 (resp. 4cc7a75) only fixed the linux side, but on OSX the result is still:

UserWarning: file $PREFIX/lib/libLLVM-18.dylib is a symlink with no target

(this is in a setting where the unversioned libLLVM.dylib intentionally isn't available yet, but libLLVM.18.1.dylib is)

[tstellar]

@h-vetinari Can you post the output of ls -l libLLVM*dylib* from the install directory?

[finagolfin]

@tstellar, I assume the libLLVM.so.18.1 library exists for you? I took a look at the Android build log and it shows exactly the two files I listed being installed, so it appears this is the result of the LLVM build, not any post-build modifications in our Termux packaging scripts.

What build flags do you pass in to CMake when building LLVM? I've linked our LLVM-specific flags above and we use more flags for all CMake builds.

I presume the problem is the difference in build flags or some platform-specific CMake issue.

[h-vetinari]

@h-vetinari Can you post the output of ls -l libLLVM*dylib* from the install directory?

Certainly:

+ ls -l $PREFIX/lib/libLLVM*dylib*
lrwxr-xr-x  1 runner  staff         18 Mar 12 06:00 $PREFIX/lib/libLLVM-18.dylib -> libLLVM.dylib.18.1
-rwxr-xr-x  1 runner  staff  128049832 Mar 12 05:56 $PREFIX/lib/libLLVM.18.1.dylib
lrwxr-xr-x  1 runner  staff         18 Mar 12 06:00 $PREFIX/lib/libLLVM.dylib -> libLLVM.18.1.dylib

The issue is the symlink pointing to libLLVM-18.dylib -> libLLVM.dylib.18.1 (note 18.1 at the end) vs. the library being called libLLVM.18.1.dylib (i.e. version in between)

[tstellar]

@finagolfin Just to be clear the problem you are seeing is that libLLVM.so is not a symlink to libLLVM.so.18.1 ?

[tstellar]

@h-vetinari Is there any reason why you can't link against libLLVM.18.1.dylib ?

[h-vetinari]

@h-vetinari Is there any reason why you can't link against libLLVM.18.1.dylib ?

No there isn't, but the point is that the CMake build as-is generates a broken symlink, and I'd prefer to fix it in LLVM rather than having to carry a manual fix.

[finagolfin]

Just to be clear the problem you are seeing is that libLLVM.so is not a symlink to libLLVM.so.18.1 ?

Yep, that would be one way to fix it. We link our Rust compiler build against libLLVM-18.so, so obviously that is broken now. I will manually fix it in our build script for now, but like @h-vetinari, I think it would be better if the LLVM CMake config generated these properly again in future releases.

[tstellar]

@h-vetinari Are you able to test this fix on MacOS: #85163

[tstellar]

@finagolfin I still can't reproduce the issue you are seeing with the symlinks. Do you have a build log or something I can pull the cmake invocation from?

[h-vetinari]

@h-vetinari Are you able to test this fix on MacOS: #85163

I'm currently trying out the following

diff --git a/llvm/cmake/modules/AddLLVM.cmake b/llvm/cmake/modules/AddLLVM.cmake
index 3bc78b0dc935..b3505e2b749e 100644
--- a/llvm/cmake/modules/AddLLVM.cmake
+++ b/llvm/cmake/modules/AddLLVM.cmake
@@ -2089,7 +2089,13 @@ function(llvm_install_library_symlink name dest type)

   set(full_name ${CMAKE_${type}_LIBRARY_PREFIX}${name}${CMAKE_${type}_LIBRARY_SUFFIX})
   if (ARG_SOVERSION)
-    set(full_dest ${CMAKE_${type}_LIBRARY_PREFIX}${dest}${CMAKE_${type}_LIBRARY_SUFFIX}.${ARG_SOVERSION})
+    if(${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
+      # macOS puts the SOVERSION in the middle, i.e. we need to point to libLLVM.18.1.dylib
+      set(full_dest ${CMAKE_${type}_LIBRARY_PREFIX}${dest}.${ARG_SOVERSION}${CMAKE_${type}_LIBRARY_SUFFIX})
+    else()
+      # linux has the SOVERSION at the end, i.e. libLLVM.so.18.1
+      set(full_dest ${CMAKE_${type}_LIBRARY_PREFIX}${dest}${CMAKE_${type}_LIBRARY_SUFFIX}.${ARG_SOVERSION})
+    endif()
   else()
     set(full_dest ${CMAKE_${type}_LIBRARY_PREFIX}${dest}${CMAKE_${type}_LIBRARY_SUFFIX})
   endif()

I expect this to work, because the only problem AFAICT is the hard-coded format not being adapted to macOS. If it doesn't work, I'll try your suggestion. If it works I'm happy to raise a PR (or try your suggestion nevertheless, if you like your approach better).

[h-vetinari]

So my patch works, but I think yours is more elegant. I'll try it next.

[finagolfin]

I still can't reproduce the issue you are seeing with the symlinks.

OK, if you are not seeing it, maybe it is something specific to Android. I will manually create the symlink for now and look into the underlying issue in the coming weeks.

[h-vetinari]

So my patch works, but I think yours is more elegant. I'll try it next.

Yours works as well, so I suggest to merge #85163 - thanks for the quick handling!

[finagolfin]

I can no longer reproduce with 18.1.2, which I just shipped for Android:

> ls -l $PREFIX/lib/libLLVM*so*
lrwxrwxrwx 1 u0_a318 u0_a318        18 Mar 26 20:24 /data/data/com.termux/files/usr/lib/libLLVM-18.so -> libLLVM.so
-rw------- 1 u0_a318 u0_a318 114525472 Mar 26 18:06 /data/data/com.termux/files/usr/lib/libLLVM.so

Was something merged to fix this or should I look into it, to make sure it wasn't some fluke that might break again?

[tstellar]

@finagolfin I'm not sure what is going on in your build system, but having libLLVM-18.so be a symlink to libLLVM.so is a (bug)[https://github.com//issues/82647]. However, this was fixed in 18.1.0-rc4. Do you have a complete build log for your build?

[finagolfin]

Do you have a complete build log for your build?

Here is where the symlink is created on our CI for the Android AArch64 library, not sure if that's the level of detail you want though.

I will spend some time looking into that CMake config to see what changed and why it behaves differently on Android.

[tstellar]

Can you update the build so that the logs record the full cmake command that was used? Without that, it's going to be hard to debug this.

[finagolfin]

I will look into this manually and report back instead.

[h-vetinari]

I think this issue can be closed

[finagolfin]

No, @tstellar says the current behavior is a bug, even though it works for me on Android now. I will look into why it behaves the way it does now and report back, then we can decide if anything needs to be done.