search

pypa / distutils

distutils as found in cpython
MIT License
45 stars 65 forks source link

[BUG] self.linker_ex: TypeError: 'NoneType' object is not subscriptable #283

Open daniel-larraz opened 2 months ago

daniel-larraz commented 2 months ago

setuptools version

setuptools==72.2.0

Python version

Python 3.8.16 (PyPy 7.3.11)

OS

Ubuntu 20.04

Additional environment information

This issue occurs only when I run setuptools with PyPy; it works fine with CPython. Downgrading to setuptools version 72.1.0 resolves the issue for both PyPy and CPython.

Description

When building a Cython extension that depends on a shared library, setuptools triggers a TypeError during the linking process.

Expected behavior

setuptools doesn't fail.

How to Reproduce

Create a file named hello_world.cpp:

// hello_world.cpp
#include <iostream>

extern "C" void print_hello() {
    std::cout << "Hello, World!" << std::endl;
}

Create a file named hello_world.h:

// hello_world.h
#ifndef HELLO_WORLD_H
#define HELLO_WORLD_H

extern "C" void print_hello();

#endif // HELLO_WORLD_H

On Linux, compile this into a shared library using:

g++ -c -fPIC hello_world.cpp -o hello_world.o
g++ -shared hello_world.o -o libhello_world.so

Create a file named hello.pyx:

# hello.pyx
cdef extern from "hello_world.h":
    void print_hello()

def call_print_hello():
    print_hello()

Create a setup.py file to build the Cython extension:

# setup.py
from setuptools import setup, Extension
from Cython.Build import cythonize

extensions = [
    Extension(
        "hello",
        ["hello.pyx"],
        language="c++",
        include_dirs=["."],
        library_dirs=["."],
        libraries=["hello_world"],
        extra_compile_args=["-std=c++11"]
    )
]

setup(
    name="hello",
    ext_modules=cythonize(extensions),
)

Run the following command in the terminal:

python setup.py build_ext --inplace

Output

