Merge pull request #1416 from gevent/issue1395

Make the CFFI backends work with non-embedded libev/libuv

.travis.yml

@@ -50,6 +50,7 @@ matrix:
 
 before_install:
   - export PATH=$BUILD_RUNTIMES/snakepit/$TRAVIS_PYTHON_VERSION.d/bin:$PATH
+  - export G_SITE=$BUILD_RUNTIMES/snakepit/$TRAVIS_PYTHON_VERSION.d/lib/*/site-packages/
 
 before_script:
   # Show some details of interest
@@ -130,6 +131,7 @@ jobs:
         # Install the Python runtime
         - *build-gevent-python
         - *build-gevent-deps
+        - ls -l $G_SITE
         # Install the C dependencies to a known location. This is used
         # to test 'no embed' cases. It might seem like it would prime
         # the CCache for when we *do* embed if we did it as part of the generic build stage,
@@ -141,8 +143,16 @@ jobs:
         # libev builds a manpage each time, and it includes today's date, so it frequently changes.
         # delete to avoid repacking the archive
         - rm -rf $BUILD_LIBS/share/man/
-        - ls -l $BUILD_LIBS $BUILD_LIBS/lib
+        - ls -l $BUILD_LIBS $BUILD_LIBS/lib $BUILD_LIBS/include
         - pip install --no-build-isolation .[test]
+        # Test that we're actually linking
+        # to the .so file.
+        - objdump -p $G_SITE/gevent/libev/_corecffi*so | grep "NEEDED.*libev.so"
+        - objdump -p $G_SITE/gevent/libuv/_corecffi*so | grep "NEEDED.*libuv.so"
+      script:
+        # Verify that we got non-embedded builds
+        - python -c 'import gevent.libev.corecffi as CF; assert not CF.LIBEV_EMBED'
+        - python -c 'import gevent.libuv.loop as CF; assert not CF.libuv.LIBUV_EMBED'
 
     # Ok, now we switch to the test stage. These are all in addition
     # to the jobs created by the matrix (and should override the `script` command).
@@ -238,17 +248,18 @@ jobs:
 
     # 2.7, no-embed. Run the tests that exercise the libraries we
     # linked to.
-    # XXX: The CFFI backends, as exercised by test-lib[eu]v-jobs
-    # don't really support this!
     - <<: *test-ares-jobs
+      # This job exercises both the non-embedded ares resolver
+      # and the non-embedded Cython libev loop.
       env: TRAVIS_PYTHON_VERSION=2.7 GEVENTSETUP_EMBED=0
       name: ares27-noembed
-    # - <<: *test-libuv-jobs
-    #   env: TRAVIS_PYTHON_VERSION=2.7 GEVENTSETUP_EMBED=0
-    #   name: libuv27-noembed
-    # - <<: *test-libev-jobs
-    #   env: TRAVIS_PYTHON_VERSION=2.7 GEVENTSETUP_EMBED=0
-    #   name: libev27-noembed
+    # These exercise the CFFI versions.
+    - <<: *test-libuv-jobs
+      env: TRAVIS_PYTHON_VERSION=2.7 GEVENTSETUP_EMBED=0
+      name: libuv27-noembed
+    - <<: *test-libev-jobs
+      env: TRAVIS_PYTHON_VERSION=2.7 GEVENTSETUP_EMBED=0
+      name: libev27-noembed
 
     # PyPy 2.7
     - <<: *test-dnspython-jobs

CHANGES.rst

@@ -90,6 +90,9 @@
   variables. Instead, use ``GEVENTSETUP_EMBED`` and
   ``GEVENTSETUP_EMBED_LIBEV``. See :issue:`1402`.
 
+- The CFFI backends now respect the embed build-time setting. This allows
+  building the libuv backend without embedding libuv (except on Windows).
+
 - Support test resources. This allows disabling tests that use the
   network. See :ref:`limiting-test-resource-usage` for more.
 

_setuplibev.py

@@ -1,6 +1,9 @@
 # -*- coding: utf-8 -*-
 """
 setup helpers for libev.
+
+Importing this module should have no side-effects; in particular,
+it shouldn't attempt to cythonize anything.
 """
 
 from __future__ import print_function, absolute_import, division
@@ -19,7 +22,6 @@
 from _setuputils import glob_many
 from _setuputils import dep_abspath
 from _setuputils import should_embed
-from _setuputils import cythonize1
 
 
 LIBEV_EMBED = should_embed('libev')
@@ -60,35 +62,42 @@ def configure_libev(bext, ext):
     finally:
         os.chdir(cwd)
 
-CORE = Extension(name='gevent.libev.corecext',
-                 sources=[
-                     'src/gevent/libev/corecext.pyx',
-                     'src/gevent/libev/callbacks.c',
-                 ],
-                 include_dirs=['src/gevent/libev'] + [dep_abspath('libev')] if LIBEV_EMBED else [],
-                 libraries=list(LIBRARIES),
-                 define_macros=list(DEFINE_MACROS),
-                 depends=glob_many('src/gevent/libev/callbacks.*',
-                                   'src/gevent/libev/stathelper.c',
-                                   'src/gevent/libev/libev*.h',
-                                   'deps/libev/*.[ch]'))
-if WIN:
-    CORE.define_macros.append(('EV_STANDALONE', '1'))
-# QQQ libev can also use -lm, however it seems to be added implicitly
-
-if LIBEV_EMBED:
-    CORE.define_macros += [('LIBEV_EMBED', '1'),
-                           ('EV_COMMON', ''),  # we don't use void* data
-                           # libev watchers that we don't use currently:
-                           ('EV_CLEANUP_ENABLE', '0'),
-                           ('EV_EMBED_ENABLE', '0'),
-                           ("EV_PERIODIC_ENABLE", '0')]
-    CORE.configure = configure_libev
-    if sys.platform == "darwin":
-        os.environ["CPPFLAGS"] = ("%s %s" % (os.environ.get("CPPFLAGS", ""), "-U__llvm__")).lstrip()
-    if os.environ.get('GEVENTSETUP_EV_VERIFY') is not None:
-        CORE.define_macros.append(('EV_VERIFY', os.environ['GEVENTSETUP_EV_VERIFY']))
-else:
-    CORE.libraries.append('ev')
-
-CORE = cythonize1(CORE)
+def build_extension():
+    # Return the un-cythonized extension.
+    # This can be used to access things like `libraries` and `include_dirs`
+    # and `define_macros` so we DRY.
+    CORE = Extension(name='gevent.libev.corecext',
+                     sources=[
+                         'src/gevent/libev/corecext.pyx',
+                         'src/gevent/libev/callbacks.c',
+                     ],
+                     include_dirs=['src/gevent/libev'] + [dep_abspath('libev')] if LIBEV_EMBED else [],
+                     libraries=list(LIBRARIES),
+                     define_macros=list(DEFINE_MACROS),
+                     depends=glob_many('src/gevent/libev/callbacks.*',
+                                       'src/gevent/libev/stathelper.c',
+                                       'src/gevent/libev/libev*.h',
+                                       'deps/libev/*.[ch]'))
+    if WIN:
+        CORE.define_macros.append(('EV_STANDALONE', '1'))
+    # QQQ libev can also use -lm, however it seems to be added implicitly
+
+    if LIBEV_EMBED:
+        CORE.define_macros += [('LIBEV_EMBED', '1'),
+                               # we don't use void* data in the cython implementation;
+                               # the CFFI implementation does and removes this line.
+                               ('EV_COMMON', ''),
+                               # libev watchers that we don't use currently:
+                               ('EV_CLEANUP_ENABLE', '0'),
+                               ('EV_EMBED_ENABLE', '0'),
+                               ("EV_PERIODIC_ENABLE", '0')]
+        CORE.configure = configure_libev
+        if sys.platform == "darwin":
+            os.environ["CPPFLAGS"] = ("%s %s" % (os.environ.get("CPPFLAGS", ""), "-U__llvm__")).lstrip()
+        if os.environ.get('GEVENTSETUP_EV_VERIFY') is not None:
+            CORE.define_macros.append(('EV_VERIFY', os.environ['GEVENTSETUP_EV_VERIFY']))
+    else:
+        CORE.define_macros += [('LIBEV_EMBED', '0')]
+        CORE.libraries.append('ev')
+
+    return CORE

_setuputils.py

@@ -87,7 +87,6 @@ def _bool_from_environ(key):
     raise ValueError('Environment variable %r has invalid value %r. '
                      'Please set it to 1, 0 or an empty string' % (key, value))
 
-IGNORE_CFFI = _bool_from_environ("GEVENTSETUP_NO_CFFI_BUILD")
 
 def _check_embed(key, defkey, path=None, warn=False):
     """

docs/installing_from_source.rst

@@ -107,11 +107,6 @@ yes/no.
   In general, setting ``CPPFLAGS`` is more general and can contain
   other macros recognized by libev.
 
-``GEVENTSETUP_NO_CFFI_BUILD``
-  A boolean; when set to true, this disables all attempts at building
-  the CFFI modules. *This disables libuv.* (TODO: verify that.)
-  Ignored on PyPy and ignored on Windows.
-
 
 Embedding Libraries
 -------------------
@@ -124,8 +119,7 @@ embedding, especially in the case of libev, can be more efficient as
 features not needed by gevent can be disabled, resulting in smaller or
 faster libraries or runtimes.
 
-However, this can be disabled (TODO: verify how this interacts with
-CFFI; see NO_CFFI_BUILD), either for all libraries at once or for
+However, this can be disabled, either for all libraries at once or for
 individual libraries.
 
 When embedding a library is disabled, the library must already be

setup.py

@@ -17,7 +17,6 @@
 from _setuputils import read_version
 from _setuputils import system
 from _setuputils import PYPY, WIN
-from _setuputils import IGNORE_CFFI
 from _setuputils import ConfiguringBuildExt
 from _setuputils import GeventClean
 from _setuputils import BuildFailed
@@ -46,10 +45,11 @@
 
 from _setuplibev import libev_configure_command
 from _setuplibev import LIBEV_EMBED
-from _setuplibev import CORE
-
+from _setuplibev import build_extension as build_libev_extension
 from _setupares import ARES
 
+CORE = cythonize1(build_libev_extension())
+
 # Get access to the greenlet header file.
 # The sysconfig dir is not enough if we're in a virtualenv
 # See https://github.com/pypa/pip/issues/4610
@@ -284,15 +284,6 @@
 del _to_cythonize
 
 
-if IGNORE_CFFI and not PYPY and not WIN:
-    # Allow distributors to turn off CFFI builds
-    # even if it's available, because CFFI always embeds
-    # our copy of libev/libuv and they may not want that.
-    # Not allowed on PyPy and not allowed on Windows, because those
-    # backends are required there.
-    # TODO: CONFIRM if this is still the case.
-    del cffi_modules[:]
-
 ## Extras
 
 EXTRA_DNSPYTHON = [

src/gevent/libev/_corecffi_build.py

@@ -10,21 +10,32 @@
 import sys
 import os
 import os.path # pylint:disable=no-name-in-module
+from cffi import FFI
+
+sys.path.append(".")
+try:
+    import _setuplibev
+except ImportError:
+    print("This file must be imported with setup.py in the current working dir.")
+    raise
+
+thisdir = os.path.dirname(os.path.abspath(__file__))
+setup_dir = os.path.abspath(os.path.join(thisdir, '..', '..', '..'))
 
 
 __all__ = []
 
 
-from cffi import FFI
 ffi = FFI()
 
-thisdir = os.path.dirname(os.path.abspath(__file__))
+
 def read_source(name):
     with open(os.path.join(thisdir, name), 'r') as f:
         return f.read()
 
+# cdef goes to the cffi library and determines what can be used in
+# Python.
 _cdef = read_source('_corecffi_cdef.c')
-_source = read_source('_corecffi_source.c')
 
 # These defines and uses help keep the C file readable and lintable by
 # C tools.
@@ -35,6 +46,14 @@ def read_source(name):
                       'typedef int... nlink_t;')
 _cdef = _cdef.replace('GEVENT_ST_NLINK_T', 'nlink_t')
 
+if _setuplibev.LIBEV_EMBED:
+    _cdef += """
+struct ev_loop {
+    int backend_fd;
+    int activecnt;
+    ...;
+};
+"""
 
 if sys.platform.startswith('win'):
     # We must have the vfd_open, etc, functions on
@@ -49,19 +68,50 @@ def read_source(name):
 void vfd_free(int);
 """
 
+# source goes to the C compiler
+_source = read_source('_corecffi_source.c')
 
 
-include_dirs = [
-    thisdir, # libev_vfd.h
-    os.path.abspath(os.path.join(thisdir, '..', '..', '..', 'deps', 'libev')),
-]
+distutils_ext = _setuplibev.build_extension()
+
+macros = list(distutils_ext.define_macros)
+try:
+    # We need the data pointer.
+    macros.remove(('EV_COMMON', ''))
+except ValueError:
+    pass
+
 ffi.cdef(_cdef)
-ffi.set_source('gevent.libev._corecffi', _source, include_dirs=include_dirs)
+ffi.set_source(
+    'gevent.libev._corecffi',
+    _source,
+    include_dirs=distutils_ext.include_dirs + [thisdir], # "libev.h"
+    define_macros=macros,
+    libraries=distutils_ext.libraries,
+)
 
 if __name__ == '__main__':
     # XXX: Note, on Windows, we would need to specify the external libraries
     # that should be linked in, such as ws2_32 and (because libev_vfd.h makes
     # Python.h calls) the proper Python library---at least for PyPy. I never got
     # that to work though, and calling python functions is strongly discouraged
     # from CFFI code.
-    ffi.compile()
+
+    # On macOS to make the non-embedded case work correctly, against
+    # our local copy of libev:
+    #
+    # 1) configure and make libev
+    # 2) CPPFLAGS=-Ideps/libev/ LDFLAGS=-Ldeps/libev/.libs GEVENTSETUP_EMBED_LIBEV=0 \
+    #     python setup.py build_ext -i
+    # 3) export DYLD_LIBRARY_PATH=`pwd`/deps/libev/.libs
+    #
+    # The DYLD_LIBRARY_PATH is because the linker hard-codes
+    # /usr/local/lib/libev.4.dylib in the corecffi.so dylib, because
+    # that's the "install name" of the libev dylib that was built.
+    # Adding a -rpath to the LDFLAGS doesn't change things.
+    # This can be fixed with `install_name_tool`:
+    #
+    # 3) install_name_tool -change /usr/local/lib/libev.4.dylib \
+    #    `pwd`/deps/libev/.libs/libev.4.dylib \
+    #     src/gevent/libev/_corecffi.abi3.so
+    ffi.compile(verbose=True)

src/gevent/libev/_corecffi_cdef.c

@@ -1,3 +1,7 @@
+/* access whether we built embedded or not */
+
+#define LIBEV_EMBED ...
+
 /* libev interface */
 
 #define EV_MINPRI ...
@@ -55,11 +59,9 @@
 #define GEVENT_STRUCT_DONE int
 #define GEVENT_ST_NLINK_T int
 
-struct ev_loop {
-    int backend_fd;
-    int activecnt;
-    GEVENT_STRUCT_DONE _;
-};
+/* Note that we don't declare the ev_loop struct and fields here. */
+/* If we don't embed libev, we can't access those fields, libev */
+/* keeps it opaque.  */
 
 // Watcher types
 // base for all watchers

src/gevent/libev/_corecffi_source.c

@@ -1,5 +1,9 @@
-// passed to the real C compiler
+/* passed to the real C compiler */
+#ifndef LIBEV_EMBED
+/* We're normally used to embed libev, assume that */
+/* When this is defined, libev.h includes ev.c */
 #define LIBEV_EMBED 1
+#endif
 
 #ifdef _WIN32
 #define EV_STANDALONE 1