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

ctypes should be able to link with installed libffi #43157

Closed
doko42 opened this issue Apr 4, 2006 · 14 comments
Closed

ctypes should be able to link with installed libffi #43157

doko42 opened this issue Apr 4, 2006 · 14 comments
Assignees
@theller
Labels
extension-modules C modules in the Modules dir

Comments

@doko42
Copy link
Member

doko42 commented Apr 4, 2006

BPO 1464444
Nosy @loewis, @theller, @doko42
PRs
  • bpo-30912: Don't check ffi.h for specific contents #2687
    		•
    Files
  • libffi.diff: patch to check for system libffi (v3)
  • libffi-doc.diff: documentation patch
    		•

    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/theller'
closed_at = <Date 2017-07-13.17:16:35.983>
created_at = <Date 2006-04-04.19:42:46.000>
labels = ['extension-modules']
title = 'ctypes should be able to link with installed libffi'
updated_at = <Date 2017-07-13.17:16:35.983>
user = 'https://github.com/doko42'

    bugs.python.org fields:

    activity = <Date 2017-07-13.17:16:35.983>
actor = 'shlomif'
assignee = 'theller'
closed = True
closed_date = None
closer = None
components = ['Extension Modules']
creation = <Date 2006-04-04.19:42:46.000>
creator = 'doko'
dependencies = []
files = ['1949', '1950']
hgrepos = []
issue_num = 1464444
keywords = []
message_count = 14.0
messages = ['28062', '28063', '28064', '28065', '28066', '28067', '28068', '28069', '28070', '28071', '28072', '28073', '28074', '28075']
nosy_count = 3.0
nosy_names = ['loewis', 'theller', 'doko']
pr_nums = ['2687']
priority = 'normal'
resolution = 'accepted'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue1464444'
versions = []
    @doko42
    Copy link
    Member Author

    doko42 commented Apr 4, 2006

    ctypes always uses the included libffi implementation
    (which should be updated to current GCC HEAD). Pleae
    add an option to build and link against a libffi, which
    is installed on the system.

    attached is a hack to just always build using the
    installed libffi. I'm unsure how to find the correct
    ffi implementation (maybe check for LIBFFI_H defined in
    the ffi header?), and where to implement this check
    (configure.ac --with-system-ffi=/usr?).

    @doko42 doko42 closed this as completed Apr 4, 2006
    @doko42 doko42 added the extension-modules C modules in the Modules dir label Apr 4, 2006
    @doko42 doko42 closed this as completed Apr 4, 2006
    @doko42 doko42 added the extension-modules C modules in the Modules dir label Apr 4, 2006
    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Apr 4, 2006

    Logged In: YES
    user_id=21627

    Why is it desirable to link with the system ffi? That just
    adds an external dependency, for a negligable space saving
    (on x86, libffi.so.4.0.1 is 7840 bytes).

    @doko42
    Copy link
    Member Author

    doko42 commented Apr 4, 2006

    Logged In: YES
    user_id=60903

    It's not an external dependency, if it's not found. The size
    is not the argument. For distributors it's an additional
    maintainance burden to update copies of libraries. Security
    advisories for zlib (and pcre?) show this.

    patch updated.

    @theller
    Copy link

    theller commented Apr 6, 2006

    Logged In: YES
    user_id=11105

    I tend to accept this patch. Martin, any objections?

    Matthias: do any of the buildbot slaves have a system libffi
    installed?

    About the code:
    And, shouldn't it be enough to let the compiler find ffi.h?
    I assume that the presence of this file should indicate
    that libffi.so is installed - why do we have to scan this
    file for '#define LIBFFI_H', and why do we have to search
    for the ffi shared library?

    Would updating the included libffi code to GCC HEAD bring
    any advantages? The currently included code has been
    patched in a few places, to avoid some of the compiler
    warnings, and to allow compilation on OS X 10.3 (which does
    not know the 'live_support' segment attribute, see Python
    bug 1460514).

    @doko42
    Copy link
    Member Author

    doko42 commented Apr 6, 2006

    Logged In: YES
    user_id=60903

    do any of the buildbot slaves have a system libffi
    installed?

    the Debian and Ubuntu bots have libffi from gcc-4.1 installed.

    I think in the past, there was a complaint about a
    conflicting ffi.h, or maybe was this ffitarget.h? not sure
    if the check for LIBFFI_H really helps with this. yes, if
    gcc and ffi.h are installed from the same source / into the
    same path, then the check for include dirs and libraries are
    obsolete. the check for the different library names comes
    from the observation that libgcj is linked against libffi as
    a convenience library (for performance reasons?). These are
    checked first so that not another external dependency is
    introduced, if these libraries are available (not installed
    by a standard gcc installation).

    updating the code would at least make the library buildable
    on hppa (or maybe this is just the failed (it's really a
    32bit runtime, but hppa64-linux is detected).

    @theller
    Copy link

    theller commented Apr 6, 2006

    Logged In: YES
    user_id=11105

    I tried your patch on an AMD64 ubuntu 5.10, after I
    installed the 4.0.1-4ubuntu9 libffi package. It crashes in
    ctypes/test/test_python_api.py, test_PyOS_snprintf. IIRC,
    libffi did not support varargs functions, but Andreas Degert
    added support for them (for x86_64), and it works fine.

    Ok, new features in libffi might not be the norm, but I see
    no way how I can check whether a system-installed libffi
    supports this feature or not.

    Do *you* see a solution for that?

    For the actual changes to setup.py, if the patch really is a
    good idea: Since I don't even know what a convience library
    is I'll trust you fully that your patch does the right thing.

    @doko42
    Copy link
    Member Author

    doko42 commented Apr 6, 2006

    Logged In: YES
    user_id=60903

    maybe a configure switch --with-system-ffi could do it?
    How to pass this to the setup.py file?

    @theller
    Copy link

    theller commented Apr 6, 2006

    Logged In: YES
    user_id=11105

    I do not really know. Maybe using
    distutils.sysconfig.get_config_var() ?

    A pragmatic approach would be to parse pyconfig.h itself in
    the setup script.

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Apr 6, 2006

    Logged In: YES
    user_id=21627

    If this configuration needs an operator decision, I think
    this decision is best made in Modules/Setup.local. Anybody
    who wants to use the local libffi installation can do so, by
    writing a Setup.local that lists the source files, and gives
    -lffi as a library option. Setup.dist could provide a
    commented version of such a configuration.

    setup.py *should* then skip over building _ctypes, as it
    should find out that _ctypes was already built through Makefile.

    @theller
    Copy link

    theller commented Apr 7, 2006

    Logged In: YES
    user_id=11105

    I'm astonished how flexible the build-system is :-), once
    you leave Windows.

    setup.py needs a change, though, so that the _ctypes/libffi
    configure script is not run when not needed.

    One possible advange of Modules/Setup.local is that one can
    define additional compilation flags for ctypes - for
    example, if ctypes would support it, to enable/disable
    certain features (varargs function calls, for example).

    @doko42
    Copy link
    Member Author

    doko42 commented Apr 10, 2006

    Logged In: YES
    user_id=60903

    Setup.local has the disadvantage that you have to edit the
    file. I updated the patch to only have an effect, if
    configure is passed --with-system-ffi.

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Apr 10, 2006

    Logged In: YES
    user_id=21627

    The code is fine, but it lacks documentation. There should
    be some way to learn about --with-system-ffi, preferably by
    either reading ./configure --help output, or reading README.

    @doko42
    Copy link
    Member Author

    doko42 commented Apr 10, 2006

    Logged In: YES
    user_id=60903

    documentation added.

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Apr 11, 2006

    Logged In: YES
    user_id=21627

    Thanks for the patch. Committed as r45278.

    @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

    @theller theller

    Labels
    extension-modules C modules in the Modules dir
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @theller @doko42