Traceback (most recent call last):
  File "setup.py", line 17, in <module>
    setup(
  File "/home/daniel/.pyenv/versions/pypy3.8-7.3.11/lib/pypy3.8/site-packages/setuptools/__init__.py", line 108, in setup
    return distutils.core.setup(**attrs)
  File "/home/daniel/.pyenv/versions/pypy3.8-7.3.11/lib/pypy3.8/site-packages/setuptools/_distutils/core.py", line 184, in setup
    return run_commands(dist)
  File "/home/daniel/.pyenv/versions/pypy3.8-7.3.11/lib/pypy3.8/site-packages/setuptools/_distutils/core.py", line 200, in run_commands
    dist.run_commands()
  File "/home/daniel/.pyenv/versions/pypy3.8-7.3.11/lib/pypy3.8/site-packages/setuptools/_distutils/dist.py", line 964, in run_commands
    self.run_command(cmd)
  File "/home/daniel/.pyenv/versions/pypy3.8-7.3.11/lib/pypy3.8/site-packages/setuptools/dist.py", line 945, in run_command
    super().run_command(command)
  File "/home/daniel/.pyenv/versions/pypy3.8-7.3.11/lib/pypy3.8/site-packages/setuptools/_distutils/dist.py", line 983, in run_command
    cmd_obj.run()
  File "/home/daniel/.pyenv/versions/pypy3.8-7.3.11/lib/pypy3.8/site-packages/setuptools/command/build_ext.py", line 93, in run
    _build_ext.run(self)
  File "/home/daniel/.pyenv/versions/pypy3.8-7.3.11/lib/pypy3.8/site-packages/setuptools/_distutils/command/build_ext.py", line 359, in run
    self.build_extensions()
  File "/home/daniel/.pyenv/versions/pypy3.8-7.3.11/lib/pypy3.8/site-packages/setuptools/_distutils/command/build_ext.py", line 476, in build_extensions
    self._build_extensions_serial()
  File "/home/daniel/.pyenv/versions/pypy3.8-7.3.11/lib/pypy3.8/site-packages/setuptools/_distutils/command/build_ext.py", line 502, in _build_extensions_serial
    self.build_extension(ext)
  File "/home/daniel/.pyenv/versions/pypy3.8-7.3.11/lib/pypy3.8/site-packages/setuptools/command/build_ext.py", line 254, in build_extension
    _build_ext.build_extension(self, ext)
  File "/home/daniel/.pyenv/versions/pypy3.8-7.3.11/lib/pypy3.8/site-packages/Cython/Distutils/build_ext.py", line 135, in build_extension
    super(build_ext, self).build_extension(ext)
  File "/home/daniel/.pyenv/versions/pypy3.8-7.3.11/lib/pypy3.8/site-packages/setuptools/_distutils/command/build_ext.py", line 581, in build_extension
    self.compiler.link_shared_object(
  File "/home/daniel/.pyenv/versions/pypy3.8-7.3.11/lib/pypy3.8/site-packages/setuptools/_distutils/ccompiler.py", line 757, in link_shared_object
    self.link(
  File "/home/daniel/.pyenv/versions/pypy3.8-7.3.11/lib/pypy3.8/site-packages/setuptools/_distutils/unixccompiler.py", line 269, in link
    self.linker_exe
TypeError: 'NoneType' object is not subscriptable (key slice(None, None, None))
QuLogic commented 2 months ago

We are seeing the same problem with building Matplotlib wheels, though not directly. It's actually failing on kiwisolver, but this is a bit annoying to work around, since the setuptools install is under cibuildwheel's test command + build isolation.

A bisect points to 0ab156c3e12054503a0dac0ce172e241262881f4, which is not too surprising, I think. Bisecting distutils itself points to pypa/distutils@2c937116cc0dcd9b26b6070e89a3dc5dcbedc2ae

QuLogic commented 2 months ago

It looks like PyPy only just added LDCXXSHARED (and there's been no releases with it) but the fetch from sysconfig to distutils's linker_so_cxx does not have any handling of that. Something simple like this might work:

diff --git a/distutils/sysconfig.py b/distutils/sysconfig.py
index fbdd5d73..19ea6aa0 100644
--- a/distutils/sysconfig.py
+++ b/distutils/sysconfig.py
@@ -321,6 +321,9 @@ def customize_compiler(compiler):
         )

         cxxflags = cflags
+        if ldcxxshared is None:
+            # Older versions of PyPy do not set LDCXXSHARED.
+            ldcxxshared = ldshared

         if 'CC' in os.environ:
             newcc = os.environ['CC']
QuLogic commented 2 months ago

Actually, looking at the commit again, it appears that CygwinCCompiler and UnixCCompiler both have a default for linker_so_cxx, but it's customize_compiler that overwrites it. So instead of trying to set the default in customize_compiler, it should just not set it, perhaps:

diff --git a/distutils/sysconfig.py b/distutils/sysconfig.py
index fbdd5d73..29a2ba36 100644
--- a/distutils/sysconfig.py
+++ b/distutils/sysconfig.py
@@ -362,11 +362,13 @@ def customize_compiler(compiler):
             compiler_cxx=cxx_cmd,
             compiler_so_cxx=cxx_cmd + ' ' + ccshared,
             linker_so=ldshared,
-            linker_so_cxx=ldcxxshared,
             linker_exe=cc,
             linker_exe_cxx=cxx,
             archiver=archiver,
         )
+        if ldcxxshared is not None:
+            # Older versions of PyPy do not set LDCXXSHARED.
+            compile.set_executable('linker_so_cxx', ldcxxshared)

         if 'RANLIB' in os.environ and compiler.executables.get('ranlib', None):
             compiler.set_executables(ranlib=os.environ['RANLIB'])
abravalheri commented 2 months ago

Thank you @QuLogic for the investigation.

@jaraco , @lazka do you think this patch is viable in the pypa/distutils repository? Should we move this issue there?

lazka commented 2 months ago

The patches above need a few more None checks for CXXFLAGS and LDCXXSHARED cases, but yeah, moving to distutils sounds good to me.

I wonder why CI didn't catch this.

jaraco commented 2 months ago

See #228 where this behavior was added. That PR and the one that preceded it were tricky to get right, especially due to the nuances of manipulating the sysconfig variables and the different locations where those variables are defined.

I see now how that PR caused a regression. Prior to this change, distutils and setuptools would treat C++ like C, only honoring the C configs and environment and disregarding the C++ settings. With the change, it now honors the C++ settings for C++ extensions, but there are some environments that don't yet have C++ settings configured. In that case, we want distutils to fall back to the old behavior (C settings).

I wonder why CI didn't catch this.

We should definitely determine the answer. If the current tests don't cover the condition (and it seems they don't), we'll want to manifest a test that does capture the missed expectation.

QuLogic commented 2 months ago

Looking at the original PR, test_customize_compiler does not set LDCXXSHARED, but it only checks compiler_cxx, and none of the new compiler_so_cxx, linker_so_cxx, linker_exe_cxx, so it neither confirms that the environment variable works, nor confirms that the fallback works.

hugovk commented 2 months ago

It looks like PyPy only just added LDCXXSHARED (and there's been no releases with it)

This has now been released in last week's PyPy3.10 v7.3.17:

GitHub Actions is still on 7.3.16:

Reported: https://github.com/actions/setup-python/issues/936

mattip commented 2 months ago

Could you add check-latest: true to the setup-python action options? I don't know how expensive it is, in any case you can comment it out once the image updates.

hugovk commented 2 months ago

Thanks, that does force 7.3.17 to be installed and now it passes for Ubuntu but fails for macOS with a different error:

ld: unknown options: -Bsymbolic-function

https://github.com/hugovk/ultrajson/actions/runs/10676841128

mattip commented 2 months ago

I wonder where that is coming from. As far as I can tell, it is not here (distutils) nor in PyPy. Is it an xcode default?