-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: allow setting platform for scratch image #5670
base: master
Are you sure you want to change the base?
Conversation
I'm not sure I still understand what case this is. If you want to build
We have some warnings already when we detect that the build result doesn't match what user asked in the build request. Atm we detect misbehaving frontends, but in the future, we could check binaries etc. |
I want to use it for a case when I want to cross-compile, so my target is arm64, but compiler has to run on i386. And the compiler itself is to be copied from context, therefore that won't set LLB to cross-compilation. PTAL at stage3 from my linked commit as an example. It pulls in stage2, a cross-compiler which has i386 binaries and aarch64 output |
Possibly related #5691 |
Looking at #5691 that's more of an issue with the emulators fallback. If emulators would be installed in kernel via |
I observe to issue only on our GH Action. Locally it works for me with both moby and podman. I think on my machine kernel level binfmt is in use so respective |
Yeah, but FROM platform also allows buildkit to properly detect that it's a cross build. This should be more future-proof |
now see that |
For some scenarios you cannot just add binfmt to your environment. And honestly it seems more like a workaround rather than letting buildkit know what you're going to run so that it handles the build properly |
From my understanding, this fixes the case where FROM scratch AS build-386
COPY --from=context-386 . /
ARG TARGETARCH # arm64 |
@tonistiigi it looks like adding |
Build contexts are selected based on the current step arch, so this won't work unless the current step is primed for needed arch either by FROM with a platform, or my proposed solution |
I think we can bring this in, but we need an integration test. |
Okay, will work on this
|
BuildKit refers to platform set in the current LLB node for determining whether or not executing commands should use QEMU. This makes some build flows working fine without BuildKit to fail with it. This is of particular importance for cross-compilation cases. Make scratch image a no exception and allow platform to be set, therefore determining the architecture of binaries planned to be ran (e.g. if they are COPYed from contexts) Signed-off-by: Dmitry Sharshakov <[email protected]>
@@ -6310,6 +6311,61 @@ COPY --from=build out . | |||
require.Equal(t, "freebsd", string(dt)) | |||
} | |||
|
|||
func testPlatformArgsExplicitContext(t *testing.T, sb integration.Sandbox) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not so sure about correctness of this one and didn't have time to run this through. I ran unit tests though and they feel to be sufficient for the change
An issue faced during practical usage: So it appears this is not sufficient to solve the problem without using FROM directly and not using COPY from image-based contexts at all during cross stages. Note: once bootstrap-stage1 image was not built with |
BuildKit refers to platform set in the current LLB node for determining whether or not executing commands should use QEMU. This makes some build flows working fine without BuildKit to fail with it. This is of particular importance for cross-compilation cases.
Make scratch image a no exception and allow platform to be set, therefore determining the architecture of binaries planned to be ran (e.g. if they are COPYed from contexts)
Example use case: siderolabs/stagex@eec19a8 - here we build for arm64 and amd64 targets, but use linux/386 compiler binary, as these are cross-compilation stages. So if scratch is used, then buildkit would use target platform and assume QEMU must be called to run /bin/sh, which crashed the build, since BuildKit called qemu-aarch64, which errored on i386 ELF file supplied. But with this patch I'm able to declare platform build happens in, even though I only add images later on (and for example stage3 pulls in multiplatform stage2, so in this case FROM stage2 directly won't work either)
Example build (listing logs with
linux/386->arm64
) https://github.com/siderolabs/stagex/actions/runs/12834542603/job/35792080087