Skip to content

Support durable base-image build context#3004

Merged
markphelps merged 2 commits into
mainfrom
fix-base-image-build-context
May 6, 2026
Merged

Support durable base-image build context#3004
markphelps merged 2 commits into
mainfrom
fix-base-image-build-context

Conversation

@markphelps
Copy link
Copy Markdown
Collaborator

Summary

  • Add a hidden base-image dockerfile --build-context-dir flag for external base-image builders.
  • Keep the default temporary build-context behavior unchanged.
  • Persist generated cog_build artifacts, such as requirements.txt, when callers provide a durable context directory.

Root Cause

base-image dockerfile can emit Dockerfiles that reference a named BuildKit context:

COPY --from=cog_build requirements.txt /tmp/requirements.txt

That path is needed for Torch base images because the base-image generator writes Torch, torchvision, torchaudio, and OpenCV requirements into requirements.txt.

After the .cog/build refactor, base-image dockerfile stages those files in a temporary .cog/build directory created via dotcog.OpenTemp(). The temp directory is cleaned up before the command exits, so external builders like cog-base-images cannot provide the required cog_build context to Docker.

This PR lets external callers opt into a durable staging directory without changing the default cleanup behavior.

Verification

  • go test ./pkg/dockerfile ./pkg/cli -count=1
  • Generated a Torch base-image Dockerfile with --build-context-dir /tmp/cog-base-context-check and verified:
    • Dockerfile contains COPY --from=cog_build requirements.txt /tmp/requirements.txt
    • /tmp/cog-base-context-check/requirements.txt exists after command exit
    • staged requirements include torch==2.11.0
  • git diff --check

@markphelps markphelps requested a review from a team as a code owner May 6, 2026 20:05
@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk Bot commented May 6, 2026

I've reviewed the changes in PR #3004. The implementation looks correct and well-tested:

  1. Clean API design: The hidden --build-context-dir flag allows external builders to opt into durable staging without changing default behavior
  2. Proper resource management: The buildDir() method returns appropriate cleanup functions - no-op for durable directories, dotcog.Close() for temp directories
  3. Good test coverage: The new test TestGenerateDockerfileWritesDurableBuildContext verifies both the Dockerfile content and the persisted requirements.txt
  4. Backward compatibility: Default temp directory behavior unchanged, cleanup happens via deferred function

The code follows existing patterns in the codebase and properly handles the edge case where Torch base images need their requirements.txt to persist for external Docker builds.

LGTM

github run

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk Bot commented May 6, 2026

LGTM

github run

@markphelps markphelps enabled auto-merge May 6, 2026 20:12
Copy link
Copy Markdown
Contributor

@michaeldwan michaeldwan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@markphelps markphelps added this pull request to the merge queue May 6, 2026
@markphelps markphelps removed this pull request from the merge queue due to a manual request May 6, 2026
@markphelps markphelps merged commit 3ec21f6 into main May 6, 2026
43 checks passed
@markphelps markphelps deleted the fix-base-image-build-context branch May 6, 2026 20:21
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.

2 participants