-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Beginnings of native Android build #110471
base: main
Are you sure you want to change the base?
Changes from 11 commits
90c22cb
6f1b917
c25fbd9
bfcbb3e
97f2555
3d9da10
9e173df
bb9f323
b0f3d55
f9eb950
a69fcfd
54fbb50
ee4ec72
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -78,16 +78,24 @@ | |
<ItemGroup Condition="('$(TargetsAndroid)' == 'true' or '$(TargetsLinuxBionic)' == 'true') and '$(ANDROID_NDK_ROOT)' != ''"> | ||
<_CoreClrBuildArg Include="-DCMAKE_TOOLCHAIN_FILE=$(ANDROID_NDK_ROOT)/build/cmake/android.toolchain.cmake"/> | ||
<_CoreClrBuildArg Include="-DANDROID_NDK=$(ANDROID_NDK_ROOT)"/> | ||
<_CoreClrBuildArg Include="-DANDROID_STL=none"/> | ||
<_CoreClrBuildArg Include="-DANDROID_CPP_FEATURES="no-rtti no-exceptions""/> | ||
<_CoreClrBuildArg Include="-DANDROID_STL=c++_static"/> | ||
<_CoreClrBuildArg Include="-DANDROID_CPP_FEATURES="no-rtti exceptions""/> | ||
Comment on lines
-81
to
+82
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this affect size of native AOT runtime packs? (Build with |
||
<_CoreClrBuildArg Include="-DANDROID_PLATFORM=android-$(AndroidApiLevelMin)"/> | ||
<_CoreClrBuildArg Include="-DANDROID_NATIVE_API_LEVEL=$(AndroidApiLevelMin)"/> | ||
<_CoreClrBuildArg Condition="'$(Platform)' == 'arm64'" Include="-DANDROID_ABI=arm64-v8a" /> | ||
<_CoreClrBuildArg Condition="'$(Platform)' == 'arm'" Include="-DANDROID_ABI=armeabi-v7a" /> | ||
<_CoreClrBuildArg Condition="'$(Platform)' == 'x86'" Include="-DANDROID_ABI=x86" /> | ||
<_CoreClrBuildArg Condition="'$(Platform)' == 'x64'" Include="-DANDROID_ABI=x86_64" /> | ||
|
||
<!-- No LTTNG on Android --> | ||
<_CoreClrBuildArg Include="-cmakeargs -DFEATURE_EVENT_TRACE=0"/> | ||
<!-- | ||
TODO: No LTTNG on Android, it's disabled at the eventprovider level. Will need an answer. | ||
Disabling FEATURE_EVENT_TRACE is too wide. | ||
--> | ||
<!-- | ||
Unsure if something like this is actually defined. We need this to differentiate between | ||
features that are enabled in mono android, but not in coreclr. | ||
--> | ||
<_CoreClrBuildArg Include="-cmakeargs -DRUNTIME_CORECLR=1" /> | ||
</ItemGroup> | ||
|
||
<PropertyGroup> | ||
|
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.
@grendello has mentioned in #110470 that even API level 28 is unfortunate and that we will want to support older API level, e.g. the originally used level 21.
So I wonder if moving to the level 29 is a good thing.
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.
It's not good in the long term and is something we should fix. Moving it to 29 for now allows us to not have to deal with emulated TLS usage.
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.
Unless we can fix it now, I don't think this should block progress. We can do it sometime down the road.
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.
In perspective, this would drop 12.8% of the devices (with API level from 21 to 29) according to https://apilevels.com/
Then again:
Level 21 is Android 5 and 6 years EOL
Level 29 is Android 10 and 2 years EOL (https://endoflife.date/android)
Level 34 or higher is necessary to publish apps to the Play Store (https://developer.android.com/google/play/requirements/target-sdk)
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.
I think I can make the emulated TLS work without too much hassle. The question though is if we are worried about the perf implications of the emulated TLS vs the standard TLS and thus we would like to end up having support for both.