[PATCH] D134990: [CMake] Provide Findzstd module

Michał Górny via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 30 23:32:01 PDT 2022


mgorny added a comment.

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.



================
Comment at: llvm/cmake/modules/Findzstd.cmake:13
+find_path(ZSTD_INCLUDE_DIR NAMES zstd.h)
+find_library(ZSTD_LIBRARY NAMES zstd)
+
----------------
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)
```


================
Comment at: llvm/cmake/modules/Findzstd.cmake:17
+find_package_handle_standard_args(
+    ZSTD DEFAULT_MSG
+    ZSTD_LIBRARY ZSTD_INCLUDE_DIR
----------------
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134990/new/

https://reviews.llvm.org/D134990



More information about the llvm-commits mailing list