[Lldb-commits] [PATCH] D70764: build: reduce CMake handling for zlib

hans added a comment. There were some concerns raised on this patch, and also in PR44780. I think at this point it's safer to revert these changes and start over. I've pushed the revert in 916be8fd6a0a0feea4cefcbeb0c22c65848d7a2e

[Lldb-commits] [PATCH] D70764: build: reduce CMake handling for zlib

raj.khem added a comment. In D70764#1807405 , @smeenai wrote: > In D70764#1803559 , @raj.khem wrote: > > > this is now in master, and I am seeing build failures in cross-building > > clang, e.g. when building clang

[Lldb-commits] [PATCH] D70764: build: reduce CMake handling for zlib

smeenai added a comment. In D70764#1803559 , @raj.khem wrote: > this is now in master, and I am seeing build failures in cross-building > clang, e.g. when building clang for arm on a x86_64 host. its resorting to > finding, libz from buildhost instead of

[Lldb-commits] [PATCH] D70764: build: reduce CMake handling for zlib

raj.khem added a comment. this is now in master, and I am seeing build failures in cross-building clang, e.g. when building clang for arm on a x86_64 host. its resorting to finding, libz from buildhost instead of target sysroot ( using --sysroot) and failing in link step. e.g. FAILED: bin/llvm

[Lldb-commits] [PATCH] D70764: build: reduce CMake handling for zlib

compnerd closed this revision. compnerd added a comment. GIT 68a235d07f9 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70764/new/ https://reviews.llvm.org/D70764

[Lldb-commits] [PATCH] D70764: build: reduce CMake handling for zlib

beanz accepted this revision. beanz added a comment. This revision is now accepted and ready to land. I think this is good, and it has been sitting a while. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70764/new/ https://reviews.llvm.org/D70764

[Lldb-commits] [PATCH] D70764: build: reduce CMake handling for zlib

beanz added a comment. I conferred off-list with @compnerd. He and I talked through my concerns, and I believe this patch is fine as-is, so LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70764/new/ https://reviews.llvm.org/D70764 _

[Lldb-commits] [PATCH] D70764: build: reduce CMake handling for zlib

labath added a comment. `/usr/lib/x86_64-linux-gnu/libz.so` is definitely better than `-l/usr/lib/x86_64-linux-gnu/libz.so`, as it's at least a valid link line. It's not completely equivalent, and may not work in all cases -- last time I accidentally changed this, I got about a dozen emails fro

[Lldb-commits] [PATCH] D70764: build: reduce CMake handling for zlib

beanz added a comment. @labath sorry for not replying on-list. I've actually been discussing offline with @compnerd. If `llvm-config` is printing an absolute path, I'm concerned about how that will interact with binary package distributions for the llvm-dev packages. Not sure if @tstellar has a

[Lldb-commits] [PATCH] D70764: build: reduce CMake handling for zlib

JDevlieghere added a comment. In D70764#1767413 , @compnerd wrote: > In D70764#1767395 , @JDevlieghere > wrote: > > > Having one canonical variable controlling zlib support seems indeed > > desirable. > > > > In D

[Lldb-commits] [PATCH] D70764: build: reduce CMake handling for zlib

compnerd added a comment. In D70764#1767395 , @JDevlieghere wrote: > Having one canonical variable controlling zlib support seems indeed desirable. > > In D70519#1754618 , @labath wrote: > > > With this patch, what

[Lldb-commits] [PATCH] D70764: build: reduce CMake handling for zlib

JDevlieghere added a comment. Having one canonical variable controlling zlib support seems indeed desirable. In D70519#1754618 , @labath wrote: > With this patch, what is the output of `llvm-config --system-libs` ? @compnerd What's the answer to this fo

[Lldb-commits] [PATCH] D70764: build: reduce CMake handling for zlib

compnerd added a comment. @labath I think you are misunderstanding the patch. This is not autoselecting the dependencies. It is simply doing that based on an existing option that we have - `LLVM_ENABLE_ZLIB`. We could always search for zlib and override the results with `LLVM_ENABLE_ZLIB` as

[Lldb-commits] [PATCH] D70764: build: reduce CMake handling for zlib

labath added a comment. Please take a look at D70519 for the issues with this approach. Also, while I do agree with you that we should not auto-select dependencies, I think this runs contrary to the llvm philosophy that the default built should "just work" (I

[Lldb-commits] [PATCH] D70764: build: reduce CMake handling for zlib

compnerd created this revision. compnerd added reviewers: beanz, smeenai. Herald added subscribers: lldb-commits, Sanitizers, hiraditya, mgorny. Herald added projects: clang, Sanitizers, LLDB, LLVM. Rather than handling zlib handling manually, use `find_package` from CMake to find zlib properly.