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

[android]: bump sdk, switch to gradle, add support for runtime permissions, fix some memleaks. #5096

Merged
merged 1 commit into from Jun 29, 2019

Conversation

@pazos
Copy link
Contributor

commented Jun 25, 2019

Requires koreader/android-luajit-launcher#149 and koreader/koreader-base#927

In order to build we need to add the valid sdk and ndk paths into platform/android/luajit-launcher/local.properties

I'm using:

ndk.dir=/home/pazos/Escritorio/koreader/base/toolchain/android-ndk-r15c
sdk.dir=/home/pazos/Escritorio/koreader/base/toolchain/android-sdk-linux
@pazos

This comment has been minimized.

Copy link
Contributor Author

commented Jun 26, 2019

Cool!

Just a minor nit, it seems that calling make clean doesn't work as expected and we need to call ( cd platform/android/luajit-launcher && ./mk-luajit.sh clean) between builds for different abis.

BTW, I think that we need to make a few changes after merging this:

update Makefile commit in https://github.com/koreader/virdevenv/blob/master/docker/ubuntu/koandroid/fetch_android_tc.sh

adjust pruning in https://github.com/koreader/virdevenv/blob/master/docker/ubuntu/koandroid/build_android_tc.sh

leaving only NDK14 platform as it is the minimun target and what is used to build luajit and to build the nativeActivity

All the rest should work as is!

@Frenzie: could you take care of these changes?

@pazos

This comment has been minimized.

Copy link
Contributor Author

commented Jun 26, 2019

Just a minor nit, it seems that calling make clean doesn't work as expected and we need to call ( cd platform/android/luajit-launcher && ./mk-luajit.sh clean) between builds for different abis.

That happened with ant build as well. I'm using a script to test all builds. I can skip the clean_luajit step between debug/release builds of the same arch but not between archs.

#!/bin/sh

clean_luajit() {
    ( cd platform/android/luajit-launcher && make clean )
}


cd /home/pazos/Escritorio/koreader
find koreader-android* -exec rm -rf {} \; 2>/dev/null

# arm release
clean_luajit
./kodev release --ignore-translation android

# arm debug
clean_luajit
./kodev release --ignore-translation --debug android

# x86 release
clean_luajit
ANDROID_ARCH=x86 ./kodev release --ignore-translation android

# x86 debug
clean_luajit
ANDROID_ARCH=x86 ./kodev release --ignore-translation --debug android
@Frenzie

This comment has been minimized.

Copy link
Member

commented Jun 26, 2019

@pazos I'll hopefully have some time to look at the Docker images tomorrow. :-)

@pazos pazos force-pushed the pazos:gradle-bump branch 3 times, most recently from 07b1746 to 6efc151 Jun 26, 2019

@pazos

This comment has been minimized.

Copy link
Contributor Author

commented Jun 26, 2019

Updated to include notch support for Android Pie+.

Fixes #5020 too!

@pazos pazos changed the title [wip][android]: bump sdk, switch to gradle, add support for runtime permissions, fix some memleaks. [android]: bump sdk, switch to gradle, add support for runtime permissions, fix some memleaks. Jun 26, 2019

@pazos pazos added this to the 2019.07 milestone Jun 26, 2019

@pazos pazos force-pushed the pazos:gradle-bump branch from 6efc151 to 93fcaa5 Jun 26, 2019

Frenzie added a commit to Frenzie/virdevenv that referenced this pull request Jun 28, 2019

@Frenzie

This comment has been minimized.

Copy link
Member

commented Jun 28, 2019

@pazos Incidentally, that whole pruning business wasn't doing anything because the directory is named android-ndk-r15c. So fixing that up may speed up the Android build slightly, although even up to half a minute extra pulling the Docker image (and probably a lot less) is still negligible compared to the full build time.

Anyway, I pushed a koreader/koandroid:0.3 image, so feel free to merge when ready. Thanks to the weekend I'll be able to fix any issues that might crop up tomorrow.

@pazos

This comment has been minimized.

Copy link
Contributor Author

