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

Refactor Cython usage #3693

Closed
asvetlov opened this issue Apr 15, 2019 · 5 comments
Closed

Refactor Cython usage #3693

asvetlov opened this issue Apr 15, 2019 · 5 comments
Assignees

Comments

@asvetlov
Copy link
Member

asvetlov commented Apr 15, 2019

pip 19+ uses isolated virtual environments to install a package.
It breaks a build, please read #3581 report

The idea is:

  1. We don't need Cython to be installed in venv if all .pyx files are already precompiled into .c sources.
  2. .c are already present in distribution tarball. Binary wheels don't need to compile sources at all.
  3. Precompilation step can be done in Makefile before installing aiohttp library.
  4. Pure-python installation should require AIOHTTP_NO_EXTENSIONS=1 env var. Otherwise, a compilation error is raised. It prevents a silent performance degradation if aiohttp installed on Alpine or OS without C compiler toolchain.

#3694 addresses points 1-3, it can be backported to aiohttp 3.5.
The 4th bullet is not backward compatible. It should be implemented in a separate PR and land on aiohttp 4.0 only.

@webknjaz
Copy link
Member

webknjaz commented Apr 15, 2019

See also: pypa/pip#6144

I think I've actually got it figured out in #3581.
https://www.python.org/dev/peps/pep-0517/#in-tree-build-backends would work for us but it's not yet implemented by Pip.

The current implementation relies on an undefined requires filtering behavior as per pypa/pip#6410.

So the only thing left to do is to put it into a separate dist. (And it works!)

@asvetlov
Copy link
Member Author

manylinux is broken still but #3694 solves almost everything

@webknjaz webknjaz self-assigned this May 15, 2019
@webknjaz
Copy link
Member

I'd keep this open anyway. I'm still thinking of improvements.

@asvetlov
Copy link
Member Author

Up to you, the PR will hang for years then.
I have a feeling that supporting both brand new and 2-years-old pip versions is a good idea.

@asvetlov
Copy link
Member Author

asvetlov commented Sep 8, 2019

The issue is solved by adding explicit make cythonize step.

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

2 participants