pypa / setuptools

Official project repository for the Setuptools build system
https://pypi.org/project/setuptools/
MIT License
2.5k stars 1.19k forks source link

error: each element of 'ext_modules' option must be an Extension instance or 2-tuple #309

Open ghost opened 9 years ago

ghost commented 9 years ago

Originally reported by: floppym (Bitbucket: floppym, GitHub: floppym)


On Gentoo Linux, we are running into a problem building pyzmq with recent versions of setuptools.

https://bugs.gentoo.org/show_bug.cgi?id=532708

We get this error:

error: each element of 'ext_modules' option must be an Extension instance or 2-tuple

From what I can tell, it has to do with the order in which distutils.extenstion.Extension and setuptools are imported. setuptools replaces the Extension module with its own implementation. If this happens after setup.py has imported distutils.extension.Extension, it causes this problem.

Here is a reduced test case:

% cat setup.py
from distutils.core import setup
from distutils.extension import Extension
import setuptools

setup(ext_modules=[
        Extension('demo', ['demo.c'])
])
% python3.4 setup.py build
running build
running build_ext
error: each element of 'ext_modules' option must be an Extension instance or 2-tuple

The obvious workaround is to always import setuptools first. However, it is not always obvious where setuptools is being imported.

For pyzmq, it seems like setuptools is being imported indirectly; setup.py does not import it anywhere. By trial and error, it looks like importing nose brings in setuptools.


ghost commented 8 years ago

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


The problem is that, if cython is installed, then setuptools.extension.Extension is defined twice. If cython is absent, then the error does not happen.

I may have an idea what's going on here, and unfortunately, it may be a different bug than the originally reported bug (which is specifically about packages that import from distutils before importing setuptools). I've opened #488 to track it separately.

ghost commented 8 years ago

Original comment by johnyf (Bitbucket: johnyf, GitHub: johnyf):


This error arises also in a different case, without importing distutils at all. The problem is that, if cython is installed, then setuptools.extension.Extension is defined twice. In particular, distutils.command.build_ext.build_ext.check_extensions_list fails when checking whether isinstance(ext, Extension). If cython is absent, then the error does not happen.

First reported in psutil #723.

A minimal setup.py that demonstrates the issue is:

from setuptools import setup, Extension

print(id(Extension))
name = 'hoho'
version = '0.0.1'
ext_modules = [Extension('foo', ['hoho/foo.c'])]
setup(
    name=name,
    version=version,
    packages=[name],
    package_dir={name: name},
    install_requires=['psutil'],
    ext_modules=ext_modules,
    dependency_links=['file://localhost/some/path/psutil/dist/psutil-3.3.0.tar.gz#egg=psutil-3.3.0'])

where hoho contains an empty __init__.py, and a trivial hehe.py, and foo.c. The dependency_links is irrelevant to the issue, I use it to print debugging information from psutil/setup.py. An error trace can be seen in psutil #723. Copying the tail here:

pip install cython
python setup.py install  # hoho
4344374408  # id(Extension) at top of `hoho/setup.py`
running install
...
...
Installed /Users/ifilippi/.virtualenvs/test/lib/python2.7/site-packages/hoho-0.0.1-py2.7-macosx-10.4-x86_64.egg
Processing dependencies for hoho==0.0.1
Searching for psutil
Best match: psutil 3.3.0
Processing psutil-3.3.0.tar.gz
Writing /var/folders/cy/b25_dhcn0bj1s5j95zf0j9_h0000gp/T/easy_install-8GZlT5/psutil-3.3.0/setup.cfg
Running psutil-3.3.0/setup.py -q bdist_egg --dist-dir /var/folders/cy/b25_dhcn0bj1s5j95zf0j9_h0000gp/T/easy_install-8GZlT5/psutil-3.3.0/egg-dist-tmp-dHQ3sI
in psutil setup.py, id(Extension):
4381298488  # id(Extension) at top of `psutil/setup.py` 
4381298488  # id(Extension) before instantiating extensions
4381298488  # id(Extension) after instantiating extensions
4297012928  # id(type(ext)), where `ext` is one of the extensions
type(ext): <type 'instance'>
<setuptools.extension.Extension instance at 0x1052b0f60>
type(Extension): <type 'classobj'>
setuptools.extension.Extension
id(type(ext)) = 4297012928 and id(Extension) = 4344374408
id(ext.__class__) = 4381298488 and id(Extension) = 4344374408
---------------------------------------------------------------------------
UnpickleableException                     Traceback (most recent call last)
/Users/ifilippi/.../hoho/setup.py in <module>()
     12     install_requires=['psutil'],
     13     ext_modules=ext_modules,