commented Jun 29, 2019

@Frenzie: cool. I'm testing a few other stuff in https://github.com/pazos/android-luajit-launcher/tree/moar_classes, but I'll wait a bit.

This particular PR is ready, but I wonder if we can skip ci tests on the autogenerated gradlew script entirely (my last two commits on luajit-launcher were for that, unsucessfully)

@Frenzie

This comment has been minimized.

Copy link
Member

commented Jun 29, 2019

Sure, that's this line:

mapfile -t shellscript_locations < <({ git grep -lE '^#!(/usr)?/bin/(env )?(bash|sh)' && git submodule --quiet foreach '[ "$path" = "base" ] || git grep -lE "^#!(/usr)?/bin/(env )?(bash|sh)" | sed "s|^|$path/|"' && git ls-files ./*.sh; } | sort | uniq)

@pazos

This comment has been minimized.

Copy link
Contributor Author

commented Jun 29, 2019

Sure, that's this line:

mapfile -t shellscript_locations < <({ git grep -lE '^#!(/usr)?/bin/(env )?(bash|sh)' && git submodule --quiet foreach '[ "$path" = "base" ] || git grep -lE "^#!(/usr)?/bin/(env )?(bash|sh)" | sed "s|^|$path/|"' && git ls-files ./*.sh; } | sort | uniq)

Wow, to much information, I'm not sure how to fix it :/. I will wait for your help (again) 🆘

@Frenzie

This comment has been minimized.

Copy link
Member

commented Jun 29, 2019

Oh, haha, sorry. It's just that [ "$path" = "base" ] part excluding base. So instead it should be something like:

[ "$path" = "base" -o "$path" = "platform/android/luajit-launcher/shittygradlescript" ]
switch from ant to gradle,
add support for runtime permissions on api23+
add support for devices with a notch on api28+
fix some potential memory leaks
do not check luajit-launcher shell scripts

@pazos pazos force-pushed the pazos:gradle-bump branch from f3af683 to e4b239d Jun 29, 2019

@Frenzie Frenzie merged commit bcac5b8 into koreader:master Jun 29, 2019

1 check passed

ci/circleci Your tests passed on CircleCI!
Details
@Frenzie

This comment has been minimized.

Copy link
Member

commented Jun 29, 2019

@pazos

In order to build we need to add the valid sdk and ndk paths into platform/android/luajit-launcher/local.properties

There's no way to get gradle to pick that stuff up automatically the way it used to be? I'm not really following how this is supposed to work I'm afraid. It's not generating a local.properties and it's not abiding by any NDK/ANDROID_NDK environment variable.

> Configure project :eink-test
NDK is missing a "platforms" directory.
If you are using NDK, verify the ndk.dir is set to a valid NDK directory.  It is currently set to /home/frans/src/kobo/koreader/base/toolchain/android-toolchain-x86.
If you are not using NDK, unset the NDK variable from ANDROID_NDK_HOME or local.properties to remove this warning.


> Configure project :launcher
NDK is missing a "platforms" directory.
If you are using NDK, verify the ndk.dir is set to a valid NDK directory.  It is currently set to /home/frans/src/kobo/koreader/base/toolchain/android-toolchain-x86.
If you are not using NDK, unset the NDK variable from ANDROID_NDK_HOME or local.properties to remove this warning.

Checking the license for package SDK Patch Applier v4 in /home/frans/src/kobo/koreader/base/toolchain/android-sdk-linux/licenses
Warning: License for package SDK Patch Applier v4 not accepted.
Checking the license for package NDK in /home/frans/src/kobo/koreader/base/toolchain/android-sdk-linux/licenses
Warning: License for package NDK not accepted.

FAILURE: Build failed with an exception.

* What went wrong:
A problem occurred configuring project ':launcher'.
> com.android.builder.sdk.LicenceNotAcceptedException: Failed to install the following Android SDK packages as some licences have not been accepted.
     patcher;v4 SDK Patch Applier v4
     ndk-bundle NDK
  To build this project, accept the SDK license agreements and install the missing components using the Android Studio SDK Manager.
  Alternatively, to transfer the license agreements from one workstation to another, see http://d.android.com/r/studio-ui/export-licenses.html
  
  Using Android SDK: /home/frans/src/kobo/koreader/base/toolchain/android-sdk-linux

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. Run with --scan to get full insights.

* Get more help at https://help.gradle.org

BUILD FAILED in 0s

Manually creating a local.properties somewhat works, but it's still giving the same error. Except in this case it's completely nonsensical because there is a platforms directory full of all the platforms:

> Configure project :launcher
NDK is missing a "platforms" directory.
If you are using NDK, verify the ndk.dir is set to a valid NDK directory.  It is currently set to /home/frans/kobo/koreader/base/toolchain/android-ndk-r15c.
If you are not using NDK, unset the NDK variable from ANDROID_NDK_HOME or local.properties to remove this warning.

Checking the license for package SDK Patch Applier v4 in /home/frans/src/kobo/koreader/base/toolchain/android-sdk-linux/licenses
Warning: License for package SDK Patch Applier v4 not accepted.
Checking the license for package NDK in /home/frans/src/kobo/koreader/base/toolchain/android-sdk-linux/licenses
Warning: License for package NDK not accepted.

FAILURE: Build failed with an exception.

* What went wrong:
A problem occurred configuring project ':launcher'.
> com.android.builder.sdk.LicenceNotAcceptedException: Failed to install the following Android SDK packages as some licences have not been accepted.
     patcher;v4 SDK Patch Applier v4
     ndk-bundle NDK
  To build this project, accept the SDK license agreements and install the missing components using the Android Studio SDK Manager.
  Alternatively, to transfer the license agreements from one workstation to another, see http://d.android.com/r/studio-ui/export-licenses.html
  
  Using Android SDK: /home/frans/src/kobo/koreader/base/toolchain/android-sdk-linux

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. Run with --scan to get full insights.

* Get more help at https://help.gradle.org
@pazos

This comment has been minimized.

Copy link
Contributor Author

commented Jun 29, 2019

Er, will check when I arrive home. But probably we need to update the sdk tools and use sdkmanager to set root path.

@Frenzie

This comment has been minimized.

Copy link
Member

commented Jun 29, 2019

Thanks. :-)

@pazos

This comment has been minimized.

Copy link
Contributor Author

commented Jun 30, 2019

@Frenzie:

Ok, we need to update sdk tools to a version higher than 25.2.5 and use sdkmanager to install sdk packages and aprove the license(s).

But, there are two problems:

  1. you cannot approve a license of a package you didn't install.
  2. sdkmanager only provides the latest version of the ndk (incompatible with api14-15)

Google sucks badly, it is not fun anymore.

Possible solution: install patch applier v4 and ndk-bundle with sdkmanager (to accept their licenses) and then just ignore the bundled ndk path and use our own.

@pazos

This comment has been minimized.

Copy link
Contributor Author

commented Jun 30, 2019

Sadly my internet speed is not good enough to test changes on the makefile, as the entire ndk + sdk download takes more than an hour on my machine.

@pazos

This comment has been minimized.

Copy link
Contributor Author

commented Jun 30, 2019

There's no way to get gradle to pick that stuff up automatically the way it used to be?

No, they force you to use android studio (which will recreate local.properties based on the paths buried with the IDE) or they force you to update local.properties yourself.

We can make a few changes to Makefile -> koreader/android-luajit-launcher@bc76d79

for SDK: use $(SDK) -> use $(ANDROID_HOME) -> use base/toolchain/android-sdk-linux
for NDK: use $(NDK) -> use $(ANDROID_NDK_HOME) -> use base/toolchain/android-ndk-r15c

And tune the toolchain makefile -> koreader/koreader-base@3e0b259

Frenzie added a commit to koreader/virdevenv that referenced this pull request Jul 2, 2019

Docker/Android: bump TC creation (#40)
Cf. koreader/koreader#5096

* Create fake dir to silence gradle complaint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.