fix(oci): render arm64 platform description without redundant v8 - #783
Merged
jglogan merged 1 commit intoAug 7, 2026
Merged
Conversation
Halvanhelv
force-pushed
the
fix/platform-description-canonical-arm64
branch
from
June 27, 2026 23:09
1e8f0c7 to
7eb0c7d
Compare
Contributor
|
Thanks for the change! Can you please rebase the change before we merge |
Halvanhelv
force-pushed
the
fix/platform-description-canonical-arm64
branch
from
August 6, 2026 19:39
7eb0c7d to
05af1ac
Compare
Contributor
Author
Done |
adityaramani
approved these changes
Aug 6, 2026
adityaramani
left a comment
adityaramani
left a comment
Contributor
There was a problem hiding this comment.
LGTM! Only nit I have is do we need to remove the references to the issues in the code. Will wait for another maintainer to weigh in
Contributor
Author
Great, I'll try to delete it quickly if that's the decision |
Contributor
|
@Halvanhelv Yep, go ahead and take the issue reference in the test comment and we'll build, approve, and merge. Thank you! |
jglogan
requested changes
Aug 7, 2026
jglogan
left a comment
jglogan
left a comment
Contributor
There was a problem hiding this comment.
Just the issue comment, otherwise looks good.
Halvanhelv
force-pushed
the
fix/platform-description-canonical-arm64
branch
from
August 7, 2026 08:12
05af1ac to
20816d0
Compare
`Platform.description` rendered the same arm64 platform two different ways depending on how the value was constructed: `linux/arm64` when the variant was `nil`, and `linux/arm64/v8` when the variant was set to `"v8"`. These are the same platform — `==`, `hash`, and Set membership already treat an arm64 `nil` variant as equivalent to `"v8"` — so two equal values produced different descriptions and drifted between `arm64` and `arm64/v8` across stages of a single build (apple/container#1542). Omit the redundant `v8` variant for arm64 so equal platforms always describe as `linux/arm64`, matching how Docker and containerd display it. Only the rendered description changes; the stored variant and Codable encoding are untouched, so OCI content digests remain stable.
Halvanhelv
force-pushed
the
fix/platform-description-canonical-arm64
branch
from
August 7, 2026 08:15
59f2d1e to
ac63cf2
Compare
Contributor
Author
done |
jglogan
approved these changes
Aug 7, 2026
Contributor
|
@Halvanhelv Thanks! Merged. |
Merged
5 tasks
henrywang
added a commit
to henrywang/Berthly
that referenced
this pull request
Aug 28, 2026
…eme (#133) Closes #129. ## What Adopts apple/container **1.3.0** (and its required containerization **0.41.0**). ### `RequestScheme.auto` removal ([apple/container#2100](apple/container#2100)) 1.3.0 deletes `RequestScheme.auto` and the internal-host detection behind it, so `schemeFor` now returns `https` for every host unless the caller already chose `http`. Without the old heuristic a plain-HTTP registry on `localhost` or a private network is unreachable unless the user ticks "Allow insecure registry". New `RegistrySchemeResolver` ports that detection **verbatim** — `localhost`, the daemon's internal DNS domain, and the RFC 1918 / loopback IPv4 ranges resolve to `http`; everything else to `https` — and drives the paths where Berthly controls a single host: `resolveRegistryConnectionTarget` (login), `pullImage`, `pushImage`, `recreateContainer`. `runRegistryFlags` / `machineRegistryFlags` can't use it: their one `Flags.Registry` scheme fans out to the init-image fetch too (`Utility.containerConfigFromFlags`), so per-host detection isn't safe there. The insecure toggle keeps its "force http" meaning and becomes the only path to `http` for `run` / `machine create` against an untoggled internal registry — documented as a deliberate gap in `PARITY.md` (`pull` then `run` for the same result without the toggle). The vminit base-image pull is hardcoded to `.https` (Apple's registry, not routed through the resolver so a user's internal DNS domain can't match it). ### containerization 0.41.0 Required by 1.3.0. [apple/containerization#783](apple/containerization#783) drops the redundant `v8` variant from arm64's `Platform.description` (matching Docker/containerd) — one test assertion updated. The `Platform` equality fix ([#833](apple/containerization#833)) doesn't affect `builderPlatform` (line 2433 mirrors 1.3.0's own `BuilderStart.swift:116` verbatim). ### Compatibility floor Stays at 1.2 — nothing in 1.3.0 adds an API Berthly newly calls, so a 1.2.x daemon still works. Only the SPM pin moved. ## Test plan - [x] `xcodebuild build` — succeeds - [x] `xcodebuild build-for-testing` (all test targets incl. UITests/E2E) — succeeds - [x] `BerthlyTests` — 532 pass, 0 failures (new `RegistrySchemeResolverTests`, `runRegistryFlagsDefaultToHTTPS`) - [x] `swiftlint lint --strict` — 0 violations - [ ] Not verified without a local daemon: disk-usage volume-name validation ([#2107](apple/container#2107) / [#2136](apple/container#2136)) on the `fetchDiskUsage` path — stricter input checks, Berthly passes real volume names, no expected impact. ## Follow-ups (separate issues) #130 tmpfs fix verification · #131 k8s PARITY.md rationale · #132 mock kernel fixtures
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
3 participants
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.
Summary
Platform.descriptionrenders the same arm64 platform two different waysdepending on how the value was constructed:
These are the same platform —
==,hash(into:), andSetmembershipalready treat an arm64
nilvariant as equivalent to"v8"— yet theyserialize differently, so a single platform drifts between
linux/arm64andlinux/arm64/v8across stages of one build (apple/container#1542).Relation to #764
#764 fixed the
Hashableside of this: it stoppedhash(into:)from usingdescriptionand canonicalized arm64nil→v8in the hash. That workedaround the inconsistent
descriptionbut did not fix it — its own summarynames
description(linux/arm64vslinux/arm64/v8) as the root cause.This PR fixes that remaining gap at the source.
Change
Omit the redundant
v8variant for arm64 when renderingdescription, soequal arm64 platforms always describe as
linux/arm64— matching how Dockerand containerd display the platform. Other variants (
arm/v7) andarchitectures (
amd64) are unaffected.Only the rendered
descriptionchanges. The storedvariantand theCodableencoding are untouched, so OCI content digests remain stable.Testing
Added
OCIPlatformTestscases for description consistency (equal arm64platforms describe identically;
arm64/v8renders aslinux/arm64;arm/v7andamd64preserved).swift test --filter ContainerizationOCITestspasses (55 tests);
swift format lint --strictclean.Closes apple/container#1542 (normalization-consistency aspect).