[PATCH] D47560: [CMake] Use find_package to find zlib

[Petr Hosek via Phabricator via llvm-commits]

phosek added inline comments.


================
Comment at: llvm/lib/Support/CMakeLists.txt:3
+if ( HAVE_LIBZ )
   set(system_libs ${system_libs} ${ZLIB_LIBRARIES})
 endif()
----------------
labath wrote:
> delcypher wrote:
> > labath wrote:
> > > What would be the new value of `ZLIB_LIBRARIES` here? One of my previous attempts to refactor this failed because we have code which assumes `system_libs` is a list bare library names `z dl ...` that can then be prefixed with `-l` and passed to the linker. This fails horribly if you specify a full path to a library (which cmake likes to to).
> > > Please make sure that `llvm-config --system-libs` produces something reasonable after this patch.
> > Sounds like we should have a better test case then. It looks like `test/tools/llvm-config/system-libs.test` is too weak to check what you asking for.
> Yes, a better test case would be useful certainly (but don't worry you will get test failures, they will just be in compiler-rt, or other projects which use llvm-config to invoke the linker :)).
> 
> Anyway, I am not asking for anything. I don't have a horse in this game; as long as the main cmake project builds I am happy. I'm just giving you a heads up that you are likely to run into issues down this road.
Isn't this actually more correct? In our case, I'd like to use our own version of zlib and libxml2 rather than the system ones using the `CMAKE_FIND_ROOT_PATH` option. If `llvm-config --system-libs` prints `-lz -lxml2`, it's ambiguous which zlib and libxml2 it's referring to. Furthermore, in our case we want to link zlib and libxml2 statically, so presumably `llvm-config --system-libs` shouldn't print `-lz -lxml2` at all but that's not currently the case.


Repository:
  rL LLVM

https://reviews.llvm.org/D47560