search

mesonbuild / meson-python

Meson PEP 517 Python build backend
https://mesonbuild.com/meson-python/
MIT License
118 stars 59 forks source link

BUG: Fix wheels not being compressed #520

Closed lazka closed 8 months ago

lazka commented 8 months ago

As pointed out in https://docs.python.org/3/library/zipfile.html#zipfile.ZipFile.writestr when creating a ZipInfo it defaults to ZIP_STORED and when passing it to writestr it will use the compression info from the ZipInfo instance instead of the ZipFile. This leads to the wheel file being uncompressed.

To fix this explicitely specify the compression type and level for writestr() so it gets applied to the ZipInfo instance before writing. While the compression level currently isn't used I also added it to avoid future surprises.

Fixes #519

lazka commented 8 months ago

In my case this reduced the file size for my wheel from ~500kb to ~130kb

lazka commented 8 months ago

Good catch. I'm surprised we didn't notice this before: meson-python is used for some large projects!Thanks for the patch. It looks good, but i think you missed one instance of .writestr() that needs handling, and the test needs to be updated to catch it.

I can only see two calls to writestr() and both are handled :/

Other than that, can you please wrap the commit message at 72 characters? And, just to be extra picky, we use a convention where "Fix" in the commit message subject line is lowercase not capitalized. Thanks!

done

dnicolodi commented 8 months ago

I can only see two calls to writestr() and both are handled :/

Yes, sorry, my bad. Not enough coffee this morning yet.

lazka commented 8 months ago

Thanks!

rgommers commented 8 months ago

Thanks a lot, great catch!

I'm surprised we didn't notice this before: meson-python is used for some large projects!

I think why I didn't notice is that we always run numpy/scipy wheels through auditwheel & co, and those tools will write out a new compressed wheel with dependencies vendored, so in those cases it didn't matter. Without that, the difference is very large indeed.