Repository navigation
fix(build): let a builder stage inherit its base stage's platform - #4545
Conversation
The cross-compile check read only the literal FROM line, so a stage built FROM an earlier --platform=$BUILDPLATFORM stage counted as emulated even though it runs natively. The shadowbox webpack stage is such a stage, and it failed the check on main. The check now follows FROM <stage> to its base and inherits its platform. ARCH_NEUTRAL also accepts "<file>:<stage>", so the webpack stage can be marked as producing no architecture without excusing the Go stage in the same Dockerfile. A lookup failure now fails the stage instead of passing it. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <[email protected]>
|
Warning Review limit reachedNext included review available in 18 minutes. View limit detailsLimit details: You’ve used all 8 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository: cozystack/cozystack/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
IvanHunters
left a comment
There was a problem hiding this comment.
LGTM. The check modeled only the literal FROM line, so FROM source AS app in the shadowbox Dockerfile read as emulated even though source is pinned to --platform=$BUILDPLATFORM. Tracking native stages and carrying that to stages built from them is the right fix, and it matches BuildKit's own behavior: a stage takes the platform of the stage it starts from (docker/buildx#3034).
I ran the base check against the current Dockerfiles and it failed on exactly one stage, shadowbox/Dockerfile: FROM source AS app; the new check passes and no other stage changed verdict, so the scope is exactly that one false positive. The Go ss-server stage in the same file stays native and targeted and still has to use TARGETARCH, which the :app-scoped neutral entry correctly leaves alone. Dropping the inheritance line turns app back to non-native, so the new test genuinely holds it.
Three non-blocking notes:
-
Mirror case, and it predates this PR:
FROM --platform=$BUILDPLATFORM <earlier-emulated-stage>gets marked native by the literal flag, but BuildKit ignores--platformwhen the base is a prior stage and the child inherits the parent's (emulated) platform (moby/buildkit#4929, podman-container-tools/buildah#4464). No Dockerfile here hits it today; worth a comment since it is the exact inverse of what this change models. -
base in nativestageis case-sensitive while Docker stage names are not, soFROM SourceagainstAS sourcewould miss the inheritance and flag a native stage. That errs on the strict side and nothing in the tree triggers it. -
Minor: the comment on lines 14-15 became one long line after the edit.
What this PR does
The check that builder stages compile natively is red on main, and this is a false positive. The shadowbox Dockerfile builds its webpack stage as
FROM source AS app, wheresourceis a--platform=$BUILDPLATFORMstage. The check reads only the literal FROM line, so it reportsappas emulated. BuildKit gives a stage the platform of the stage it starts from, soappalready runs natively.The check now tracks which named stages are native and carries that to any stage built from one of them. A stage built from an emulated stage stays emulated. The arch-neutral list now also takes a
<file>:<stage>entry. With it only the shadowbox JS stage is excused, and its Go stage still has to use TARGETARCH.Screenshots
Not a UI change.
Downstream repositories
Release note