---> 14     dependency_links=['file://localhost/some/path/psutil/dist/psutil-3.3.0.tar.gz#egg=psutil-3.3.0'])

/Users/ifilippi/python/lib/python2.7/distutils/core.pyc in setup(**attrs)
    149     if ok:
    150         try:
--> 151             dist.run_commands()
    152         except KeyboardInterrupt:
    153             raise SystemExit, "interrupted"
...
...
/Users/ifilippi/python/lib/python2.7/distutils/command/build_ext.py in check_extensions_list(self, extensions)
    370             if not isinstance(ext, tuple) or len(ext) != 2:
    371                 raise DistutilsSetupError, \
--> 372                       ("each element of 'ext_modules' option must be an "
    373                        "Extension instance or 2-tuple")
    374

UnpickleableException: DistutilsSetupError("each element of 'ext_modules' option must be an Extension instance or 2-tuple",)

The above output is obtained by patching psutil/setup.py with

from setuptools import setup, Extension
print('in psutil setup.py, id(Extension):')
print(id(Extension))

near the top, where setuptools.Extension is imported, and

# OS X
elif sys.platform.startswith("darwin"):
    print(id(Extension))
    ext = Extension(
        'psutil._psutil_osx',
        sources=[
            'psutil/_psutil_osx.c',
            'psutil/_psutil_common.c',
            'psutil/arch/osx/process_info.c'
        ],
        define_macros=[VERSION_MACRO],
        extra_link_args=[
            '-framework', 'CoreFoundation', '-framework', 'IOKit'
        ])
    print(id(Extension))
    print(id(type(ext)))
    print(id(type(posix_extension)))
    assert isinstance(ext, Extension)
    assert ext.__class__ is Extension
    assert type(ext) is not Extension
    extensions = [ext, posix_extension]

later, where the extensions of psutil are constructed.

Note that distutils.extension.Extension is an old-style class, so id(type(ext)) does not match id(Extension), though ext.__class__ is Extension, see discussion here.

The last part of the output is from the following patch to distutils.command.build_ext:

if isinstance(ext, Extension):
    continue  # OK! (assume type-checking done
                    # by Extension constructor)
print('type(ext): {t}'.format(t=type(ext)))
print(ext)
print('type(Extension): {t}'.format(t=type(Extension)))
print(Extension)
print('id(type(ext)) = {i} and id(Extension) = {j}'.format(
    i=id(type(ext)), j=id(Extension)))
print('id(ext.__class__) = {i} and id(Extension) = {j}'.format(
    i=id(ext.__class__), j=id(Extension)))

Conclusion: The instance check fails inside distutils.command.build_ext.build_ext.check_extensions.list because Extension does not match ext.__class__. The class Extension is imported from distutils.extension.Extension. It is the same instance as the setuptools.Extension imported at the top of hoho/setup.py. This is seen from the id(Extension) reported at the start and end of the above output.

The instance ext has ext.__class__ different from the Extension above (different ids towards end of output). As expected, this ext instance has class equal to the setuptools.Extension imported at the top of psutil. To confirm this, compare the ids reported at the middle and bottom of the output above.

Therefore, the setuptools.Extension imported in hoho/setup.py is a different instance from the setuptools.Extension imported in psutil/setup.py when building the dependency psutil.

This behavior is triggered only when Cython is installed. Also, it doesn't happen if hoho does not have any extension modules. Cython is not directly used anywhere in hoho nor psutil. It is imported only by setuptools at a single place: setuptools.command.build_ext. So, that import is likely part of the problem. Cython imports distutils itself to subclass build_ext and Extension, in cython.distutils.

Finally, note that this behavior was revealed in setuptools == 18.8.1, after an earlier infinite recursion was fixed in #440, as described here.

So, the question is what should be done to fix this ? I am willing to help reach a fix, but I have little experience with setuptools. I think that the problem may be that setuptools imports Cython, which re-imports distutils.extension.Extension (by then the patched one), but somehow can see the setuptools.Extension that was patched by the original requirement hoho, instead of the later instance imported by the dependency in psutil/setup.py.

ghost commented 9 years ago

Original comment by Merwok (Bitbucket: Merwok, GitHub: Merwok):


The same issue happens with the Distribution class (see CPython ticket).

ghost commented 9 years ago

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


@floppym Thanks for pointing that out. That's definitely an unintentional regression. I've filed #311 to address that separate issue.

ghost commented 9 years ago

