-
Notifications
You must be signed in to change notification settings - Fork 984
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
DIVIDE APK SIZE BY 2 IN 2 LINES YOU WON'T BELIEVE THAT ONE TRICK #8331
Conversation
Pull Request Checklist
|
Jenkins BuildsClick to see older builds (36)
|
I am in awe of your oh so subtle marketing prowess. |
@jakubgs Can you elaborate what should be tested here? |
0% of end-end tests have passed
Failed tests (16)Click to expand
|
@jakubgs lmao seems like statustestbot is an x86 lover |
0% of end-end tests have passed
Failed tests (16)Click to expand
|
@jakubgs something wrong with e2e build. all tests are failed, please take a look. I was able to install .apk on my real device btw |
Actually, I just checked that build with zipinfo -1 StatusIm-190604-174951-7d9566-nightly.apk | grep \.so$ | awk -F'/' '{print $2}' | uniq
armeabi-v7a
x86 It should contain only Though now that I think about it that would cause problems for e2e release builds anyway. I need to think about this. |
93d929c
to
5dce6a4
Compare
Okay, instead I've introduced a
By default in |
The manual nightly build for Android seems to prove this approach works: $ zipinfo -1 StatusIm-190604-210633-5dce6a-nightly.apk | grep \.so$ | awk -F'/' '{print $2}' | uniq
armeabi-v7a |
98% of end-end tests have passed
Failed tests (1)Click to expand
Passed tests (48)Click to expand
|
I like how this reduced |
alright seems it works now, is there anything you are planning to achieve in this branch or we can merge it? @jakubgs |
@@ -200,7 +197,7 @@ android { | |||
reset() | |||
enable enableSeparateBuildPerCPUArchitecture | |||
universalApk false // If true, also generate a universal APK | |||
include "armeabi-v7a", "x86" | |||
include "armeabi-v7a", "arm64-v8a", "x86" |
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.
so you still include x86 here after all?
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.
The configuration section isn't used right now since enableSeparateBuildPerCPUArchitecture
is set to false
:
https://github.com/status-im/status-react/blob/c2b55233225f6e4d59c9ca5174b5261cc1137fec/android/app/build.gradle#L97
And that means that this section, which normally would have caused Gradle to build a separate APK for each platform, does nothing. I'm just adding arm64-v8a
to match what we have set in ndk.abiFilters
.
Now, since it seems to be necessary for dev builds, because devs use the emulator, x86
should be included by default, which is the ndk.abiFilters
setting, but for pr, release, and nightly builds(except for e2e ones) it will be removed by setting the NDK_ABI_FILTERS
env variable in android.gradlke
.
See:
https://github.com/status-im/status-react/pull/8331/files#diff-921ddb8a284f72367049b92eff26b9b6R10
98% of end-end tests have passed
Failed tests (1)Click to expand
Passed tests (48)Click to expand |
Let's see if rebasing on latest develop will fix the remaining 2% on E2E tests... |
100% of end-end tests have passed
Passed tests (49)Click to expand |
4f34219
to
4d59575
Compare
Signed-off-by: Jakub Sokołowski <jakub@status.im>
Not sure why we build for x86 and x86_64 considering almost no one uses x86 and we exclude the libraries for x86_64 anyway.
This also reduces the APK size from 51 MB to 31 MB.
Related to #8292 and #8326.