Support permission properties (maxSdkVersion and usesPermissionFlags) + remove WRITE_EXTERNAL_STORAGE permission, which has been previously declared by default
#2725
Conversation
… + remove WRITE_EXTERNAL_STORAGE permission, which has been previously required by default
misl6
force-pushed
the
fix/reverse-2694-and-add-a-way-to-set-sdkversion
branch
from
ced6f70 to
c28ae30
Compare
| else: | ||
| _permissions.append(dict(name=f"android.permission.{permission}")) |
There was a problem hiding this comment.
Do we want to add a warning in this case?
There was a problem hiding this comment.
Do you mean a deprecation warning? I thought the same, but then I decided to go with the slow path instead in order to avoid a large increase in support requests (users may start panicking 😀).
I did not officially deprecated it by purpose, instead, I've changed the docs and added a warning regarding "name-only" permissions.
There's a lot of documentation, posts, videos, etc ... which is referring to "name-only" permissions (And we even need to migrate buildozer and python-for-android examples).
What do you think about it?
tests/test_bootstrap_build.py
Outdated
| try: | ||
| parsed_permissions = buildpy.parse_permissions(args.permissions) # noqa: F841 | ||
| except ValueError as _e: | ||
| assert "Property 'propertyThatFails' is not supported." in str(_e) |
There was a problem hiding this comment.
If the exception isn't raised, the test won't catch it.
So we either want to make it fail explicitly after the parse_permissions() something like self.fail("ValueError not raised") or use something like the pytest.raises context manager e.g. with pytest.raises(ValueError, match="Property 'propertyThatFails' is not supported."):
There was a problem hiding this comment.
Thank you for catching this beginner-like error! 😓
Addressed.
…gs`) + remove `WRITE_EXTERNAL_STORAGE` permission, which has been previously declared by default (kivy#2725) * Support permission properties (maxSdkVersion and usesPermissionFlags) + remove WRITE_EXTERNAL_STORAGE permission, which has been previously required by default * Fix test for ValueError
The
--permissionargument historically only accepted something like--permission android.permission.WRITE_EXTERNAL_STORAGEand--permission VIBRATEwhich is not enough in certain use cases.As an example, when dealing with bluetooth permissions and targeting API 31, the legacy
android.permission.BLUETOOTHandandroid.permission.BLUETOOTH_ADMINpermissions need the propertyandroid:maxSdkVersionbe set to30, andthe
"android.permission.BLUETOOTH_SCANneeds to strongly assert that the app never derives physical location from Bluetooth scan results viaandroid:usesPermissionFlags="neverForLocation".Basically, nowadays, android permissions declarations need a lot of fine-tuning, which is not possible at this time via
python-for-android, unless an extra manifest is used.This PR introduces a new syntax:
--permission (name=android.permission.WRITE_EXTERNAL_STORAGE;maxSdkVersion=18)that allows the user to set every single (and currently available/supported) property of the<uses-permission>element.The old syntaxes are migrated before the template gets processed, so users will only notice this change when needed, and when reading the docs.
I also started to add some tests for our
build.pyscript. I needed to add a switch to skip some things as ATM are not patchable (at least easily). The whole script needs some refactoring in order to be well-tested.