Build/install broken without cython

[apatrushev]

Long story short

Latest master cannot be installed without cython.

clang: error: no such file or directory: 'aiohttp/_websocket.c'

Expected behaviour

pip install git+http://github.com/aio-libs/aiohttp

Should install aiohttp even without cython. It worked at least in previous versions.

Actual behaviour

Build failed:

    running build_ext
    building 'aiohttp._websocket' extension
    creating build/temp.macosx-10.13-x86_64-3.7
    creating build/temp.macosx-10.13-x86_64-3.7/aiohttp
    gcc -Wno-unused-result -Wsign-compare -Wunreachable-code -fno-common -dynamic -DNDEBUG -g -fwrapv -O3 -Wall -I/Users/tosha/.pythonz/pythons/CPython-3.7.0/Frameworks/Python.framework/Versions/3.7/include/python3.7m -c aiohttp/_websocket.c -o build/temp.macosx-10.13-x86_64-3.7/aiohttp/_websocket.o
    clang: error: no such file or directory: 'aiohttp/_websocket.c'
    clang: error: no input files
    error: command 'gcc' failed with exit status 1

Steps to reproduce

Run

pip install git+http://github.com/aio-libs/aiohttp

in fresh empty venv.

Your environment

macOS 10.13.6/Python 3.7/empty vevn
I am sure that the problem is OS/Python version independent.

[webknjaz]

Clone with recursive submodules and install from there

[webknjaz]

Ref #3138

[apatrushev]

Sorry, my fault. May be it will make a sense to write the link to CONTRIBUTING.txt (the only place where I found recursive word) somewhere in issue template?

[apatrushev]

Yeah, and before it was possible to install aiohttp with command I mentioned above:

pip install git+http://github.com/aio-libs/aiohttp

I think that vendoring with submodules is a little step back. Just an opinion.

[webknjaz]

GitHub shows the link to the CONTRIBUTING in the side bar when you create an issue or PR

[asvetlov]

Make sense.
Contributing is awailable from aiohttp docs but if you want a new link -- please add it.

[asvetlov]

Sorry, nevermind

[webknjaz]

Try pip==18, I saw some code handling submodules there. Also this simplifies tracking of external sources.

[apatrushev]

Just reproduced everything with --recurse-submodules and it does not work anyway. Broken dep mentioned in the error is aiohttp/_websocket.c and it has no relationship with submodules AFAICS.

[apatrushev]

The problem introduced here 28f1519 by unhiding CCompilerError in setup.py.

[apatrushev]

New finding: no actual tests occurs in travis without cython installed. It is installed in all envs AFAICS. I am not a big expert in tox/travis in fact, but it looks like that for me. That's why this regression was not caught by the tests. Should I open new issue for that?

[asvetlov]

We should have a clean separation between aiohttp developing mode and usage.
In development, I'd like to be as strict as possible.
C Compiler error exception was removed after I spent several hours trying to figure out why my C changes are not visible by tests. The answer was trivial: I had suppressed compilation error.

So restriction to have a properly prepared environment (cython, submodules, whatever) when hacking aiohttp is perfectly fine.

Installing aiohttp from PyPI for usage in a project is different.
We have binary wheels for most popular platforms, they work fine.
If a user has a not supported platform pip downloads and installs tarball.
The tarball already has cython compiled files (it is a part of our deployment process), no need to install Cython on user's machine.
setup.py tries to compile C extensions and falls back to pure python implementation if C compiler is not available. Compilation errors are reported carefully and break the build. We can add verbosity to reported error messages (say, recommend to install python-dev package if Python includes are not available) -- but I don't care too much now. Prefer to wait for a problem report first.

We have no pure python environment for testing, that's true. I check it manually after every release.
Tests automation is absent because I don't want to slow down our already not fast Travis bots.
We can do it on nightly builds but again I don't care too much and don't want to invest my own tie to make it work.

[apatrushev]

Is the version without binary extensions at all supposed to be working? If so, I can create an update for setup.py and make it a little bit more sophisticated to check availability of C files and switch between version without-ext/with-c-ext/cython. It will not rely on compiler errors, but will check availability of preprocessed cython files.

[apatrushev]

@asvetlov to avoid misunderstanding - what do you mean by tarball already has cython compiled files? Which tarballs? I just tried to download last 3 releases from https://github.com/aio-libs/aiohttp/releases and did not find cython compiled files there.

[asvetlov]

Please check out PyPI release: https://files.pythonhosted.org/packages/72/6a/5bbf3544fe8de525f4521506b372dc9c3b13070fe34e911c976aa95631d7/aiohttp-3.3.2.tar.gz

Generated C files are always here (as well as http_parser from submodule). Version without C extensions should work: we wrote the library this way, do check behavior equality of pure Python and C Accelerator functionality.
The only thing that we don't perform is checking installation on systems without Cython and C compiler

[webknjaz]

@asvetlov we should probably attach dists to gh releases from CI on publish as well. I've suggested earlier that those could also be used as a synchronization primitive allowing to release all dists simultanously.

[apatrushev]

I totally agree with @webknjaz because currently it is confusing situation that you can download "wrong" release tarball.

[webknjaz]

Well, technically it's not a release tarball, it's just git-archive

[apatrushev]

Tar file on release page is not a release tarball. That's what I name confusion.

[webknjaz]

Yes, because it's automatically generated by GitHub for every (tagged?) commit, it's basically git archive, nothing more than that.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a [new issue] for related bugs.
If you feel like there's important points made in this discussion, please include those exceprts into that [new issue].
[new issue]: https://github.com/aio-libs/aiohttp/issues/new