media-types: Define layer media types with and without '+gzip' #388

wking · 2016-10-15T05:27:47Z

This is a version of #332 that doesn't define a structured syntax suffix (and just defines layer types with and without +gzip). I prefer #332, because it automatically provides compressed and uncompressed versions of any existing media type and is exactly the sort of thing that the RFC 6839 approach is useful for. But for reasons that I still don't understand, the maintainer consensus seems to be against involving structured syntax suffixes.

git diff b9173d5..581cc54 shows the current tips of this PR and #332 are largely the same, with this PR being a bit wordier in places that list media types (because it has to list both the bare and +gzip forms) and #332 being a bit wordier where it defines the structured syntax suffix. Currently this PR introduces fewer new words than #332, but I expect that as the number of potentially-compressed media types grows this approach will become more tedious. But we may not see further potentially-compressed media types, and if we do we can always return to the structured syntax suffix approach later.

jonboulle · 2016-10-17T15:00:24Z

LGTM, but I think you have others to convince

philips · 2016-10-19T04:10:43Z

This Looks Good To Me but, I will defer to those with stronger opinions from #332.

wking · 2016-10-21T19:48:55Z

Rebased around #317 and #337 with 581cc54 → fd97198.

vbatts · 2016-11-30T21:30:58Z

hmmm. I'm more inclined for this one. It would shift around media type variable names for some. And there are no MUSTs around implementations supporting all (though that really seems like not a huge deal to support a media type difference like this).

stevvooe · 2017-01-19T00:09:28Z

config.md

@@ -28,7 +28,8 @@ Changing it means creating a new derived image, instead of changing the existing
 A layer DiffID is a SHA256 digest over the layer's uncompressed tar archive and serialized in the descriptor digest format, e.g., `sha256:a9561eb1b190625c9adb5a9513e72c4dedafc1cb2d4c5236c9a6957ec7dfd5a9`.
 Layers must be packed and unpacked reproducibly to avoid changing the layer DiffID, for example by using tar-split to save the tar headers.

