Add a reference Dockerfile for building aiohttp from a git clone - #13347
Closed
ri1wik wants to merge 2 commits into
Closed
Add a reference Dockerfile for building aiohttp from a git clone#13347ri1wik wants to merge 2 commits into
ri1wik wants to merge 2 commits into
Conversation
The dockerization run for aio-libs#11290 died inside `pip install -e .` with "Install submodules when building from git clone". setup.py exits 2 when vendor/llhttp/README.md is missing, and the build ordered `git submodule update --init --recursive` after the install that needs it, so it never ran. Reordering on its own isn't enough. setup.py lists the Cython-generated .c files as Extension sources while the repo only ships the .pyx originals ("NOTE: makefile cythonizes all Cython modules"), so the next thing you hit is gcc failing on a missing aiohttp/_websocket/mask.c. fixed.Dockerfile generates the llhttp sources in a throwaway Node stage, runs `make cythonize-nodeps`, and installs requirements/test.txt rather than hand-listing packages - the lockfile already has isal, zlib_ng, blockbuster, freezegun and pytest-mock, which the original run rediscovered one failed attempt at a time. original-reconstructed.Dockerfile is kept alongside it: it reproduces the original error verbatim, so the before/after can be diffed. Verified on the aio-libs#11290 tree - image builds, C extensions load, and tests/test_payload.py plus tests/test_client_functional.py pass (63 and 278 passed, 0 failures).
Says three changes, there are four. Also notes which of them are actually about the recorded failure and which are not.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #13347 +/- ##
==========================================
- Coverage 98.98% 98.88% -0.11%
==========================================
Files 132 130 -2
Lines 49073 49029 -44
Branches 2553 2543 -10
==========================================
- Hits 48576 48483 -93
- Misses 373 421 +48
- Partials 124 125 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
Member
|
Duplicate of #13331 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Name: Ritwik Ritu Parn
Email:
What it adds
fixed.Dockerfile— builds the repo from a clone with the C extensions enabled, and runs a smoke test that asserts the accelerated build actually loaded.original-reconstructed.Dockerfile— the same build with the steps in the wrong order, kept as executable documentation of the failure mode..dockerignore— notably does not exclude.git, for reasons below.Why it might be worth having
Building from a clone inside a container has three ordering constraints that aren't obvious from
CONTRIBUTING.rst, and each one fails with an error that points somewhere unhelpful:Submodules before install.
pip install -e .beforegit submodule update --initgivesexit code: 2 / Install submodules when building from git clonefrom the guard atsetup.py:24-31. Clear enough, but easy to get the ordering wrong in a Dockerfile where eachRUNlooks independent.Cython codegen before compile.
setup.pylistsaiohttp/_websocket/mask.c,_http_parser.c,_http_writer.cand_websocket/reader_c.casExtensionsources, but only the.pyxoriginals are tracked — the note atsetup.py:34says the makefile handles it. Missmake cythonizeand you getcc1: fatal error: aiohttp/_websocket/mask.c: No such file or directory, which reads like a missing compiler dep rather than a missing build step.llhttp needs Node.
vendor/llhttp/build/c/llhttp.ccomes fromnpm ci && make generatein the submodule. Here that runs in anode:20builder stage so npm never reaches the final image.The
.dockerignorekeeps.gitdeliberately:setup.pybranches onIS_GIT_REPO = (HERE / ".git").exists(), so dropping it silently disables the submodule guard and the build fails later and less legibly, inside gcc.Installing
requirements/test.txtrather than hand-listing packages also avoids rediscoveringisal,zlib_ng,blockbuster,freezegunandpytest-mockone import error at a time.Verified
parser=aiohttp._http_parseris the part that matters — that's the compiled parser, not the pure-Python fallback you get fromAIOHTTP_NO_EXTENSIONS=1.Built and tested on arm64 against 9b0153c. No changelog entry since nothing user-facing changes; happy to add one, adjust the layout, or drop
original-reconstructed.Dockerfileif only the working one is of interest.