Merge pull request #1078 from gevent/issue1076

Run cythonize from setup.py

.gitignore

@@ -7,8 +7,14 @@ build/
 *.egg-info
 gevent.*.[ch]
 src/gevent/__pycache__
+src/gevent/_semaphore.c
+src/gevent/local.c
+src/gevent/libev/corecext.c
+src/gevent/libev/corecext.h
 src/gevent/libev/_corecffi.c
 src/gevent/libev/_corecffi.o
+src/gevent/ares.c
+src/gevent/ares.h
 Makefile.ext
 MANIFEST
 *_flymake.py

CHANGES.rst

@@ -61,6 +61,22 @@ Platform Support
   still be installed on them. Supporting code will be removed in the
   next version of gevent. See :issue:`1073`.
 
+Build Changes
+-------------
+
+- When building gevent from a source checkout (*not* a distributed
+  source distribution), ``make`` is no longer required and the
+  ``Makefile`` is not used. Neither is an external ``cython`` command.
+  Instead, the ``cythonize`` function is used, as recommended by
+  Cython. See :issue:`1076`.
+
+- :class:`gevent.local.local` is compiled with Cython on CPython. It
+  was already 5 to 6 times faster due to the work on :issue:`1020`,
+  and compiling it with Cython makes it another 5 to 6 times faster, for a
+  total speed up of about 35 times. It is now in the same ballpark as
+  the native :class:`threading.local` class. See :pr:`1024`.
+
+
 Other Changes
 -------------
 
@@ -120,12 +136,6 @@ Other Changes
   implementing more of the attribute protocols directly. Please open
   an issue if you have any compatibility problems. See :issue:`1020`.
 
-- :class:`gevent.local.local` is compiled with Cython on CPython. It
-  was already 5 to 6 times faster due to the work on :issue:`1020`,
-  and compiling it with Cython makes it another 5 to 6 times faster, for a
-  total speed up of about 35 times. It is now in the same ballpark as
-  the native :class:`threading.local` class. See :pr:`1024`.
-
 - More safely terminate subprocesses on Windows with
   :meth:`gevent.subprocess.Popen.terminate`. Reported in :issue:`1023`
   by Giacomo Debidda.

Makefile

@@ -12,38 +12,11 @@ export PATH:=$(BUILD_RUNTIMES)/snakepit:$(TOOLS):$(PATH)
 export LC_ALL=C.UTF-8
 
 
-all: src/gevent/libev/gevent.corecext.c src/gevent/gevent.ares.c src/gevent/gevent._semaphore.c src/gevent/gevent._local.c
-
-src/gevent/libev/gevent.corecext.c: src/gevent/libev/corecext.pyx src/gevent/libev/libev.pxd src/gevent/libev/libev.h
-	$(CYTHON) -o gevent.corecext.c src/gevent/libev/corecext.pyx
-	echo '#include "callbacks.c"' >> gevent.corecext.c
-	mv gevent.corecext.* src/gevent/libev/
-
-src/gevent/gevent.ares.c: src/gevent/ares.pyx src/gevent/*.pxd
-	$(CYTHON) -o gevent.ares.c src/gevent/ares.pyx
-	mv gevent.ares.* src/gevent/
-
-src/gevent/gevent._semaphore.c: src/gevent/_semaphore.py src/gevent/_semaphore.pxd
-# On PyPy, if we wanted to use Cython to compile _semaphore.py, we'd
-# need to have _semaphore named as a .pyx file so it doesn't get
-# loaded in preference to the .so. (We want to keep the definitions
-# separate in a .pxd file for ease of reading, and that only works
-# with .py files, so we'd have to copy them back and forth.)
-#	cp src/gevent/_semaphore.pyx src/gevent/_semaphore.py
-	$(CYTHON) -o gevent._semaphore.c src/gevent/_semaphore.py
-	mv gevent._semaphore.* src/gevent/
-#	rm src/gevent/_semaphore.py
-
-src/gevent/gevent._local.c: src/gevent/local.py
-	$(CYTHON) -o gevent._local.c src/gevent/local.py
-	mv gevent._local.* src/gevent/
-
-
 clean:
-	rm -f gevent.corecext.c gevent.corecext.h src/gevent/libev/gevent.corecext.c src/gevent/libev/gevent.corecext.h
-	rm -f gevent.ares.c gevent.ares.h src/gevent/gevent.ares.c src/gevent/gevent.ares.h
-	rm -f gevent._semaphore.c gevent._semaphore.h src/gevent/gevent._semaphore.c src/gevent/gevent._semaphore.h
-	rm -f gevent._local.c gevent._local.h src/gevent/gevent._local.c src/gevent/gevent._local.h
+	rm -f src/gevent/libev/corecext.c src/gevent/libev/corecext.h
+	rm -f src/gevent/ares.c src/gevent/ares.h
+	rm -f src/gevent/_semaphore.c src/gevent/_semaphore.h
+	rm -f src/gevent/local.c src/gevent/local.h
 	rm -f src/gevent/*.so src/gevent/libev/*.so src/gevent/libuv/*.so
 	rm -rf src/gevent/libev/*.o src/gevent/libuv/*.o src/gevent/*.o
 	rm -rf src/gevent/__pycache__ src/greentest/__pycache__ src/gevent/libev/__pycache__
@@ -60,7 +33,7 @@ doc:
 	cd doc && PYTHONPATH=.. make html
 
 whitespace:
-	! find . -not -path "*.pem" -not -path "./.eggs/*" -not -path "./src/greentest/htmlcov/*" -not -path "./src/greentest/.coverage.*" -not -path "./.tox/*" -not -path "*/__pycache__/*" -not -path "*.so" -not -path "*.pyc" -not -path "./.git/*" -not -path "./build/*"  -not -path "./src/gevent/libev/*" -not -path "./src/gevent.egg-info/*" -not -path "./dist/*" -not -path "./.DS_Store" -not -path "./deps/*" -not -path "./src/gevent/gevent.*.[ch]" -not -path "./src/gevent/corecext.pyx" -not -path "./doc/_build/*" -not -path "./doc/mytheme/static/*" -type f | xargs egrep -l " $$"
+	! find . -not -path "*.pem" -not -path "./.eggs/*" -not -path "./src/greentest/htmlcov/*" -not -path "./src/greentest/.coverage.*" -not -path "./.tox/*" -not -path "*/__pycache__/*" -not -path "*.so" -not -path "*.pyc" -not -path "./.git/*" -not -path "./build/*"  -not -path "./src/gevent/libev/*" -not -path "./src/gevent.egg-info/*" -not -path "./dist/*" -not -path "./.DS_Store" -not -path "./deps/*" -not -path "./src/gevent/libev/corecext.*.[ch]" -not -path "./src/gevent/ares.*" -not -path "./doc/_build/*" -not -path "./doc/mytheme/static/*" -type f | xargs egrep -l " $$"
 
 prospector:
 	which prospector
@@ -69,7 +42,7 @@ prospector:
 #	pylint --rcfile=.pylintrc --init-hook="import sys, code; sys.excepthook = lambda exc, exc_type, tb: print(tb.tb_next.tb_next.tb_next.tb_next.tb_next.tb_next.tb_next.tb_next.tb_next.tb_next.tb_frame.f_locals['self'])" gevent src/greentest/* || true
 	${PYTHON} scripts/gprospector.py -X
 
-lint: whitespace prospector
+lint: prospector
 
 test_prelim:
 	which ${PYTHON}
@@ -139,7 +112,7 @@ travis_test_linters:
 	coveralls --rcfile=src/greentest/.coveragerc
 
 
-.PHONY: clean all doc prospector whitespace lint travistest travis
+.PHONY: clean doc prospector lint travistest travis
 
 # Managing runtimes
 

_setupares.py

@@ -24,6 +24,7 @@ from _setuputils import DEFINE_MACROS
 from _setuputils import glob_many
 from _setuputils import dep_abspath
 from _setuputils import RUNNING_ON_CI
+from _setuputils import cythonize1
 
 
 CARES_EMBED = should_embed('c-ares')
@@ -79,8 +80,8 @@ def configure_ares(bext, ext):
 
 
 ARES = Extension(name='gevent.ares',
-                 sources=['src/gevent/gevent.ares.c'],
-                 include_dirs=[dep_abspath('c-ares')] if CARES_EMBED else [],
+                 sources=['src/gevent/ares.pyx'],
+                 include_dirs=['src/gevent'] + [dep_abspath('c-ares')] if CARES_EMBED else [],
                  libraries=list(LIBRARIES),
                  define_macros=list(DEFINE_MACROS),
                  depends=glob_many('src/gevent/dnshelper.c',
@@ -107,3 +108,5 @@ if CARES_EMBED:
 else:
     ARES.libraries.append('cares')
     ARES.define_macros += [('HAVE_NETDB_H', '')]
+
+ARES = cythonize1(ARES)

_setuplibev.py

@@ -19,6 +19,7 @@ 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,8 +61,8 @@ def configure_libev(bext, ext):
         os.chdir(cwd)
 
 CORE = Extension(name='gevent.libev.corecext',
-                 sources=['src/gevent/libev/gevent.corecext.c'],
-                 include_dirs=[dep_abspath('libev')] if LIBEV_EMBED else [],
+                 sources=['src/gevent/libev/corecext.pyx'],
+                 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.*',
@@ -86,3 +87,15 @@ if LIBEV_EMBED:
         CORE.define_macros.append(('EV_VERIFY', os.environ['GEVENTSETUP_EV_VERIFY']))
 else:
     CORE.libraries.append('ev')
+
+CORE = cythonize1(CORE)
+
+# XXX The include of callbacks.c must go at the end of the
+# file because it references things cython generates.
+# How can we do this automatically, or relax that restriction?
+with open(CORE.sources[0]) as f:
+    core_data = f.read()
+
+if '#include "callbacks.c"' not in core_data:
+    with open(CORE.sources[0], 'a') as f:
+        f.write('\n#include "callbacks.c"\n')

_setuputils.py

@@ -12,8 +12,9 @@ import sys
 from subprocess import check_call
 from glob import glob
 
+from setuptools import Extension as _Extension
 from setuptools.command.build_ext import build_ext
-from setuptools.command.sdist import sdist
+
 
 ## Exported configurations
 
@@ -143,6 +144,35 @@ def system(cmd, cwd=None, env=None, **kwargs):
         sys.exit(1)
 
 
+# Cython
+
+
+try:
+    from Cython.Build import cythonize
+except ImportError:
+    # The .c files had better already exist. Based on code from
+    # http://cython.readthedocs.io/en/latest/src/reference/compilation.html#distributing-cython-modules
+    def cythonize(extensions):
+        for extension in extensions:
+            sources = []
+            for sfile in extension.sources:
+                path, ext = os.path.splitext(sfile)
+                if ext in ('.pyx', '.py'):
+                    ext = '.c'
+                    sfile = path + ext
+                sources.append(sfile)
+            extension.sources[:] = sources
+        return extensions
+
+def cythonize1(ext):
+    new_ext = cythonize([ext], include_path=['src/gevent', 'src/gevent/libev'])[0]
+    for optional_attr in ('configure', 'optional'):
+        if hasattr(ext, optional_attr):
+            setattr(new_ext, optional_attr,
+                    getattr(ext, optional_attr))
+    return new_ext
+
+
 ## Distutils extensions
 class BuildFailed(Exception):
     pass
@@ -170,55 +200,7 @@ class ConfiguringBuildExt(build_ext):
         return result
 
 
-class MakeSdist(sdist):
-    """
-    An sdist that runs make if needed, and makes sure
-    that the Makefile doesn't make it into the dist
-    archive.
-    """
 
-    _ran_make = False
-
-    @classmethod
-    def make(cls, targets=''):
-        # NOTE: We have two copies of the makefile, one
-        # for posix, one for windows. Our sdist command takes
-        # care of renaming the posix one so it doesn't get into
-        # the .tar.gz file (we don't want to re-run make in a released
-        # file). We trigger off the presence/absence of that file altogether
-        # to skip both posix and unix branches.
-        # See https://github.com/gevent/gevent/issues/757
-        if cls._ran_make:
-            return
-
-        if os.path.exists('Makefile'):
-            if WIN:
-                # make.cmd handles checking for PyPy and only making the
-                # right things, so we can ignore the targets
-                system("appveyor\\make.cmd")
-            else:
-                if "PYTHON" not in os.environ:
-                    os.environ["PYTHON"] = sys.executable
-                # Let the user specify the make program, helpful for BSD
-                # where GNU make might be called gmake
-                make_program = os.environ.get('MAKE', 'make')
-                system(make_program + ' ' + targets)
-        cls._ran_make = True
-
-    def run(self):
-        renamed = False
-        if os.path.exists('Makefile'):
-            self.make()
-            os.rename('Makefile', 'Makefile.ext')
-            renamed = True
-        try:
-            return sdist.run(self)
-        finally:
-            if renamed:
-                os.rename('Makefile.ext', 'Makefile')
-
-
-from setuptools import Extension as _Extension
 
 class Extension(_Extension):
     # This class exists currently mostly to make pylint

appveyor.yml

@@ -165,8 +165,7 @@ test_script:
 after_test:
   # We already built the wheel during build_script, because it's
   # much faster to do that and install from the wheel than to
-  # rebuild it here (because we wind up re-building all the cython
-  # code, even though it's already built on disk; our make.cmd is not smart)
+  # rebuild it here
   #- "%CMD_IN_ENV% %PYEXE% setup.py bdist_wheel bdist_wininst"
   - ps: "ls dist"
 

appveyor/make.cmd

-IF "%PYTHON_EXE%" == "python" (
-    cython -o gevent.corecext.c src\gevent\libev\corecext.pyx
-	type src\gevent\libev\callbacks.c >> gevent.corecext.c
-    move gevent.corecext.* src\gevent\libev
-)
-cython -o gevent.ares.c src\gevent\ares.pyx
-move gevent.ares.* src\gevent
-cython -o gevent._semaphore.c src\gevent\_semaphore.py
-move gevent._semaphore.* src\gevent
-cython -o gevent._local.c src\gevent\local.py
-move gevent._local.c src\gevent

deps/README.rst

+When updating c-ares, remember to copy ares.h to cares.h.
+
+The original ares.h conflicts with the ares.h generated automatically
+by cython for src/gevent/ares.pyx.

setup.py

@@ -3,6 +3,14 @@
 from __future__ import print_function
 import sys
 import os
+import os.path
+
+# setuptools is *required* on Windows
+# (https://bugs.python.org/issue23246) and for PyPy. No reason not to
+# use it everywhere. v24.2.0 is needed for python_requires
+from setuptools import Extension, setup
+from setuptools import find_packages
+
 
 from _setuputils import read
 from _setuputils import read_version
@@ -10,14 +18,10 @@ from _setuputils import system
 from _setuputils import PYPY, WIN
 from _setuputils import IGNORE_CFFI
 from _setuputils import ConfiguringBuildExt
-from _setuputils import MakeSdist
 from _setuputils import BuildFailed
+from _setuputils import cythonize1
+
 
-# setuptools is *required* on Windows
-# (https://bugs.python.org/issue23246) and for PyPy. No reason not to
-# use it everywhere. v24.2.0 is needed for python_requires
-from setuptools import Extension, setup
-from setuptools import find_packages
 
 if WIN:
     # Make sure the env vars that make.cmd needs are set
@@ -45,11 +49,14 @@ from _setupares import ARES
 
 
 SEMAPHORE = Extension(name="gevent._semaphore",
-                      sources=["src/gevent/gevent._semaphore.c"])
+                      sources=["src/gevent/_semaphore.py"],
+                      depends=['src/gevent/_semaphore.pxd'])
+SEMAPHORE = cythonize1(SEMAPHORE)
 
 LOCAL = Extension(name="gevent.local",
-                  sources=["src/gevent/gevent._local.c"])
-
+                  sources=["src/gevent/local.py"],
+                  depends=['src/gevent/local.pxd'])
+LOCAL = cythonize1(LOCAL)
 
 EXT_MODULES = [
     CORE,
@@ -140,8 +147,6 @@ def run_setup(ext_modules, run_make):
             if LIBEV_CFFI_MODULE in cffi_modules and not WIN:
                 system(libev_configure_command)
 
-        MakeSdist.make()
-
     setup(
         name='gevent',
         version=__version__,
@@ -158,7 +163,7 @@ def run_setup(ext_modules, run_make):
         packages=find_packages('src'),
         include_package_data=True,
         ext_modules=ext_modules,
-        cmdclass=dict(build_ext=ConfiguringBuildExt, sdist=MakeSdist),
+        cmdclass=dict(build_ext=ConfiguringBuildExt),
         install_requires=install_requires,
         setup_requires=setup_requires,
         # It's always safe to pass the CFFI keyword, even if

src/gevent/cares.pxd

-cdef extern from "ares.h":
+cdef extern from "cares.h":
     struct ares_options:
         int flags
         void* sock_state_cb

src/gevent/dnshelper.c

@@ -8,7 +8,7 @@
 #include <netdb.h>
 #endif
 
-#include "ares.h"
+#include "cares.h"
 
 #include "cares_ntop.h"
 #include "cares_pton.h"

src/gevent/local.pxd

 # cython: auto_pickle=False
+
+cimport cython
+
+@cython.final
+@cython.internal
 cdef class _wrefdict(dict):
    cdef object __weakref__
 
+@cython.final
+@cython.internal
 cdef class _localimpl:
     cdef str key
     cdef dict dicts