Merge pull request #7497 from soltanmm/be-nicer

Be more intelligent about Cython in grpcio

3 files changed, 91 insertions, 42 deletions

diff --git a/setup.py b/setup.py
index 2e342a96d3..cad300c144 100644
--- a/setup.py
+++ b/setup.py
@@ -168,55 +168,27 @@ if 'darwin' in sys.platform and PY3:
         r'macosx-10.7-\1',
         util.get_platform())
 
-
-def cython_extensions():
-  module_names = list(CYTHON_EXTENSION_MODULE_NAMES)
-  extra_sources = list(CYTHON_HELPER_C_FILES) + list(CORE_C_FILES)
-  include_dirs = list(EXTENSION_INCLUDE_DIRECTORIES)
-  libraries = list(EXTENSION_LIBRARIES)
-  define_macros = list(DEFINE_MACROS)
-  build_with_cython = bool(BUILD_WITH_CYTHON)
-  # Set compiler directives linetrace argument only if we care about tracing;
-  # this is due to Cython having different behavior between linetrace being
-  # False and linetrace being unset. See issue #5689.
-  cython_compiler_directives = {}
-  if ENABLE_CYTHON_TRACING:
-    define_macros = define_macros + [('CYTHON_TRACE_NOGIL', 1)]
-    cython_compiler_directives['linetrace'] = True
-  pyx_module_files = [os.path.join(PYTHON_STEM,
-                                   name.replace('.', '/') + '.pyx')
-                      for name in module_names]
-  c_module_files = [os.path.join(PYTHON_STEM,
-                                 name.replace('.', '/') + '.c')
-                    for name in module_names]
-  if not build_with_cython:
-    for module_file in c_module_files:
-      if not os.path.isfile(module_file):
-        sys.stderr.write('Cython-generated files are missing; '
-                         'forcing Cython build...\n')
-        build_with_cython = True
-        break
-  module_files = pyx_module_files if build_with_cython else c_module_files
+def cython_extensions_and_necessity():
+  cython_module_files = [os.path.join(PYTHON_STEM,
+                               name.replace('.', '/') + '.pyx')
+                  for name in CYTHON_EXTENSION_MODULE_NAMES]
   extensions = [
       _extension.Extension(
           name=module_name,
-          sources=[module_file] + extra_sources,
-          include_dirs=include_dirs, libraries=libraries,
-          define_macros=define_macros,
+          sources=[module_file] + list(CYTHON_HELPER_C_FILES) + list(CORE_C_FILES),
+          include_dirs=list(EXTENSION_INCLUDE_DIRECTORIES),
+          libraries=list(EXTENSION_LIBRARIES),
+          define_macros=list(DEFINE_MACROS),
           extra_compile_args=list(CFLAGS),
           extra_link_args=list(LDFLAGS),
-      ) for (module_name, module_file) in zip(module_names, module_files)
+      ) for (module_name, module_file) in zip(list(CYTHON_EXTENSION_MODULE_NAMES), cython_module_files)
   ]
-  if build_with_cython:
-    import Cython.Build
-    return Cython.Build.cythonize(
-        extensions,
-        include_path=include_dirs,
-        compiler_directives=cython_compiler_directives)
-  else:
-    return extensions
+  need_cython = BUILD_WITH_CYTHON
+  if not BUILD_WITH_CYTHON:
+    need_cython = need_cython or not commands.check_and_update_cythonization(extensions)
+  return commands.try_cythonize(extensions, linetracing=ENABLE_CYTHON_TRACING, mandatory=BUILD_WITH_CYTHON), need_cython
 
