Skip to content
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

Use layer digest in the description of DiffID #328

Merged
merged 1 commit into from
Oct 24, 2016

Conversation

coolljt0725
Copy link
Member

Signed-off-by: Lei Jitang [email protected]

manifest digest means the digest of manifest, I think this is wrong.

@@ -32,7 +32,7 @@ This specification uses the following terms:
<dd>
A layer DiffID is a SHA256 digest over the layer's uncompressed tar archive and serialized in the descriptor digest format, e.g., <code>sha256:a9561eb1b190625c9adb5a9513e72c4dedafc1cb2d4c5236c9a6957ec7dfd5a9</code>.
Layers must be packed and unpacked reproducibly to avoid changing the layer ID, 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 <code>application/vnd.oci.image.layer.tar+gzip</code> types.
NOTE: the DiffID is different than the digest in the manifest list because the digest in the manifest is taken over the gzipped layer for <code>application/vnd.oci.image.layer.tar+gzip</code> types.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to avoiding “manifest digest”. But to make it extra clear that we're comparing with layer digests, how about a separate paragraph:

The DiffID is different than the layer digest in the <a href="manifest.md#image-manifest-property-descriptions">manifest's <code>layers</code></a> because the layer digest is taken over the blob regardless of compression, while the DiffID is taken after removing any compression.
For an <code>application/vnd.oci.image.layer.tar+gzip</code> layer.md, the layer digest is taken over the <code>application/vnd.oci.image.layer.tar+gzip</code> content, while the DiffID is take over the <code>application/vnd.oci.image.layer.tar</code> content.

Although that gets into defining a +gzip suffix (more on that in #316).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wking You suggestion make it much more clear, but the application/vnd.oci.image.layer.tar is not explicit define at the moment.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mentioning manifests here is the real problem. We should probably just say this is the content of the uncompressed layer tar.

@wking
Copy link
Contributor

wking commented Sep 20, 2016

On Mon, Sep 19, 2016 at 07:35:57PM -0700, Lei Jitang wrote:

  • 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.tar+gzip types.
  • NOTE: the DiffID is different than the digest in the manifest list because the digest in the manifest is taken over the gzipped layer for application/vnd.oci.image.layer.tar+gzip types.

@wking You suggestion make it much more clear, but the
application/vnd.oci.image.layer.tar is not explicit define at the
moment.

So we wait until #316 or similar lands with a definition of the +gzip
suffix? I'll work up a PR for the suffix.

@vbatts
Copy link
Member

vbatts commented Sep 21, 2016

On 20/09/16 01:37 -0700, W. Trevor King wrote:

So we wait until #316 or similar lands with a definition of the +gzip
suffix? I'll work up a PR for the suffix.

This change does not require waiting on the +gzip wording. That is one
of the ways that removing it here causes confusion.

@vbatts
Copy link
Member

vbatts commented Sep 21, 2016 via email

@wking
Copy link
Contributor

wking commented Sep 21, 2016

On Wed, Sep 21, 2016 at 12:57:02PM -0700, Vincent Batts wrote:

On 20/09/16 01:37 -0700, W. Trevor King wrote:

So we wait until #316 or similar lands with a definition of the
+gzip suffix? I'll work up a PR for the suffix.

This change does not require waiting on the +gzip wording. That is
one of the ways that removing it here causes confusion.

With the suffix defined, we could use the wording I've proposed in
1. @coolljt0725 liked that wording, but (correctly) pointed out
that it only makes sense once application/vnd.oci.image.layer.tar has
been defined 2. With the +gzip PR (#332), both
application/vnd.oci.image.layer.tar and
application/vnd.oci.image.layer.tar+gzip will be defined, and we can
use the wording from 1.

@@ -32,7 +32,7 @@ This specification uses the following terms:
<dd>
A layer DiffID is a SHA256 digest over the layer's uncompressed tar archive and serialized in the descriptor digest format, e.g., <code>sha256:a9561eb1b190625c9adb5a9513e72c4dedafc1cb2d4c5236c9a6957ec7dfd5a9</code>.
Layers must be packed and unpacked reproducibly to avoid changing the layer ID, 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 <code>application/vnd.oci.image.layer.tar+gzip</code> types.
NOTE: the DiffID is different than the digest in the manifest list because the digest in the manifest is taken over the gzipped layer for <code>application/vnd.oci.image.layer.tar+gzip</code> types.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mentioning manifests here is the real problem. We should probably just say this is the content of the uncompressed layer tar.

@coolljt0725
Copy link
Member Author

updated
@vbatts @stevvooe

@coolljt0725 coolljt0725 changed the title Replace 'manifest digest' with 'the digest in manifest' Use layer digest in the description of DiffID Sep 22, 2016
@@ -32,7 +32,7 @@ This specification uses the following terms:
<dd>
A layer DiffID is a SHA256 digest over the layer's uncompressed tar archive and serialized in the descriptor digest format, e.g., <code>sha256:a9561eb1b190625c9adb5a9513e72c4dedafc1cb2d4c5236c9a6957ec7dfd5a9</code>.
Layers must be packed and unpacked reproducibly to avoid changing the layer ID, 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 <code>application/vnd.oci.image.layer.tar+gzip</code> types.
NOTE: the DiffID is different than the layer digest because the digest is taken over the content of the uncompressted layer tar for <code>application/vnd.oci.image.layer.tar+gzip</code> types.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uncompressed

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vbatts fixed, thx

@coolljt0725 coolljt0725 force-pushed the digest branch 2 times, most recently from 3b5a4b2 to a319777 Compare September 28, 2016 01:39
@coolljt0725
Copy link
Member Author

@vbatts @stevvooe rebased after #333

@vbatts
Copy link
Member

vbatts commented Oct 6, 2016

LGTM

Approved with PullApprove

@vbatts
Copy link
Member

vbatts commented Oct 19, 2016

the needs another LGTM

@@ -28,7 +28,7 @@ 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.
NOTE: the DiffID is different than the layer digest because the layer digest is taken over the content of the gzipped layer for `application/vnd.oci.image.layer.v1.tar+gzip` types.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@wking wking Oct 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your suggested language points out the DiffID being over the uncompressed content. @coolljt0725's current wording points out the layer digest being over the (raw) gzipped content. Both of these are important for understanding the difference, and while the uncompressed-ness of DiffID is mentioned in the previous paragraph, I think it's worth talking about both of them in this note. I have wording for that in my earlier suggestion. Until something like #332 or #388 lands, we could just use “over the uncompressed content” instead of “over the application/vnd.oci.image.layer.v1.tar content”.

@coolljt0725
Copy link
Member Author

@stevvooe updated

@stevvooe
Copy link
Contributor

stevvooe commented Oct 21, 2016

LGTM

Approved with PullApprove

@coolljt0725
Copy link
Member Author

ping @vbatts needs re-LGTM after rebasing

@vbatts
Copy link
Member

vbatts commented Oct 24, 2016

LGTM

Approved with PullApprove

@vbatts vbatts merged commit 48e3743 into opencontainers:master Oct 24, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants