-
Notifications
You must be signed in to change notification settings - Fork 993
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
Use "android-xx" format for ANDROID_PLATFORM #11799
Conversation
Old NDK versions only support "android-<level>" format for the ANDROID_PLATFORM argument.
Hi @DDoSolitary Thanks for your contribution! The PR looks good! One point about it, let me add a little check in our tests to verify that we're setting the variable correctly. Let me know if you have any other doubt. |
conans/test/integration/toolchains/cmake/test_cmaketoolchain.py
Outdated
Show resolved
Hide resolved
@@ -313,7 +313,7 @@ def context(self): | |||
android_ndk_path = android_ndk_path.replace("\\", "/") | |||
|
|||
ctxt_toolchain = { | |||
'android_platform': self._conanfile.settings.os.api_level, | |||
'android_platform': 'android-' + str(self._conanfile.settings.os.api_level), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Android is not sufficiently tested in our CI.
Are we completely sure that this format android-xxx
does not break for more modern API levels? It is compatible for all current (and hopefully future) levels?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as it's shown in the official documentation, https://developer.android.com/ndk/guides/cmake?hl=es-419#android_platform is not saying anything about "restrictions" in terms of using android-xxx
for modern versions, so it should work as usual.
From official docs:
Multiple formats are accepted for this parameter:
* android-$API_LEVEL
* $API_LEVEL
* android-$API_LETTER
The $API_LETTER format allows you to specify android-N without the need to determine the number associated with that release. Note that some releases received an API increase without a letter increase. These APIs can be specified by appending the -MR1 suffix. For example, API level 25 is android-N-MR1.
Changelog: Bugfix: Use "android-" format for the ANDROID_PLATFORM argument to be compatible with old NDK versions.
Docs: omit
Closes: #11798
develop
branch, documenting this one.Note: By default this PR will skip the slower tests and will use a limited set of python versions. Check here how to increase the testing level by writing some tags in the current PR body text.
Fixes #11798