Skip to content

Conversation

@austinvazquez
Copy link
Member

This change updates the build to split artifacts into host/guest. By default, build the host artifacts to match the developers local platform.

Signed-off-by: Austin Vazquez <austin.vazquez@docker.com>
Copilot AI review requested due to automatic review settings December 12, 2025 00:54
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the build system to differentiate between host and guest artifacts, enabling developers to build artifacts for their local platform by default while maintaining support for guest (Linux) artifacts.

Key changes:

  • Introduced HOST_OS variable and platform-specific build targets (_host_common, _guest_common)
  • Modified the default build group to include separate host and guest binary groups
  • Updated the Makefile to detect and use the local platform for builds

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

File Description
docker-bake.hcl Adds host/guest platform distinction with new variables and target groups, reorganizes default build targets
Makefile Updates platform detection to use lowercase OS names and passes HOST_OS to the build system
Dockerfile Changes cache key from TARGETPLATFORM to explicit TARGETOS/TARGETARCH format

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 107 to 108
output = ["${DESTDIR}"]
}
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

The libkrun target is missing an inherits declaration. Since it's part of the guest-binaries group (line 77), it should inherit from _guest_common to ensure consistent platform targeting.

Copilot uses AI. Check for mistakes.
$(BUILDX) bake
build:
@echo "$(WHALE) $@"
HOST_OS=$(OS) KERNEL_ARCH=$(ARCH) $(BUILDX) bake
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

Variable naming inconsistency: KERNEL_ARCH is used here but TARGETARCH is the standard Docker build argument. Consider renaming to HOST_ARCH for consistency with HOST_OS, or document why a different convention is used.

Suggested change
HOST_OS=$(OS) KERNEL_ARCH=$(ARCH) $(BUILDX) bake
HOST_OS=$(OS) HOST_ARCH=$(ARCH) $(BUILDX) bake

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

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

I need to check this config. Putting this into draft.

@austinvazquez austinvazquez marked this pull request as draft December 12, 2025 01:31
@austinvazquez
Copy link
Member Author

Superseded by #74

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