Merge pull request #1937 from JonasT/fix-cython-calls

Call Cython via `python -m Cython` rather than system-wide binary

Author: JonasT

Dockerfile.py2

@@ -122,8 +122,6 @@ RUN usermod -append --groups sudo ${USER}
 RUN echo "%sudo ALL=(ALL) NOPASSWD: ALL" >> /etc/sudoers
 
 
-RUN pip install --upgrade cython==0.28.6
-
 WORKDIR ${WORK_DIR}
 COPY --chown=user:user . ${WORK_DIR}
 RUN chown --recursive ${USER} ${ANDROID_SDK_HOME}
@@ -132,4 +130,5 @@ USER ${USER}
 # install python-for-android from current branch
 RUN virtualenv --python=python venv \
     && . venv/bin/activate \
+    && pip install --upgrade cython==0.28.6 \
     && pip install -e .

Dockerfile.py3

@@ -94,7 +94,7 @@ ENV WORK_DIR="${HOME_DIR}" \
 # install system dependencies
 RUN ${RETRY} apt -y install -qq --no-install-recommends \
         python3 virtualenv python3-pip python3-venv \
-        wget lbzip2 patch sudo \
+        wget lbzip2 patch sudo python python-pip \
     && apt -y autoremove
 
 # build dependencies
@@ -122,8 +122,8 @@ RUN useradd --create-home --shell /bin/bash ${USER}
 RUN usermod -append --groups sudo ${USER}
 RUN echo "%sudo ALL=(ALL) NOPASSWD: ALL" >> /etc/sudoers
 
-
-RUN pip3 install --upgrade cython==0.28.6
+# install cython for python 2 (for python 3 it's inside the venv)
+RUN pip2 install --upgrade Cython==0.28.6
 
 WORKDIR ${WORK_DIR}
 COPY --chown=user:user . ${WORK_DIR}
@@ -133,4 +133,5 @@ USER ${USER}
 # install python-for-android from current branch
 RUN virtualenv --python=python3 venv \
     && . venv/bin/activate \
+    && pip3 install --upgrade Cython==0.28.6 \
     && pip3 install -e .

pythonforandroid/build.py

@@ -27,14 +27,6 @@
     RECOMMENDED_NDK_API, RECOMMENDED_TARGET_API)
 
 
-def get_cython_path():
-    for cython_fn in ("cython", "cython3", "cython2", "cython-2.7"):
-        cython = sh.which(cython_fn)
-        if cython:
-            return cython
-    raise BuildInterruptingException('No cython binary found.')
-
-
 def get_ndk_platform_dir(ndk_dir, ndk_api, arch):
     ndk_platform_dir_exists = True
     platform_dir = arch.platform_dir
@@ -111,7 +103,6 @@ class Context(object):
     use_setup_py = False
 
     ccache = None  # whether to use ccache
-    cython = None  # the cython interpreter name
 
     ndk_platform = None  # the ndk platform directory
 
@@ -374,7 +365,14 @@ def prepare_build_environment(self,
         if not self.ccache:
             info('ccache is missing, the build will not be optimized in the '
                  'future.')
-        self.cython = get_cython_path()
+        try:
+            subprocess.check_output([
+                "python3", "-m", "cython", "--help",
+            ])
+        except subprocess.CalledProcessError:
+            warning('Cython for python3 missing. If you are building for '
+                    ' a python 3 target (which is the default)'
+                    ' then THINGS WILL BREAK.')
 
         # This would need to be changed if supporting multiarch APKs
         arch = self.archs[0]

pythonforandroid/recipe.py

@@ -1024,8 +1024,11 @@ def cythonize_file(self, env, build_dir, filename):
             del cyenv['PYTHONPATH']
         if 'PYTHONNOUSERSITE' in cyenv:
             cyenv.pop('PYTHONNOUSERSITE')
-        cython_command = sh.Command(self.ctx.cython)
-        shprint(cython_command, filename, *self.cython_args, _env=cyenv)
+        python_command = sh.Command("python{}".format(
+            self.ctx.python_recipe.major_minor_version_string.split(".")[0]
+        ))
+        shprint(python_command, "-m", "Cython.Build.Cythonize",
+                filename, *self.cython_args, _env=cyenv)
 
     def cythonize_build(self, env, build_dir="."):
         if not self.cythonize:

tests/test_toolchain.py

@@ -68,8 +68,6 @@ def test_create(self):
         ) as m_get_toolchain_versions, mock.patch(
             'pythonforandroid.build.get_ndk_platform_dir'
         ) as m_get_ndk_platform_dir, mock.patch(
-            'pythonforandroid.build.get_cython_path'
-        ) as m_get_cython_path, mock.patch(
             'pythonforandroid.toolchain.build_recipes'
         ) as m_build_recipes, mock.patch(
             'pythonforandroid.bootstraps.service_only.'
@@ -84,7 +82,6 @@ def test_create(self):
             mock.call('/tmp/android-sdk')]
         assert m_get_toolchain_versions.call_args_list == [
             mock.call('/tmp/android-ndk', mock.ANY)]
-        assert m_get_cython_path.call_args_list == [mock.call()]
         build_order = [
             'hostpython3', 'libffi', 'openssl', 'sqlite3', 'python3',
             'genericndkbuild', 'setuptools', 'six', 'pyjnius', 'android',