Original comment by floppym (Bitbucket: floppym, GitHub: floppym):


I don't believe pkg_resources is implicated here. pkg_resources is specifically kept partitioned from the setuptools namespace and never imports or invokes behavior from the setuptools package. Did you simply mean "setuptools" in place of "pkg_resources"?

pkg_resources.py currently does this:

import setuptools._vendor.packaging.version
import setuptools._vendor.packaging.specifiers

This causes setuptools to be imported, and therefore causes the Extension class to be replaced.

Also see my stack trace in the Gentoo bug report.

https://bugs.gentoo.org/show_bug.cgi?id=532708#c17

In the same bug report, I bisected the issue with pyzmq to dcd552da643c, which is where that setuptools import was introduced in pkg_resources.py.

ghost commented 9 years ago

Original comment by Merwok (Bitbucket: Merwok, GitHub: Merwok):


A fix in setuptools can also reach users sooner than a change in the stdlib.

ghost commented 9 years ago

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


A couple of ideas I have:

Of course it could be fixed in distutils.

I can see distutils being updated to provide a more rigorous API and extension mechanism for its classes, such as Extension which setuptools could leverage rather than monkey patching. I'd be in favor of that in general, but thinking about the practical concerns, I'm not sure there's an elegant solution. To avoid monkey-patching, but still allow a distutils Extension to be used might prove difficult.

ghost commented 9 years ago

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


just because some bit of code, somewhere, imports pkg_resources does not at all seem to reliably signify that setup.py is setuptools-base

I don't believe pkg_resources is implicated here. pkg_resources is specifically kept partitioned from the setuptools namespace and never imports or invokes behavior from the setuptools package. Did you simply mean "setuptools" in place of "pkg_resources"?

ghost commented 9 years ago

Original comment by gmturner (Bitbucket: gmturner, GitHub: gmturner):


http://bugs.python.org/issue23102. The basic recipe:

step 1) in setup.py, do lots of from distutils.foo.bar import baz type stuff. step 2) import something that imports (one of a subset of all the) setuptools modules step 3) x=Extension(...) step 4) call setup(..., ext_modules=[x,...]) step 5) crash

Monsters are hiding under the beds of steps one, three, and four. First we save off a reference to the un-monkey-patched distutils.extension.Extension, then we (indirectly) cause the monkey patch by importing some setuptools modules, and then we create the Extension instance, using the un-monkey class reference stored in the global namespace.

Now we're off to the races: hand it to distutils (not setuptools), and it will iterate ext_modules checking that all the things in there are two-ples, or else distutils.extension.Extension instances. But since that is monkey patched, now, they aren't. Distutils therefore branches directly to step five.

Resonable people could disagree, but to me, it seems like a setuptools bug, insofar as, i.e., just because some bit of code, somewhere, imports pkg_resources does not at all seem to reliably signify that setup.py is setuptools-base (and that therefore it's a good idea to start monkey-patching distutils).

I have no good ideas for how to fix this in setuptools without some novel innovation. Then again I really don't know how setuptools works. But it just "sounds hard". I.e., we could:

Make setuptools Extension a true subclass of the un-monkey-patched distutils.extension.Extension; do not monkey patch it.

Make setuptools Extension a virtual subclass of the un-monkey-patched distutils.extension.Extension (but is that even possible in 2.7, where it's an old-style class?)

Monkey patch distutils.command.check_extensions_list too, to check more liberally.

...?

But, I dunno about the first one, presumably there is a good reason it's being monkey patched, the second one seems ... well all-but-guaranteed to cause stupid regressions, and the third kinda feels like throwing good money after bad somehow... I'm not an expert so I'll shut up and hope some of those can chime in.

Of course it could be fixed in distutils, btw, and imo should be, but the cat is well out of the bag for many years on this one -- setuptools won't be able to rely on that fix for many years.

bsolomon1124 commented 4 years ago

Bug still exists in Cython 0.29.19 / setuptools 46.4.0. And this occurs when following an example virtually straight out of the Cython docs. Amazing!

Edit: watch out for the fact that cythonize() returns a list even if you feed it a single Extension instance. E.g.:

# BAD
extension = Extension("example", ["example"+ext])
if USE_CYTHON:
    from Cython.Build import cythonize
    extensions = cythonize(extensions)
setup(ext_modules = [extension])

This will fail because you're actually passing a List[List[Extension]].

Correct version:

extensions = [Extension("example", ["example"+ext])]
if USE_CYTHON:
    from Cython.Build import cythonize
    extensions = cythonize(extensions)
setup(ext_modules = extensions)