search

taka-no-me / android-cmake

CMake toolchain file and other scripts for the Android NDK
1.22k stars 437 forks source link

Make ANDROID a cache variable. #42

Closed efidler closed 9 years ago

efidler commented 9 years ago

Some projects that use CMake and support Android have something in their CMakeLists.txt like: OPTION( ANDROID "Build for Android NDK" OFF )

Using this toolchain file won't override the ANDROID option the first time CMake runs unless ANDROID is a cache variable.

wonder-mice commented 9 years ago

If project has such an option, then probably it has some internal support for ANDROID build and doesn't require a toolchain. And if it requires a toolchain, then it doesn't need that option, because toolchain already defines ANDROID variable.

efidler commented 9 years ago

You've described the problem exactly. Some projects require a toolchain and have conditional stuff in their CMakeLists.txt that depend on the value of the ANDROID variable. Without the toolchain making ANDROID a cache variable, the conditional stuff in the CMakeLists.txt won't happen on the first build.

https://github.com/libgit2/libgit2/blob/master/CMakeLists.txt#L252 is an example.

wonder-mice commented 9 years ago

I still consider it an issue in the project, but not in the toolchain. Option could be changed to (in libgit2): option(ANDROID "Bla-Bla" "${ANDROID}")

Anyway it's just a coincidence that both names match. What if libgit2 was using LIBGIT2_ANDROID variable for that? Do we need to add it as a cache variable to the toolchain anyway?

I think the project should be aware of the toolchain, not vice versa.

efidler commented 9 years ago

Fair enough, but ANDROID is used widely enough that I'd think it's worth supporting this case. We do set ANDROID in the toolchain already, after all.

Do you think there's a downside to ANDROID being a cache variable?

wonder-mice commented 9 years ago

Well, it's up to @taka-no-me to decide.

But when you specified an android toolchain for the project - you ARE building for android. What the point of having ANDROID cache variable that could be set to NO? That will put a project into weird state where android toolchain is used, but ANDROID is set to NO. I don't like the possibility of such inconsistency.

taka-no-me commented 9 years ago

The ANDROID is designed to serve the same purpose as standard WIN32 or UNIX variables. So it should not be cached.

Moreover it can not work as an option because it is impossible to change its value after the first run.