Repository navigation
[container]: add commit command - #1762
Merged
Merged
Conversation
saehejkang
force-pushed
the
container-commit-command
branch
from
June 20, 2026 01:36
ed952e3 to
daf20b6
Compare
mostlyobvious
added a commit
to mostlyobvious/mrs
that referenced
this pull request
Jul 3, 2026
Keep the doc to what is built and what is a tracked follow-up, per YAGNI. * Delete the "Likely Future Changes" section. Broader-macOS support, richer logs, and multi-container DNS/network improvements were invented roadmap with no tracked work. Promote the one real item — the shipped commit emulation — to a top-level section and fold the native-commit follow-up (apple/container#1762) into a single pointer. * Drop the "Container-to-container DNS" risk; ociman does not do multi-container, so those issues are not risks to this backend. * De-duplicate: the build/registry/DNS issue lists appeared in both Known Risks and Likely Future Changes; commit emulation and the port/host mechanics were described two or three times. Condense the Known Risks entries that merely restated Implementation Seams into pointers. * Remove residual future hints (framework "escape hatches", stdin-build 'if a future version supports it').
mostlyobvious
added a commit
to mostlyobvious/mrs
that referenced
this pull request
Jul 4, 2026
Keep the doc to what is built and what is a tracked follow-up, per YAGNI. * Delete the "Likely Future Changes" section. Broader-macOS support, richer logs, and multi-container DNS/network improvements were invented roadmap with no tracked work. Promote the one real item — the shipped commit emulation — to a top-level section and fold the native-commit follow-up (apple/container#1762) into a single pointer. * Drop the "Container-to-container DNS" risk; ociman does not do multi-container, so those issues are not risks to this backend. * De-duplicate: the build/registry/DNS issue lists appeared in both Known Risks and Likely Future Changes; commit emulation and the port/host mechanics were described two or three times. Condense the Known Risks entries that merely restated Implementation Seams into pointers. * Remove residual future hints (framework "escape hatches", stdin-build 'if a future version supports it').
mostlyobvious
added a commit
to mostlyobvious/mrs
that referenced
this pull request
Jul 4, 2026
The Apple backend is implemented and its acceptance suite passes, so the document is no longer a plan. Rename PLAN.md to APPLE_BACKEND.md, retitle, and rewrite the prescriptive sections to describe what shipped. * Drop the BackendKind and PrivilegeMode proposals, which were never built: unsupported-feature errors are Apple-only and hardcode "apple", and is_rootless() returns false for Apple. * Collapse the contradictory commit sections into the emulation reality and correct the CommitError snippet. The plan body previously said commit was unsupported / must not be emulated while the code shipped a full export-and-build emulation. * Note the InspectError::FormatUnsupported and ResolveHostnameError ::Unsupported / CommitError::Unsupported variants, and that the latter two are currently unreachable. * Rename the internal ContainerRecord seam to the Apple-only AppleRecord that was actually built. * Collapse the completed Suggested Milestones roadmap into a short Implementation History note. Drop draft-history narration from Apple backend doc The doc's evolution is recorded in git history, so describe only the current design. Remove the BackendKind/PrivilegeMode/ContainerRecord "earlier draft / considered but not built" asides, the "(shipped)" marker, and the Implementation History build-order section. Trim speculation and repetition from Apple backend doc Keep the doc to what is built and what is a tracked follow-up, per YAGNI. * Delete the "Likely Future Changes" section. Broader-macOS support, richer logs, and multi-container DNS/network improvements were invented roadmap with no tracked work. Promote the one real item — the shipped commit emulation — to a top-level section and fold the native-commit follow-up (apple/container#1762) into a single pointer. * Drop the "Container-to-container DNS" risk; ociman does not do multi-container, so those issues are not risks to this backend. * De-duplicate: the build/registry/DNS issue lists appeared in both Known Risks and Likely Future Changes; commit emulation and the port/host mechanics were described two or three times. Condense the Known Risks entries that merely restated Implementation Seams into pointers. * Remove residual future hints (framework "escape hatches", stdin-build 'if a future version supports it'). State Apple supported/unsupported surface in present tense The backend is shipped, so describe what it supports now rather than what to build first. Rename "Supported Initial Surface" to "Supported Surface" and "Explicitly Unsupported Initially" to "Explicitly Unsupported", and drop the stale conditionals: inline builds use the temporary-directory fallback (the target release rejects stdin contexts), and pull carries Apple-aware not-found classification. Drop redundant test-passing claims from Apple backend doc A shipped backend implies its suite passes, so assertions to that effect add nothing. Remove the "acceptance suite passes" / "is green" / "after green tests" phrasings from the intro, Test Plan, and Decision Status, keeping the acceptance command itself (and its serial-run constraint) as a how-to reference. Drop deferred decisions from Apple backend doc The list mixed non-decisions ("exact error enum shapes" is not a pending choice) and YAGNI speculation (exec builder fallibility has no Apple-unsupported option forcing it) with two genuinely open questions. Rather than curate, drop the section; open questions live in the tracker, not a settled design doc. Collapse the remaining "Settled decisions" list into the Decision Status body. Drop redundant unreleased-PR non-goal Targeting the last released Apple Container version already implies not depending on unreleased PRs. Pin Apple Container 1.0.0 baseline in docs Name the current released Apple Container version (1.0.0) instead of "the latest released version", and note the version is detected but not gated. Update the stale 0.3.0 example in the parse_version comment to match. Document why Apple integration CI is deferred Not the macOS skip: platform::support() already returns Ok(()) for OCIMAN_BACKEND=apple, so an Apple job would attempt resolution, not skip. The blocker is the runner CPU. Apple `container` runs Linux containers as lightweight virtual machines, so on a GitHub-hosted runner (itself a VM) it needs nested virtualization, which Apple supports only on M3+. GitHub's standard macos-26 runner is M1 and its larger macOS runners are M2 Pro, so a hosted job would fail at `container system start`, not pass. Record the revisit triggers (a self-hosted M3+ macOS 26 runner, or GitHub offering M3+ hosted runners) and correct the prior text, which wrongly implied hosted macos-26 runners could run the suite. Cite sources for the runs-as-VM, hosted-runner-VM, M3+, and M1 claims.
stephenlclarke
added a commit
to stephenlclarke/container-compose
that referenced
this pull request
Jul 12, 2026
Treat omitted compose commit --index and --index=0 like Docker Compose's unset service-container selection, while keeping explicit positive indexes targeted and rejecting negative indexes. Refresh commit parity docs and runtime blocker references now that apple/containerization#685 has landed and apple/container#1630 plus apple/container#1762 track the remaining live export/commit path.
saehejkang
force-pushed
the
container-commit-command
branch
from
August 26, 2026 05:54
daf20b6 to
0b4a47a
Compare
saehejkang
force-pushed
the
container-commit-command
branch
from
August 26, 2026 06:09
0b4a47a to
d6de569
Compare
saehejkang
force-pushed
the
container-commit-command
branch
2 times, most recently
from
August 26, 2026 06:25
bedbc0e to
aa0728f
Compare
Contributor
|
@saehejkang Build's failing for this one, looks like a simple fix: /Users/runner/actions-runner/_work/container/container/Sources/ContainerCommands/Image/ImageLoad.swift:83:73: error: cannot find type 'Logger' in scope
81 | }
82 |
83 | static func load(from path: FilePath, force: Bool = false, log: Logger) async throws {
| `- error: cannot find type 'Logger' in scope
84 | let progressConfig = try ProgressConfig(
85 | showTasks: true,
[3208/3318] Compiling ContainerCommands ContainerStop.swift |
Contributor
Author
Fixed. Also this PR is almost ready for a full review, but I need to go through some things before I am confident in the changes I made. |
saehejkang
force-pushed
the
container-commit-command
branch
2 times, most recently
from
September 9, 2026 04:07
f10c7d6 to
f29bb2f
Compare
saehejkang
force-pushed
the
container-commit-command
branch
from
September 9, 2026 04:48
f29bb2f to
bfc92c3
Compare
saehejkang
marked this pull request as ready for review
September 9, 2026 05:02
Code Coverage
|
jglogan
approved these changes
Sep 29, 2026
1 task done
henrywang
added a commit
to henrywang/Berthly
that referenced
this pull request
Oct 26, 2026
Closes #206. ## What - **Commit to Image…**: a button in the container detail header (`containerCommitButton`) and an item in the Compute row's context menu. Enabled for running or stopped containers, the states `export` accepts. - **Flow** matches the CLI's `container commit` (apple/container#1762, 1.6.0): 1. normalize the reference 2. export the rootfs 3. wrap it as a one-layer OCI archive built from the source image's config 4. load and unpack it through the existing `loadImages` - `ContainerCommitArchive` ports upstream's `OCIImageArchive`, which lives in the unlinked `ContainerCommands` target. `imageConfig(...)` is split out as a pure function: the init process becomes the entrypoint and cmd, and container labels override the source image's. - **Sheet:** prefilled with `<name>:snapshot`. - Validation reuses the Tag sheet's `tagTargetIssue`. An invalid reference blocks Commit. - An existing name shows a warning. I confirmed with the 1.6.1 CLI that committing onto an existing reference replaces it rather than failing. - A failure stays editable so you can retry. Cancel is disabled while the daemon-side export and load run. - **Scope note:** volumes and mounts aren't included. The server's `exportRootfs` reads only the container's rootfs block device. - PARITY.md: `commit` added to the Containers table, and the now-empty "Tracked for container 1.6.x" section is removed. ## Notes - `ContainerizationArchive` and `ContainerizationOCI` are imported transitively, with no new package product in the pbxproj. The local build and tests are fine. If CI fails with "No such module", that's the reason. - A commit temporarily uses about 3× the rootfs size in the temp dir (export tar, blob copy, image tar). It's cleaned up afterwards, and upstream's flow costs the same. - No version gate: running-container export predates the 1.6.1 floor, and nothing else in the flow is new on the daemon side. ## Testing - **Unit:** `ContainerCommitTests` covers the image-config mapping and label precedence. It also builds an archive and checks that `index.json` has one manifest with the reference annotations and platform, that the layer blob equals the rootfs bytes, and that the config carries the architecture, variant, author, diffIDs and entrypoint. A mock test checks that a commit adds the image, recommitting replaces it, and an unknown container throws. Full `BerthlyTests` passes. - **UI (mock):** `CommitContainerSheetUITests` checks that an invalid reference shows the issue and disables Commit, and that a valid one finishes on Done. - **E2E (real 1.6.1 daemon):** `CommitJourneyTests` writes a marker into a running alpine container and commits it from the detail button. A container run from the committed image sees the marker and runs `sleep` as PID 1. No `berthly-e2e` containers or images were left behind. Only the running path is tested end to end. A stopped container uses the same archive and load code, and stopped export is already covered by `ExportJourneyTests`. - `swiftlint lint --strict`: clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
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.
Type of Change
Motivation and Context
Closes #1399
Testing