Merge pull request #1417 from gevent/issue1411

Clean up configuring for embedded CFFI modules.

.gitignore

@@ -73,6 +73,7 @@ deps/libev/Makefile
 deps/libev/config.log
 deps/libev/config.h
 deps/libev/config.status
+deps/libev/configure-output.txt
 deps/libev/libtool
 deps/libev/stamp-h1
 deps/libev/.libs

_setuplibev.py

@@ -14,6 +14,7 @@ import os.path
 from _setuputils import Extension
 
 from _setuputils import system
+from _setuputils import dep_abspath
 from _setuputils import quoted_dep_abspath
 from _setuputils import WIN
 from _setuputils import make_universal_header
@@ -26,41 +27,38 @@ from _setuputils import should_embed
 
 LIBEV_EMBED = should_embed('libev')
 
-# Configure libev in place; but cp the config.h to the old directory;
-# if we're building a CPython extension, the old directory will be
-# the build/temp.XXX/libev/ directory. If we're building from a
-# source checkout on pypy, OLDPWD will be the location of setup.py
-# and the PyPy branch will clean it up.
+# Configure libev in place
 libev_configure_command = ' '.join([
     "(cd ", quoted_dep_abspath('libev'),
-    " && sh ./configure ",
-    " && cp config.h \"$OLDPWD\"",
+    " && sh ./configure > configure-output.txt",
     ")",
-    '> configure-output.txt'
 ])
 
 
-def configure_libev(bext, ext):
+def configure_libev(build_command=None, extension=None): # pylint:disable=unused-argument
+    # build_command is an instance of ConfiguringBuildExt.
+    # extension is an instance of the setuptools Extension object.
+    #
+    # This is invoked while `build_command` is in the middle of its `run()`
+    # method.
+
+    # Both of these arguments are unused here so that we can use this function
+    # both from a build command and from libev/_corecffi_build.py
+
     if WIN:
         return
 
-    bdir = os.path.join(bext.build_temp, 'libev')
-    ext.include_dirs.insert(0, bdir)
+    libev_path = dep_abspath('libev')
+    config_path = os.path.join(libev_path, 'config.h')
+    if os.path.exists(config_path):
+        print("Not configuring libev, 'config.h' already exists")
+        return
 
-    if not os.path.isdir(bdir):
-        os.makedirs(bdir)
+    system(libev_configure_command)
+    if sys.platform == 'darwin':
+        make_universal_header(config_path,
+                              'SIZEOF_LONG', 'SIZEOF_SIZE_T', 'SIZEOF_TIME_T')
 
-    cwd = os.getcwd()
-    os.chdir(bdir)
-    try:
-        if os.path.exists('config.h'):
-            return
-        system(libev_configure_command)
-        if sys.platform == 'darwin':
-            make_universal_header('config.h',
-                                  'SIZEOF_LONG', 'SIZEOF_SIZE_T', 'SIZEOF_TIME_T')
-    finally:
-        os.chdir(cwd)
 
 def build_extension():
     # Return the un-cythonized extension.
@@ -99,5 +97,6 @@ def build_extension():
     else:
         CORE.define_macros += [('LIBEV_EMBED', '0')]
         CORE.libraries.append('ev')
+        CORE.configure = lambda *args: print("libev not embedded, not configuring")
 
     return CORE

_setuputils.py

@@ -233,6 +233,28 @@ ext_errors = (CCompilerError, DistutilsExecError, DistutilsPlatformError, IOErro
 
 class ConfiguringBuildExt(build_ext):
 
+    # CFFI subclasses this class with its own, that overrides run()
+    # and invokes a `pre_run` method, if defined. The run() method is
+    # called only once from setup.py (this class is only instantiated
+    # once per invocation of setup()); run() in turn calls
+    # `build_extension` for every defined extension.
+
+    # For extensions we control, we let them define a `configure`
+    # callable attribute, and we invoke that before building. But we
+    # can't control the Extension object that CFFI creates. The best
+    # we can do is provide a global hook that we can invoke in pre_run().
+
+    gevent_pre_run_actions = ()
+
+    @classmethod
+    def gevent_add_pre_run_action(cls, action):
+        # Actions should be idempotent.
+        cls.gevent_pre_run_actions += (action,)
+
+    def finalize_options(self):
+        self.parallel = True # pylint: disable=attribute-defined-outside-init
+        build_ext.finalize_options(self)
+
     def gevent_prepare(self, ext):
         configure = getattr(ext, 'configure', None)
         if configure:
@@ -247,6 +269,13 @@ class ConfiguringBuildExt(build_ext):
                 raise BuildFailed()
             raise
 
+    def pre_run(self, *_args):
+        # Called only from CFFI.
+        # With mulitple extensions, this probably gets called multiple
+        # times.
+        for action in self.gevent_pre_run_actions:
+            action()
+
 
 class Extension(_Extension):
     # This class exists currently mostly to make pylint
@@ -362,6 +391,7 @@ class GeventClean(clean):
                     'config.h',
                     'config.log',
                     'config.status',
+                    'configure-output.txt',
                     '.libs'
             ):
                 yield os.path.join('deps', dep, f)

setup.py

@@ -15,7 +15,6 @@ from setuptools import find_packages
 
 from _setuputils import read
 from _setuputils import read_version
-from _setuputils import system
 from _setuputils import PYPY, WIN
 from _setuputils import ConfiguringBuildExt
 from _setuputils import GeventClean
@@ -43,8 +42,6 @@ if PYPY and sys.pypy_version_info[:3] < (2, 6, 1): # pylint:disable=no-member
 __version__ = read_version()
 
 
-from _setuplibev import libev_configure_command
-from _setuplibev import LIBEV_EMBED
 from _setuplibev import build_extension as build_libev_extension
 from _setupares import ARES
 
@@ -307,18 +304,6 @@ EXTRA_RECOMMENDED = [
     CFFI_DEP,
 ] + EXTRA_DNSPYTHON + EXTRA_EVENTS + EXTRA_MONITOR
 
-# If we are running info / help commands, or we're being imported by
-# tools like pyroma, we don't need to build anything
-_BUILDING = True
-if ((len(sys.argv) >= 2
-     and ('--help' in sys.argv[1:]
-          or sys.argv[1] in ('--help-commands',
-                             'egg_info',
-                             '--version',
-                             'clean',
-                             '--long-description')))
-        or __name__ != '__main__'):
-    _BUILDING = False
 
 def make_long_description():
     readme = read('README.rst')
@@ -332,27 +317,7 @@ def make_long_description():
     return readme
 
 
-def run_setup(ext_modules, run_make):
-    if run_make:
-        if (not LIBEV_EMBED and not WIN and cffi_modules) or PYPY:
-            # We're not embedding libev but we do want
-            # to build the CFFI module. We need to configure libev
-            # because the CORE Extension won't.
-            # TODO: Generalize this.
-            if LIBEV_CFFI_MODULE in cffi_modules and not WIN:
-                system(libev_configure_command)
-                # This changed to the libev directory, and ran configure .
-                # It then copied the generated config.h back to the previous
-                # directory, which happened to be beside us. In the embedded case,
-                # we're building in a different directory, so it copied it back to build
-                # directory, but here, we're building in the embedded directory, so
-                # it gave us useless files.
-                bad_file = None
-                for bad_file in ('config.h', 'configure-output.txt'):
-                    if os.path.exists(bad_file):
-                        os.remove(bad_file)
-                del bad_file
-
+def run_setup(ext_modules):
     setup(
         name='gevent',
         version=__version__,
@@ -452,11 +417,10 @@ if os.getenv('READTHEDOCS'):
     os.environ['PATH'] = new_path
 
 try:
-    run_setup(EXT_MODULES, run_make=_BUILDING)
+    run_setup(EXT_MODULES)
 except BuildFailed:
     if ARES not in EXT_MODULES or not ARES.optional:
         raise
-    EXT_MODULES.remove(ARES)
-    run_setup(EXT_MODULES, run_make=_BUILDING)
-if ARES not in EXT_MODULES and __name__ == '__main__' and _BUILDING:
     sys.stderr.write('\nWARNING: The gevent.ares extension has been disabled.\n')
+    EXT_MODULES.remove(ARES)
+    run_setup(EXT_MODULES)

src/gevent/libev/_corecffi_build.py

@@ -15,6 +15,7 @@ from cffi import FFI
 sys.path.append(".")
 try:
     import _setuplibev
+    import _setuputils
 except ImportError:
     print("This file must be imported with setup.py in the current working dir.")
     raise
@@ -27,7 +28,7 @@ __all__ = []
 
 
 ffi = FFI()
-
+distutils_ext = _setuplibev.build_extension()
 
 def read_source(name):
     with open(os.path.join(thisdir, name), 'r') as f:
@@ -47,13 +48,18 @@ _cdef = _cdef.replace('#define GEVENT_ST_NLINK_T int',
 _cdef = _cdef.replace('GEVENT_ST_NLINK_T', 'nlink_t')
 
 if _setuplibev.LIBEV_EMBED:
+    # Arrange access to the loop internals
     _cdef += """
 struct ev_loop {
     int backend_fd;
     int activecnt;
     ...;
 };
-"""
+    """
+
+# arrange to be configured.
+_setuputils.ConfiguringBuildExt.gevent_add_pre_run_action(distutils_ext.configure)
+
 
 if sys.platform.startswith('win'):
     # We must have the vfd_open, etc, functions on
@@ -71,9 +77,6 @@ void vfd_free(int);
 # source goes to the C compiler
 _source = read_source('_corecffi_source.c')
 
-
-distutils_ext = _setuplibev.build_extension()
-
 macros = list(distutils_ext.define_macros)
 try:
     # We need the data pointer.

src/gevent/libuv/_corecffi_build.py

@@ -26,7 +26,7 @@ __all__ = []
 
 WIN = sys.platform.startswith('win32')
 LIBUV_EMBED = _setuputils.should_embed('libuv')
-print("Embedding libuv?", LIBUV_EMBED)
+
 
 ffi = FFI()