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 build: rebase p4a, rebase buildozer, bump targetSdkVersion #8002

Closed
wants to merge 4 commits into from

Conversation

SomberNight
Copy link
Member

@SomberNight SomberNight commented Oct 6, 2022

This PR

  • updates the python-for-android and buildozer versions we use, and rebases our patches,
  • bumps the targetSdkVersion from 30 to 31
    • the google play store will require 31, starting 2022-11 (next month)

Note that p4a implemented multi-arch apk builds, so we could potentially build a single apk that works on both arm7 and arm8 (but it would be larger, e.g. 40 MB vs 25 MB). This is not used in this PR atm. (see APP_ANDROID_ARCHS)

TODO:

  • test build repro
  • test qml apk build

EDIT: superseded by #8545

```
Traceback (most recent call last):
  File "kivy/_clock.pyx", line 645, in kivy._clock.CyClockBase._process_events
  File "kivy/_clock.pyx", line 218, in kivy._clock.ClockEvent.tick
  File "/home/user/wspace/electrum/.buildozer_kivy/android/app/electrum/gui/kivy/uix/dialogs/password_dialog.py", line 227, in <lambda>
  File "/home/user/wspace/electrum/.buildozer_kivy/android/app/electrum/gui/kivy/uix/dialogs/password_dialog.py", line 333, in <lambda>
  File "/home/user/wspace/electrum/.buildozer_kivy/android/app/electrum/gui/kivy/main_window.py", line 721, in on_open_wallet
  File "/home/user/wspace/electrum/.buildozer_kivy/android/app/electrum/gui/kivy/main_window.py", line 687, in on_wizard_success
  File "/home/user/wspace/electrum/.buildozer_kivy/android/app/electrum/util.py", line 445, in <lambda>
  File "/home/user/wspace/electrum/.buildozer_kivy/android/app/electrum/util.py", line 441, in do_profile
  File "/home/user/wspace/electrum/.buildozer_kivy/android/app/electrum/gui/kivy/main_window.py", line 940, in load_wallet
  File "/home/user/wspace/electrum/.buildozer_kivy/android/app/electrum/gui/kivy/main_window.py", line 950, in request_focus_for_main_view
  File "jnius/jnius_export_class.pxi", line 844, in jnius.jnius.JavaMethod.__call__
jnius.jnius.JavaException: Cannot call instance method b'requestFocusForMainView' on class b'org/kivy/android/PythonActivity'
```
This is the new minimum the google play store requires.
@SomberNight
Copy link
Member Author

SomberNight commented Oct 7, 2022

This branch is in a state atm where we can build the kivy app (reproducibly), but the qml app fails to build.
The failure is when compiling Qt5, relatively early during make.
@accumulator could you have a look?

EDIT:
Interesting bits of the error:

/opt/android/android-ndk-r25b/toolchains/llvm/prebuilt/linux-x86_64/bin/aarch64-linux-android-ranlib ../../../lib/libqtpcre2_arm64-v8a.a
make[4]: /opt/android/android-ndk-r25b/toolchains/llvm/prebuilt/linux-x86_64/bin/aarch64-linux-android-ranlib: No such file or directory
make[4]: *** [Makefile:193: ../../../lib/libqtpcre2_arm64-v8a.a] Error 127

indeed there is no aarch64-linux-android-ranlib file at that path, however there is a file named llvm-ranlib:
/opt/android/android-ndk-r25b/toolchains/llvm/prebuilt/linux-x86_64/bin/llvm-ranlib

EDIT2:
In the old r22b android ndk, the aarch64-linux-android-ranlib file is present,
but not in the new r25b android ndk.

@accumulator
Copy link
Member

Qt5 requires an older NDK. Even recent Qt6.4 pins the NDK to the older r23 and that's a quite 'hard' requirement. We might be able to patch Qt5 to get the compile to complete, but I think there's a nonzero risk of weird behavior or runtime crashes.

I hope it's possible to keep using an older NDK with SDK 31, otherwise we might need to move to Qt6 (still using an older than r25 NDK) or add p4a support for our needed combination of NDK and SDK.

@SomberNight
Copy link
Member Author

Well, that sucks...

Recent python-for-android bumped the minimum NDK they support to r25 in kivy/python-for-android@7714682,
and before that to r23 in kivy/python-for-android@f7f8cea.
I've checked NDK r23b, and that version is already using the modified file layout (missing aarch64-linux-android-ranlib). :/
AFAICT, p4a, when they bumped the min NDK version to r23b (which is at the same time as introducing support for it), also made non-trivial incompatible internal changes.

Btw, we are using r22b on master, with Qt 5.15.2, but looking at the table you linked, for Qt5, I am not sure that version is "officially" supported either. Maybe it just happens to work :)

With all this in mind, our chances of being able to upstream the qt bootstrap to p4a look more grim... I expect we would have to upgrade to Qt6 at least. How difficult do you think that would be?

I hope it's possible to keep using an older NDK with SDK 31

I would be surprised if it was not possible. I only bumped the NDK version because I rebased p4a, and upstream bumped it.

I guess I will either not rebase to p4a HEAD but an older commit, or not rebase at all... Nevertheless, I need a few commits from upstream p4a. Our branch is going to diverge even further :/

@accumulator
Copy link
Member

API-wise, there should not be much difference between Qt5 and Qt6, at least nothing we cannot refactor within reasonable time, and PyQt6 is released for Qt6.4, so that shouldn't create many big issues in the app.

So that leaves the bootstrap. I think we can have a go at getting Qt6.4 to compile against r25. It looks like there was a bigger toolchain change between r22 and r23b, which is supported by Qt6.4, so if we're lucky this still works with NDK r25. If not, we can investigate how much effort it is to lower the min.ndk level in p4a down to r24 or even r23.

That would leave the potential runtime issues, and the nativeSetEnv workaround we currently use to get the environment variables sorted before starting the P4A bootstrap. The latter we need to fix anyway before release, so even if Qt6 kills our current environment variables workaround, it doesn't matter all that much.

As for the runtime issues.. I have no idea what to expect. We should just try I guess.

The next couple of days I don't have much time to investigate unfortunately, but I'll try to get Qt6 to build with some of the NDKs

@accumulator
Copy link
Member

The commit on p4a that changes the minimum ndk from r23 to r25 doesn't seem to have a clear reason (AFAICS in the git history), so excluding build errors that are quietly fixed by this tightened requirement, it is probably just to limit maintenance burden.

@SomberNight
Copy link
Member Author

note: I think it would be great if we could upgrade to Qt6 for the qml GUI
(while thinking about Qt6 in general, I opened #8007, but that is about the gui/qt/ desktop GUI, and I believe upgrading Qt5->Qt6 there should be independent)

@SomberNight
Copy link
Member Author

SomberNight commented Oct 14, 2022

note: the "bump targetSdkVersion" part is now done in 0efc881, so rebasing p4a and buildozer are not that urgent

However, my p4a rebase attempt here was non-trivial -- it would be nice to be able to do a rebase and decrease the divergence from upstream. Also, as above, we should work forward trying to upstream the qt bootstrap into p4a. Hence, I believe upgrading to Qt6 (for the qml gui) would be worthwhile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants