Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MonoVM] support building with Android NDK r22 #51876

Merged
merged 1 commit into from
Apr 28, 2021

Conversation

grendello
Copy link
Contributor

Context: android/ndk#1427 (comment)

Android NDK r22 changed the tree layout by moving sysroot from the NDK
root directory to a subdirectory in the toolchains tree and also by
removing per-platform library/header directories from their previous
location to one under the sysroot above.

MonoVM was built using the CMake's built-in Android NDK support which,
alas, is not compatible with NDK r22 before CMake version 3.19. It is
therefore better to use the Android CMake toolchain definition script
shipped with the NDK, so that the build works regardless of CMake
version installed on the system.

The r22 toolchain will complain about older CMake versions:

An old version of CMake is being used that cannot
automatically detect compiler attributes. Compiler identification is
being bypassed. Some values may be wrong or missing. Update to CMake
3.19 or newer to use CMake's built-in compiler identification.

This warning can be safely ignored.

Context: android/ndk#1427 (comment)

Android NDK r22 changed the tree layout by moving `sysroot` from the NDK
root directory to a subdirectory in the toolchains tree and also by
removing per-platform library/header directories from their previous
location to one under the sysroot above.

MonoVM was built using the CMake's built-in Android NDK support which,
alas, is not compatible with NDK r22 before CMake version 3.19.  It is
therefore better to use the Android CMake toolchain definition script
shipped with the NDK, so that the build works regardless of CMake
version installed on the system.

The r22 toolchain will complain about older CMake versions:

    An old version of CMake is being used that cannot
    automatically detect compiler attributes. Compiler identification is
    being bypassed. Some values may be wrong or missing. Update to CMake
    3.19 or newer to use CMake's built-in compiler identification.

This warning can be safely ignored.
<_MonoCMakeArgs Include="-DCMAKE_TOOLCHAIN_FILE=$(ANDROID_NDK_ROOT)/build/cmake/android.toolchain.cmake"/>
<_MonoCMakeArgs Include="-DANDROID_NDK=$(ANDROID_NDK_ROOT)"/>
<_MonoCMakeArgs Include="-DANDROID_STL=none"/>
<_MonoCMakeArgs Include="-DANDROID_CPP_FEATURES=&quot;no-rtti no-exceptions&quot;"/>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curious, does this have an effect when we already have ANDROID_STL=none?

Copy link
Contributor Author

@grendello grendello Apr 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are two separate settings. ANDROID_STL chooses libc++, stlport, system and none for the standard C++ library while ANDROID_CPP_FEATURES chooses compiler options, effectively. Those two are independent on each other

@lambdageek
Copy link
Member

@grendello @akoeplinger is this completely transparent to someone running ./build.sh --os Android ? Do we need to update anything in docs/ to specify a minimum android ndk toolchain?

@grendello
Copy link
Contributor Author

@lambdageek the cmake toolchain file has been in NDK since 2016 (NDK r13), so that would be the lowest NDK version supported, after this PR is merged. Otherwise, it is completely transparent.

@marek-safar marek-safar merged commit d44bd23 into dotnet:main Apr 28, 2021
@grendello grendello deleted the support-ndk-r22 branch April 28, 2021 21:20
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jun 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants