feature: add support for ABI3 wheels #1091
Conversation
There was a problem hiding this comment.
This is much easier to review after checking "ignore whitespace" in GH. :) The extra indentation is a minor downside - I think we should see if we can pull out each step (build, test, etc) into functions, then call them - that will keep the whitespace from growing here, and keep the logic easier to read (if this, build this, if that, test that) instead of having a massive inline function, too. Not for this PR though.
There was a problem hiding this comment.
Great stuff @mayeut. Thank you for this, it's been on my todo list for a while!
I enjoy how pragmatic it is, simpler than I'd have thought to do it.
- The normal operation of cibuildwheel is to build from oldest Python to newest. Is that what we want in this case? I.e. the abi wheel will be built on the oldest Python that supports limited api.
- Documentation - I suppose we could add a FAQ entry about limited API, and include how to do that within setup.py, since we don't allow passing the
--py_limited_apioption.
| setup_py_add=textwrap.dedent( | ||
| r""" | ||
| cmdclass = {} | ||
| extension_kwargs = {} | ||
| if sys.version_info[:2] >= (3, 8): | ||
| from wheel.bdist_wheel import bdist_wheel as _bdist_wheel | ||
|
|
||
| class bdist_wheel_abi3(_bdist_wheel): | ||
| def finalize_options(self): | ||
| _bdist_wheel.finalize_options(self) | ||
| self.root_is_pure = False | ||
|
|
||
| def get_tag(self): | ||
| python, abi, plat = _bdist_wheel.get_tag(self) | ||
| return python, "abi3", plat | ||
|
|
||
| cmdclass["bdist_wheel"] = bdist_wheel_abi3 | ||
| extension_kwargs["define_macros"] = [("Py_LIMITED_API", "0x03080000")] | ||
| extension_kwargs["py_limited_api"] = True | ||
| """ | ||
| ), | ||
| setup_py_extension_args_add="**extension_kwargs", | ||
| setup_py_setup_args_add="cmdclass=cmdclass", | ||
| ) |
There was a problem hiding this comment.
I suppose this is the way to get abi3 wheel from pip wheel/python -m build? Because we can't pass --py-limited-api? I wonder if there's a cleaner way to do it. I suppose we'd want to document this too.
There was a problem hiding this comment.
A follow-up on this question - should a reference version of this specify in the tag that it's a "3.8-ABI" wheel, rather than using whatever it's being built by?
There was a problem hiding this comment.
Most packages will hard code this into setup.py. A package either produces abi3 wheels or regular wheels, usually they don't select between the two, I think. So you'd hardcode in the define in the source code, and py_limited_api=True is passed to setup().
There was a problem hiding this comment.
I tested something a bit more complicated than what most packages will do. As @henryiii mentioned, and I think so to, "a package either produces abi3 wheels or regular wheels, usually they don't select between the two" and it'll be hardcoded in the source code.
Even for some packages that still support cp27 or some that depends on an API being made stable in a version more recent than the minimum supported, the logic will probably hardcoded.
I really don't think it's cibuildwheel's place to document how to build an abi3 wheel. Only if workarounds were to arise, specific to cibuildwheel, those should be documented then.
There was a problem hiding this comment.
I tested something a bit more complicated than what most packages will do.
Understood.
I really don't think it's cibuildwheel's place to document how to build an abi3 wheel. Only if workarounds were to arise, specific to cibuildwheel, those should be documented then.
The only thing that might be considered specific is the inability to pass --py-limited-api to bdist_wheel - most docs online still reference this. But since that calling method is deprecated, it does indeed make sense to document a more modern way across the ecosystem. Ideally, indeed, in packaging.python.org. Then we can link it from our docs.
I think that's what we want in this case. This way, when a new version of CPython is available, it'll be tested but it won't change the build so I'd say it allows more reproducibility in the long run.
I really don't think it belongs in cibuildwheel. IMHO, it belongs in https://packaging.python.org/en/latest/ and cibuildwheel should reference an entry there but it does not exist yet. |
mayeut
force-pushed
the
abi3
branch
2 times, most recently
from
7a4a5d2
to
8cc44fc
Compare
| setup_py_add=textwrap.dedent( | ||
| r""" | ||
| cmdclass = {} | ||
| extension_kwargs = {} | ||
| if sys.version_info[:2] >= (3, 8): | ||
| from wheel.bdist_wheel import bdist_wheel as _bdist_wheel | ||
|
|
||
| class bdist_wheel_abi3(_bdist_wheel): | ||
| def finalize_options(self): | ||
| _bdist_wheel.finalize_options(self) | ||
| self.root_is_pure = False | ||
|
|
||
| def get_tag(self): | ||
| python, abi, plat = _bdist_wheel.get_tag(self) | ||
| return python, "abi3", plat | ||
|
|
||
| cmdclass["bdist_wheel"] = bdist_wheel_abi3 | ||
| extension_kwargs["define_macros"] = [("Py_LIMITED_API", "0x03080000")] | ||
| extension_kwargs["py_limited_api"] = True | ||
| """ | ||
| ), | ||
| setup_py_extension_args_add="**extension_kwargs", | ||
| setup_py_setup_args_add="cmdclass=cmdclass", | ||
| ) |
There was a problem hiding this comment.
I tested something a bit more complicated than what most packages will do.
Understood.
I really don't think it's cibuildwheel's place to document how to build an abi3 wheel. Only if workarounds were to arise, specific to cibuildwheel, those should be documented then.
The only thing that might be considered specific is the inability to pass --py-limited-api to bdist_wheel - most docs online still reference this. But since that calling method is deprecated, it does indeed make sense to document a more modern way across the ecosystem. Ideally, indeed, in packaging.python.org. Then we can link it from our docs.
|
LGTM |
|
I'm currently thinking how best to document this... |
This extends the mechanism introduced in pypa#1091 for `abi3` wheels. Most of the mentions to `abi3` have been removed and replaced by a more generic `compatible_wheel`. This allows to build a wheel `foo-0.1-py3-none-win_amd64.whl` only once and still test with every configured python.
This extends the mechanism introduced in pypa#1091 for `abi3` wheels. Most of the mentions to `abi3` have been removed and replaced by a more generic `compatible_wheel`. This allows to build a wheel `foo-0.1-py3-none-win_amd64.whl` only once and still test with every configured python.
This extends the mechanism introduced in pypa#1091 for `abi3` wheels. Most of the mentions to `abi3` have been removed and replaced by a more generic `compatible_wheel`. This allows to build a wheel `foo-0.1-py3-none-win_amd64.whl` only once and still test with every configured python.
* feature: add support for `py3-none-{platform}` wheels
This extends the mechanism introduced in #1091 for `abi3` wheels.
Most of the mentions to `abi3` have been removed and replaced by a more generic `compatible_wheel`.
This allows to build a wheel `foo-0.1-py3-none-win_amd64.whl` only once and still test with every configured python.
* Add integration test for py3-none abi wheels
* Fix expected_wheels for py3-none abi
* Limit test to three pythons and check for certain log messages
* chore: add some comments to explain filter process
Co-authored-by: Joe Rickerby <joerick@mac.com>
Co-authored-by: Henry Schreiner <henryschreineriii@gmail.com>
Closes #78
While #569 errors out on duplicate abi3 wheel with an updated error message, this PR skips compatible abi3 builds and do proper testing with all python versions as specified by the build selector.
real-world usage can be seen there: https://github.com/mayeut/psutil/actions/runs/2184123889