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

Fix android module JAVA_NAMESPACE and JNI_NAMESPACE with unicode #1475

Merged
merged 1 commit into from
Nov 28, 2018

Conversation

KeyWeeUsr
Copy link
Contributor

If not switched to unicode, anything with formating such as + will trigger an error for py3 when accessing JAVA_NAMESPACE and likes:

b'1' + u'2'
u'1' + b'1'

and py2 just doesn't care.

@KeyWeeUsr KeyWeeUsr changed the title Fix android module JAVA_NAMESPACCE and JNI_NAMESPACE with unicode Fix android module JAVA_NAMESPACE and JNI_NAMESPACE with unicode Nov 21, 2018
@AndreMiras AndreMiras merged commit 6782c70 into kivy:master Nov 28, 2018
@KeyWeeUsr KeyWeeUsr deleted the fix_android branch November 28, 2018 22:03
@@ -30,16 +30,16 @@ def prebuild_arch(self, arch):
th = '#define {} {}\n'
tpy = '{} = {}\n'

bootstrap = bootstrap_name = self.ctx.bootstrap.name
bootstrap = bootstrap_name = self.ctx.bootstrap.name.decode('utf-8')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Traceback (most recent call last):
  File "/usr/lib/python3.6/runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "/usr/lib/python3.6/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/home/andre/workspace/EtherollApp/.buildozer/android/platform/python-for-android-new-toolchain/pythonforandroid/toolchain.py", line 1032, in <module>
    main()
  File "/home/andre/workspace/EtherollApp/.buildozer/android/platform/python-for-android-new-toolchain/pythonforandroid/toolchain.py", line 1028, in main
    ToolchainCL()
  File "/home/andre/workspace/EtherollApp/.buildozer/android/platform/python-for-android-new-toolchain/pythonforandroid/toolchain.py", line 550, in __init__
    getattr(self, args.subparser_name.replace('-', '_'))(args)
  File "/home/andre/workspace/EtherollApp/.buildozer/android/platform/python-for-android-new-toolchain/pythonforandroid/toolchain.py", line 149, in wrapper_func
    build_dist_from_args(ctx, dist, args)
  File "/home/andre/workspace/EtherollApp/.buildozer/android/platform/python-for-android-new-toolchain/pythonforandroid/toolchain.py", line 188, in build_dist_from_args
    build_recipes(build_order, python_modules, ctx)
  File "/home/andre/workspace/EtherollApp/.buildozer/android/platform/python-for-android-new-toolchain/pythonforandroid/build.py", line 635, in build_recipes
    recipe.prebuild_arch(arch)
  File "/home/andre/workspace/EtherollApp/.buildozer/android/platform/python-for-android-new-toolchain/pythonforandroid/recipes/android/__init__.py", line 33, in prebuild_arch
    bootstrap = bootstrap_name = self.ctx.bootstrap.name.decode('utf-8')
AttributeError: 'str' object has no attribute 'decode'

😢

Copy link
Contributor Author

@KeyWeeUsr KeyWeeUsr Nov 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is due to the Dockerfile using Python 2, therefore it worked for me on the device with py3 but it fails when you want to compile with py3 on the host. I think we should fully switch to unicode literals otherwise in case something is generated like variable = 'value' it'll always split to bytes on py2 and unicode on py3 and we either start checking each damn occurrence of this schrodinger's string or do this:

>>> b'x'.encode('utf-8').decode('utf-8')  # py2 + py3 bytes only
u'x'
>>> u'x'.encode('utf-8').decode('utf-8')  # py3 only
u'x'

and get kicked by anything that is not ASCII encodable.

Perhaps a fix for this would be to remove the decode() and enforce self.ctx.bootstrap.name to be always unicode with an assert.

Copy link
Member

@AndreMiras AndreMiras Nov 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm in to fully switch since it's one step further the migration to python3 and we can still make it compatible with python2 with e.g. unicode_literals from future.
Edit: link to some a bug report kivy/buildozer#756

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed by #1512

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a proper fix with future package, but could not test it because of gradle crash. Will do asap.

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

Successfully merging this pull request may close these issues.

2 participants