[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

[Petr Hosek via Phabricator via lldb-commits]

phosek added a comment.

In D79219#2203887 <https://reviews.llvm.org/D79219#2203887>, @lxfind wrote:

> In D79219#2201415 <https://reviews.llvm.org/D79219#2201415>, @phosek wrote:
>
>> This is correct. That target is provided by `find_package(ZLIB)`. In LLVMConfig.cmake, we invoke `find_package(ZLIB)` when zlib support is enabled. If you're using `LLVMExports.cmake`, you'll need to invoke `find_package(ZLIB)` yourself.
>
> @phosek, Thanks for the reply. I am not too familiar with this. Could you please elaborate more on why setting it to `ZLIB::ZLIB` here is more modern? And how one is expected to deal with `ZLIB::ZLIB` that shows up in LLVMExports.cmake` (in the modern way)? Should one string pattern matching for `ZLIB::ZLIB` and invoke `find_package(ZLIB)` again? (previously one could simply concatenate `-l` with the library name in there.

@lxfind `ZLIB::ZLIB` is the name of the imported target that's defined by CMake when you invoke `find_dependency(ZLIB)`. Ideally, you'd just use `LLVMConfig.cmake` which invokes that for you if needed. https://pabloariasal.github.io/2018/02/19/its-time-to-do-cmake-right/ has some more details.

`ZLIB::ZLIB` includes the correct include and library directory, so you no longer have to manually append `-I` or `-l`, all you need is to include `ZLIB::ZLIB` in the list of your dependencies, which is one advantage. Another advantage is that `ZLIB::ZLIB` can expand to different things, for example some users may want to statically link zlib in which case they would configure their build accordingly and `ZLIB::ZLIB` would expand into `/path/to/libz.a` (this is for example what we do in our toolchain build), others may be fine using the default system library in which case `ZLIB::ZLIB` would typically expand to just `z`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219