fix: pack one binary per platform into python wheels#1181
fix: pack one binary per platform into python wheels#1181mrexox merged 3 commits intoevilmartians:masterfrom
Conversation
danfimov
force-pushed
the
fix-pypi-distribution
branch
from
8aa2b73 to
9bf9812
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the PyPI packaging to generate platform-specific wheels instead of bundling all platform binaries in a single universal wheel. The change creates separate wheel distributions for each OS/architecture combination (Linux, Darwin, FreeBSD, OpenBSD, Windows × x86_64, arm64).
- Introduces platform detection logic and custom wheel building to package only the relevant binary per platform
- Updates the Ruby packaging script to build separate wheels for each platform combination
- Removes the MIT License classifier from setup.py while adding
license_filesdirective
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packaging/pypi/setup.py | Adds platform detection, custom wheel class, and dynamic package_data generation to create platform-specific wheels |
| packaging/pack.rb | Updates PyPI publishing to build separate wheels for each platform using environment variables |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Maybe it will be even better to migrate from But let's think about it in a different MR after fixing this issue with platform-specific binaries) |
|
Thank you for preparing this PR! I will review it later this week 👍 |
|
I have slightly changed the script and built the packages PYTHON_PLATFORMS = ["linux", "darwin", "freebsd", "openbsd", "windows"].product(["x86_64", "arm64"])
# ...
PYTHON_PLATFORMS.each do |os, arch|
puts "Building wheel for #{os}-#{arch}..."
cd(pypi_dir)
ENV["LEFTHOOK_TARGET_PLATFORM"] = os
ENV["LEFTHOOK_TARGET_ARCH"] = arch
system("python setup.py bdist_wheel", exception: true)
end
# cd(pypi_dir)
# system("python -m twine upload --verbose --repository lefthook dist/*", exception: true)However when I try to install a package it fails: Also I noticed that logs print info about adding all binaries: Is there anything wrong? I'm not quite familiar with Python build process and will be able to take a look later. But maybe it's clear for you what went wrong 👀 |
danfimov
force-pushed
the
fix-pypi-distribution
branch
2 times, most recently
from
d7506c1 to
20001fb
Compare
|
After some more hours of debug I decided to switch to uv and hatch for package build. Now it should work. You can test build process like that:
And you can check installation of right wheel from pypi using this package from test pypi: https://test.pypi.org/project/lefthook-test/#files (I will delete it after this MR will be merrged) There is one problem: I can't build and push to pypi wheels for So I decided to build platform-specific wheels with one binary for supported platforms + one universal wheel with every binary for other platforms. If someone will try to install lefthook on linux, he will get wheel It's not a perfect solution (freebsd user still need to download 50Mb of binaries), but I see only one alternative - write in docs something like "we don't support installation from pypi for platforms other than linux/windows/macos" and just not build universal wheel. And I don't really like to do that. What you think we should do @mrexox? |
|
I think it's fine to keep 50MB giant for FreeBSD and OpenBSD. I'll try to find a solution if someone complains, but the goal is to deliver lefthook even with lots of odd binaries. I will check the changes later this week. Thank you for putting an effort into this! |
danfimov
force-pushed
the
fix-pypi-distribution
branch
from
20001fb to
3950fa1
Compare
mrexox
left a comment
mrexox
left a comment
There was a problem hiding this comment.
All good. Will merge after rebasing and removing openbsd and freebsd platforms
|
@danfimov Sorry for a long delay. Could you please rebase the PR? I'm going to merge it after that |
danfimov
force-pushed
the
fix-pypi-distribution
branch
from
81d1bba to
fff9b27
Compare
|
Rebased the PR and removed unsupported platforms. Should be good to merge now, @mrexox. |
danfimov
force-pushed
the
fix-pypi-distribution
branch
from
fff9b27 to
8decbd0
Compare
|
Awesome! Thank you for taking care for such a long time! |
|
Something isn't working well. You can see that in publishing logs the side of the uploaded pacakges is a few Kb: https://github.com/evilmartians/lefthook/actions/runs/21388823016/job/61571056398 It should've been ~10Mb instead. Do you have an idea of what could go wrong? |
|
@mrexox [tool.hatch.build.targets.wheel]
packages = ["lefthook"]
artifacts = ["lefthook/bin"] # add artifactsReference: |
|
@mrexox |
|
Thank you @npnh2! I'll test this in next release :) |
Closes #971
Context
Previously, the PyPI package was being built as a single wheel containing binaries for all architectures. This meant every user, regardless of their platform, was downloading a 50+ MB wheel with binaries they would never use. This is inefficient and not the proper way to distribute platform-specific packages on PyPI.
Changes
The fix implements platform-specific wheel generation, where each wheel contains only the binary for its target architecture.
I also replaced license classifier with
license_filesparameter, because this classifier is deprecated:I tested building process with this commands:
setup.py:cd packaging/pypisetup.pyto build wheel for my machine:python setup.py bdist_wheelLEFTHOOK_TARGET_PLATFORM=linux LEFTHOOK_TARGET_ARCH=arm64 python setup.py bdist_wheelP.S. I'm familiar with Python, but not with Ruby, so please review part with
pack.rbchanges carefully)