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

ctypes.util.find_library doesn't work with python 3 #1337

Closed
ghost opened this issue Aug 24, 2018 · 9 comments
Closed

ctypes.util.find_library doesn't work with python 3 #1337

ghost opened this issue Aug 24, 2018 · 9 comments

Comments

@ghost
Copy link

ghost commented Aug 24, 2018

We talked about this in chat before, I just wanted to make an actual issue for this: ctypes.util.find_library doesn't work when using python3crystax out of the box, because it seems to try something with a shell that doesn't work due to a missing binary.

This code makes it work when put before any ctypes.util import (it includes a PySDL2-specific hack without which PySDL2 will fail to load - this one might actually need addressing in the recipe for PySDL2):

def apply_hack():
    import ctypes.util
    orig_func = ctypes.util.find_library
    def android_find_library_hack(*args):
        import os
        name = args[0]

        # Truncate ending for easier comparison:
        if name.find(".so.") >= 0:
            name = name.partition(".so.")[0]
        if name.endswith(".so"):
            name = name[:-len(".so")]
        if not name.endswith("."):
            name += "."

        # Helper function to check lib name:
        def check_name(lib_name, search_name):
            if filename.endswith('.so') and (
                    filename.startswith("lib" + search_name) or
                    filename.startswith(search_name)):
                return True
            if search_name.endswith("-2.0."):  # PySDL2 hack
                search_name = search_name[:-len("-2.0.")] + "."
                return check_name(lib_name, search_name)
            return False

        # Check the user app lib dir and system dir:
        app_root = os.path.normpath(os.path.abspath(os.path.join(
            os.path.dirname(__file__), '..', '..', 'lib')))
        sys_dir = "/system/lib"
        for lib_search in [app_root, sys_dir]:
            if not os.path.exists(lib_search):
                continue
            for filename in os.listdir(lib_search):
                if check_name(filename, name):
                    return os.path.join(lib_search, filename)
        return orig_func(*args)
    import ctypes.util
    ctypes.util.find_library = android_find_library_hack
apply_hack()

I don't know enough to put this into python-for-android, but maybe someone with the knowledge can put this into the python launch wrapper somehow to make it magically work for everyone? Feel free to take it, consider it CC0/public domain (it's vaguely based on python-for-android's hack for Python 2 anyway)

@AndreMiras
Copy link
Member

Thanks for making a bug report out of it.
FWIW I also monkey patch it in a similar fashion here:
https://github.com/AndreMiras/EtherollApp/blob/v20180617/src/etheroll/utils.py#L172
And for records, yes indeed this is already patched in the Python2 recipe here https://github.com/kivy/python-for-android/blob/58f7618/pythonforandroid/recipes/python2/patches/ctypes-find-library-updated.patch
We may eventually need to do the same for Crystax Python3. That could be an easy one.

@ghost
Copy link
Author

ghost commented Aug 25, 2018

While no longer an issue for me, it is extremely counter-intuitive for beginners that it doesn't work. Also, I'd like to get that hackish code out of my repository since it feels quite misplaced there. So I'd love to see it go in 👍

@AndreMiras
Copy link
Member

I totally agree and indeed p4a is not beginner friendly. We're doing our best to go this way and would love to fill the huge entry gap, but there's still a long way to go. Any help (good bug report like you're doing included) is always appreciated 😄

@inclement
Copy link
Member

I think there wouldn't be an issue with applying this monkey patch in the start.c, it would be nicer than having to patch python itself.

@ghost
Copy link
Author

ghost commented Aug 25, 2018

Yup, I'm also patching way after the whole init stuff right in the application itself, so there's no need to pre-apply this to python I think. It can just run in any sort of bootstrap code that runs before the user code.

I totally agree and indeed p4a is not beginner friendly.

It's actually pretty easy, and would have been for me if things actually worked 😆 after all, installing the SDK/NDK and downloading crystax, providing the three folders and running a one-liner isn't that hard to do. Most of the encountered issues go back to me using Python 3, so once that's a bit stabilized it's going to be a lot easier to use

@ghost
Copy link
Author

ghost commented Oct 23, 2018

Ok so essentially I am getting constant (not super common but it happens regularly) issues due to this, because this monkey patching is quite fickle in regards to import order, so if I move modules around I'm bound to somehow screw it up and get a broken build.

This is extremely annoying, so I'd like to support moving this into python-for-android's master quicker, such that I can just wipe that annoying code out of my project and stop dealing with this.

Would finding out where to add this and a pull request be the best way to move ahead? You mentioned start.c, is that where I should start looking? (Not that I know that file yet, but I assume I'll be able to find it)

@inclement
Copy link
Member

The start.c file is this one. This defines the program that is actually executed on Android, and one of the things it does is start up the Python interpreter. As you can see from what it currently does, it's quite easy to make it run python code passed as strings, or to interpret a file.

The start.c is actually quite hacky and badly structured at the moment, it doesn't get much love since it's almost never necessary to change it. However, it should be clear how you can insert your code at some appropriate place.

I'm assuming that the performance penalty for doing this is negligible, even on Android.

@inclement
Copy link
Member

Fixed by #1624

@ghost
Copy link
Author

ghost commented Jan 31, 2019

Oh it had the ticket number #1337 😍 haha

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

No branches or pull requests

2 participants