-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Initial support for PySide6 and Qt #2918
Conversation
Hi, I am a Qt for Python developer. A while ago, I discussed in the p4a discord channel about the possibility of adding a new bootstrap that uses Qt instead of SDL2, and I had a positive response from you guys. This would be the first among possibly many series of patches. The Qt boostrap would enable PySide to use the entire p4a infrastructure to package PySide application to Android .apk. There would also be recipes for PySide6 and shiboken6, but this would live inside PySide repository. How would we use p4a and use this new bootstrap? We provide a tool called pyside6-android-deploy which is basically a interface to setup the recipes for PySide6 and shiboken6, and create a custom buildozer.spec file with all the dependencies added. The tool then further calls "buildozer android debug" which uses the created buildozer.spec file. The entire working of this tool is detailed here: https://www.qt.io/blog/taking-qt-for-python-to-android. That said, the tool is ofcourse still under technical preview. Among other attributes from
What does the recipes do? Since we create PySide6 and shiboken6 wheels for Android, the only thing the recipes does is to unzip the wheels into the |
So, the entire thing seems to work well at the moment as i tried to deploy quite a few PySide examples. Nevetheless, I am sure I am missing a ton of things and there maybe some code sections which are not really necessary since a lot of the bootstrap code was copied and adapted by looking at the other bootstraps. |
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.
Incredible stuff, thanks this contribution ❤️
Would you mind documenting the bootstrap here doc/source/buildoptions.rst
, updating the documentation here doc/source/commands.rst
and here doc/source/quickstart.rst
.
Ideally we should also update the CI to build an app on that bootstrap, see Makefile
and .github/workflows/push.yml
.
I probably won't have a chance to play with it, but it overall looks good to me, hopefully a core dev can look this up more in depth.
Until then let's see what the CI thinks about it
Hi @shyamnathp What is the license status for mobile apps using QT? If it was LGPL, it would be very difficult to comply? Is a commercial license required? |
The tool under question (pyside6-android-deploy) that uses this new bootstrap comes under the LGPLv3/GPLv3 and not the commercial license. The only use of this Qt bootstrap is to deploy PySide6 applications to Android. So, since the user uses PySide6 API they will have to comply with LGPL anyway irrespective of using the tool. |
Thanks a lot :). I can do all of these things. |
My understanding is that it is impossible to deploy an app to the Google Play Store in a fashion that the app can satisfy the LGPLv3 requirement for library substitution. So this requires the developer to purchase a QT license, or make their project fully open source. Are you contributing this pull request under the terms of the MIT license used by python-for-android? |
I asked around and you are absolutely right here. But, that comes under the licensing terms of Qt/PySide6. As long as one does not use the PySide6 recipe, they don't have to adhere to these licenses. This is generally similar to other recipes included in p4a like "psycopg2" which comes with only LGPLv3 license, and user will have to make their code open source if that recipe is used.
Yes. Great question. There is a part in the commited code where I am doing a "public class PythonActivity extends QtActivity" and this could have raised your question. This is also pretty similar to the question here: https://opensource.stackexchange.com/questions/7904/can-your-mit-library-use-an-lgpl-library and I believe the answer applies. |
Hi @shyamnathp, thank you for your helpful reply. I now have a much better understanding of the situation. |
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.
Hi @shyamnathp !
Nice work, and can't wait to add the qt
bootstrap to our bootstrap
set.
Can you take a look at suggestions and comments?
Agree with @AndreMiras about tests and docs. (and better to target both in this PR)
@@ -222,7 +222,7 @@ def compile_py_file(python_file, optimize_python=True): | |||
|
|||
def make_package(args): | |||
# If no launcher is specified, require a main.py/main.pyc: | |||
if (get_bootstrap_name() != "sdl" or args.launcher is None) and \ | |||
if (get_bootstrap_name() in ["sdl", "qt"] or args.launcher is None) and \ |
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.
should not be not in
(at least for our sdl
provider) ?
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.
done
{% if args.enable_androidx %} | ||
sourceCompatibility JavaVersion.VERSION_1_8 | ||
targetCompatibility JavaVersion.VERSION_1_8 | ||
{% else %} | ||
{% else %} | ||
sourceCompatibility JavaVersion.VERSION_1_7 | ||
targetCompatibility JavaVersion.VERSION_1_7 | ||
{% endif %} | ||
{% endif %} |
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.
This is likely due to an autoformatter which is not playing well with a gradle
file managed via a cookiecutter.
Can you please revert these style changes and the ones just before?
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.
done
@@ -107,7 +105,7 @@ android { | |||
} | |||
|
|||
aaptOptions { | |||
noCompress "tflite" | |||
noCompress "tflite", "rcc" |
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.
Minor:
Can you please add a comment regarding rcc
? (what is, why it needs noCompress
, etc ..)
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 just realized that the rcc part was not really needed in the context of Python. That only makes sense for C++.
Anyway, .rcc files are qt resource file binaries that can be loaded at runtime - https://doc.qt.io/qt-6/resources.html
# this is needed because the recipes PySide6 and shiboken6 resides in the PySide Qt respository | ||
# - https://code.qt.io/cgit/pyside/pyside-setup.git/ | ||
# Without this some tests will error because it cannot find the recipes within pythonforandroid | ||
# respository |
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.
Minor typo: respository
--> repository
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.
done
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.
Why this change is needed?
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.
removed.
if 'sqlite3' not in self.ctx.recipe_build_order: | ||
with open('blacklist.txt', 'a') as fileh: | ||
fileh.write('\nsqlite3/*\nlib-dynload/_sqlite3.so\n') |
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.
Copied from our service_only
bootstrap? Better to avoid that.
We can introduce a feature to achieve the same (or a better result)
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.
Done.
The sdl bootstrap also does this. Does that mean if i build both PySide recipe and use the Qt bootstrap, these sqlite3 files will get included into the .apk from the built CPython files? Should this be avoided?
# unused kivy files (platform specific) | ||
kivy/input/providers/wm_* | ||
kivy/input/providers/mactouch* | ||
kivy/input/providers/probesysfs* | ||
kivy/input/providers/mtdev* | ||
kivy/input/providers/hidinput* | ||
kivy/core/camera/camera_videocapture* | ||
kivy/core/spelling/*osx* | ||
kivy/core/video/video_pyglet* | ||
kivy/tools | ||
kivy/tests/* | ||
kivy/*/*.h | ||
kivy/*/*.pxi |
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.
Maybe needs to be cleaned up?
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.
right. coped from the other bootstraps. I guess, the kivy related files don't make sense. I will leave the rest as it is, as they come from built Python interpreter?
@@ -0,0 +1,107 @@ | |||
<?xml version="1.0" encoding="utf-8"?> | |||
<manifest xmlns:android="http://schemas.android.com/apk/res/android" | |||
package="{{ args.package }}" |
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.
A recent change, require to not have package
:
#2887
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.
done
df6469b
to
4e58cce
Compare
sorry for the delay guys. I had a couple of conference talks in between and then I was sick. I am back on this now. Will add the documentation soon. also, sorry for the forced pushes. Should have put the changes in a new commit. |
@misl6 @AndreMiras the documentation has been added. Do let me know if this is sufficient. |
I see we have a conflict, can you please take care of it @shyamnathp ? |
- Add a new bootstrap for Qt - This bootstrap will be used by `pyside6-android-deploy` tool shipped with PySide6, which interally calls pythonforandroid using buildozer. - The Qt bootstrap depends on recipes PySide6 and shiboken6 among other mandatory recipes. The recipes for PySide6 and shiboken6 resides in the PySide repository - https://code.qt.io/cgit/pyside/pyside-setup.git/tree/sources/pyside-tools/deploy_lib/android/recipes - The PythonActivity entrypoint class is derived from QtActivity class which is the main acitivty class when a Qt C++ application is packaged for Android. The jar containing QtActivity class is supplied through buildozer `android.add_jars` option. - The C wrapper binary to the application main.py is named as `main_{abi_name}` instead of just `main` for other bootstraps. - Multi architecture deployment is not supported at the moment. - Adapt tests based on the new Qt bootstrap
- update the docs to include sections depicting the Qt boostrap.
- Sometimes a flaky Java heap out of memory error is throw. By, tweaking the memory setting we can get rid of that error.
- Qt boostrap removed from comparison in the changed line because its expects a value is args.launcher, which is not applicable for Qt boostrap. Hence, it exits with an value not found exception. - Removing Qt boostrap from the comparison leads to checking for main.py or --private, which is to be done for the Qt boostrap.
f5f8620
to
99250f1
Compare
- check if empty, otherwise store empty string
should be fixed now. I added a couple more commits. As suggested by Andre, I am also working on a test app to test in your CI. Should have it ready by Tuesday. Was waiting on a server to host the wheels. Until we (at PySide) provision our CI to continously deliver the Android wheels (either to PyPI or our own serves), the idea is to host a set of test wheels on our servers so that I can enable the test app to build the recipes for PySide. |
- for the purpose of testing, the pyside6 and shiboken6 wheels, the extra .jar files needed and the recipes for pyside6 and shiboken6 are manually added into testapps/on_device_unit_tests/test_qt. These files are normally generated by the `pyside6-android-deploy` tool that is shipped with PySide. Generating the wheels and the .jar files belongs to the scope of PySide and not python-for-android. Hence, they are not done here. This also reduces the load on the CI which will otherwise have to cross-compile CPython and PySide. - The Android aarch64 wheels for testing are downloaded from Qt servers. These wheels are for testing purposes only and the download link will be updated when official PySide6 Android wheels are generated. - Tests were added in test_requirements.py so that when running the apk the current date and time are printed on the terminal. The tests also checks shiboken6 and PySide6 module imports.
- This was introduced by a VSCode setting and is unrealated to this patch. Although this might be good, this has to be introduced through a different patch. - pyside6 recipe typo adapted in buildoptions.rst
A non-gui test was added which tests with module imports. I am not sure if testing the PySide6 GUI itself is within the scope of python-for-android. I have tried testing the generated artifacts and seems to work. |
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.
Hi @shyamnathp ,
Sorry for the late reply, but as you might have seen, we're quite busy releasing Kivy 2.3.0, while we're preparing the soil for the new major version.
We just have a minor typo (I left a suggestion), when fixed, the PR could be merged.
Thank you for adding the docs and tests!
Co-authored-by: Mirko Galimberti <me@mirkogalimberti.com>
Thank you so much. I wish you a happy new year :) |
@misl6 any possibility of merging this soon? Sorry for pestering |
No worries! Next time: please re-request a review so it ends up in my task list again. |
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.
LGTM. Thank you!
pyside6-android-deploy
tool shipped with PySide6, which interally calls pythonforandroid using buildozer.android.add_jars
option.main_{abi_name}
instead of justmain
for other bootstraps.