Fix build without cython (#3162)

[apatrushev]

What do these changes do?

Fixes build/install without cython

Are there changes in behavior for the user?

Nope

Related issue number

#3162

Checklist

• I think the code is well written
• [-] Unit tests for the changes exist
• [-] Documentation reflects the changes
• If you provide code modification, please add yourself to CONTRIBUTORS.txt
  • The format is <Name> <Surname>.
  • Please keep alphabetical order, the file is sorted by names.
• Add a new news fragment into the CHANGES folder
  • name it <issue_id>.<type> for example (588.bugfix)
  • if you don't have an issue_id change it to the pr id after creating the pr
  • ensure type is one of the following:
    • .feature: Signifying a new feature.
    • .bugfix: Signifying a bug fix.
    • .doc: Signifying a documentation improvement.
    • .removal: Signifying a deprecation or removal of public API.
    • .misc: A ticket has been closed, but it is not of interest to users.
  • Make sure to use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files."

[webknjaz]

We also might want to have some test for this (missing deps).

[webknjaz]

This is a workaround. We need a real fix, finding out why the file is missing or having a clear explanation of why this is happening.

[apatrushev]

Explanation is easy - build process relied on assumption that if we have no cython, than we will have compiler error and ignore extensions bundling. Later runtime will work with pure python implementations. This file is missing because cython is not available during build. So I can not understand what do you mean by real fix?

[apatrushev]

One more note: I just reintroduced the way how it worked before and was broken in 28f1519, as I mentioned in the issue.

[webknjaz]

Okay, it might make sense. Let's wait for @asvetlov.
P.S. I still want to see this tested in CI.

[apatrushev]

Nope. We already discussed the problem with @asvetlov in #3162 and I probably caught his idea and pain about development environment. Though the build is broken anyway: it is impossible to build/install from sources without gcc. So I plan to improve setup.py.

[webknjaz]

And while you're on it you might want to extend docs to mention how things work.

[apatrushev]

I'll require adding env combos to Travis CI as mandatory anyway.

@asvetlov mentioned that this is the bad idea, because it will increase even more the testing time.

[webknjaz]

Not really, just add it and then I'll make sure that it doesn't hit performance. I'll basically do to it what I did to OS X and it'll be a fine compromise: running those only on cron as opposed to having them in each build. Andrew trusts me to handle CI.

P.S. If we recognize that we need it on each commit we could always move it to another CI service.

[webknjaz]

Remark: while working on changing setup.py it might be worth adding some env var for toggling strict mode for dev.

It could be smth like:

import os
...
class ve_build_ext(build_ext):
    ...
    def build_extension(self, ext):
        valid_build_exceptions = {CCompilerError, DistutilsExecError, DistutilsPlatformError, ValueError}
        suppressed_build_exceptions = set()

        if os.getenv('AIOHTTP_DEV_MODE'):
            suppressed_build_exceptions = {CCompilerError, DistutilsExecError, DistutilsPlatformError}

        try:
            build_ext.build_extension(self, ext)
        except tuple(valid_build_exceptions - suppressed_build_exceptions) as build_exc:
            raise BuildFailed() from build_exc

cc: @asvetlov

[apatrushev]

Yeah, that's what I planned to implement.

[codecov-io]

Codecov Report

Merging #3164 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #3164   +/-   ##
=======================================
  Coverage   98.09%   98.09%           
=======================================
  Files          43       43           
  Lines        7833     7833           
  Branches     1351     1351           
=======================================
  Hits         7684     7684           
  Misses         57       57           
  Partials       92       92

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4f9f142...493a435. Read the comment docs.

[webknjaz]

It's also useful to add from original_exception part.

[asvetlov]

Superseded by #3189

[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