search

noloader / cryptopp-cmake

CMake files for Crypto++ project
BSD 3-Clause "New" or "Revised" License
92 stars 68 forks source link

Fix Android and iOS builds #51

Closed TomTasche closed 4 years ago

TomTasche commented 4 years ago

Thank you so much for maintaining this project! It helped us a lot for an app we're developing: https://opendocument.app/

There's one small change I had to apply to make it work on iOS and one bigger change that was necessary to make it build on Android too. I'd be happy to see those changes go upstream and help others too.

noloader commented 4 years ago

Thanks @TomTasche..

One question... Where does `ANDROID_ABI come from? Is that a CMake variable?

One comment... I believe that should be ${ANDROID_NDK_ROOT}. Also see Recommended NDK Directory? on the Android NDK mailing list.

TomTasche commented 4 years ago

Found out about ANDROID_ABI on StackOverflow and decided to use it as an indicator if the current build is for Android. It seems to be coming from the Android CMake toolchain.

I'll give ANDROID_NDK_ROOT a try and report back.

noloader commented 4 years ago

It looks like you can use CMAKE_SYSTEM_NAME to detect Android builds.

Also see cmake-variables(7) in the Cmake docs.

TomTasche commented 4 years ago

ANDROID_NDK_ROOT doesn't work unfortunately (empty on my machine). I could use CMAKE_ANDROID_NDK though: https://cmake.org/cmake/help/latest/manual/cmake-toolchains.7.html#cross-compiling-for-android

noloader commented 4 years ago

Thanks @TomTasche,

I'd like to setup the situation where things just work for folks doing thing the right way (i.e., setting ANDROID_NDK_ROOT and friends).

Is it possible for you to wire-in ANDROID_NDK_ROOT? Something like:

if (NOT ANDROID_NDK)
    if (ANDROID_NDK_ROOT)
        set (ANDROID_NDK "${ANDROID_NDK_ROOT}")
    endif()
endif()

(Remember, ANDROID_NDK_ROOT comes from the environment).

Let me send you a collaborator invite so you can modify things without waiting for me. It also ensures there are admins available if I get hit by a bus.

TomTasche commented 4 years ago

Sorry for the radio silence on my end! I saw there was another pull request addressing Android, I'm glad that was merged. I tested it and it works fine for me, therefore I'm going to close this pull request for now. I might have to reopen it in the future for the iOS change that's included here too, but I'm still investigating that...