Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to Ignore Cython, even if found #1229

Closed
ax3l opened this issue Dec 6, 2017 · 11 comments
Closed

How to Ignore Cython, even if found #1229

ax3l opened this issue Dec 6, 2017 · 11 comments

Comments

@ax3l
Copy link

@ax3l ax3l commented Dec 6, 2017

Currently, importing setuptools.command.build_ext pulls in a Cython dependency if it finds any Cython on a system.

That is not in all cases ideal, e.g. in a package manager (such as spack) or a virtual python environment that does not have a cython installed, while at the same time a system-wide Cython exists. In such a setup, nearly always binary incompatibilities with the system-wide python install will occur.

Is there any way to express "try not to import cython (even when found)" to setuptools?

@jaraco
Copy link
Member

@jaraco jaraco commented Dec 6, 2017

Can you expand a little on

nearly always binary incompatibilities with the system-wide python install will occur.

I'd assumed that the Cython build_ext class only extended the distutils build_ext class and would fall back to the default behavior if .pyx files weren't involved.

How is it that builds are affected by the presence of Cython?

@ax3l
Copy link
Author

@ax3l ax3l commented Dec 6, 2017

The problem I am facing is that

try:
    # Attempt to use Cython for building extensions, if available
    from Cython.Distutils.build_ext import build_ext as _build_ext

succeeds but a later call to build_ext, e.g. in the form of

from setuptools.command.build_ext import build_ext as BuildExtCommand

class BuildExtraLibraries(BuildExtCommand):
    def run(self):
        return BuildExtCommand.run(self)

in a setup.py (in this case of mpl) will raise a distutils.errors.DistutilsPlatformError with missing symbols. The cython symbols are then taken from the first found cython of a system, e.g.

failed to import Cython: /home/axel/.local/lib/python2.7/site-packages/Cython/Compiler/Scanning.so: undefined symbol: PyUnicodeUCS4_DecodeUTF8
Unexpected error: <class 'distutils.errors.DistutilsPlatformError'>

instead of something from the python I am currently working with (which has no cython package installed).

I am trying to build a little, not too complex reproducer but it's a bit complex in this situation (need to install two incompatible python versions, etc.).

@tacaswell
Copy link

@tacaswell tacaswell commented Dec 6, 2017

If you do not have Cython installed why is the import succeeding? Do you have extra things in sys.path that should not be there?

@ax3l
Copy link
Author

@ax3l ax3l commented Dec 6, 2017

All right, I have a demonstrating Dockerfile now ready.
It prepares the following, common setting for a user that might use conda or virtual env:

System-Wide Software

A default CPython 2.7 build with some version of gcc (e.g. 5) and building some version of cython.

Virtual / Per-User Software

A conda provided CPython 2.7 build with some other version of gcc (4.8) and without cython.

The Issue

The problem now is, that setuptools will pick on build_ext the system-wide install of cython. This breaks e.g. the build of matplotlib although matplotlib does not even need Cython (but setuptools does suddenly depend on it).

This effect is commonly known as side-effect during install of e.g. C/C++ libraries. Instead of a "pick any" of a dependency an option to disable/pick-a-specific cython would be needed for better control. (e.g. modern CMake scripts usually expose an "AUTO/ON/OFF" option to users [example]).

Long story short: as soon as you do a

try:
    import Cython

inside setuptools, Cython needs to be considered a public, optional dependency of setuptools and build systems / package managers / non-isolated virtual environments need the control to explicitly set the path to it or to disable it fully even if an install exists (in e.g. /usr/ or .local/).

Demonstrator

bash

docker build -t setuptools .

docker

# updated: forgot to copy-paste the base image I used :)
FROM       ubuntu:16.04

# general environment for docker
ENV        DEBIAN_FRONTEND=noninteractive \
           FORCE_UNSAFE_CONFIGURE=1

RUN        apt-get update \
           && apt-get install -y --no-install-recommends \
              bzip2 \
              build-essential \
              pkg-config \
              libc6-dev \
              python-minimal \
              python-dev \
              python-pip \
              ca-certificates \
              poppler-utils \
              coreutils \
              curl \
              git \
              lsb-release \
              wget \
           && rm -rf /var/lib/apt/lists/*

# this is the "SYSTEM-WIDE" install of python and cython
RUN        pip install setuptools --user --no-binary :all:
RUN        pip install cython --user --no-binary :all:
RUN        python --version && which python
RUN        /root/.local/bin/cython --version

# here starts the "USER-SCOPE": either in $HOME, conda, a virtual env, or e.g. spack
RUN        wget http://repo.continuum.io/miniconda/Miniconda-latest-Linux-x86_64.sh -O miniconda.sh && \
           chmod +x miniconda.sh && \
           ./miniconda.sh -b && \
           ls

RUN        git clone --depth 10 https://github.com/matplotlib/matplotlib.git

RUN        /root/miniconda2/bin/conda create -y -p /tmp/py27 python=2.7 && \
           /bin/bash -l -c "source /root/miniconda2/bin/activate /tmp/py27 && \
           /root/miniconda2/bin/conda install -y gcc libgcc=4.8.5 numpy pkgconfig libpng freetype && \
           cd matplotlib && \
           which python && \
           python setup.py build && \
           python setup.py install"

error

UPDATING build/lib.linux-x86_64-2.7/matplotlib/_version.py
set build/lib.linux-x86_64-2.7/matplotlib/_version.py to '0+untagged.49.g9a935e7'
running build_ext
failed to import Cython: /root/.local/lib/python2.7/site-packages/Cython/Compiler/Code.so: undefined symbol: PyFPE_jbuf
error: Cython does not appear to be installed
INFO[0098] The command [/bin/sh -c /root/miniconda2/bin/conda create -y -p /tmp/py27 python=2.7 &&            /bin/bash -l -c "source /root/miniconda2/bin/activate /tmp/py27 &&            /root/miniconda2/bin/conda install -y gcc libgcc=4.8.5 numpy pkgconfig libpng freetype &&            cd matplotlib &&            which python &&            python setup.py build &&            python setup.py install"] returned a non-zero code: 1
@ax3l
Copy link
Author

@ax3l ax3l commented Dec 6, 2017

If you do not have Cython installed why is the import succeeding? Do you have extra things in sys.path that should not be there?

No, I do have a cython installed - system wide. And it should not be picked inside a virtual env or conda env or spack env that is binary incompatible. The problem is, that all three are not isolating the user, so a "try to pick any cython" strategy in setuptools has this very confusing side-effect for users inside a virtual env/conda/spack.

@ax3l
Copy link
Author

@ax3l ax3l commented Dec 7, 2017

As a quick work-around one could do e.g. the following: inside the try: block importing Cython, also test the import and catch the distutils.errors.DistutilsPlatformError if thrown.

from distutils.command.build_ext import build_ext as _du_build_ext

try:
    # Attempt to use Cython for building extensions, if available
    from Cython.Distutils.build_ext import build_ext as _build_ext
    # Test if _build_ext is usable/compatible
    class TestBuildExt(_build_ext):
        def run(self):
            return _build_ext.run(self)
    TestBuildExt().run() # not sure if some cleanup will be needed now
except ImportError, distutils.errors.DistutilsPlatformError:
    _build_ext = _du_build_ext

This will still not expose (imho needed) control over the use of Cython's build_ext impl. to the user, but at least will not load an unusable Cython :)

@jaraco
Copy link
Member

@jaraco jaraco commented Dec 7, 2017

Thanks @ax3l for the reproducer. I've uploaded it as this gist. I then made a couple of revisions. The first was to specify a base image, which I suspect was necessary because I'm testing on macOS. At that point, I was able to run the build and get the error you described.

But I noticed one thing - that you're not in fact installing Cython system-wide; you're installing it in the user site (pip install --user). And the way user-local works is it's shared across different Python installs... which clarifies the answer to the question, how is import cython succeeding.

In the next revision, I installed Cython system-wide, and the issue goes away. Now that Cython is installed only to the system Python 2.7, it no longer appears in the miniconda install of Python 2.7, and all is well.

In rev 4, I took another tack and disabled that user-local environment before invoking the build. This also bypasses the issue. One could alternatively have set PYTHONNOUSERSITE=1.

I think this demonstrates that the issue originates from an incorrect usage of user-local environments and presents a couple of plausible workarounds.

I do sympathize with the issue and thinking about the presented workaround in Setuptools, I'm reluctant to go with that approach because of the complexity of behavior it adds on import. If on the other hand cython has or could add a call to check that it's viable in this environment, like cython.assert_viable(), that is something I would be willing to add, something like:

try:
    # Attempt to use Cython for building extensions, if available
    from Cython.Distutils.build_ext import build_ext as _build_ext
    __import__('cython').assert_viable()
except (ImportError, AssertionError):
    _build_ext = _du_build_ext

In other words, if Cython is willing to support this use-case, then setuptools will support it as well.

@ax3l
Copy link
Author

@ax3l ax3l commented Dec 7, 2017

Thank you for your detailed answer!

The first was to specify a base image

Ups, sorry - copy & paste omitted on my side. Updated the original Dockerfile with the missing line (just for reference: ubuntu:16.04).

not in fact installing Cython system-wide; you're installing it in the user site [...]
which clarifies the answer to the question, how is import cython succeeding

yes, absolutely. My wording was sloppy, I just meant "it's somewhere outside (of virtual env/conda/...) and can be found".

In the next revision, I installed Cython system-wide, and the issue goes away.

Same for me, it then can not be found anymore as expected.

PYTHONNOUSERSITE

interesting, didn't know that option exists. looks like a very good candidate for a package manager like spack! I wonder why conda is not using this by default as well (on activate).

incorrect usage of user-local environments

I agree that this is probably not intended but very, very common. Users start with some environment they have when creating a new virtual environment or installing conda. I've not yet seen a manual recommending "delete your ~/.local/ before creating a virtual env or installing conda" - so "incorrect" is probably just "common" :-)

cython.assert_viable()

Absolutely, this would also be a great-solution if Cython wants to support this.
Or do we come up with an other solution to probe the import is valid?

Orthogonal: Maybe also enable users to even skip the try import itself on a setup-tools specific environment var / config / parameter / ...?

@ax3l
Copy link
Author

@ax3l ax3l commented Jan 8, 2018

@jaraco @tacaswell has anyone of you connections to the Cython folks so we can propose the post-import test upstream?

@jaraco
Copy link
Member

@jaraco jaraco commented Jan 8, 2018

@ax3l: I don't. I'd suggest starting with the mailing list or maybe file an issue with their tracker to explain the issue and get advice on how to proceed.

@ax3l
Copy link
Author

@ax3l ax3l commented Jan 25, 2018

Upstream we got the recommendation that we should just include Cython.Compiler.Main or from Cython.Build import cythonize for a proper check of a compatible/working Cython in our try block.

Also, build_ext seems to be outdated and should be replaced with new_build_ext

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants