[CMake] Provide Findzstd module

[phosek]

Summary

This module is used to find the system zstd library. The imported
targets intentionally use the same name as the generate zstd config
CMake file so these can be used interchangeably.

[MaskRay]

Thanks for adding the support! However, I get such a warning and my built ninja lld does not link against libzstd...

CMake Warning (dev) at /usr/share/cmake-3.24/Modules/FindPackageHandleStandardArgs.cmake:438 (message):
  The package name passed to `find_package_handle_standard_args` (ZSTD) does
  not match the name of the calling package (zstd).  This can lead to
  problems in calling code that expects `find_package` result variables
  (e.g., `_FOUND`) to follow a certain pattern.
Call Stack (most recent call first):
  cmake/modules/Findzstd.cmake:16 (find_package_handle_standard_args)
  cmake/config-ix.cmake:149 (find_package)
  CMakeLists.txt:843 (include)
This warning is for project developers.  Use -Wno-dev to suppress it.

[mgorny]

Thanks a lot for doing this. Please consider the more fancy suggestions non-obligatory, I think just find_package_handle_standard_args needs to be fixed.

By the way, you may also look at FindLibEdit.cmake on how to grab directory hints from pkg-config.

However, if you don't feel like doing all the extra things, I can look into making a followup patches for them.

llvm/cmake/modules/Findzstd.cmake

14

Well, I think this is good enough for Gentoo since we hate static libraries but given all the fancy logic in LLVM you may want to try finding both shared and static. Thinking about it, you could issue a second find_library call like:

find_library(zstd_STATIC_LIBRARY_NAMES libzstd.a)

18

As @MaskRay pointed out, you probably need to make the zstd lowercase. I'd also make all the other ZSTD_* vars lowercase for consistency but not sure if that's necessary.

[mgorny]

llvm/cmake/modules/Findzstd.cmake

23

Pointed out by Rolf Eike Beer, the man with the coolest name in Gentoo ;-).

30

[mgorny]

I'm a little impatient and need a distraction, so I hope you don't mind if I try to finish your patch.

[mgorny]

Submitted my version as D135153.

[phosek]

I apologize about the delay, I implemented the suggested changes, but then I decided to also test this module on Windows and found out that more changes are going to be needed and haven't had time to implement those. I hope to find some time today or tomorrow.

[mgorny]

I think it's good enough for us. I wish you'd have implemented the improvements from my version but I can work on top of this.

[phosek]

In D134990#3836334, @mgorny wrote:

I think it's good enough for us. I wish you'd have implemented the improvements from my version but I can work on top of this.

I'd prefer adding functionality such as the pkg-config support in a follow up change. We would need to ensure it works correctly on systems that don't have pkg-config like Windows. I'd also prefer doing this consistently across all Find*.cmake modules; we currently don't support pkg-config in any of them.

[nikic]

I've reverted this patch because it breaks running LLVM tests with:

llvm-lit: /home/npopov/repos/llvm-project/llvm/utils/lit/lit/TestingConfig.py:138: fatal: unable to parse config file '/home/npopov/repos/llvm-project/build/test/lit.site.cfg.py', traceback: Traceback (most recent call last):
  File "/home/npopov/repos/llvm-project/llvm/utils/lit/lit/TestingConfig.py", line 127, in load_from_path
    exec(compile(data, path, 'exec'), cfg_globals, None)
  File "/home/npopov/repos/llvm-project/build/test/lit.site.cfg.py", line 48, in <module>
    config.have_zstd = FALSE
NameError: name 'FALSE' is not defined

I expect this is just a matter of a missing llvm_canonicalize_cmake_booleans entry, but it's not completely obvious to me how this is related to this patch.

[mgorny]

In D134990#3839541, @nikic wrote:

I expect this is just a matter of a missing llvm_canonicalize_cmake_booleans entry, but it's not completely obvious to me how this is related to this patch.

I've noticed this earlier today but couldn't submit patch then. Now D135357.

[mgorny]

My fix has landed, so I suppose you can try again.

[MaskRay]

Thanks to both of you! A regular build of clang/lld/etc now use zstd.

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_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)
+
+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)
+    add_library(zstd::libzstd_shared SHARED IMPORTED)
+    set_target_properties(zstd::libzstd_shared PROPERTIES
+          INTERFACE_INCLUDE_DIRECTORIES "${zstd_INCLUDE_DIR}"
+          IMPORTED_LOCATION "${zstd_LIBRARY}")
+  endif()
+  if(zstd_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_LIBRARY}")
+  endif()
+endif()
+
+unset(zstd_SHARED_LIBRARY_SUFFIX)
+unset(zstd_STATIC_LIBRARY_SUFFIX)
+
+mark_as_advanced(zstd_INCLUDE_DIR zstd_LIBRARY)