pypa / wheel

The official binary distribution format for Python
MIT License
504 stars 151 forks source link

Regression: wheel 0.41 creates an extension module with empty name #566

Closed hroncok closed 3 weeks ago

hroncok commented 1 year ago

When running the impact check of wheel 0.41 in Fedora, we have noticed a regression since 0.40 in our mplcairo package. To reproduce, run:

$ sudo dnf install python3.12-devel  # or python3.11-devel
$ sudo dnf builddep python-mplcairo  # this only works on Fedora and ensures the extension module can be built, it installs cairo-devel etc.

$ wget https://files.pythonhosted.org/packages/source/m/mplcairo/mplcairo-0.5.tar.gz
...
$ tar xf mplcairo-0.5.tar.gz 
$ cd mplcairo-0.5/

[mplcairo-0.5]$ python3.12 -m venv __venv__
[mplcairo-0.5]$ . __venv__/bin/activate
(__venv__) [mplcairo-0.5]$ pip install setuptools wheel
...
Successfully installed setuptools-68.1.2 wheel-0.41.2

The content of setup.cfg does not matter:

(__venv__) [mplcairo-0.5]$ cat setup.cfg 
[egg_info]
tag_build = 
tag_date = 0

(__venv__) [mplcairo-0.5]$ rm setup.cfg  # or not

Build the wheel. The extension module is called (empty string) and the file will be named just .cpython-312-x86_64-linux-gnu.so. Using pip also reproduces it.

(__venv__) [mplcairo-0.5]$ python setup.py bdist_wheel
...
running build_ext
building '' extension
gcc -shared -Wl,-z,relro -Wl,--as-needed -Wl,-z,now -Wl,--build-id=sha1 -Wl,-z,relro -Wl,--as-needed -Wl,-z,now -Wl,--build-id=sha1 -L/usr/lib64 -o build/lib.linux-x86_64-cpython-312/.cpython-312-x86_64-linux-gnu.so
...
copying build/lib.linux-x86_64-cpython-312/.cpython-312-x86_64-linux-gnu.so -> build/bdist.linux-x86_64/wheel
...
creating 'dist/mplcairo-0.5-cp312-cp312-linux_x86_64.whl' and adding 'build/bdist.linux-x86_64/wheel' to it
adding '.cpython-312-x86_64-linux-gnu.so'
...

(__venv__) [mplcairo-0.5]$ unzip -vl dist/mplcairo-0.5-cp312-cp312-linux_x86_64.whl | grep cpython
   15320  Defl:N     1570  90% 09-01-2023 09:55 d5cc7582  .cpython-312-x86_64-linux-gnu.so

Using an older version of wheel, the extension module is called mplcairo._mplcairo and the file will be named mplcairo/_mplcairo.cpython-312-x86_64-linux-gnu.so.

(__venv__) [mplcairo-0.5]$ rm dist/ build/ -rf
(__venv__) [mplcairo-0.5]$ pip install wheel==0.40.0
...
Successfully installed wheel-0.40.0

