Add Apple Container as a build-capable runtime#509
Conversation
Add the `container` binary (Apple Virtualization framework) to DetectContainerRuntime() on macOS, alongside docker and podman. Switch tag/push commands to use the `image` subcommand form (`image tag`, `image push`) which is compatible with all three runtimes. Update error messages and UI text to mention Apple Container.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for the "container" runtime (Apple Container) on macOS. It updates the runtime detection logic, error messages, and UI onboarding text to include this runtime, and transitions the container CLI commands (tag, push) to use the image subcommand prefix (e.g., image tag, image push). The review feedback suggests updating the build commands to also use the image build form for consistency, and updating a dropdown label in the onboarding UI to match the "Apple Container" terminology.
| <strong>No container runtime detected.</strong> | ||
| <p> | ||
| Install Docker or Podman to pull or build images. You can skip this | ||
| Install Docker, Podman, or Apple Container to pull or build images. You can skip this |
There was a problem hiding this comment.
For consistency and clarity, please also update the label for the "container" option in the dropdown menu (around line 742) from "Container (generic)" to "Apple Container" or "Apple Container (macOS)". Since the warning message now explicitly mentions "Apple Container", keeping "Container (generic)" in the dropdown can be confusing for users.
|
|
||
| if buildPush { | ||
| pushExec := exec.CommandContext(cmd.Context(), runtimeBin, "push", outputImage) | ||
| pushExec := exec.CommandContext(cmd.Context(), runtimeBin, "image", "push", outputImage) |
There was a problem hiding this comment.
Since the tag and push commands have been updated to use the image subcommand form (e.g., image push, image tag) for compatibility across all runtimes, the build command (on line 114) should also be updated to use the image build form (i.e., image, build) for consistency and to ensure compatibility with runtimes that require the image prefix for all image-related operations.
| pushImage := registry + "/" + outputImage | ||
| _, _ = fmt.Fprintf(logger, "Tagging %s as %s...\n", outputImage, pushImage) | ||
| tagCmd := exec.CommandContext(ctx, runtimeBin, "tag", outputImage, pushImage) | ||
| tagCmd := exec.CommandContext(ctx, runtimeBin, "image", "tag", outputImage, pushImage) |
There was a problem hiding this comment.
Similar to the tag and push commands, the build command on line 556 should also be updated to use the image build form (i.e., image, build) for consistency and compatibility across all runtimes.
1 participant
Add the
containerbinary (Apple Virtualization framework) to DetectContainerRuntime() on macOS, alongside docker and podman.Switch tag/push commands to use the
imagesubcommand form (image tag,image push) which is compatible with all three runtimes. Update error messages and UI text to mention Apple Container.Fixes #<issue_number_goes_here>