-NOTE: the DiffID is different than the digest in the manifest list because the manifest digest is taken over the gzipped layer for `application/vnd.oci.image.layer.v1.tar+gzip` types.
+The difference between DiffIDs and the layer digests in the [manifest's `layers`](manifest.md#image-manifest-property-descriptions) is that the layer digest is taken over the blob regardless of compression, while the DiffID is taken after removing any compression.
+For an `application/vnd.oci.image.layer.tar+gzip` layer, the layer digest is taken over the `application/vnd.oci.image.layer.tar+gzip` content, while the DiffID is take over the `application/vnd.oci.image.layer.tar` content.


This may not always be a given. A layer digest may be taken over the uncompressed content. The only limitation is that DiffID is always uncompressed.

This may not always be a given.

What may not be given? Are you just talking about the last sentence? It's qualified with “For an application/vnd.oci.image.layer.tar+gzip layer” (although that's missing the v1 added in #347), and I think the rest of that sentence is always true in that case. Of course, there may be layers that are not application/vnd.oci.image.layer.v1.tar+gzip…

A layer digest may be taken over the uncompressed content. The only limitation is that DiffID is always uncompressed

Isn't that what I'm saying (e.g. with “regardless of compresson” for layers and “after removing any compression” for DiffIDs)? Which wording in particular do you think is saying something different, and what do you think it's saying. Once we identify the sticky phrasing we can reword it for additional clarity.

Only one side of this statement is true: "the DiffID is take over the application/vnd.oci.image.layer.tar content."

For this PR to make even a remote bit of sense, a layer digest may be taken over application/vnd.oci.image.layer.tar or application/vnd.oci.image.layer.tar+gzip.

For this PR to make even a remote bit of sense, a layer digest may be taken over application/vnd.oci.image.layer.tar or application/vnd.oci.image.layer.tar+gzip.

For a generic layer yes, and those types are still not exhaustive because we only SHOULD layer media types. This sentence, on the other hand, is qualified with “For an application/vnd.oci.image.layer.tar+gzip layer”, and in that case the layer hash MUST be taken over the application/vnd.oci.image.layer.tar+gzip content.

@wking Could you please stop arguing every little point? I would like for this PR to land but you seem to just want to waste everyone's time. Please get this under control or I am going to start closing your PRs so we can focus on changes that are more likely to succeed.

A DiffID is narrow. It is part of the config, in rootfs. A DiffID is only ever taken over the uncompressed content.

A layer digest may be taken over the uncompressed or compressed version of the content. Arguably, this is not relevant to this document.

Arguably, this is not relevant to this document.

Perhaps this is a way forward? I've dropped the non-normative paragraph in 63ce6dd → 46ec151. If you'd rather have an informative paragraph with different wording, please just paste in the wording you'd like to see or file a PR against this branch.

stevvooe · 2017-01-19T00:10:05Z

layer.md

@@ -311,11 +311,10 @@ Any given image is likely to be composed of several of these Image Filesystem Ch
 Certain layers, due to legal requirements, may not be regularly distributable.
 Typically, such layers are downloaded directly from a distributor but are never uploaded.

-Layers that have these restrictions SHOULD be tagged with an alternative mediatype of `application/vnd.oci.image.layer.nondistributable.v1.tar+gzip`.
+Layers that have these restrictions SHOULD be tagged with an alternative media type of `application/vnd.oci.image.layer.nondistributable.v1.tar`.


Or +gzip.

This is true of many instances where I currently only talk about the uncompressed type (which is one reason why I prefer the structured syntax suffix approach from #332). With a structured syntax suffix off the table, this PR tries to stay clean by only defining the +gzip forms in media-types.md. For example, this section defines a nondistributable, uncompressed layer. If you're wondering what a compressed, nondistributable layer is, you look it up in the media type list, and see that it's a gzipped version of the uncompressed, nondistributable layer (linking to this section). You can also look up the uncompressed, nondistributable layer in the media-type table, and that also links to this section.

If, on the other hand, you're reading this section and think “hmm, I wonder what media type this would be if I gzip it?”, the current state of this PR requires to you to grep or remember that we define +gzip forms for these specific types. If we want to continually remind readers that we define those +gzip types, I'll reluctantly inject whatever boilerplate you need in most locations where we reference an uncompressed layer type; just tell me what you want that boilerplate to be. Maybe (or ['…+gzip'][media-types.md#oci-image-media-types) almost everywhere we list an uncompressed-layer media type? Do you really want that?

stevvooe · 2017-01-19T00:10:27Z

manifest.md

+        - [`application/vnd.oci.image.layer.v1.tar`](layer.md)
+        - [`application/vnd.oci.image.layer.v1.tar+gzip`](media-types.md#oci-image-media-types)
+        - [`application/vnd.oci.image.layer.nondistributable.v1.tar`](layer.md#non-distributable-layers)
+        - [`application/vnd.oci.image.layer.nondistributable.v1.tar+gzip`](media-types.md#oci-image-media-types)


This link doesn't look right.

What doesn't look right about it? We list both the uncompressed and +gzip layer types there, and define the +gzip form as a gzipped version of the uncompressed layer. I don't think we need to define the +gzip forms anywhere else, although there are other cases where it makes sense to list them as “MUST be supported”.

Why wouldn't it link to layer.md like the one above?

Because layer.md has nothing to say about gzipping (which happens on top of the layer tar archive described in layer.md):

$ git grep gzip 63ce6ddc -- layer.md …no hits…

So you link them to a random table? Link to what is contained. For all intents and purposes, a reader can infer that +gzip is nearly the same.

Link to what is contained.

I'm having trouble parsing this.

For all intents and purposes, a reader can infer that +gzip is nearly the same.

Why infer anything when we have a normative definition of the +gzip types in media-types.md? Maybe you want me to move that normative description into layers.md?

Maybe you want me to move that normative description into layers.md?

In case this was what you were looking for, I've worked it up as a fixup commit with a387698 → b0105cb. If the changes in b0105cb look good to you, let me know and I'll squash them in. If they don't look like what you want, just tell me what to change (or file a fixup PR against this branch).

After spinning off #515, the fixup commit which shifts the gzip definition is efade57 (and not b0105cb). The change made by the commit is the same. Let me know if you want it squashed, dropped, or changed.

Link to layer.md.

Link to layer.md.

I am. It may be easier to just look at the fixup commit (efade57).

stevvooe · 2017-01-19T00:13:33Z

While there will be complexities in making this a reality, I'm more in favor of this today than before.

For this PR to work it needs to make it clear that a layer DiffID is always the uncompressed hash, while the digest may be compressed or uncompressed based on the media type.

Please clean this up and rebase and let's see if we can make it work.

wking · 2017-01-19T07:33:17Z

On Wed, Jan 18, 2017 at 04:13:34PM -0800, Stephen Day wrote: For this PR to work it needs to make it clear that a layer `DiffID` is _always_ the uncompressed hash, while the digest may be compressed or uncompressed based on the media type.

I read my currently proposed wording as clear on this point. If you have a particular phrase that is bothering you, please point it out.

Please clean this up and rebase and let's see if we can make it work.

Rebased onto master with fd97198 → 63ce6dd. This also fixed some missing ‘.v1’s [1] and simplified some Markdown links (e.g. ‘[…](./media-types.md)’ → ‘[…](media-types.md)’), but didn't make major changes to my proposed wording. Points that still need discussion include: * What portion of my proposed DiffID vs. layer description is unclear, and how it should be modified for increased clarity [2] * Whether we need +gzip references at most locations where we currently have an uncompressed-layer type reference [3], and if so, what that +gzip reference should look like. * Whether the +gzip forms should have their definitions moved to a location besides media-types.md [4]. [1]: #388 (comment) [2]: #388 (comment) [3]: #388 (comment) [4]: #388 (comment)

wking · 2017-01-19T07:36:27Z

On Wed, Nov 30, 2016 at 01:30:58PM -0800, Vincent Batts wrote: And there are no MUSTs around implementations supporting all (though that really seems like not a huge deal to support a media type difference like this).

I'm not quite clear on what you're saying here (are you calling for “MUST support”? Suggesting we avoid MUST support?), but we currently REQUIRE support for the layer types we define [1], and with this PR we will continue that [2] (although now we are defining more layer types). [1]: https://github.com/opencontainers/image-spec/blame/v1.0.0-rc3/manifest.md#L55 [2]: https://github.com/opencontainers/image-spec/pull/388/files#diff-bf8d9e5146f89ef0a8b9bf8cd1026901R55

stevvooe · 2017-01-19T22:42:41Z

config.md

@@ -28,7 +28,8 @@ Changing it means creating a new derived image, instead of changing the existing
 A layer DiffID is a SHA256 digest over the layer's uncompressed tar archive and serialized in the descriptor digest format, e.g., `sha256:a9561eb1b190625c9adb5a9513e72c4dedafc1cb2d4c5236c9a6957ec7dfd5a9`.
 Layers must be packed and unpacked reproducibly to avoid changing the layer DiffID, for example by using tar-split to save the tar headers.

-NOTE: the DiffID is different than the layer digest because it is taken over the uncompressed gzipped layer for `application/vnd.oci.image.layer.v1.tar+gzip` types.
+The difference between DiffIDs and the layer digests in the [manifest's `layers`](manifest.md#image-manifest-property-descriptions) is that the layer digest is taken over the blob regardless of compression, while the DiffID is taken after removing any compression.


@wking Right here: this sentence is super unclear. It is backwards. Assert the positive notion (what a DiffID is taken over), then qualify it. Clean this up.

Assert the positive notion (what a DiffID is taken over)…

This happens in the preceding paragraph (“A layer DiffID is…”).

…then qualify it.

This paragraph is not attempting to qualify what a DiffID is. It is just underlining the difference between what a DiffID is (defined in the previous paragraph) and what a layer digest is (defined in manifest.md). A given uncompressed blob may have multiple compressed forms, but “the uncompressed blob associated with this potentially compressed blob” is unambiguous, so it made more sense to me to work it in the layer digest → DiffID direction.

However, if you would prefer different wording for this section, I'm happy to push a commit with whatever you want. Just tell me what it needs to be in a comment here or file a PR against my branch.

This can probably remain as a note. It is a little odd to mention a manifest at this level and introducing this alternative without more context is jarring.

This should probably be along the lines of "Do not confuse DiffID with layer digests, often referenced in the manifest, which allow digests over compressed or uncompressed content."

On Thu, Jan 19, 2017 at 02:48:16PM -0800, Stephen Day wrote [1]: > A DiffID is narrow. It is part of the config, in `rootfs`. A DiffID > is only ever taken over the uncompressed content. > > A layer digest may be taken over the uncompressed or compressed > version of the content. I agree with both of these points, but have been unable to convince Stephen that this is what I'm trying to show with this example. > Arguably, this is not relevant to this document. This paragraph was intended as a clarifying example, but it was never normative. Since we can't agree on clear wording, we can hopefully agree that it's not needed ;). [1]: opencontainers#388 (comment) Signed-off-by: W. Trevor King <[email protected]>

stevvooe · 2017-01-19T22:59:01Z

config.md

@@ -28,8 +28,6 @@ Changing it means creating a new derived image, instead of changing the existing
 A layer DiffID is a SHA256 digest over the layer's uncompressed tar archive and serialized in the descriptor digest format, e.g., `sha256:a9561eb1b190625c9adb5a9513e72c4dedafc1cb2d4c5236c9a6957ec7dfd5a9`.
 Layers must be packed and unpacked reproducibly to avoid changing the layer DiffID, for example by using tar-split to save the tar headers.

-NOTE: the DiffID is different than the layer digest because it is taken over the uncompressed gzipped layer for `application/vnd.oci.image.layer.v1.tar+gzip` types.


Do not confuse DiffID with layer digests, often referenced in the manifest, which allow digests over compressed or uncompressed content.

Works for me. Should I make a commit with you as an author and your usual Signed-off-by?

that is fine

Done with 46ec151 → a387698. Let me know if you want anything changed about that commit (e.g. the commit message).

Actually this new wording no longer mentions media types at all, so we can spin it off into an orthogonal PR. Done with #515 and dropped from here.

Trevor and I couldn't agree on more detailed wording [1], and this paragraph was never normative anyway. [1]: opencontainers#388 (comment) Signed-off-by: Stephen J Day <[email protected]> Signed-off-by: W. Trevor King <[email protected]>

stevvooe · 2017-01-19T23:43:09Z

LGTM after merging #515.

I'd prefer defining this as a structured syntax suffix following RFC 6839, and have filed a pull request to that effect [1]. However, the current maintainer consensus seems to be to define the compressed and uncompressed types directly without declaring a structured syntax suffix pattern [2]. I'm not clear on the reason for avoiding the structured syntax suffix, but that's the route I've taken in this commit. Now that you can choose both compressed or uncompressed media types, it is easy to clarify DiffIDs by comparing types with and without the +gzip compression. media type. It also allows you to create image-layout instances where the layers are stored uncompressed, which may be useful for cases such as: * Binary diffing between layer blobs for cheaper updates of large layers [3]. * Compressing an image-layout tarball for a smaller smaller overall tarball (by avoiding the unnecessary fragmentation of compressing the individual blob entries). [1]: opencontainers#332 [2]: opencontainers#332 (comment) [3]: http://ircbot.wl.linuxfoundation.org/eavesdrop/%23opencontainers/%23opencontainers.2016-08-16.log.html#t2016-08-16T23:35:43 Signed-off-by: W. Trevor King <[email protected]>

Generated with: $ make img/media-types.png and Graphviz version 2.38.0. Signed-off-by: W. Trevor King <[email protected]>

wking · 2017-01-19T23:50:11Z

On Thu, Jan 19, 2017 at 03:43:09PM -0800, Stephen Day wrote: LGTM after merging #515.

I've just rebased this on top of #515 with efade57 → 35b9fae. Assuming #515 lands unchanged and I don't have to re-rebase here, and assuming nobody chimes in asking for further changes, we'll be able to land 35b9fae as it stands.

vbatts · 2017-01-25T22:48:16Z

LGTM

stevvooe · 2017-01-25T22:48:43Z

LGTM

Catch up with aad7f24 (media-types: Define layer media types with and without '+gzip', 2016-09-20, opencontainers#388). Signed-off-by: W. Trevor King <[email protected]>

Trevor and I couldn't agree on more detailed wording [1], and this paragraph was never normative anyway. [1]: opencontainers/image-spec#388 (comment) Signed-off-by: Stephen J Day <[email protected]> Signed-off-by: W. Trevor King <[email protected]>

wking mentioned this pull request Oct 15, 2016

media-types: Define '+gzip' structured syntax suffix #332

Closed

wking mentioned this pull request Oct 19, 2016

Use layer digest in the description of DiffID #328

Merged

wking force-pushed the uncompressed-layer-media-types branch from 581cc54 to fd97198 Compare October 21, 2016 19:48

wking mentioned this pull request Oct 28, 2016

manifest: Explicitly make 'layers' optional #407

Closed

stevvooe reviewed Jan 19, 2017

View reviewed changes

wking force-pushed the uncompressed-layer-media-types branch from fd97198 to 63ce6dd Compare January 19, 2017 07:22

stevvooe reviewed Jan 19, 2017

View reviewed changes

wking force-pushed the uncompressed-layer-media-types branch from 46ec151 to a387698 Compare January 19, 2017 23:08

wking force-pushed the uncompressed-layer-media-types branch from a387698 to a929074 Compare January 19, 2017 23:19

wking force-pushed the uncompressed-layer-media-types branch from a929074 to b0105cb Compare January 19, 2017 23:21

wking mentioned this pull request Jan 19, 2017

config: Simplify the paragraph contrasting diffIDs with layer digests #515

Merged

wking force-pushed the uncompressed-layer-media-types branch from b0105cb to efade57 Compare January 19, 2017 23:32

wking added 2 commits January 19, 2017 15:46

img: Rebuild media-types.png

35b9fae

Generated with: $ make img/media-types.png and Graphviz version 2.38.0. Signed-off-by: W. Trevor King <[email protected]>

wking force-pushed the uncompressed-layer-media-types branch from efade57 to 35b9fae Compare January 19, 2017 23:46

wking mentioned this pull request Jan 23, 2017

layer is compressed tar format #518

Closed

stevvooe merged commit 84072d8 into opencontainers:master Jan 25, 2017

wking deleted the uncompressed-layer-media-types branch January 26, 2017 05:17

vbatts mentioned this pull request Jan 27, 2017

Bump version for v1.0.0-rc4 #537

Merged

wking mentioned this pull request Feb 2, 2017

schema/manifest_backwards_compatibility_test: Fix gzip compat type #550

Merged

media-types: Define layer media types with and without '+gzip' #388

media-types: Define layer media types with and without '+gzip' #388

Conversation

wking commented Oct 15, 2016

jonboulle commented Oct 17, 2016 • edited by caniszczyk Loading

philips commented Oct 19, 2016

wking commented Oct 21, 2016

vbatts commented Nov 30, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevvooe commented Jan 19, 2017

wking commented Jan 19, 2017 via email

wking commented Jan 19, 2017 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevvooe commented Jan 19, 2017 • edited by caniszczyk Loading

wking commented Jan 19, 2017 via email

vbatts commented Jan 25, 2017 • edited by caniszczyk Loading

stevvooe commented Jan 25, 2017 • edited by caniszczyk Loading

jonboulle commented Oct 17, 2016 •

edited by caniszczyk

Loading

stevvooe commented Jan 19, 2017 •

edited by caniszczyk

Loading

vbatts commented Jan 25, 2017 •

edited by caniszczyk

Loading

stevvooe commented Jan 25, 2017 •

edited by caniszczyk

Loading