Respect CMAKE_SYSROOT and CMAKE_CROSSCOMPILING when searching for libxml2.

[srhines]

Summary

If we ignore these variables, we end up always using the host
information for libxml2, when it is not appropriate. This makes builds
less hermetic than they should ideally be.

[srhines]

Rui, I added you to this review (and the other corresponding one), as you reviewed ecbeckmann's original work here. Can you take a quick look at these? Thanks.

[smeenai]

I think it might be more appropriate to pass NO_SYSTEM_ENVIRONMENT_PATH to find_package in case CMAKE_SYSROOT or CMAKE_CROSSCOMPILING are set? That way, we won't search the host system for libxml2 in those cases, but we'll still be able to find it in the custom sysroot or cross-compilation SDK if it's present.

[smeenai]

Actually, I would imagine that if you're cross-compiling or using a custom sysroot, you should probably also specify CMAKE_FIND_ROOT_PATH and set CMAKE_FIND_ROOT_PATH_MODE_PACKAGE to ONLY, to limit all these searches to the desired directories?

(I'm actually playing around with a bunch of this stuff myself right now. I'm curious about your CMake setup for a custom sysroot or cross-compilation and what issues you've come across.)

[smeenai]

Ah, the documentation is confusing, but CMAKE_FIND_ROOT_PATH and CMAKE_FIND_ROOT_PATH_MODE_PACKAGE only have an effect when using the config mode of find_package, whereas this invocation is using the module mode. That's a bummer, and definitely makes custom sysroots and cross-compilation harder than it should be. Sorry for the noise.

[smeenai]

Okay, upon playing with this further, the following seems to do what I want (for a custom sysroot), and seems to be a pretty common pattern as well:

set(CMAKE_FIND_ROOT_PATH_MODE_INCLUDE ONLY)
set(CMAKE_FIND_ROOT_PATH_MODE_LIBRARY ONLY)
set(CMAKE_FIND_ROOT_PATH_MODE_PACKAGE ONLY)
set(CMAKE_FIND_ROOT_PATH_MODE_PROGRAM NEVER)

[phosek]

CMakeLists.txt

191

This isn't going to cause a problem for us since we always build LLVM against explicit sysroot even building for host, but we would still like to link against libxml2, except it should come from our sysroot rather than host.

[srhines]

In D48459#1143012, @smeenai wrote:

Actually, I would imagine that if you're cross-compiling or using a custom sysroot, you should probably also specify CMAKE_FIND_ROOT_PATH and set CMAKE_FIND_ROOT_PATH_MODE_PACKAGE to ONLY, to limit all these searches to the desired directories?

(I'm actually playing around with a bunch of this stuff myself right now. I'm curious about your CMake setup for a custom sysroot or cross-compilation and what issues you've come across.)

https://android.googlesource.com/toolchain/llvm_android/+/master/build.py is what we use to wrap all the CMake invocations that we do to create Android's toolchain. We have to currently iterate because we need to create multiple different runtimes for all of our supported architectures. I don't think we have hit many other problems. This is something that is only affecting some of our users, but I wanted to try to fix it better than "just uninstall libxml2".

As far as this patch goes, I am trying to experiment with those other variables you mentioned. If that is enough to solve all of our problems, I'm probably just going to abandon these two patches. CMake remains terribly painful to use, especially for these cross-compiling scenarios, which are not that well-documented in general.

[srhines]

Removed this in favor of the suggestions here. Setting the CMAKE_FIND_ROOT_PATH_MODE* variables does make this properly hermetic.

CMakeLists.txt

 endif()
 
 # Make sure that our source directory is on the current cmake module path so that
 # we can include cmake files from this directory.
 list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake/modules")
 
 # Don't look for libxml if we're using MSan, since uninstrumented third party
 # code may call MSan interceptors like strlen, leading to false positives.
-if(NOT LLVM_USE_SANITIZER MATCHES "Memory.*")
+# Similarly, don't look for libxml on the host if we have a specific sysroot or
+# are cross-compiling.
+if(NOT CMAKE_SYSROOT AND NOT CMAKE_CROSSCOMPILING AND NOT LLVM_USE_SANITIZER MATCHES "Memory.*")
   set (LIBXML2_FOUND 0)
   find_package(LibXml2 2.5.3 QUIET)
   if (LIBXML2_FOUND)
     set(CLANG_HAVE_LIBXML 1)
   endif()
 endif()
 
 include(CheckIncludeFile)