NDK cmake toolchain overrides CMAKE_FIND_ROOT_PATH

[aberaud]

Since NDKr19b (#890), android.toolchain.cmake overrides CMAKE_FIND_ROOT_PATH to points to the NDK directory.

This prevents projects building multiple libraries depending on each other to find other built libraries using find_path, find_library etc.

Indeed, CMAKE_FIND_ROOT_PATH is used to prefix every other cmake search path, so find_library would only be able to find libraries under the NDK path.

It's possible to specify multiple root paths, so instead of overriding, the NDK might set itself as the first root path ?

set(CMAKE_FIND_ROOT_PATH "${ANDROID_NDK};${CMAKE_FIND_ROOT_PATH}")

or something similar.

[DanAlbert]

That won't do because we do need to make sure that the host system isn't being searched (which is why this changed, causing this new bug), but if the user is supposed to be able to override that then obviously the r19b approach isn't working either.

I'll need to play with this a bit to see what we can do. Could you give me an example of how you use CMAKE_FIND_ROOT_PATH? I don't use CMake much myself, and this is a corner I'm rather unfamiliar with. I want to get a test case added to make sure we don't regress, but I don't know what that test case looks like. I was under the impression that CMAKE_MODULE_PATH and whatnot were the right things to override from the user side.

[aberaud]

We use CMAKE_FIND_ROOT_PATH to point to the cross-compilation root, where built libraries are installed.
This allows cmake to find other installed headers and built libraries with find_path, find_library etc.

According to the cmake documentation this is what CMAKE_FIND_ROOT_PATH is meant for:

This variable is most useful when cross-compiling. CMake uses the paths in this list as alternative roots to find filesystem items with find_package(), find_library() etc.

I think CMAKE_MODULE_PATH is specific to CMake modules and unrelated. CMAKE_PREFIX_PATH and al are simply appended to CMAKE_FIND_ROOT_PATH so they don't allow to find stuff outside of the root paths.

Test case

A project that builds two libraries:

• Library A, installed to some arbitrary rootpath $(PREFIX) (might just be some header file include/test.h)
• Library B, depends on A, uses CMake to find A with CMAKE_FIND_ROOT_PATH set to the rootpath where A is installed.

CMakeLists.txt of library B:

find_path(test_INCLUDE test.h)

if (test_INCLUDE)
    message(STATUS "Found test include at: ${test_INCLUDE}")
else ()
    message(FATAL_ERROR "Failed to locate test dependency.")
endif ()

Library B configured with:

cmake . -DCMAKE_TOOLCHAIN_FILE=$(ANDROID_NDK)/build/cmake/android.toolchain.cmake \
		-DANDROID_PLATFORM=$(ANDROID_API) \
		-DANDROID_ABI=$(ANDROID_ABI) \
		-DCMAKE_FIND_ROOT_PATH=$(PREFIX) \
		-DCMAKE_INSTALL_PREFIX=$(PREFIX)

I guess a full testcase might also make library B depend on some system library..

Possible solutions

By default, CMAKE_FIND_ROOT_PATH is empty. If set to any value, the host system won't be searched. This means the proposed solution of appending the NDK path instead of overriding would likely work fine. In that case the project would look in priority in the NDK directory and fallback to the project rootpath, if set.

The NDK might also set CMAKE_SYSROOT and let projets set CMAKE_FIND_ROOT_PATH themself. Not certain but this seems like the intended use of CMake (sysroot for system headers/libraries, root path for the project). This means more work for projects, but that's what projects already do for cross-compiling for other platforms.

[DanAlbert]

By default, CMAKE_FIND_ROOT_PATH is empty. If set to any value, the host system won't be searched. This means the proposed solution of appending the NDK path instead of overriding would likely work fine. In that case the project would look in priority in the NDK directory and fallback to the project rootpath, if set.

Yeah, given the explanation I think you're right. I'll make sure all our existing tests pass with that change and then see if I can work up a test case that makes use of CMAKE_FIND_ROOT_PATH so we don't regress (such a test will be... complicated in our infrastructure, but I'll give it a shot).

[DanAlbert]

https://android-review.googlesource.com/c/platform/ndk/+/907834 doesn't break any of our existing tests and does seem to fix the problem. For the moment I've only added a trivial test to make sure that we don't clobber user input, but it may be worth trying to build a more complete test at some point.

Thanks for the report and suggestions on this!

[aberaud]

Many thanks !

[DanAlbert]

Merged into r19c.