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

Require cython when building from git clone only #3189

Merged
merged 16 commits into from
Aug 20, 2018
Merged

Require cython when building from git clone only #3189

Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
33d8a7b
Fix #3162: require cython when building from git clone only
asvetlov Aug 12, 2018
ee4ebf9
Don't install inself when building docs
asvetlov Aug 12, 2018
bb04d42
Simplify git hint
asvetlov Aug 12, 2018
c6f015f
Improve white list
asvetlov Aug 12, 2018
f05f688
Reorganize requirements
asvetlov Aug 12, 2018
a1342c2
Use stderr for error messages
asvetlov Aug 12, 2018
21b2e8f
Improve error text
asvetlov Aug 12, 2018
fc9f4eb
Fix build
asvetlov Aug 12, 2018
f6d1052
Add upgrade_python_toolset
asvetlov Aug 12, 2018
f856d64
Extract cython into a separate requirement file
asvetlov Aug 12, 2018
6a1a1c2
Configure more
asvetlov Aug 12, 2018
d7b67e7
Merge remote-tracking branch 'origin/master' into relax-build
asvetlov Aug 19, 2018
189186e
Install cython on tests
asvetlov Aug 19, 2018
01a98e6
Update cython from makefile
asvetlov Aug 19, 2018
c874bc7
Install cython on Windows
asvetlov Aug 19, 2018
86479c3
Add cython requirements to linux wheels
asvetlov Aug 19, 2018
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
15 changes: 14 additions & 1 deletion setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,27 @@
if sys.version_info < (3, 5, 3):
raise RuntimeError("aiohttp 3.x requires Python 3.5.3+")

here = pathlib.Path(__file__).parent

try:
from Cython.Build import cythonize
USE_CYTHON = True
except ImportError:
USE_CYTHON = False

if (here / '.git').exists() and not USE_CYTHON:
print("Install cython when building from git clone")
Copy link
Member

Choose a reason for hiding this comment

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

Please use stderr

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch

print("pip install cython")
Copy link
Member

Choose a reason for hiding this comment

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

Add this to pyproject.toml and it'll be autoinstalled for pip 10+ users.

sys.exit(1)


if (here / '.git').exists() and not (here / 'vendor/http-parser/README.md'):
print("Install submodules when building from git clone")
print("git submodule init")
print("git submodule update")
Copy link
Member

Choose a reason for hiding this comment

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

It's simpler to do git submodule update --init.

Copy link
Member

Choose a reason for hiding this comment

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

Also, how about to do that automagically if condition is met? Since we know which commands are need to run. Suddenly, it wouldn't be useful for Cython - setup_requires can't handle wheels /:

Copy link
Member Author

Choose a reason for hiding this comment

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

It is for dev module only.
I prefer don't hide git commands from user here.
There is a rare possibility to have no git command in PATH, better to give a hint only

Copy link
Member

Choose a reason for hiding this comment

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

Ok, that's fine for me.

sys.exit(2)


ext = '.pyx' if USE_CYTHON else '.c'


Expand Down Expand Up @@ -62,7 +76,6 @@ def build_extension(self, ext):
raise BuildFailed()


here = pathlib.Path(__file__).parent

txt = (here / 'aiohttp' / '__init__.py').read_text('utf-8')
try:
Expand Down