docker: copy the build context below the dependency installs - #2140
Open
sujeito-operator wants to merge 1 commit into
Open
docker: copy the build context below the dependency installs#2140sujeito-operator wants to merge 1 commit into
sujeito-operator wants to merge 1 commit into
Conversation
`COPY . /tmp/project/` sat four instructions above `pip install -r requirements.txt` and the optional torch/transformers install, neither of which reads the build context. Any change to any file in the repository therefore invalidated both. Nothing between the old and the new position reads /tmp/project. /tmp/install.sh does, but it is written above by a `RUN echo` and not executed until below the new position, so the ordering still holds.
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What this PR does / why we need it:
COPY . /tmp/project/sits at line 139, four instructions above the layers that installdependencies. Those layers do not read the build context, so today a change to any file
in the repository — a doc, a test, a README — invalidates:
RUN pip install --no-cache-dir -r requirements.txt(the 16 server requirements), andRUN if [ "$INSTALL_TYPE" = "all" ] ; then pip install torch torchvision torchaudio scikit-learn nltk transformers tokenizers ...This moves the copy down to just above the first instruction that actually reads
/tmp/project. Two lines removed, four added, and no behaviour change: I walkedeverything between the old and new positions and none of it touches
/tmp/project./tmp/install.shdoes read it, but it is only written above (aRUN echo) — it is notexecuted until the layer below the new position, so the ordering still holds.
Some numbers, all of them things you can re-measure rather than take from me:
developat3cc28d8is 180 MB —.git149 MB, working tree31 MB, of which
docs/alone is 22 MB. There is no.dockerignore, so the whole ofthat is the build context and a docs-only commit busts the dependency layers exactly
as a source commit does.
INSTALL_TYPE=allline are 562.0 MB of wheelsbefore their own dependencies — PyPI's published sizes for the current manylinux
x86_64 wheels, read 2026-08-13: torch 2.13.0 526.6 MB, transformers 5.15.0 11.7 MB,
scikit-learn 1.9.0 9.1 MB, torchvision 0.28.0 7.7 MB, tokenizers 0.23.1 3.3 MB,
torchaudio 2.11.0 1.8 MB, nltk 3.10.3 1.8 MB. torch's own CUDA runtime dependencies
are on top of that and I have not tried to total them.
Where that lands, concretely:
docker-compose.ymlbuilds from this Dockerfile withINSTALL_TYPE: ${INSTALL_TYPE:-default}, so anyone iterating locally pays it on everyedit; and
docker-release.ymlbuilds two platforms withcache-from: type=gha/cache-to: type=gha,mode=max, where the release commit always changes the context andtherefore always misses on these layers.
Special notes for your reviewers:
I did not build the image. There is no Docker daemon on the machine this was
written on, so I have no
docker imagesdiff and I am not claiming a wall-clock orimage-size saving — only the layer-invalidation change, which is visible in the
Dockerfile itself. What I would run to check it, on
developand then on this branch:On
developthepip install -r requirements.txtstep re-runs; on this branch itshould report
CACHED. Your CONTRIBUTING asks that Docker changes be tested locally,so I would rather say plainly that I could not than imply I had.
The installs that remain below the copy (
pip install "/tmp/project/...",/tmp/install.sh,crawl4ai-setup,playwright install) install the project itselfand genuinely depend on the source, so they stay where they are. A generic
copy-before-install linter will still flag the new position; that is the linter being
unable to tell "installs the copied project" from "installs third-party
dependencies", not a sign the move is incomplete.
A separate finding I have deliberately not put in this diff.
/tmp/projectisnever removed and this is a single-stage build, so the whole 180 MB context — the
149 MB
.gitincluded — ships inside the published image. A.dockerignorewould fixthat and would also stop docs edits busting the cache at all. I have left it out
because choosing what to exclude is your call, not mine, and one PR should do one
thing. Happy to send it as a follow-up if you want it. (For what it is worth: the
build reads the version from
crawl4ai/__version__.py, not from git metadata, so.gitis not needed at build time.)Provenance, since it bears on how much you should trust the above: the position was
flagged by a Dockerfile linter I maintain
(https://github.com/sujeito-operator/dockerfile-sanity) across a scan of 500
repositories, then re-read against
developby hand before anything was written. Thepatch was prepared by an automated agent. Close it without ceremony if it is not
useful to you.
If applicable:
this PR contains user facing changes - docs added
No user-facing change: the same files are installed in the same order by the same
commands, only the layer boundaries move. Nothing under
docs/mentions/tmp/projector this ordering — I grepped before writing that.this PR contains unit tests
No test added. This is a build-cache ordering change with no runtime behaviour to
assert, and I did not want to add a test I could not run against a real build.