[lldb] [test] Fix tests to use more portable LLVM_ENABLE_ZLIB

[mgorny]

Summary

The HAVE_LIBZ variable is not exported by LLVM, and therefore is not
available in stand-alone builds of other tools. Use LLVM_ENABLE_ZLIB
which is the name under which the effective value is exported.

Additional, use llvm_canonicalize_cmake_booleans() to make sure that
a correct (Python-safe) boolean value is passed down to lit.

[labath]

Maybe we could just change the lit script to use the value of LLVM_ENABLE_ZLIB as the trigger? That should work in both standalone and non-standalone builds (and it seems more clean -- I'm not sure why llvm does not do the same)?

[mgorny]

Maybe we could. I presumed people are using the separate variables for some reason. We'd have to check that it's properly sanitized when building in-tree and given to cmake though.

[labath]

In D41725#969183, @mgorny wrote:

Maybe we could. I presumed people are using the separate variables for some reason. We'd have to check that it's properly sanitized when building in-tree and given to cmake though.

I would expect that reason is purely historical. While working on that patch I got the feeling that there is a lot of confusion in how to check for zlib presence. All everyone wants to know is whether zlib is present and working, so there should not be a need for different checks. We certainly don't need anything fancy here.

[mgorny]

Well, it's actually more complex. In Gentoo we want to let the user choose whether he wants to build against zlib or without zlib support. So in the end there are two uses to be considered:

1. When the value is merely passed down from LLVM to indicate state of compression support in LLVM. AFAIR that was the case with clang -- we merely wanted to know if we can run tests that require LLVM to have compression support.
2. When the value is actually used to use zlib locally. To be honest, in this case I believe we should have a separate check & switch. FWICS, this is the case with LLDB.

So I don't think using LLVM_ENABLE_ZLIB here is correct, and my patch isn't correct as well.

That said, is there a reason for LLDB to use zlib directly instead of using the LLVM support compression library?

[labath]

In D41725#969189, @mgorny wrote:

That said, is there a reason for LLDB to use zlib directly instead of using the LLVM support compression library?

We aren't using zlib directly. We only need to know whether llvm was configured with zlib support, so we can know whether we can expect some tests to pass. That is what this piece of code does.

[mgorny]

I was talking of:

source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
tools/debugserver/source/RNBRemote.cpp

[mgorny]

Updated the patch to use LLVM_ENABLE_ZLIB directly.

[labath]

In D41725#969388, @mgorny wrote:

I was talking of:

source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

Ah, right. These uses were introduced before we developed a preference for llvm primitives. That said, I see a lot of compression stuff there, but I don't see mention of zlib directly.

tools/debugserver/source/RNBRemote.cpp

Debugserver does not use llvm on purpose, but that should only worry you if your targetting darwin.

In any case, lgtm.

[mgorny]

In D41725#970926, @labath wrote:

In D41725#969388, @mgorny wrote:

I was talking of:

source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

Ah, right. These uses were introduced before we developed a preference for llvm primitives. That said, I see a lot of compression stuff there, but I don't see mention of zlib directly.

In the first of those files there are two HAVE_LIBZ conditionals that use zlib. There's also HAVE_LIBCOMPRESSION for the other library, I suppose it's relevant to Darwin.

Therefore, I have three questions:

1. Should we add a separate check/switch for zlib here, or just use LLVM_ENABLE_ZLIB?
2. Should I post a patch rewriting this little zlib bit to reuse libLLVMSupport?
3. Should I look into merging libcompression support into libLLVMSupport?

tools/debugserver/source/RNBRemote.cpp

Debugserver does not use llvm on purpose, but that should only worry you if your targetting darwin.

In any case, lgtm.

Thanks, merged.

lit/CMakeLists.txt

 set(LLVM_SOURCE_DIR "${LLVM_MAIN_SRC_DIR}")
 set(LLVM_BINARY_DIR "${LLVM_BINARY_DIR}")
 set(LLVM_BUILD_MODE "%(build_mode)s")
 set(LLVM_TOOLS_DIR "${LLVM_TOOLS_BINARY_DIR}/%(build_config)s")
 set(LLVM_LIBS_DIR "${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/%(build_config)s")
 set(CLANG_SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/..")
 set(CLANG_BINARY_DIR "${CMAKE_CURRENT_BINARY_DIR}/..")
 if(BUILD_SHARED_LIBS)
   set(ENABLE_SHARED 1)
 else()
   set(ENABLE_SHARED 0)
 endif(BUILD_SHARED_LIBS)
+if(LLDB_BUILT_STANDALONE)
+  # Set HAVE_LIBZ according to recorded LLVM_ENABLE_ZLIB value. This
+  # value is forced to 0 if zlib was not found, so it is fine to use it
+  # instead of HAVE_LIBZ (not recorded).
+  if(LLVM_ENABLE_ZLIB)
+    set(HAVE_LIBZ 1)
+  endif()
+endif()
+
+llvm_canonicalize_cmake_booleans(
+  HAVE_LIBZ)
 
 configure_lit_site_cfg(
   ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.in
   ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg)
 configure_lit_site_cfg(
   ${CMAKE_CURRENT_SOURCE_DIR}/Unit/lit.site.cfg.in
   ${CMAKE_CURRENT_BINARY_DIR}/Unit/lit.site.cfg
   )