JDevlieghere marked an inline comment as done. JDevlieghere added inline comments.
================ Comment at: lldb/cmake/modules/FindLibXml28.cmake:14 + if (APPLE) + set(LIBXML2_LIBRARIES xml2) + endif() ---------------- compnerd wrote: > 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? Right, with the special `if(APPLE)` logic gone there there's no good reason to have a custom module anymore. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72290/new/ https://reviews.llvm.org/D72290 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
