-
Notifications
You must be signed in to change notification settings - Fork 636
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
media-types.md: clarify differences from Docker media types #817
Conversation
@dmcgowan @tonistiigi @ktock PTAL |
e77583e
to
008d344
Compare
media-types.md
Outdated
- `.mediaType`: only present on Docker | ||
- `.annotations`: only present on OCI | ||
- `.[]manifests.annotations`: only present on OCI | ||
- `.[]manifests.platform`: ARM v6 is represented as `{"architecture":"arm","variant":"v6"}` on OCI, while represented as `{"variant:armv6l"}` on Docker |
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.
This needs to be a mistake in https://github.com/distribution/distribution/pull/1592/files#diff-6b5275ccafa2776e05d590a9e729b0ec6762300317c928d34adc856d9f11714fR108, right? I'm quite sure no implementation has ever supported anything like armv6l
. Quotes are wrong as well.
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.
Improved text. PTAL.
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.
Just fix that erroneous example in upstream. No need to copy it around more. Afaics platform was moved to OCI exactly how it was implemented in Docker (with all the os.* weirdnesses). Theoretically, you can use any value in there. OCI spec even makes it clear that it is implementation-defined. The values that OCI recommends for arm are the same values that Docker uses (and used before that docs example was changed to an invalid value).
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.
Removed the note about .[]manifests.platform
and opened PR distribution/distribution#3371 for fixing Docker spec.
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.
Same for StopSignal
Labels
. These are not OCI concepts but copied from Docker. The fix is to update docs where needed. Issue with these fields seems to be that Docker reuses container.Config
struct in here that is documented under API docs and needs to be kept in sync with your docs link.
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.
Updated to remove StopSignal
and Labels
The fix is to update docs where needed.
Do we need to update all v1.md
, v1.1.md
, and v1.2.md
in https://github.com/moby/moby/tree/v20.10.8/image/spec ? Or maybe we should have v1.2.1.md
?
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 don't think we need to increase version. If you want, you can look at the publish date for the old ones as I do think at least most of them are very old but just updating v1.2
is good enough for me.
008d344
to
705b1fa
Compare
705b1fa
to
e57b202
Compare
e57b202
to
cb8a8b3
Compare
rebased |
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.
A couple of nits, but is looking good
38bb9a8
to
13b8355
Compare
OCI media types are slightly different from Docker ones, e.g., Docker manifests must have `.mediaType` field while OCI may not have. Also, OCI descriptors may have `.annotations` while Docker may not. Also updates to compare the spec with Docker Image Spec v1.2, not v1.0. OCI Image Spec v1 is more akin to Docker Image Spec v1.2 rather than v1.0, which lacked content addressability. See also https://github.com/moby/moby/blob/v20.10.1/image/spec/README.md for differencces between Docker Image Spec v1.2 and v1.0. Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
13b8355
to
08dd547
Compare
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.
LGTM
OCI media types are slightly different from Docker ones,
e.g., Docker manifests must have
.mediaType
field while OCI may not have.Also, OCI descriptors may have
.annotations
while Docker may not.