Remove string decoding in _android.pyx for Python3 compatibility

[darosior]

#1747 broke Python3 compatibility by decoding a string. Here is a fix.

[tshirtman]

i'm thinking now maybe it would be better to make them explicitely unicode (by adding u before the strings) at definition time.

edit: though i'm not sure why the pygame ones are defined as explicitely bytes.

[darosior]

i'm thinking now maybe it would be better to make them explicitely unicode (by adding u before the strings) at definition time.

edit: though i'm not sure why the pygame ones are defined as explicitely bytes.

In Python3 strings are unicode by default.

[gabrielbrise]

Seems to be the right approach to this issue since it is still compatible with python 2.
Fixed the issues I was having while trying to compile my apk with buildozer/kivy/python3.

[AndreMiras]

I also second what @tshirtman says regarding explicitly forcing unicode so it would behave the same for python2 and python3. The thing is we already have the future unicode literal import. That's why I was suspecting initial issue from @tshirtman was with pygame because indeed it's the only path I could see where the type may differ. See #1747 (comment) comment.
But now before touching anything, I want to be sure we can reproduce both cases so we can trace it and provide proof the new situation works on both paths. Also see same comment here #1747 (comment)
What's also not clear to me, is it seems one failing path is at compile time, while the other is at on-device run time, is it?

[darosior]

What's also not clear to me, is it seems one failing path is at compile time, while the other is at on-device run time, is it?

Yes. It seems that (with Python3) the error occurs when trying to ask runtime permissions (I got it with ZbarCam).

[tshirtman]

@darosior i know python strings are unicode by default in python3, that's also why i was suprised by this issue, because i did build targetting python3, and it failed at runtime for me, but since this is a cython file (.pyx), it generated code for python2, (as tito suggested in the PR i did), that could explain why people get different results, i was pretty sure i had built everything in python3, but seems cython --version results correspond to pip show cython not to pip3 show cython (which is an older version), so it seems i messed up by using the wrong cython and generate C code for python2 instead of python3.

edit: ideally, p4a should be able to check that the cython it calls corresponds to the python version it targets, and raise a warning when it's not the case, one hacky way to do that could be

$(head -n1 $(which cython)|sed 's/#!//')  -c "import sys; print(sys.version_info[0])"

that's a bit hacky, but i don't know of a better one yet.

re-edit: but also, just adding u in front of the strings in

python-for-android/pythonforandroid/recipes/android/__init__.py

Lines 48 to 49 in 7bb8c8d

	java_ns = 'org.kivy.android'
	jni_ns = 'org/kivy/android'

would ensure the same behavior whatever python version.

[darosior]

The proposal in the re edit seems great, would you like me to edit the commit to make this change instead of modifying the cython file ?

[tshirtman]

the thing that puzzle me is why is it marked as bytes for the pygame alternative, but otherwise yeah, i think that's the best route.

[darosior]

the thing that puzzle me is why is it marked as bytes for the pygame alternative, but otherwise yeah, i think that's the best route.

Indeed, that's weird. To my understanding (limited, first time going through the code) there is no need for that, the difference would be handled with, I think, IS_PYGAME ?

python-for-android/pythonforandroid/recipes/android/__init__.py

Lines 60 to 67 in 7bb8c8d

	config = {
	'BOOTSTRAP': bootstrap,
	'IS_SDL2': int(is_sdl2),
	'IS_PYGAME': int(is_pygame),
	'PY2': int(will_build('python2')(self)),
	'JAVA_NAMESPACE': java_ns,
	'JNI_NAMESPACE': jni_ns,
	}

But it is not handled in any of these files :

$ grep -ri "is_pygame" pythonforandroid/recipes/android/src/
pythonforandroid/recipes/android/src/android/_android_jni.c:#if IS_PYGAME
pythonforandroid/recipes/android/src/android/_android_jni.c:#endif // IS_PYGAME
pythonforandroid/recipes/android/src/android/_android.pyx:IF IS_PYGAME:
pythonforandroid/recipes/android/src/setup.py:if int(os.environ['IS_PYGAME']):

The other thing I found is that there does not seem to be any error in other files where JAVA_NAMESPACE is concatenated with a string without u'' :

$ grep -ri "JAVA_NAMESPACE" pythonforandroid/recipes/android/src/
pythonforandroid/recipes/android/src/android/runnable.py:from android.config import JAVA_NAMESPACE
pythonforandroid/recipes/android/src/android/runnable.py:_PythonActivity = autoclass(JAVA_NAMESPACE + '.PythonActivity')
pythonforandroid/recipes/android/src/android/broadcast.py:from android.config import JAVA_NAMESPACE, JNI_NAMESPACE
pythonforandroid/recipes/android/src/android/broadcast.py:        GenericBroadcastReceiver = autoclass(JAVA_NAMESPACE + '.GenericBroadcastReceiver')
pythonforandroid/recipes/android/src/android/broadcast.py:            PythonService = autoclass(JAVA_NAMESPACE + '.PythonService')
pythonforandroid/recipes/android/src/android/broadcast.py:        PythonActivity = autoclass(JAVA_NAMESPACE + '.PythonActivity')
pythonforandroid/recipes/android/src/android/activity.py:from android.config import JAVA_NAMESPACE, JNI_NAMESPACE
pythonforandroid/recipes/android/src/android/activity.py:_activity = autoclass(JAVA_NAMESPACE + '.PythonActivity').mActivity
pythonforandroid/recipes/android/src/android/_android.pyx:python_act = autoclass(JAVA_NAMESPACE + u'.PythonActivity')

[AndreMiras]

I would not care too much about pygame guys since it's pretty legacy right? I would just make it behave the same as sdl2

[darosior]

I would not care too much about pygame guys since it's pretty legacy right? I would just make it behave the same as sdl2

Thus removing this condition ?

python-for-android/pythonforandroid/recipes/android/__init__.py

Lines 50 to 51 in 7bb8c8d

	elif is_pygame:
	java_ns = b'org.renpy.android'

[AndreMiras]

Yes I would be really tempted to remove that condition indeed

[darosior]

It seems reasonnable to me as well. Should we just change the b'' to '' since the url is different ?

[tshirtman]

in that case i would remove the condition and put them both as unicode (by adding u in front), so it's true whatever python/cython is used.

[AndreMiras]

Yes @tshirtman I would explicitly force it to be the same on both with a comment explaining why.
As for the url @darosior yes you're right we still need the condition to have it correct

[darosior]

Ok I did it in the last commit, we are good then.

[darosior]

put them both as unicode (by adding u in front), so it's true whatever python/cython is used.

Why ? Other string to which it is concatenated don't have it and it seems to work well (cf above)

[tshirtman]

this code is compiled to C by cython, which might (and in my case, was), be interpreting it differently than python itself. hinting it to make sure it's not ambigious can only help, no?

[darosior]

As you want, did it :)

[tshirtman]

thanks, let's see what happens with that 👍

[AndreMiras]

But wait guys, then the call on decode() would still be failing. There's no decode() method on unicode strings right?

[tshirtman]

🤦‍♂️ indeed we do need to remove this part too, i somehow though it had been done, although it's not in the diff. my bad.

[tshirtman]

#1752

[darosior]

I just forgot to git add _android.pyx.. Thanks