-CYTHON_EXTENSION_MODULES = cython_extensions()
+CYTHON_EXTENSION_MODULES, need_cython = cython_extensions_and_necessity()
 
 PACKAGE_DIRECTORIES = {
     '': PYTHON_STEM,
@@ -236,6 +208,15 @@ SETUP_REQUIRES = INSTALL_REQUIRES + (
     'sphinx_rtd_theme>=0.1.8',
     'six>=1.10',
 )
+if BUILD_WITH_CYTHON:
+  sys.stderr.write(
+    "You requested a Cython build via GRPC_PYTHON_BUILD_WITH_CYTHON, "
+    "but do not have Cython installed. We won't stop you from using "
+    "other commands, but the extension files will fail to build.\n")
+elif need_cython:
+  sys.stderr.write(
+      'We could not find Cython. Setup may take 10-20 minutes.\n')
+  SETUP_REQUIRES += ('cython>=0.23',)
 
 COMMAND_CLASS = {
     'doc': commands.SphinxDocumentation,
diff --git a/src/python/grpcio/.gitignore b/src/python/grpcio/.gitignore
index 7cd8fab273..3309795948 100644
--- a/src/python/grpcio/.gitignore
+++ b/src/python/grpcio/.gitignore
@@ -14,3 +14,4 @@ doc/
 _grpcio_metadata.py
 htmlcov/
 grpc/_cython/_credentials
+poison.c
diff --git a/src/python/grpcio/commands.py b/src/python/grpcio/commands.py
index 86a73fa836..d36ac23305 100644
--- a/src/python/grpcio/commands.py
+++ b/src/python/grpcio/commands.py
@@ -184,6 +184,71 @@ class BuildPy(build_py.build_py):
     build_py.build_py.run(self)
 
 
+def _poison_extensions(extensions, message):
+  """Includes a file that will always fail to compile in all extensions."""
+  poison_filename = os.path.join(PYTHON_STEM, 'poison.c')
+  with open(poison_filename, 'w') as poison:
+    poison.write('#error {}'.format(message))
+  for extension in extensions:
+    extension.sources = [poison_filename]
+
+def check_and_update_cythonization(extensions):
+  """Replace .pyx files with their generated counterparts and return whether or
+     not cythonization still needs to occur."""
+  for extension in extensions:
+    generated_pyx_sources = []
+    other_sources = []
+    for source in extension.sources:
+      base, file_ext = os.path.splitext(source)
+      if file_ext == '.pyx':
+        generated_pyx_source = next(
+            (base + gen_ext for gen_ext in ('.c', '.cpp',)
+             if os.path.isfile(base + gen_ext)), None)
+        if generated_pyx_source:
+          generated_pyx_sources.append(generated_pyx_source)
+        else:
+          sys.stderr.write('Cython-generated files are missing...\n')
+          return False
+      else:
+        other_sources.append(source)
+    extension.sources = generated_pyx_sources + other_sources
+  sys.stderr.write('Found cython-generated files...\n')
+  return True
+
+def try_cythonize(extensions, linetracing=False, mandatory=True):
+  """Attempt to cythonize the extensions.
+
+  Args:
+    extensions: A list of `distutils.extension.Extension`.
+    linetracing: A bool indicating whether or not to enable linetracing.
+    mandatory: Whether or not having Cython-generated files is mandatory. If it
+      is, extensions will be poisoned when they can't be fully generated.
+  """
+  try:
+    # Break import style to ensure we have access to Cython post-setup_requires
+    import Cython.Build
+  except ImportError:
+    if mandatory:
+      sys.stderr.write(
+          "This package needs to generate C files with Cython but it cannot. "
+          "Poisoning extension sources to disallow extension commands...")
+      _poison_extensions(
+          extensions,
+          "Extensions have been poisoned due to missing Cython-generated code.")
+    return extensions
+  cython_compiler_directives = {}
+  if linetracing:
+    additional_define_macros = [('CYTHON_TRACE_NOGIL', '1')]
+    cython_compiler_directives['linetrace'] = True
+  return Cython.Build.cythonize(
+    extensions,
+    include_path=[
+      include_dir for extension in extensions for include_dir in extension.include_dirs
+    ],
+    compiler_directives=cython_compiler_directives
+  )
+
+
 class BuildExt(build_ext.build_ext):
   """Custom build_ext command to enable compiler-specific flags."""
 
@@ -201,6 +266,8 @@ class BuildExt(build_ext.build_ext):
     if compiler in BuildExt.LINK_OPTIONS:
       for extension in self.extensions:
         extension.extra_link_args += list(BuildExt.LINK_OPTIONS[compiler])
+    if not check_and_update_cythonization(self.extensions):
+      self.extensions = try_cythonize(self.extensions)
     try:
       build_ext.build_ext.build_extensions(self)
     except Exception as error: