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

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

Open
hroncok opened this issue Sep 1, 2023 · 9 comments
Open

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

hroncok opened this issue Sep 1, 2023 · 9 comments

Comments

@hroncok
Copy link

hroncok commented Sep 1, 2023

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
Copy link
Author

hroncok commented Sep 1, 2023

Reverting e43f2fc makes it go away. I don't know what exactly makes the name go blank in this commit.

@hroncok
Copy link
Author

hroncok commented Sep 1, 2023

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

@agronholm
Copy link
Contributor

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

@hroncok
Copy link
Author

hroncok commented Sep 1, 2023

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

@agronholm
Copy link
Contributor

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

@hroncok
Copy link
Author

hroncok commented Sep 1, 2023

cc @QuLogic and @anntzer from mplcairo

@anntzer
Copy link

anntzer commented Sep 1, 2023

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
Copy link
Contributor

henryiii commented Sep 1, 2023

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
Copy link

anntzer commented Sep 1, 2023

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

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

No branches or pull requests

4 participants