fix(build): honor CODEGEN_IMAGE in Makefile codegen target#1529
Open
anxkhn wants to merge 1 commit into
Open
Conversation
The generated-files CI workflow builds the codegen image once via buildx with GitHub Actions layer caching and loads it as codegen-env:latest, then runs "CODEGEN_IMAGE=codegen-env:latest make codegen" to reuse it. Since the codegen target was simplified to always run "docker build -q -t codegen-env", that variable became a no-op: the workflow rebuilt the image instead of reusing the cached one, and the buildx build step served no purpose. Skip the docker build and run the caller-supplied image when CODEGEN_IMAGE is set, falling back to a locally built codegen-env otherwise. This keeps "make codegen" self-contained for local use while letting CI consume the image it already built.
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
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
The generated-files workflow builds the codegen image once, with GitHub Actions
layer caching, and loads it into the daemon as
codegen-env:latest:The
CODEGEN_IMAGE=codegen-env:latestprefix is meant to makemake codegenreuse that pre-built, cache-warmed image instead of building it again. But the
codegentarget no longer readsCODEGEN_IMAGE; it always runs its owndocker build:So the variable is a no-op today: CI rebuilds the image from scratch on every run
and the buildx caching step above serves no purpose.
grep -rn CODEGEN_IMAGE(excluding
node_modules) currently returns a single hit, the workflow line,with nothing consuming it.
How this happened
CODEGEN_IMAGEwas originally introduced on both sides in the same change (theworkflow began passing it and the Makefile honored it via a
$${CODEGEN_IMAGE:-$$(docker build ...)}fallback), sitting next to the buildxcache step. A later "simplify codegen" change rewrote the
codegentarget to thehardcoded
docker build -q -t codegen-envform and dropped the variable, but theworkflow kept passing it, leaving the two sides out of sync.
The fix
Skip the
docker buildand run the caller-supplied image whenCODEGEN_IMAGEisset, falling back to a locally built
codegen-envotherwise:This keeps
make codegenself-contained for local use (unchanged behavior whenCODEGEN_IMAGEis unset) while letting CI consume the image it already built andcached. It restores the pre-simplification semantics in the current target's
readable multi-line style, so no workflow change is needed.
Behavior
make codegen, no variable): buildscodegen-env, then runs it.Unchanged.
CODEGEN_IMAGE=codegen-env:latest make codegen): skips the build and runsthe pre-built
codegen-env:latest, reusing the buildx GHA cache.The generated output is identical either way (same image contents, same
make generate), so the "Check for uncommitted changes" step is unaffected. TheGenerated filesjob itself is the regression guard here and stays green.Alternative considered
The mechanical alternative is to drop the now-dead prefix from the workflow
(
run: make codegen), which also makes the two sides consistent. I went withrestoring
CODEGEN_IMAGEinstead because the buildx cache step and itsload: truecomment ("makes the image available fordocker run") existspecifically to feed a reused image; dropping the prefix would leave that step
building an image nothing consumes. Happy to switch to the workflow-only change
if you'd rather keep the
codegentarget strictly hardcoded.