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.

Sign up for GitHub

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

Jump to bottom

DNM: Ensure manual builds use at least the same minimum Cython version used to build our wheels #9876

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

DNM: Ensure manual builds use at least the same minimum Cython version used to build our wheels #9876

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
[build-system]
requires = [
"setuptools >= 46.4.0",
"Cython >= 3.0.11",
Copy link
Member

@webknjaz webknjaz Nov 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't do anything since the current packaging implementation in aiohttp differs from the other libs. setuptools is configured to build from C-files. And those C-files have to exist. So the contributors have to call cythonize. And we call it when publishing. The end-users either get pre-built wheels or hit the sdist that contains those C-files.
So everything is dependent on whatever https://github.com/aio-libs/aiohttp/blob/9e3a328/requirements/cython.txt outputs at the time of release (this is also known to cause problems with older Cython-produced C-files not being compatible with bleeding-edge CPythons).
At some point, I'd like to change the build process but this is how things are right now.

Hence, we shouldn't add this here as it's never getting invoked in the current process.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I wonder how build will go on Python distro where Cython is not available?
python for java, .net, whatever...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cython is not the problem. It can convert PYX into C with its pure-python code. I'd worry about the following step (calling GCC/CLANG).

]
build-backend = "setuptools.build_meta"

Expand Down
Loading