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

 # -*- 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 DEFINE_MACROS
 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
 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 @@ __version__ = read_version()
 
 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 @@ for mod in _to_cythonize:
 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 @@ from __future__ import absolute_import, print_function
 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 @@ _cdef = _cdef.replace('#define GEVENT_ST_NLINK_T int',
                       '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,14 +68,27 @@ vfd_socket_t vfd_get(int);
 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
@@ -64,4 +96,22 @@ if __name__ == '__main__':
     # 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

+/* 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

-// 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

src/gevent/libev/corecffi.py

@@ -389,7 +389,9 @@ class loop(AbstractLoop):
             libev.gevent_reset_sigchld_handler()
 
     def fileno(self):
-        if self._ptr:
+        if self._ptr and LIBEV_EMBED:
+            # If we don't embed, we can't access these fields,
+            # the type is opaque
             fd = self._ptr.backend_fd
             if fd >= 0:
                 return fd
@@ -398,7 +400,9 @@ class loop(AbstractLoop):
     def activecnt(self):
         if not self._ptr:
             raise ValueError('operation on destroyed loop')
-        return self._ptr.activecnt
+        if LIBEV_EMBED:
+            return self._ptr.activecnt
+        return -1
 
 
 @ffi.def_extern()
@@ -424,4 +428,4 @@ def set_syserr_cb(callback):
 
 __SYSERR_CALLBACK = None
 
-LIBEV_EMBED = True
+LIBEV_EMBED = libev.LIBEV_EMBED

src/gevent/libev/libev.h

-#if defined(LIBEV_EMBED)
-#include "ev.c"
-#undef LIBEV_EMBED
-#define LIBEV_EMBED 1
-#define gevent_ev_loop_origflags(loop) ((loop)->origflags)
-#define gevent_ev_loop_sig_pending(loop) ((loop))->sig_pending
-#define gevent_ev_loop_backend_fd(loop) ((loop))->backend_fd
-#define gevent_ev_loop_activecnt(loop) ((loop))->activecnt
-#if EV_USE_SIGNALFD
-#define gevent_ev_loop_sigfd(loop) ((loop))->sigfd
-#else
-#define gevent_ev_loop_sigfd(loop) -1
-#endif /* !EV_USE_SIGNALFD */
+#if defined(LIBEV_EMBED) && LIBEV_EMBED
+  #include "ev.c"
+  #undef LIBEV_EMBED
+  #define LIBEV_EMBED 1
+  #define gevent_ev_loop_origflags(loop) ((loop)->origflags)
+  #define gevent_ev_loop_sig_pending(loop) ((loop))->sig_pending
+  #define gevent_ev_loop_backend_fd(loop) ((loop))->backend_fd
+  #define gevent_ev_loop_activecnt(loop) ((loop))->activecnt
+  #if EV_USE_SIGNALFD
+    #define gevent_ev_loop_sigfd(loop) ((loop))->sigfd
+  #else
+    #define gevent_ev_loop_sigfd(loop) -1
+  #endif /* !EV_USE_SIGNALFD */
 #else /* !LIBEV_EMBED */
 #include "ev.h"
 

src/gevent/libuv/_corecffi_build.py

@@ -11,16 +11,29 @@ import sys
 import os
 import os.path # pylint:disable=no-name-in-module
 
+from cffi import FFI
+
+sys.path.append(".")
+
+try:
+    import _setuputils
+except ImportError:
+    print("This file must be imported with setup.py in the current working dir.")
+    raise
+
 
 __all__ = []
 
 WIN = sys.platform.startswith('win32')
+LIBUV_EMBED = _setuputils.should_embed('libuv')
+print("Embedding libuv?", LIBUV_EMBED)
 
-
-from cffi import FFI
 ffi = FFI()
 
 thisdir = os.path.dirname(os.path.abspath(__file__))
+setup_py_dir = os.path.abspath(os.path.join(thisdir, '..', '..', '..'))
+libuv_dir = os.path.abspath(os.path.join(setup_py_dir, 'deps', 'libuv'))
+
 def read_source(name):
     with open(os.path.join(thisdir, name), 'r') as f:
         return f.read()
@@ -49,12 +62,9 @@ _void_pointer_as_integer = 'intptr_t'
 _cdef = _cdef.replace("GEVENT_UV_OS_SOCK_T", 'int' if not WIN else _void_pointer_as_integer)
 
 
-setup_py_dir = os.path.abspath(os.path.join(thisdir, '..', '..', '..'))
-libuv_dir = os.path.abspath(os.path.join(setup_py_dir, 'deps', 'libuv'))
 
 
 LIBUV_INCLUDE_DIRS = [
-    thisdir, # libev_vfd.h
     os.path.join(libuv_dir, 'include'),
     os.path.join(libuv_dir, 'src'),
 ]
@@ -192,7 +202,9 @@ elif sys.platform.startswith('sunos'):
     ]
 
 
-LIBUV_MACROS = []
+LIBUV_MACROS = [
+    ('LIBUV_EMBED', int(LIBUV_EMBED)),
+]
 
 def _define_macro(name, value):
     LIBUV_MACROS.append((name, value))
@@ -239,6 +251,11 @@ elif WIN:
     _add_library('userenv')
     _add_library('ws2_32')
 
+if not LIBUV_EMBED:
+    del LIBUV_SOURCES[:]
+    del LIBUV_INCLUDE_DIRS[:]
+    _add_library('uv')
+
 ffi.cdef(_cdef)
 ffi.set_source('gevent.libuv._corecffi',
                _source,
@@ -249,4 +266,9 @@ ffi.set_source('gevent.libuv._corecffi',
                define_macros=list(LIBUV_MACROS))
 
 if __name__ == '__main__':
-    ffi.compile()
+    # See notes in libev/_corecffi_build.py for how to test this.
+    #
+    # Other than the obvious directory changes, the changes are:
+    #
+    # CPPFLAGS=-Ideps/libuv/include/
+    ffi.compile(verbose=True)

src/gevent/libuv/_corecffi_cdef.c

+/* access whether we built embedded or not */
+
+#define LIBUV_EMBED ...
+
 /* markers for the CFFI parser. Replaced when the string is read. */
 #define GEVENT_STRUCT_DONE int
 #define GEVENT_ST_NLINK_T int