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.

Sign up for GitHub

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

Jump to bottom

gethostbyname_r() is broken on android #71044

Closed
xdegaye mannequin opened this issue Apr 26, 2016 · 13 comments
Closed

gethostbyname_r() is broken on android #71044

xdegaye mannequin opened this issue Apr 26, 2016 · 13 comments
Assignees
@skrah
Labels
build The build process and cross-build type-bug An unexpected behavior, bug, or error

Comments

@xdegaye
Copy link
Mannequin

xdegaye mannequin commented Apr 26, 2016

BPO 26857
Nosy @skrah, @xdegaye, @moreati, @yan12125
Files
  • socketmodule.patch
  • socketmodule_2.patch: conditional on ANDROID_API value
  • issue26857.diff
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/skrah'
closed_at = <Date 2016-05-22.15:37:26.493>
created_at = <Date 2016-04-26.13:33:19.200>
labels = ['type-bug', 'build']
title = 'gethostbyname_r() is broken on android'
updated_at = <Date 2016-05-22.15:37:26.491>
user = 'https://github.com/xdegaye'

    bugs.python.org fields:

    activity = <Date 2016-05-22.15:37:26.491>
actor = 'skrah'
assignee = 'skrah'
closed = True
closed_date = <Date 2016-05-22.15:37:26.493>
closer = 'skrah'
components = ['Cross-Build']
creation = <Date 2016-04-26.13:33:19.200>
creator = 'xdegaye'
dependencies = []
files = ['42609', '42809', '42944']
hgrepos = []
issue_num = 26857
keywords = ['patch']
message_count = 13.0
messages = ['264280', '264290', '264296', '265292', '265309', '265310', '265315', '265321', '265323', '266077', '266079', '266082', '266083']
nosy_count = 5.0
nosy_names = ['skrah', 'xdegaye', 'python-dev', 'Alex.Willmer', 'yan12125']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue26857'
versions = ['Python 3.6']
    @xdegaye
    Copy link
    Mannequin Author

    xdegaye mannequin commented Apr 26, 2016

    HAVE_GETHOSTBYNAME_R is defined on android API 21, but importing the _socket module fails with:

    ImportError: dlopen failed: cannot locate symbol "gethostbyaddr_r" referenced by "_socket.cpython-36m-i386-linux-gnu.so"

    Patch attached.
    The patch does not take into account the fact that this may be fixed in future versions of android.

    @xdegaye xdegaye mannequin added build The build process and cross-build type-bug An unexpected behavior, bug, or error labels Apr 26, 2016
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Apr 26, 2016

    New changeset eb19ad1918cd by Stefan Krah in branch 'default':
    Issue bpo-26857: Workaround for missing symbol "gethostbyaddr_r" on Android.
    https://hg.python.org/cpython/rev/eb19ad1918cd

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Apr 26, 2016

    Thanks, fixed.

    @skrah skrah mannequin closed this as completed Apr 26, 2016
    @skrah skrah mannequin self-assigned this Apr 26, 2016
    @xdegaye
    Copy link
    Mannequin Author

    xdegaye mannequin commented May 11, 2016

    gethostbyaddr_r() is implemented now on Android 6.0 (API 23). The attached patch has been tested on the android-21-x86 emulator (API 21) and android-23-x86 emulator (API 23). No new NDK has been released at Android 5.1 (API 22) so there is no need to test the patch for this release.

    @xdegaye xdegaye mannequin reopened this May 11, 2016
    @skrah
    Copy link
    Mannequin

    skrah mannequin commented May 11, 2016

    How about supporting API >= 23 only? Can people upgrade their devices or do they have to buy a new one?

    @yan12125
    Copy link
    Mannequin

    yan12125 mannequin commented May 11, 2016

    Can people upgrade their devices or do they have to buy a new one?

    AFAIK most models other than Nexus won't get updates with a bumped major version. (5.x => 6.x for example)

    @yan12125
    Copy link
    Mannequin

    yan12125 mannequin commented May 11, 2016

    By the way, socketmodule_2.patch is problematic. Developers may choose to build CPython against API level 21 and run it on all devices with API level >= 21. In general Android keeps ABI compatibility between consecutive versions. That is, most binaries built for API 21 can be run on API 21, 22, 23 and 24. With this patch, developers have to build two versions of CPython, one for API < 23 and one for API >= 23, or gethostbyname_r() may not be used.

    @xdegaye
    Copy link
    Mannequin Author

    xdegaye mannequin commented May 11, 2016

    So what is problematic with this new patch ? Obviously you need to build with API 23 to get gethostbyname_r() since it was not supported by android before, with the previous patch you don't get gethostbyname_r(), even when building with API 23.

    @yan12125
    Copy link
    Mannequin

    yan12125 mannequin commented May 11, 2016

    I was thinking the (somewhat hacky) dlopen() approach so that gethostbyname_r() works in API 21 builds. If a universal build is not necessary this patch is OK.

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented May 22, 2016

    Okay thanks, let's assume api-level >= 21 for now. I've moved the include into pyport.h in order to save a little space everywhere. Could you check if that works?

    @xdegaye
    Copy link
    Mannequin Author

    xdegaye mannequin commented May 22, 2016

    I've moved the include into pyport.h

    and also fixed the error in my patch, thanks :)

    With bpo-26857.diff, importing the socket module does not fail both with an API 21 emulator and an API 23 emulator.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 22, 2016

    New changeset 09af54099973 by Stefan Krah in branch 'default':
    Issue bpo-26857: The gethostbyaddr_r() workaround is no longer needed with
    https://hg.python.org/cpython/rev/09af54099973

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented May 22, 2016

    Thanks! Closing again.

    @skrah skrah mannequin closed this as completed May 22, 2016
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @skrah skrah

    Labels
    build The build process and cross-build type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    0 participants