Skip to content

docker: copy the build context below the dependency installs - #2140

Open
sujeito-operator wants to merge 1 commit into
unclecode:developfrom
sujeito-operator:docker-context-below-the-dependency-installs
Open

docker: copy the build context below the dependency installs#2140
sujeito-operator wants to merge 1 commit into
unclecode:developfrom
sujeito-operator:docker-context-below-the-dependency-installs

Conversation

@sujeito-operator

Copy link
Copy Markdown

What this PR does / why we need it:

COPY . /tmp/project/ sits at line 139, four instructions above the layers that install
dependencies. 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), and
  • RUN 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 walked
everything between the old and new positions and none of it touches /tmp/project.
/tmp/install.sh does read it, but it is only written above (a RUN echo) — it is not
executed 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:

  • A full clone of develop at 3cc28d8 is 180 MB.git 149 MB, working tree
    31 MB, of which docs/ alone is 22 MB. There is no .dockerignore, so the whole of
    that is the build context and a docs-only commit busts the dependency layers exactly
    as a source commit does.
  • The seven packages named on the INSTALL_TYPE=all line are 562.0 MB of wheels
    before 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.yml builds from this Dockerfile with
INSTALL_TYPE: ${INSTALL_TYPE:-default}, so anyone iterating locally pays it on every
edit; and docker-release.yml builds two platforms with cache-from: type=gha /
cache-to: type=gha,mode=max, where the release commit always changes the context and
therefore 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 images diff and I am not claiming a wall-clock or
    image-size saving — only the layer-invalidation change, which is visible in the
    Dockerfile itself. What I would run to check it, on develop and then on this branch:

    docker build -t c4ai .            # warm the cache
    touch docs/md_v2/index.md         # a change that touches no code
    docker build -t c4ai .            # watch for CACHED on the pip install steps
    

    On develop the pip install -r requirements.txt step re-runs; on this branch it
    should 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 itself
    and 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/project is
    never removed and this is a single-stage build, so the whole 180 MB context — the
    149 MB .git included — ships inside the published image. A .dockerignore would fix
    that 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
    .git is 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 develop by hand before anything was written. The
    patch 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/project or 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.

`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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant