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

[Shoaib Meenai via Phabricator via lldb-commits]

smeenai added inline comments.


================
Comment at: lld/test/lit.site.cfg.py.in:16
 config.target_triple = "@TARGET_TRIPLE@"
-config.python_executable = "@Python3_EXECUTABLE@"
-config.have_zlib = @HAVE_LIBZ@
+config.python_executable = "@PYTHON_EXECUTABLE@"
+config.have_zlib = @LLVM_ENABLE_ZLIB@
----------------
This seems like an unintended/unrelated change.


================
Comment at: llvm/cmake/config-ix.cmake:506
 
-if (LLVM_ENABLE_ZLIB )
-  # Check if zlib is available in the system.
-  if ( NOT HAVE_ZLIB_H OR NOT HAVE_LIBZ )
-    set(LLVM_ENABLE_ZLIB 0)
+if(LLVM_ENABLE_ZLIB)
+  if(LLVM_ENABLE_ZLIB STREQUAL FORCE_ON)
----------------
How come you're moving this out of the MSan check block? Isn't that a behavior change?


================
Comment at: llvm/lib/Support/CMakeLists.txt:207
+  get_filename_component(zlib_library ${zlib_library} NAME)
+  if(CMAKE_STATIC_LIBRARY_PREFIX AND
+     zlib_library MATCHES "^${CMAKE_STATIC_LIBRARY_PREFIX}.*${CMAKE_STATIC_LIBRARY_SUFFIX}$")
----------------
Even if there's no prefix, wouldn't you still wanna strip the suffix?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219