CMake: CMAKE_FIND_ROOT_PATH_MODE variables should not override values

[rcdailey]

The current android toolchain file does this:

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

This overwrites values specified at the command line. For example:

cmake -G Ninja -D CMAKE_FIND_ROOT_PATH_MODE_PACKAGE=NEVER -D CMAKE_TOOLCHAIN_FILE=....

The justification for this being specified at the command line has to do with how I manage my third party dependencies. For example, I build zlib myself instead of using the NDK-provided zlib (I also do not load the libz.so provided in /system/lib). I literally want none of the find_*() functions I perform to find anything in the NDK dir.

As such, the toolchain file should respect parameters already set from outside. The below changes fix this for me:

if(NOT CMAKE_FIND_ROOT_PATH_MODE_PROGRAM)
    set(CMAKE_FIND_ROOT_PATH_MODE_PROGRAM NEVER)
endif()

if(NOT CMAKE_FIND_ROOT_PATH_MODE_LIBRARY)
    set(CMAKE_FIND_ROOT_PATH_MODE_LIBRARY ONLY)
endif()

if(NOT CMAKE_FIND_ROOT_PATH_MODE_INCLUDE)
    set(CMAKE_FIND_ROOT_PATH_MODE_INCLUDE ONLY)
endif()

if(NOT CMAKE_FIND_ROOT_PATH_MODE_PACKAGE)
    set(CMAKE_FIND_ROOT_PATH_MODE_PACKAGE ONLY)
endif()

Would this be an acceptable modification of the NDK toolchain file?

[DanAlbert]

Yeah, that looks right to me. (If you want to upload the change yourself feel free, but it's a bit of a hassle to setup and build the NDK)

[rcdailey]

I definitely can't build the NDK. The only testing I can do is to modify the toolchain file in an installed version of the NDK and test it in my own project. I'm not sure if this is sufficient.

[DanAlbert]

Don't worry about it then. Easy enough for me to make the change on your behalf :)

[rcdailey]

Thanks Dan; and sorry for the trouble!

[enh]

I build zlib myself instead of using the NDK-provided zlib (I also do not load the libz.so provided in /system/lib)

off-topic, but i'd love to hear your rationale for this. switching folks off the system libz is something we talk about internally (because it's not entirely ABI stable, and that causes very advanced users some trouble), but i'm curious to know if there's any stronger motivation than that. (because obviously this would be likely to upset app developers, so i'm not keen to do it unless i get time machine access to just prevent libz from ever having appeared in the NDK, or there's some stronger motivation.)

[rcdailey]

Ultimately our reason is simple: We carry all of our third party dependencies across multiple platforms (Linux OS, Windows OS, stuff outside of android). As such, we need consistent versions of those dependencies across platforms. Furthermore, we need the ability to customize how our libraries are built (shared versus static, etc). The only way I've found that we can guarantee that is by not having dependencies on libraries outside of our control. This is why I need to override the NDK toolchain to stop forcing CMake to search in CMAKE_SYSROOT.

An even better example would be OpenSSL. Imagine if you guys packaged that in /system/lib, and we're using that because "it's there and that's convenient for us". Out software has to follow PCI guidelines because we process payments, so secure sockets are required and we have strict rules we have to follow, especially when it comes to responding to exploits. If the shipped version of OpenSSL in Android (let's assume it had one, for the sake of argument) had those exploits, we couldn't use it and would have to build it ourselves anyway to get the newest version which has fixes. So it's a false convenience, sadly.

I understand that not everyone builds their own third party libs, that is an exceptional case for us. However, Android OS doesn't have a package manager in the ubuntu sense, so we cannot control what version of zlib is on the system across multiple OS versions. So we package all dependencies in our APKs and keep things self-sufficient.

Hope that helps.

[enh]

yeah, thanks. that's basically the case we've already seen (and definitely makes sense).

[DanAlbert]

https://android-review.googlesource.com/#/c/platform/ndk/+/484980

[DanAlbert]

This was merged into r16.