[Lldb-commits] [PATCH] D72290: [lldb/CMake] Use LLDB's autodetection logic for libxml2

Tue Jan 7 16:39:53 PST 2020

compnerd added inline comments.

================
Comment at: lldb/cmake/modules/FindLibXml28.cmake:14
+    if (APPLE)
+      set(LIBXML2_LIBRARIES xml2)
+    endif()
----------------
labath wrote:
> JDevlieghere wrote:
> > kwk wrote:
> > > labath wrote:
> > > > Why is this under `if(APPLE)` ?
> > > To me this looks as if this module file `FileLibXml28.cmake` is only relevant for Apple. In line 10 it falls back to the standard CMake find-module file (`find_package(LibXml2 QUIET)`). Is this correct?
> > I'm not sure, I just kept the old behavior. I'm pretty sure it doesn't matter. Do you prefer to make it unconditional or just remove it altogether? 
> Ah, right, I think I now see where this is coming from. I don't think it makes sense to keep this if xml is disabled then linking against it is useless. Ideally, I'd just remove it...
Why the custom module for this?  Why not use `find_package(LibXml2 2.8 QUIET)` and ensure that libxml2 is >= 2.8?

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

https://reviews.llvm.org/D72290