[MonoVM] support building with Android NDK r22 #51876
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
lambdageek
reviewed
Apr 26, 2021
akoeplinger
approved these changes
Apr 26, 2021
| <_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="no-rtti no-exceptions""/> |
Member
There was a problem hiding this comment.
curious, does this have an effect when we already have ANDROID_STL=none?
Contributor
Author
There was a problem hiding this comment.
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
Member
|
@grendello @akoeplinger is this completely transparent to someone running |
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. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context: android/ndk#1427 (comment)
Android NDK r22 changed the tree layout by moving
sysrootfrom the NDKroot 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:
This warning can be safely ignored.