[cmake] Fix Findzstd.cmake to handle OpenBSD shared libraries

[mgorny]

Summary

Fix Findzstd CMake to handle shared libraries on OpenBSD correctly.
This userland does not use shared library symlinks without SOVERSION,
so the result of find_library() does not ever end with
zstd_SHARED_LIBRARY_SUFFIX. To work around this, reverse the logic
to compare the result against zstd_STATIC_LIBRARY_SUFFIX and assume
shared library otherwise.

While at it, fix the conditions not to fall back to "result is static
library" path if it actually was recognized as a shared library but
zstd_shared target already existed.

Fixes #59056

[mgorny]

@phosek, could you test whether this doesn't break MSVC?

[MaskRay]

I don't know much of the cmake mechanism, so I can only provide tests on Debian testing. I'll leave it to @brad and @phosek
This patch works with the libzstd-dev package.

% grep -i ZSTD /tmp/out/custom2/CMakeCache.txt
//Use zstd for compression/decompression if available. Can be ON,
LLVM_ENABLE_ZSTD:STRING=ON
//Use static version of zstd. Can be TRUE, FALSE
LLVM_USE_STATIC_ZSTD:BOOL=FALSE
zstd_INCLUDE_DIR:PATH=/usr/include
zstd_LIBRARY:FILEPATH=/usr/lib/x86_64-linux-gnu/libzstd.so
zstd_STATIC_LIBRARY:FILEPATH=/usr/lib/x86_64-linux-gnu/libzstd.a
//ADVANCED property for variable: zstd_INCLUDE_DIR
zstd_INCLUDE_DIR-ADVANCED:INTERNAL=1
//ADVANCED property for variable: zstd_LIBRARY
zstd_LIBRARY-ADVANCED:INTERNAL=1
//ADVANCED property for variable: zstd_STATIC_LIBRARY
zstd_STATIC_LIBRARY-ADVANCED:INTERNAL=1

# with a -DCMAKE_PREFIX_PATH=/tmp/p/zstd/out/release build
% grep -i ZSTD /tmp/out/custom2/CMakeCache.txt
CMAKE_PREFIX_PATH:UNINITIALIZED=/tmp/p/zstd/out/release
//Use zstd for compression/decompression if available. Can be ON,
LLVM_ENABLE_ZSTD:STRING=ON
//Use static version of zstd. Can be TRUE, FALSE
LLVM_USE_STATIC_ZSTD:BOOL=FALSE
zstd_INCLUDE_DIR:PATH=/usr/include
zstd_LIBRARY:FILEPATH=/tmp/p/zstd/out/release/lib/libzstd.so
zstd_STATIC_LIBRARY:FILEPATH=/tmp/p/zstd/out/release/lib/libzstd.a
//ADVANCED property for variable: zstd_INCLUDE_DIR
zstd_INCLUDE_DIR-ADVANCED:INTERNAL=1
//ADVANCED property for variable: zstd_LIBRARY
zstd_LIBRARY-ADVANCED:INTERNAL=1
//ADVANCED property for variable: zstd_STATIC_LIBRARY
zstd_STATIC_LIBRARY-ADVANCED:INTERNAL=1

[brad]

humpty$ grep -i zstd CMakeCache.txt
//Use zstd for compression/decompression if available. Can be ON,
LLVM_ENABLE_ZSTD:STRING=ON
//Use static version of zstd. Can be TRUE, FALSE
LLVM_USE_STATIC_ZSTD:BOOL=FALSE
zstd_INCLUDE_DIR:PATH=/usr/local/include
zstd_LIBRARY:FILEPATH=/usr/local/lib/libzstd.so.6.1
zstd_STATIC_LIBRARY:FILEPATH=/usr/local/lib/libzstd.a
//ADVANCED property for variable: zstd_INCLUDE_DIR
zstd_INCLUDE_DIR-ADVANCED:INTERNAL=1
//ADVANCED property for variable: zstd_LIBRARY
zstd_LIBRARY-ADVANCED:INTERNAL=1
//ADVANCED property for variable: zstd_STATIC_LIBRARY
zstd_STATIC_LIBRARY-ADVANCED:INTERNAL=1

No error with the patch applied. Looking at build.ninja it appears to be linking against the zstd shared library. I am running a build.

[phosek]

llvm/cmake/modules/Findzstd.cmake

34–55

This could be combined to an elseif block.

56–62

I'd consider combining this if block with the if block above.

[mgorny]

Combine else+if into elseif.

[mgorny]

llvm/cmake/modules/Findzstd.cmake

56–62

Do you have anything specific in mind? What we do here depends on the branch taken in the above if but it needs to happen for both branches, i.e. if zstd_LIBRARY was a static lib, then we use that, otherwise we rely on separate static lib lookup result.

[brad]

In D138361#3939762, @mgorny wrote:

Combine else+if into elseif.

This appears to be good.

[mgorny]

@phosek, very gentle ping ;-).

[phosek]

LGTM

llvm/cmake/modules/Findzstd.cmake

56–62

I misread the condition, thanks for clarifying.

[mgorny]

Thank you!

llvm/cmake/modules/Findzstd.cmake

 # Try to find the zstd library
 #
 # If successful, the following variables will be defined:
 # zstd_INCLUDE_DIR
 # zstd_LIBRARY
 # zstd_STATIC_LIBRARY
 # zstd_FOUND
 #
 # Additionally, one of the following import targets will be defined:
 # zstd::libzstd_shared
 # zstd::libzstd_static
 
 if(MSVC)
-  set(zstd_SHARED_LIBRARY_SUFFIX "\\${CMAKE_LINK_LIBRARY_SUFFIX}$")
   set(zstd_STATIC_LIBRARY_SUFFIX "_static\\${CMAKE_STATIC_LIBRARY_SUFFIX}$")
 else()
-  set(zstd_SHARED_LIBRARY_SUFFIX "\\${CMAKE_SHARED_LIBRARY_SUFFIX}$")
   set(zstd_STATIC_LIBRARY_SUFFIX "\\${CMAKE_STATIC_LIBRARY_SUFFIX}$")
 endif()
 
 find_path(zstd_INCLUDE_DIR NAMES zstd.h)
 find_library(zstd_LIBRARY NAMES zstd zstd_static)
 find_library(zstd_STATIC_LIBRARY NAMES
   zstd_static
   "${CMAKE_STATIC_LIBRARY_PREFIX}zstd${CMAKE_STATIC_LIBRARY_SUFFIX}")
 
 include(FindPackageHandleStandardArgs)
 find_package_handle_standard_args(
     zstd DEFAULT_MSG
     zstd_LIBRARY zstd_INCLUDE_DIR
 )
 
 if(zstd_FOUND)
-  if(zstd_LIBRARY MATCHES "${zstd_SHARED_LIBRARY_SUFFIX}$" AND
-     NOT TARGET zstd::libzstd_shared)
+  if(zstd_LIBRARY MATCHES "${zstd_STATIC_LIBRARY_SUFFIX}$")
+    set(zstd_STATIC_LIBRARY "${zstd_LIBRARY}")
+  else()
+    if (NOT TARGET zstd::libzstd_shared)
       add_library(zstd::libzstd_shared SHARED IMPORTED)
       if(MSVC)
         # IMPORTED_LOCATION is the path to the DLL and IMPORTED_IMPLIB is the "library".
         get_filename_component(zstd_DIRNAME "${zstd_LIBRARY}" DIRECTORY)
         string(REGEX REPLACE "${CMAKE_INSTALL_LIBDIR}$" "${CMAKE_INSTALL_BINDIR}" zstd_DIRNAME "${zstd_DIRNAME}")
         get_filename_component(zstd_BASENAME "${zstd_LIBRARY}" NAME)
         string(REGEX REPLACE "\\${CMAKE_LINK_LIBRARY_SUFFIX}$" "${CMAKE_SHARED_LIBRARY_SUFFIX}" zstd_BASENAME "${zstd_BASENAME}")
         set_target_properties(zstd::libzstd_shared PROPERTIES
             INTERFACE_INCLUDE_DIRECTORIES "${zstd_INCLUDE_DIR}"
             IMPORTED_LOCATION "${zstd_DIRNAME}/${zstd_BASENAME}"
             IMPORTED_IMPLIB "${zstd_LIBRARY}")
         unset(zstd_DIRNAME)
         unset(zstd_BASENAME)
       else()
         set_target_properties(zstd::libzstd_shared PROPERTIES
             INTERFACE_INCLUDE_DIRECTORIES "${zstd_INCLUDE_DIR}"
             IMPORTED_LOCATION "${zstd_LIBRARY}")
       endif()
-  else()
-    set(zstd_STATIC_LIBRARY "${zstd_LIBRARY}")
+    endif()
   endif()
   if(zstd_STATIC_LIBRARY MATCHES "${zstd_STATIC_LIBRARY_SUFFIX}$" AND
      NOT TARGET zstd::libzstd_static)
     add_library(zstd::libzstd_static STATIC IMPORTED)
     set_target_properties(zstd::libzstd_static PROPERTIES
         INTERFACE_INCLUDE_DIRECTORIES "${zstd_INCLUDE_DIR}"
         IMPORTED_LOCATION "${zstd_STATIC_LIBRARY}")
   endif()
 endif()
 
-unset(zstd_SHARED_LIBRARY_SUFFIX)
 unset(zstd_STATIC_LIBRARY_SUFFIX)
 
 mark_as_advanced(zstd_INCLUDE_DIR zstd_LIBRARY zstd_STATIC_LIBRARY)