(__venv__) [mplcairo-0.5]$ python setup.py bdist_wheel
...
running build_ext
building 'mplcairo._mplcairo' extension
...
gcc -fno-strict-overflow -Wsign-compare -DDYNAMIC_ANNOTATIONS_ENABLED=1 -DNDEBUG -O2 -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fstack-protector-strong -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -O2 -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fstack-protector-strong -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -O2 -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fstack-protector-strong -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -fPIC -I/home/churchyard/tmp/reproducer/mplcairo-0.5/.eggs/pycairo-1.24.0-py3.12-linux-x86_64.egg/cairo/include -I/home/churchyard/tmp/reproducer/mplcairo-0.5/.eggs/pybind11-2.11.1-py3.12.egg/pybind11/include -I/home/churchyard/tmp/reproducer/mplcairo-0.5/__venv__/include -I/usr/include/python3.12 -c src/_unity_build.cpp -o build/temp.linux-x86_64-cpython-312/src/_unity_build.o -fvisibility=hidden -g0 -std=c++17 -I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/harfbuzz -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/sysprof-4 -pthread -I/usr/include/fribidi -flto -Wall -Wextra -Wpedantic -I/usr/include/cairo -I/usr/include/libpng16 -I/usr/include/freetype2 -I/usr/include/harfbuzz -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/sysprof-4 -I/usr/include/libxml2 -pthread -I/usr/include/pixman-1
g++ -shared -Wl,-z,relro -Wl,--as-needed -Wl,-z,now -Wl,--build-id=sha1 -Wl,-z,relro -Wl,--as-needed -Wl,-z,now -Wl,--build-id=sha1 build/temp.linux-x86_64-cpython-312/src/_unity_build.o -L/usr/lib64 -o build/lib.linux-x86_64-cpython-312/mplcairo/_mplcairo.cpython-312-x86_64-linux-gnu.so -flto
...
copying build/lib.linux-x86_64-cpython-312/mplcairo/_mplcairo.cpython-312-x86_64-linux-gnu.so -> build/bdist.linux-x86_64/wheel/mplcairo
...
creating 'dist/mplcairo-0.5-cp312-cp312-linux_x86_64.whl' and adding 'build/bdist.linux-x86_64/wheel' to it
...
adding 'mplcairo/_mplcairo.cpython-312-x86_64-linux-gnu.so'
...

(__venv__) [mplcairo-0.5]$ unzip -vl dist/mplcairo-0.5-cp312-cp312-linux_x86_64.whl | grep cpython
 1004184  Defl:N   352380  65% 09-01-2023 09:59 3f9a6827  mplcairo/_mplcairo.cpython-312-x86_64-linux-gnu.so
hroncok commented 1 year ago

Reverting https://github.com/pypa/wheel/commit/e43f2fcb296c2ac63e8bac2549ab596ab79accd0 makes it go away. I don't know what exactly makes the name go blank in this commit.

hroncok commented 1 year ago

This also reproduces with Python 3.11, hence it is not Python 3.12 specific.

agronholm commented 1 year ago

Does it happen with any other projects with C extensions? If it did, someone would probably have raised an alarm earlier.

hroncok commented 1 year ago

mplcairo is the only project in Fedora that we were able to identify. I don't know yet what makes it special.

agronholm commented 1 year ago

It could be the build trickery it engages in in its setup.py.

hroncok commented 1 year ago

cc @QuLogic and @anntzer from mplcairo

anntzer commented 1 year ago

The extension module is called (empty string)

It could be the build trickery it engages in in its setup.py.

I used to do very weird things in setup.py, which date back to setup_requires and the pre-pyproject.toml days. Probably the whole thing could use a migration to pyproject.toml and a much more streamlined setup.py (I'd rather stick to setuptools for now), but I can't guarantee any timeline (a PR would be welcome).

henryiii commented 1 year ago

I think mplcario is depending on the order commands run in. This is suspect, I think:

https://github.com/matplotlib/mplcairo/blob/37f64dbbd5aa03209464f6d5ad10756c5fdd08c5/setup.py#L67

class build_ext(build_ext):

    def finalize_options(self):
        if not self.distribution.have_run.get("egg_info", 1):
            # Just listing the MANIFEST; setup_requires are not available yet.
            super().finalize_options()
            return

    # Body of build_ext here ...

If egg_info is not finalized, I think this doesn't get listed in have_run, so the default of 1 is used, causing this to run. Now it's initialized, due to

egg_info = self.distribution.get_command_obj("egg_info")
egg_info.ensure_finalized()  # needed for correct `wheel_dist_name`

So this is is 0 causing this to be skipped. That if statement, which assumes a missing "egg_info" is "True" is suspect, IMO. This looks like something that is trying to check something by looking at a totally different condition that also once happened be in sync with what it really cares about - these are always fragile.

But it could be something else in this custom build_ext that depends on the finalization order.

anntzer commented 1 year ago

Thanks for the investigation. Looks like the if check and early return are actually unnecessary since https://github.com/pypa/setuptools/pull/1150 (which installs setup_requires very early); I pushed a commit on mplcairo that removes it and the build still seems fine.