Skip to content

Python 3 fix for android package #1223

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

Closed
wants to merge 1 commit into from
Closed

Python 3 fix for android package #1223

wants to merge 1 commit into from

Conversation

wolfgangp
Copy link
Contributor

This should work with Python 2 and 3. It's tested on Python 3.5 only. I did test .decode on unicode and regular str in Python 2.7 and it's fine.
Without this change, a buildozer requirements line like this
requirements = python3crystax,kivy==master,pyjnius,android
will result in a python error stating that a string cannot be concatenated to a bytes object, i.e. JAVA_NAMESPACE is a bytes object.

@inclement
Copy link
Member

Thanks, looks good but I'll test under python2 before merging.

@jleaders
Copy link

jleaders commented Sep 2, 2018

(JAVA_NAMESPACE.decode() if type(JAVA_NAMESPACE) is bytes and type('') is not bytes else JAVA_NAMESPACE) is a ternary that would return the base string type regardless of interpreter, but im sure his patch is fine.

I was able to test python3 successfully with buildozer.spec
requirements = python3crystax,kivy,git+git://github.com/wolfgangp/python-for-android
After I did buildozer android clean

@AndreMiras
Copy link
Member

Just to make sure I understand before testing/merging, this was a problem when running on host Python3 only right?
So if I run buildozer/p4a with these requirements from host python3 I should see the issue, correct?

@wolfgangp
Copy link
Contributor Author

Yes, only running Python 3 will elicit the problem p4a had. I think tests confirming this works in Python 2 are not in.

@KeyWeeUsr
Copy link
Contributor

@AndreMiras @wolfgangp It doesn't make sense to decode it at that place because if something calls it (or the jni part), it'll fail. Ref: #1475

@wolfgangp
Copy link
Contributor Author

I'm fine with closing this, as KeyWeeUsr's fix is more to the point.

@wolfgangp wolfgangp closed this Nov 29, 2018
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.

6 participants